-
Notifications
You must be signed in to change notification settings - Fork 4
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
Conversation
48225c8
to
3ed6f97
Compare
return errors.Errorf("failed to configure the provider: %v", diag) | ||
} | ||
|
||
fwProvider := equinixprovider.CreateFrameworkProvider(version.ProviderVersion) |
There was a problem hiding this comment.
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]>
3ed6f97
to
43b8b8e
Compare
Currently failing repeatedly for all resources (as seen with
|
Signed-off-by: Marques Johansson <[email protected]>
internal/clients/equinix.go
Outdated
} | ||
} | ||
|
||
func configureNoForkEquinixClient(ctx context.Context, ps *terraform.Setup, p schema.Provider) error { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
Signed-off-by: Marques Johansson <[email protected]>
…nd retry settings Signed-off-by: Marques Johansson <[email protected]>
… provider definition values Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
Signed-off-by: Marques Johansson <[email protected]>
Description of your changes
Fixes #56
reproducing the stack traces is possible with minimal setup (and no real account/creds needed):
# from a checkout of this branch tilt up
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:
make reviewable test
to ensure this PR is ready for review.How has this code been tested