-
Notifications
You must be signed in to change notification settings - Fork 87
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
🐛 Plugin Init improvements #461
🐛 Plugin Init improvements #461
Conversation
Now we use "latest" tag, but due to recent clusterctl changes it forces to download metadata.yaml file which is not available for CAPI operator. Because of that installation fails with exceeded context deadline. To prevent this we specify a version when we create a repo object, and then pick the latest version from the list of all available versions.
✅ Deploy Preview for kubernetes-sigs-cluster-api-operator ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
cmd/plugin/cmd/init.go
Outdated
@@ -509,6 +513,9 @@ func createGenericProvider(ctx context.Context, client ctrlclient.Client, provid | |||
|
|||
log.Info("Installing provider", "Type", provider.GetType(), "Name", name, "Version", version, "Namespace", namespace) | |||
|
|||
// We have to add a short delay here to ensure that the webhooks are successfully started. | |||
time.Sleep(time.Second * 5) |
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.
are we waiting for CAPI Operator deployment to run?
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.
yes, we are waiting for webhooks to start. Sometimes when we create provider CRs, they are not ready, and therefore init fails.
but I'm going to add retries instead of the sleep.
Provider creation might fail because webhooks are not initialized yet. To prevent this, we add retries.
373502a
to
a3d27be
Compare
We have run into this issue as well trying to install the operator and then immediately trying to create a |
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.
/lgtm
LGTM label has been added. Git tree hash: 62f99005f4578d872bab0f471c32f5a321b66b11
|
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demicev The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
This PR contains two fixes for the plugin Init operation:
Get the latest operator version from the list of all available versions
Now we use "latest" tag, but due to recent clusterctl changes it forces to download metadata.yaml file which is not available for CAPI operator. Because of that installation fails with exceeded context deadline. To prevent this we specify a version when we create a repo object, and then pick the latest version from the list of all available versions.
Add a short delay before deploying providers
We have to add a short delay here to ensure that the webhooks are successfully started.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #436
/area plugin