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: remove temporary handling of CRI socket paths without URL scheme #109356
Conversation
thanks @pacoxu will double check it again before 1.25 opens for PRs. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neolit123, pacoxu 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 |
i think ...maybe we can go without release note for this one, actually. /release-note-edit
/retitle kubeadm: remove temporary handling of CRI socket paths without URL scheme |
for 1.25 (as part of this PR) we can also add URL scheme check in the e2e test: it should be fine version skew wise - i.e. kubeadm 1.25 being tested with 1.24 k8s e2e test suite 🤔 |
/retest |
LGTM, will double check early next week. |
/priority important-soon |
// TODO: Temporary workaround. Remove in 1.25: | ||
// https://github.com/kubernetes/kubeadm/issues/2426 | ||
if err := UpdateKubeletDynamicEnvFileWithURLScheme(dryRun); err != nil { | ||
return err | ||
} | ||
|
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.
note for the future.
we might want to use a similar function for other mutations in the dynamic env file.
e.g. when we have to do some cleanups of the deprecated kubelet flags such as --container-runtime=remote
kubernetes/kubeadm#2626
#106893
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
/kind cleanup
refer to kubernetes/kubeadm#2426
follow up of #107295
kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go
Lines 120 to 122 in 2118440
The warning should change to an error in the future(maybe 1.26)