kubeadm: cleanup command output #79267
Conversation
k8s-ci-robot
added
release-note-none
k8s-ci-robot
added
area/kubeadm
sig/cluster-lifecycle
|
/priority backlog |
k8s-ci-robot
added
priority/backlog
|
/test pull-kubernetes-e2e-gce-device-plugin-gpu |
|
/test pull-kubernetes-bazel-build |
| @@ -68,7 +68,7 @@ var ( | |||
| This command will print out a secure randomly-generated certificate key that can be used with | |||
| the "init" command. | |||
|
|
|||
| You can also use "kubeadm init --experimental-upload-certs" without specifying a certificate key and it will | |||
| You can also use "kubeadm init --upload-certs" without specifying a certificate key and it will | |||
There was a problem hiding this comment.
Wait... This is not the only place for changes.
Since 1.15 is out, ExperimentalUploadCerts can be safely removed in next release and using UploadCerts instead.
There was a problem hiding this comment.
Thanks @dixudx
I'll clean it up later.
There was a problem hiding this comment.
this line should have been fixed for 1.15, but we shouldn't backport non critical fixes, and --experimental-upload-certs still works there.
for 1.16 (master) this is OK.
| @@ -32,7 +32,7 @@ import ( | |||
| var joinCommandTemplate = template.Must(template.New("join").Parse(`` + | |||
| `kubeadm join {{.ControlPlaneHostPort}} --token {{.Token}} \ | |||
| {{range $h := .CAPubKeyPins}}--discovery-token-ca-cert-hash {{$h}} {{end}}{{if .ControlPlane}}\ | |||
| --experimental-control-plane {{if .CertificateKey}}--certificate-key {{.CertificateKey}}{{end}}{{end}}`, | |||
| --control-plane {{if .CertificateKey}}--certificate-key {{.CertificateKey}}{{end}}{{end}}`, | |||
There was a problem hiding this comment.
The same.
Remove ExperimentalControlPlane as well and use UploadCerts instead.
There was a problem hiding this comment.
@dixudx
I looked at the code again and found this
// NewCmdUpgradeControlPlane returns the cobra.Command for upgrading the controlplane instance on this node
// TODO: to remove when 1.16 is released
func NewCmdUpgradeControlPlane() *cobra.Command {
nodeOptions := newNodeOptions()
nodeRunner := workflow.NewRunner()
cmd := &cobra.Command{
Use: "experimental-control-plane",
Short: "Upgrade the control plane instance deployed on this node. IMPORTANT. This command should be executed after executing `kubeadm upgrade apply` on another control plane instance",
Deprecated: "this command is deprecated. Use \"kubeadm upgrade node\" instead",
Run: func(cmd *cobra.Command, args []string) {
err := nodeRunner.Run(args)
kubeadmutil.CheckErr(err)
},
}
...
Is it safe to remove this code now ?
Do we need to wait until 1.16 is released ?
/cc @neolit123 @rosti
There was a problem hiding this comment.
again we missed changing this line in 1.15, but --experimental-control-plane still works there so it's fine.
I looked at the code again and found this
i would leave command changes to a separate PR, as this one is for STDOUT changes.
in a separate PR we can remove all commands/flags that have experimental and should be removed for 1.16 (master).
e.g.
init phase upload-certs --experimental-upload-certs
upgrade experimental-control-plane
join experimental-control-plane
...
just grep for experimental under cmd/kubeadm
There was a problem hiding this comment.
yep, it is safe to remove in 1.16.
|
/test pull-kubernetes-kubemark-e2e-gce-big |
| @@ -262,7 +262,7 @@ func getSecret(client clientset.Interface) (*v1.Secret, error) { | |||
| secret, err := client.CoreV1().Secrets(metav1.NamespaceSystem).Get(kubeadmconstants.KubeadmCertsSecret, metav1.GetOptions{}) | |||
| if err != nil { | |||
| if apierrors.IsNotFound(err) { | |||
| return nil, errors.Errorf("Secret %q was not found in the %q Namespace. This Secret might have expired. Please, run `kubeadm init phase upload-certs --experimental-upload-certs` on a control plane to generate a new one", kubeadmconstants.KubeadmCertsSecret, metav1.NamespaceSystem) | |||
| return nil, errors.Errorf("Secret %q was not found in the %q Namespace. This Secret might have expired. Please, run `kubeadm init phase upload-certs --upload-certs` on a control plane to generate a new one", kubeadmconstants.KubeadmCertsSecret, metav1.NamespaceSystem) | |||
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
the
lgtm
|
/retest Review the full test history for this PR. Silence the bot with an |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
kubeadm: cleanup command output (remove experimental)
Which issue(s) this PR fixes:
Fixes kubernetes/kubeadm#1627
Special notes for your reviewer:
Does this PR introduce a user-facing change?: