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: mutate ClusterConfiguration.imageRepository to "registry.k8s.io" #110343
kubeadm: mutate ClusterConfiguration.imageRepository to "registry.k8s.io" #110343
Conversation
/priority important-soon |
[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 |
/lgtm |
Cc @pacoxu |
if err := uploadconfig.MutateImageRepository(cfg, client); err != nil { | ||
return nil, false, 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.
Do we need to make similar changes at these locations as well?
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/cmd/upgrade/diff.go#L121
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/cmd/upgrade/node.go#L142
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.
/hold
Feel free to cancel hold @neolit123
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.
good point @SataQiu
so for "diff" i think we should make the change as "diff" can be executed before "apply". for "node" we can do it too, but the upgrade sequence is always to run "apply" first and only then "node". it can be done just in case for consistency.
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.
thinking more about this, we should mutate the in cluster config only during "apply" and just mutate the local fetched struct copy during "diff" and "node".
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.
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.
/hold cancel
/lgtm
/retest |
….io" If the user runs "kubeadm upgrade apply", kubeadm can download a configuration from the cluster. If the configuration contains the legacy default imageRepository of "k8s.gcr.io", mutate it to the new default of "registry.k8s.io" and update the configuration in the config map. During "upgrade node/diff" download the configuration, mutate the image repository locally, but do not mutate the in-cluster value. That is done only on "apply". This ensures that users are migrated from the old default registry domain.
333e74e
to
1c46686
Compare
kubeadm: mutate ClusterConfiguration.imageRepository to "registry.k8s.io" kubernetes#110343
kubeadm: mutate ClusterConfiguration.imageRepository to "registry.k8s.io" kubernetes#110343
What type of PR is this?
/kind feature
/kind deprecation
What this PR does / why we need it:
If the user runs "kubeadm upgrade apply", kubeadm can download
a configuration from the cluster. If the configuration contains
the legacy default imageRepository of "k8s.gcr.io", mutate it
to the new default of "registry.k8s.io" and update the
configuration in the config map.
During "upgrade node/diff" download the configuration, mutate the
image repository locally, but do not mutate the in-cluster value.
That is done only on "apply".
This ensures that users are migrated from the old default registry
domain.
Which issue(s) this PR fixes:
xref kubernetes/kubeadm#2671
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.: