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 a new output/v1alpha2 API; deprecate output/v1alpha1 #105295
kubeadm: add a new output/v1alpha2 API; deprecate output/v1alpha1 #105295
Conversation
/kind cleanup feature deprecation |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neolit123 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 |
/retest |
/milestone v1.23 |
e61a87a
to
63b710b
Compare
The API is a copy of output/v1alpha1 with a minor difference where output/v1alpha2.BootstrapToken embeds bootstraptoken/v1.BootstrapToken instead of kubeadm/v1beta2.BootstrapToken. Embedding the later is an undesired binding between the "kubeadm" and "output" groups, preventing the eventual deprecation and removal of the kubeadm.v1beta2 API. This new output API version, unlike v1alpha1, does not include defaulting which is not needed.
Use the new API for "config images list" and "token list".
63b710b
to
77edef0
Compare
/hold for review |
Hello @neolit123 👋🏽 |
Yes this is pending review before code freeze.
|
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.
two small nits, otherwise lgtm
// TODO: move SchemeBuilder with zz_generated.deepcopy.go to k8s.io/api. | ||
// localSchemeBuilder and AddToScheme will stay in k8s.io/kubernetes. |
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.
Does this TODO still apply? Are we expected to make changes in k8s.io/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.
i don't think so. changes there are hard to argue about, thus i can just remove this TODO.
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.
added separate commit kubeadm: remove TODO about moving SchemeBuilder to k8s.io/api
for that. also cleaned up this TODO from other API versions.
(i prefer if we keep the commit set here instead of squashing)
@@ -64,3 +67,14 @@ func Convert_v1beta2_JoinConfiguration_To_kubeadm_JoinConfiguration(in *JoinConf | |||
out.NodeRegistration.ImagePullPolicy = corev1.PullIfNotPresent | |||
return nil | |||
} | |||
|
|||
// Convert_v1beta2_BootstrapToken_To_v1_BootstrapToken is required so that |
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.
Comment seems truncated
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.
indeed, now i need to remember what it was about.
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.
updated.
The addition of output/v1alpha2 made the converter-gen require an explicit converter for: kubeadm/v1beta2.BootstrapToken -> bootstraptoken/v1.BootstrapToken. Add this converter under kubeadm/v1beta. Use the converter in output/v1alpha1.
This TODO is no longer in scope, thus remove it from all register.go files under /app/apis.
77edef0
to
d3e1f87
Compare
/lgtm |
/hold cancel
|
What this PR does / why we need it:
The output/v1alpha2 API is a copy of output/v1alpha1 with a minor difference
where output/v1alpha2.BootstrapToken embeds
bootstraptoken/v1.BootstrapToken instead of
kubeadm/v1beta2.BootstrapToken.
Embedding the later is an undesired binding between the "kubeadm"
and "output" groups, preventing the eventual deprecation and removal
of the kubeadm/v1beta2 API.
This new output API version, unlike v1alpha1, does not include
defaulting which is not needed.
Which issue(s) this PR fixes:
xref kubernetes/kubeadm#2427
Special notes for your reviewer:
NONE
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: