Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix provider definition #59

Merged
merged 7 commits into from
Jul 29, 2024
Merged

fix provider definition #59

merged 7 commits into from
Jul 29, 2024

Conversation

displague
Copy link
Collaborator

@displague displague commented Jul 22, 2024

Description of your changes

Fixes #56

reproducing the stack traces is possible with minimal setup (and no real account/creds needed):

# from a kind cluster with crossplane installed
$ cat <<EOF | kubectl apply -f -
apiVersion: pkg.crossplane.io/v1
kind: Provider
metadata:
  name: provider-jet-equinix
spec:
  package: xpkg.upbound.io/equinix/provider-jet-equinix:v0.8.0
  runtimeConfigRef:
    name: no-replicas
---
apiVersion: v1
kind: Secret
metadata:
  name: example-equinix-creds
  namespace: crossplane-system
type: Opaque
stringData:
  credentials: |
    {
      "client_id": "",
      "client_secret": "",
      "auth_token": "fake-token",
      "request_timeout": "30",
      "response_max_page_size": "100",
      "endpoint": "https://api.equinix.com"
    }
---
apiVersion: equinix.jet.crossplane.io/v1beta1
kind: ProviderConfig
metadata:
  name: default
spec:
  credentials:
    source: Secret
    secretRef:
      name: example-equinix-creds
      namespace: crossplane-system
      key: credentials
---
apiVersion: pkg.crossplane.io/v1beta1
kind: DeploymentRuntimeConfig
metadata:
  name: no-replicas
spec:
  deploymentTemplate:
    spec:
      replicas: 0
      selector: {}
      template: {}
EOF
# from a checkout of this branch
tilt up
$ cat <<EOF | kubectl apply -f -
apiVersion: metal.equinix.jet.crossplane.io/v1alpha1
kind: Project
metadata:
 name: upjet-example-project
spec:
 forProvider:
   name: upjet-example-project
 providerConfigRef:
   name: default
EOF

Monitor the tilt output.

v1beta1 providerconfig is also added because it is the upstream default and allows us to remove some code. I don't think this change is required other than for the reduction of a few lines of provider setup code.

Fixes #

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

return errors.Errorf("failed to configure the provider: %v", diag)
}

fwProvider := equinixprovider.CreateFrameworkProvider(version.ProviderVersion)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version.ProviderVersion is dev unless overridden at Go compile time. We could define that in the Makefile but it may be more practical to pass TERRAFORM_PROVIDER_VERSION in from main via (SetupConfig)

Signed-off-by: Marques Johansson <[email protected]>
@displague
Copy link
Collaborator Author

displague commented Jul 24, 2024

Currently failing repeatedly for all resources (as seen with tilt up / make run):

