-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
kubeadm: add v1beta4 to scheme; add --allow-experimental-api flag #118866
kubeadm: add v1beta4 to scheme; add --allow-experimental-api flag #118866
Conversation
The highest priority is still v1beta3.
Add the flag --allow-experimental-api to the "config migrate" and "config validate" commands. The flag allows validating / migrating-to a unreleased / experimental API version. Add a new experimentalAPIVersions map in validateSupportedVersion() that contains v1beta4.
9bb802d
to
f04484f
Compare
utilruntime.Must(scheme.SetVersionPriority(v1beta3.SchemeGroupVersion)) | ||
utilruntime.Must(v1beta4.AddToScheme(scheme)) | ||
// TODO: https://github.com/kubernetes/kubeadm/issues/2890 | ||
// make v1beta4 highest priority |
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 tracked in the tasklist of #118866
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.
what happens now is that kubeadm still uses v1beta3 by default, until we adjust the priority here.
experimentalAPIVersions := map[string]string{ | ||
// TODO: https://github.com/kubernetes/kubeadm/issues/2890 | ||
// remove this from experimental once v1beta4 is released | ||
"kubeadm.k8s.io/v1beta4": "v1.28", |
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.
task tracked in #118866
gv: schema.GroupVersion{ | ||
Group: KubeadmGroupName, | ||
Version: "v1beta4", | ||
}, | ||
allowExperimental: true, | ||
}, | ||
{ | ||
gv: schema.GroupVersion{ | ||
Group: KubeadmGroupName, | ||
Version: "v1beta4", | ||
}, | ||
expectedErr: 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.
there are no /cmd/kubeadm/test integration tests currently, only unit tests.
integration tests are OK, but effectively it would only be testing if the --allow-experimental-api (BOOL CLI flag) is working.
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.
we need to see what can be done with the migration unit test
restore TestMigrateOldConfigFromFile in common_test.go once v1beta4 is added. See how we can make this test optional if only one API version is available (e.g. in the future only v1beta4 is avaiable and v1beta3 is removed)
@@ -142,4 +142,7 @@ const ( | |||
|
|||
// CleanupTmpDir flag indicates whether reset will cleanup the tmp dir | |||
CleanupTmpDir = "cleanup-tmp-dir" | |||
|
|||
// AllowExperimentalAPI flag can be used to allow experimental / work in progress APIs | |||
AllowExperimentalAPI = "allow-experimental-api" |
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.
flag naming is similar to upgrade apply, where we have --allow-foo flags
unrelated pkg/controller flake |
/assign |
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.
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
/approve
LGTM label has been added. Git tree hash: a8824ab07c03db5672aab1c899f211a4677df478
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chendave, neolit123, SataQiu 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 type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
xrefs kubernetes/kubeadm#2890
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: