-
Notifications
You must be signed in to change notification settings - Fork 89
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 unknown provider errors when using fetchConfigs #730
🐛: fix unknown provider errors when using fetchConfigs #730
Conversation
Welcome @w21froster! |
Hi @w21froster. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
✅ Deploy Preview for kubernetes-sigs-cluster-api-operator ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
027aa16
to
1ea2724
Compare
/ok-to-test |
fb16c02
to
6879721
Compare
/retest |
/test? |
@furkatgofurov7: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/test pull-cluster-api-operator-e2e-main |
This PR is ready for a final review whenever you folks have a chance, let me know if there is anything else I can fix / update here. I appreciate your help! 🙏 |
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: furkatgofurov7 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 |
chore: refactor loadCustomProviders
Co-authored-by: Furkat Gofurov <furkat.gofurov@suse.com>
5aa6a2b
to
b98394f
Compare
Hey folks, sorry for the bump, anything else needed from me on this PR to get the LGTM? I went ahead and rebased off main. Thank you 🙇 |
This is a huge patch, however, it is due to manifests included and changes are inside the /lgtm |
LGTM label has been added. Git tree hash: 68418b1740f8415fc9c29d109ac3f6358b3c423f
|
Thanks @w21froster working on the fix, appreciated 🥇 |
What this PR does / why we need it:
When using
fetchConfig
in the provider spec for multiple providers, the operator does not properly pass information about other "custom" providers to the clusterctl configuration. Currently, the operator passes repository information about the current provider only, and the default list fromclusterctl
. This causesclusterctl
to fail it's lagging provider check, which is intended to keep providers within a valid version skew to avoid a broken cluster, and prevents upgrading providers when usingfetchConfig
in multiple places.This PR adds information about other providers that are using
fetchConfig
to the MemoryReader, so that clusterctl can properly pass this check.Which issue(s) this PR fixes:
Fixes #570