crossplane-p… │ goroutine 882 [running]:
crossplane-p… │ k8s.io/apimachinery/pkg/util/runtime.logPanic({0x254c0e0, 0x43630a0})
crossplane-p… │ 	k8s.io/[email protected]/pkg/util/runtime/runtime.go:75 +0x85
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:108 +0xb2
crossplane-p… │ panic({0x254c0e0?, 0x43630a0?})
crossplane-p… │ 	runtime/panic.go:770 +0x132
crossplane-p… │ github.com/crossplane/upjet/pkg/terraform.(*WorkspaceStore).Workspace(0x0, {0x2dab560, 0xc000ab1030}, {0x2d93ed0, 0xc0019955f0}, {0x72698216d2f8, 0xc000a0d208}, {{0xc000058072, 0x5}, {{0xc00005e16a, ...}, ...}, ...}, ...)
crossplane-p… │ 	github.com/crossplane/[email protected]/pkg/terraform/store.go:220 +0x5d
crossplane-p… │ github.com/crossplane/upjet/pkg/controller.(*Connector).Connect(0xc0009dc540, {0x2dab560, 0xc000ab1030}, {0x2dd27c0, 0xc000a0d208})
crossplane-p… │ 	github.com/crossplane/[email protected]/pkg/controller/external.go:116 +0x1a2
crossplane-p… │ github.com/crossplane/crossplane-runtime/pkg/reconciler/managed.(*NopDisconnecter).Connect(0xc0009e4430?, {0x2dab560?, 0xc000ab1030?}, {0x2dd27c0?, 0xc000a0d208?})
crossplane-p… │ 	github.com/crossplane/[email protected]/pkg/reconciler/managed/reconciler.go:246 +0x2f
crossplane-p… │ github.com/crossplane/crossplane-runtime/pkg/reconciler/managed.(*Reconciler).Reconcile(0xc0009d6a00, {0x2dab4b8, 0xc000cebb90}, {{{0x0, 0x0}, {0xc0009823d8, 0x16}}})
crossplane-p… │ 	github.com/crossplane/[email protected]/pkg/reconciler/managed/reconciler.go:892 +0x1e6b
crossplane-p… │ github.com/crossplane/crossplane-runtime/pkg/ratelimiter.(*Reconciler).Reconcile(0xc00017e140, {0x2dab4b8, 0xc000cebb90}, {{{0x0?, 0x5?}, {0xc0009823d8?, 0xc000b5ed10?}}})
crossplane-p… │ 	github.com/crossplane/[email protected]/pkg/ratelimiter/reconciler.go:54 +0x151
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x2db26e0?, {0x2dab4b8?, 0xc000cebb90?}, {{{0x0?, 0xb?}, {0xc0009823d8?, 0x0?}}})
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:119 +0xb7
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc0009e83c0, {0x2dab4f0, 0xc000183b30}, {0x2638000, 0xc000c99ea0})
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:316 +0x3bc
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc0009e83c0, {0x2dab4f0, 0xc000183b30})
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266 +0x1be
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227 +0x79
crossplane-p… │ created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2 in goroutine 239
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:223 +0x50c
crossplane-p… │ 2024-07-23T20:26:53-04:00	ERROR	Reconciler error	{"controller": "managed/metal.equinix.jet.crossplane.io/v1alpha1, kind=vlan", "namespace": "", "name": "vrf-via-crossplane-xrd", "reconcileID": "a272b3c1-71ce-43de-8d15-c92a22480125", "error": "panic: runtime error: invalid memory address or nil pointer dereference [recovered]"}
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:329
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266
crossplane-p… │ sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2
crossplane-p… │ 	sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227
crossplane-p… │ E0723 20:26:53.718234 2324822 runtime.go:79] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)

}
}

func configureNoForkEquinixClient(ctx context.Context, ps *terraform.Setup, p schema.Provider) error {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only copying what I see in other providers and I'm not sure why this should be added.

The NoFork concept emerged in crossplane/upjet#294

Both clients employ an in-memory Terraform state cache as these clients no longer construct Terraform workspaces while reconciling their resources. We have an initial evaluation of the memory cost of this cache with 10k MRs and it's in the order of a few hundred megabytes (~200 MB), much smaller than the footprint of the process fork-based clients.

After the introduction of Framework support in Upjet crossplane/upjet#329, NoFork was renamed to TerraformPluginSDK. crossplane/upjet#338

limitations under the License.
*/

package v1beta1
Copy link
Collaborator Author

@displague displague Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a v1beta1 for ProviderConfig lets us remove the BasePackage overrides.
https://github.com/crossplane-contrib/provider-jet-equinix/pull/59/files#diff-483ebdb1139323e5256859b870054a99a208f4a1d60a0f5e51583c22d8600240L95-L104

I don't know of any other benefits to introducing this.

// LongProvision will set the resource to be provisioned asynchronously. Use this for resources with >1m provisions
func LongProvision() upconfig.ResourceOption {
return func(r *upconfig.Resource) {
r.UseAsync = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a premature optimization. I haven't seen any problem with deploying devices thus far.

@displague displague changed the title WIP: fix provider definition fix provider definition Jul 28, 2024
@displague displague marked this pull request as ready for review July 29, 2024 00:00
@displague displague merged commit 7f585ae into main Jul 29, 2024
7 checks passed
@displague displague deleted the fix-provider-definition branch July 29, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.8.0 can not resolve resources (Invalid provider local name)
1 participant