-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
🌱 Add support for compatible contracts to clusterctl #12018
🌱 Add support for compatible contracts to clusterctl #12018
Conversation
GetCompatibleContractVersions = func(contract string) sets.Set[string] { | ||
compatibleContracts := sets.New(contract) | ||
if contract == "v1beta2" { | ||
compatibleContracts.Insert("v1beta1") | ||
} | ||
return compatibleContracts | ||
} |
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 is the only function that we should change in future to drop compatibility with the v1beta1 contract.
Note: Test functions are not using "official" CurrentContractVersion
and GetCompatibleContractVersions
but instead they are injecting a copy of those values so we can keep testing support for compatible contracts also in case there are no compatible contracts for a given CAPI version.
e.g. See variables at the beginning of cmd/clusterctl/client/cluster/installer_test.go
/test help |
@chrischdi: The specified target(s) for
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-e2e-main |
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.
Overall looks good
All good from my side. Thx for the changes, especially the clarifications in the doc @fabriziopandini The linter and test failures only exist after the GitHub action / Prow merged your PR into main. So basically you have to rebase this PR onto main and then you can fix the findings. |
fc8f658
to
de33c61
Compare
@fabriziopandini: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
Thx! /lgtm |
LGTM label has been added. Git tree hash: 353e0b35e38b3dfeac477595d4b981d421f61657
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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:
As documented in "Improving status in CAPI resources" proposal, for the first time CAPI will not force providers to upgrade to the new API contract (v1beta2) immediately.
Instead, we are going to consider providers implementing v1beta1 contract as compatible with v1beta2 and support them until v1beta1 API will be supported (tentatively Aug 26).
This PR makes this possible by introducing in clusterctl this idea of compatible contracts: v1beta1 will be considered compatible with v1beta2.
Note: we need to merge this PR before introducing the new contract/API version, so we can use CAPD - which will not be migrated to v1beta2 immediately - to properly test the compatibility mode
Which issue(s) this PR fixes:
Part of #11947
/area clusterctl