From 016cc0c12039a1546d72c6aea746cd8204aa31fb Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Mon, 26 Dec 2022 16:01:12 +0800 Subject: [PATCH 1/4] kubeadm:[cherry-pick]backup kubelet config for "upgrade node" This addresses the TODO item so that the old kubelet config file could be recovered if something goes wrong. Signed-off-by: Dave Chen Co-authored-by: Paco Xu --- .../app/cmd/phases/upgrade/node/kubeletconfig.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index d1c66dfd02c..2eb50565203 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go @@ -71,6 +71,21 @@ func runKubeletConfigPhase() func(c workflow.RunData) error { return err } + // Create a copy of the kubelet config file in the /etc/kubernetes/tmp/ folder. + backupDir, err := constants.CreateTempDirForKubeadm(constants.KubernetesDir, "kubeadm-kubelet-config") + if err != nil { + return err + } + src := filepath.Join(kubeletDir, constants.KubeletConfigurationFileName) + dest := filepath.Join(backupDir, constants.KubeletConfigurationFileName) + if !dryRun { + fmt.Printf("[upgrade] Backing up kubelet config file to %s\n", dest) + if err := os.Rename(src, dest); err != nil { + return errors.Wrap(err, "error backing up the kubelet config file") + } + } else { + fmt.Printf("[dryrun] Would back up kubelet config file to %s\n", dest) + } // Store the kubelet component configuration. if err = kubeletphase.WriteConfigToDisk(&cfg.ClusterConfiguration, kubeletDir, data.PatchesDir(), data.OutputWriter()); err != nil { return err From c55a98fde9d0055b33df1b9e1ffa4596358259d7 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Mon, 26 Dec 2022 16:06:15 +0800 Subject: [PATCH 2/4] kubeadm: fix `invalid cross-device link` error The root cause for that error is because `rename` doesn't work across different mount points. The kubelet config file and back up directory are mounted to different file system in kinder environment. ``` df /var/lib/kubelet/config.yaml | tail -n1 | awk '{print $1}' /dev/sda2 df /etc/kubernetes/tmp/kubeadm-kubelet-configxxx | tail -n1 | awk '{print $1}' overlay ``` Call `cp` instead of `rename` to back up the kubelet file would fix that issue. Signed-off-by: Dave Chen --- .../app/cmd/phases/upgrade/node/kubeletconfig.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index 2eb50565203..196e6352f3d 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go @@ -29,6 +29,7 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/constants" kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet" "k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade" + kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun" ) @@ -80,8 +81,13 @@ func runKubeletConfigPhase() func(c workflow.RunData) error { dest := filepath.Join(backupDir, constants.KubeletConfigurationFileName) if !dryRun { fmt.Printf("[upgrade] Backing up kubelet config file to %s\n", dest) - if err := os.Rename(src, dest); err != nil { - return errors.Wrap(err, "error backing up the kubelet config file") + // call `cp` instead of `rename` here since the kubelet config file and back up directory (/etc/kubernetes/tmp/) + // might on the filesystem with differnt mount points in the test environment, such as kinder. + // This will lead to a failure to move the file from the source to dest since `rename` normally doesn't work + // across different mount points on most Unix system. + output, err := kubeadmutil.CopyDir(src, dest) + if err != nil { + return errors.Wrapf(err, "error backing up the kubelet config file, output: %q", output) } } else { fmt.Printf("[dryrun] Would back up kubelet config file to %s\n", dest) From 9e6e13e562e095fbe25f0a1d9d888faf2ca630e6 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Mon, 26 Dec 2022 16:58:59 +0800 Subject: [PATCH 3/4] kubeadm: include the err got from `PrintDryRunFile` The error was ingored which means if anything wrong from `PrintDryRunFiles`, it was sliently ignored. Signed-off-by: Dave Chen --- cmd/kubeadm/app/phases/upgrade/postupgrade.go | 2 +- .../app/phases/upgrade/postupgrade_test.go | 105 ++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index 4422b47a9c2..9484034c3a0 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade.go @@ -166,7 +166,7 @@ func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitCon } if dryRun { // Print what contents would be written - dryrunutil.PrintDryRunFile(kubeadmconstants.KubeletConfigurationFileName, kubeletDir, kubeadmconstants.KubeletRunDirectory, os.Stdout) + errs = append(errs, dryrunutil.PrintDryRunFile(kubeadmconstants.KubeletConfigurationFileName, kubeletDir, kubeadmconstants.KubeletRunDirectory, os.Stdout)) } return errorsutil.NewAggregate(errs) } diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go index e766fc60f48..f316357ba4b 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go @@ -19,12 +19,16 @@ package upgrade import ( "os" "path/filepath" + "regexp" "strings" "testing" "github.com/pkg/errors" + kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" + "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" + "k8s.io/kubernetes/cmd/kubeadm/app/preflight" testutil "k8s.io/kubernetes/cmd/kubeadm/test" ) @@ -101,3 +105,104 @@ func TestRollbackFiles(t *testing.T) { t.Fatalf("Expected error contains %q, got %v", errString, err) } } + +func TestWriteKubeletConfigFiles(t *testing.T) { + // exit early if the user doesn't have root permission as the test needs to create /etc/kubernetes directory + // while the permission should be granted to the user. + isPrivileged := preflight.IsPrivilegedUserCheck{} + if _, err := isPrivileged.Check(); err != nil { + return + } + testCases := []struct { + name string + dryrun bool + patchesDir string + errPattern string + cfg *kubeadmapi.InitConfiguration + }{ + // Be careful that if the dryrun is set to false and the test is run on a live cluster, the kubelet config file might be overwritten. + // However, you should be able to find the original config file in /etc/kubernetes/tmp/kubeadm-kubelet-configxxx folder. + // The test haven't clean up the temporary file created under /etc/kubernetes/tmp/ as that could be accidentally delete other files in + // that folder as well which might be unexpected. + { + name: "write kubelet config file successfully", + dryrun: true, + cfg: &kubeadmapi.InitConfiguration{ + ClusterConfiguration: kubeadmapi.ClusterConfiguration{ + ComponentConfigs: kubeadmapi.ComponentConfigMap{ + componentconfigs.KubeletGroup: &componentConfig{}, + }, + }, + }, + }, + { + name: "aggregate errs: no kubelet config file and cannot read config file", + dryrun: true, + errPattern: "no kubelet component config found.*no such file or directory", + cfg: &kubeadmapi.InitConfiguration{}, + }, + { + name: "only one err: patch dir does not exist", + dryrun: true, + patchesDir: "Bogus", + errPattern: "could not list patch files for path \"Bogus\"", + cfg: &kubeadmapi.InitConfiguration{ + ClusterConfiguration: kubeadmapi.ClusterConfiguration{ + ComponentConfigs: kubeadmapi.ComponentConfigMap{ + componentconfigs.KubeletGroup: &componentConfig{}, + }, + }, + }, + }, + } + for _, tc := range testCases { + err := writeKubeletConfigFiles(nil, tc.cfg, tc.patchesDir, tc.dryrun, os.Stdout) + if err != nil && tc.errPattern != "" { + if match, _ := regexp.MatchString(tc.errPattern, err.Error()); !match { + t.Fatalf("Expected error contains %q, got %v", tc.errPattern, err.Error()) + } + } + if err == nil && len(tc.errPattern) != 0 { + t.Fatalf("writeKubeletConfigFiles didn't return error expected %s", tc.errPattern) + } + } +} + +// Just some stub code, the code could be enriched when necessary. +type componentConfig struct { + userSupplied bool +} + +func (cc *componentConfig) DeepCopy() kubeadmapi.ComponentConfig { + result := &componentConfig{} + return result +} + +func (cc *componentConfig) Marshal() ([]byte, error) { + return nil, nil +} + +func (cc *componentConfig) Unmarshal(docmap kubeadmapi.DocumentMap) error { + return nil +} + +func (cc *componentConfig) Get() interface{} { + return &cc +} + +func (cc *componentConfig) Set(cfg interface{}) { +} + +func (cc *componentConfig) Default(_ *kubeadmapi.ClusterConfiguration, _ *kubeadmapi.APIEndpoint, _ *kubeadmapi.NodeRegistrationOptions) { +} + +func (cc *componentConfig) Mutate() error { + return nil +} + +func (cc *componentConfig) IsUserSupplied() bool { + return false +} +func (cc *componentConfig) SetUserSupplied(userSupplied bool) { + cc.userSupplied = userSupplied +} From 5127cbf94983c2996ea107da28d24e7ad83b120b Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Mon, 26 Dec 2022 17:20:54 +0800 Subject: [PATCH 4/4] kubeadm: backup kubelet config file for `upgrade apply` Back up kubelet config file for `kubeadm upgrade apply`, some code refactoring is done to de-dup some redundant code logic. Signed-off-by: Dave Chen --- .../cmd/phases/upgrade/node/kubeletconfig.go | 59 ++----------------- cmd/kubeadm/app/phases/upgrade/postupgrade.go | 36 ++++++++++- .../app/phases/upgrade/postupgrade_test.go | 4 +- 3 files changed, 39 insertions(+), 60 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index 196e6352f3d..b13cb77906d 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go @@ -18,19 +18,13 @@ package node import ( "fmt" - "os" - "path/filepath" "github.com/pkg/errors" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/options" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow" cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util" - "k8s.io/kubernetes/cmd/kubeadm/app/constants" - kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet" "k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade" - kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" - dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun" ) var ( @@ -66,61 +60,16 @@ func runKubeletConfigPhase() func(c workflow.RunData) error { cfg := data.Cfg() dryRun := data.DryRun() - // Set up the kubelet directory to use. If dry-running, this will return a fake directory - kubeletDir, err := upgrade.GetKubeletDir(dryRun) + // Write the configuration for the kubelet down to disk and print the generated manifests instead if dry-running. + // If not dry-running, the kubelet config file will be backed up to /etc/kubernetes/tmp/ dir, so that it could be + // recovered if there is anything goes wrong. + err := upgrade.WriteKubeletConfigFiles(cfg, data.PatchesDir(), dryRun, data.OutputWriter()) if err != nil { return err } - // Create a copy of the kubelet config file in the /etc/kubernetes/tmp/ folder. - backupDir, err := constants.CreateTempDirForKubeadm(constants.KubernetesDir, "kubeadm-kubelet-config") - if err != nil { - return err - } - src := filepath.Join(kubeletDir, constants.KubeletConfigurationFileName) - dest := filepath.Join(backupDir, constants.KubeletConfigurationFileName) - if !dryRun { - fmt.Printf("[upgrade] Backing up kubelet config file to %s\n", dest) - // call `cp` instead of `rename` here since the kubelet config file and back up directory (/etc/kubernetes/tmp/) - // might on the filesystem with differnt mount points in the test environment, such as kinder. - // This will lead to a failure to move the file from the source to dest since `rename` normally doesn't work - // across different mount points on most Unix system. - output, err := kubeadmutil.CopyDir(src, dest) - if err != nil { - return errors.Wrapf(err, "error backing up the kubelet config file, output: %q", output) - } - } else { - fmt.Printf("[dryrun] Would back up kubelet config file to %s\n", dest) - } - // Store the kubelet component configuration. - if err = kubeletphase.WriteConfigToDisk(&cfg.ClusterConfiguration, kubeletDir, data.PatchesDir(), data.OutputWriter()); err != nil { - return err - } - - // If we're dry-running, print the generated manifests - if dryRun { - if err := printFilesIfDryRunning(dryRun, kubeletDir); err != nil { - return errors.Wrap(err, "error printing files on dryrun") - } - return nil - } - fmt.Println("[upgrade] The configuration for this node was successfully updated!") fmt.Println("[upgrade] Now you should go ahead and upgrade the kubelet package using your package manager.") return nil } } - -// printFilesIfDryRunning prints the Static Pod manifests to stdout and informs about the temporary directory to go and lookup -func printFilesIfDryRunning(dryRun bool, kubeletDir string) error { - if !dryRun { - return nil - } - - // Print the contents of the upgraded file and pretend like they were in kubeadmconstants.KubeletRunDirectory - fileToPrint := dryrunutil.FileToPrint{ - RealPath: filepath.Join(kubeletDir, constants.KubeletConfigurationFileName), - PrintPath: filepath.Join(constants.KubeletRunDirectory, constants.KubeletConfigurationFileName), - } - return dryrunutil.PrintDryRunFiles([]dryrunutil.FileToPrint{fileToPrint}, os.Stdout) -} diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index 9484034c3a0..0ea680df603 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade.go @@ -18,8 +18,10 @@ package upgrade import ( "context" + "fmt" "io" "os" + "path/filepath" "github.com/pkg/errors" @@ -38,6 +40,7 @@ import ( kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet" patchnodephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/patchnode" "k8s.io/kubernetes/cmd/kubeadm/app/phases/uploadconfig" + kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun" ) @@ -59,7 +62,7 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon } // Write the new kubelet config down to disk and the env file if needed - if err := writeKubeletConfigFiles(client, cfg, patchesDir, dryRun, out); err != nil { + if err := WriteKubeletConfigFiles(cfg, patchesDir, dryRun, out); err != nil { errs = append(errs, err) } @@ -153,12 +156,36 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon return errorsutil.NewAggregate(errs) } -func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitConfiguration, patchesDir string, dryRun bool, out io.Writer) error { +func WriteKubeletConfigFiles(cfg *kubeadmapi.InitConfiguration, patchesDir string, dryRun bool, out io.Writer) error { + // Set up the kubelet directory to use. If dry-running, this will return a fake directory kubeletDir, err := GetKubeletDir(dryRun) if err != nil { // The error here should never occur in reality, would only be thrown if /tmp doesn't exist on the machine. return err } + + // Create a copy of the kubelet config file in the /etc/kubernetes/tmp/ folder. + backupDir, err := kubeadmconstants.CreateTempDirForKubeadm(kubeadmconstants.KubernetesDir, "kubeadm-kubelet-config") + if err != nil { + return err + } + src := filepath.Join(kubeletDir, kubeadmconstants.KubeletConfigurationFileName) + dest := filepath.Join(backupDir, kubeadmconstants.KubeletConfigurationFileName) + + if !dryRun { + // call `cp` instead of `rename` here since the kubelet config file and back up directory (/etc/kubernetes/tmp/) + // might on the filesystem with different mount points in the test environment, such as kinder. + // This will lead to a failure to move the file from the source to dest since `rename` normally doesn't work + // across different mount points on most Unix system. + fmt.Printf("[upgrade] Backing up kubelet config file to %s\n", dest) + output, err := kubeadmutil.CopyDir(src, dest) + if err != nil { + return errors.Wrapf(err, "error backing up the kubelet config file, output: %q", output) + } + } else { + fmt.Printf("[dryrun] Would back up kubelet config file to %s\n", dest) + } + errs := []error{} // Write the configuration for the kubelet down to disk so the upgraded kubelet can start with fresh config if err := kubeletphase.WriteConfigToDisk(&cfg.ClusterConfiguration, kubeletDir, patchesDir, out); err != nil { @@ -166,7 +193,10 @@ func writeKubeletConfigFiles(client clientset.Interface, cfg *kubeadmapi.InitCon } if dryRun { // Print what contents would be written - errs = append(errs, dryrunutil.PrintDryRunFile(kubeadmconstants.KubeletConfigurationFileName, kubeletDir, kubeadmconstants.KubeletRunDirectory, os.Stdout)) + err := dryrunutil.PrintDryRunFile(kubeadmconstants.KubeletConfigurationFileName, kubeletDir, kubeadmconstants.KubeletRunDirectory, os.Stdout) + if err != nil { + errs = append(errs, errors.Wrap(err, "error printing files on dryrun")) + } } return errorsutil.NewAggregate(errs) } diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go index f316357ba4b..5754136be77 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go @@ -156,14 +156,14 @@ func TestWriteKubeletConfigFiles(t *testing.T) { }, } for _, tc := range testCases { - err := writeKubeletConfigFiles(nil, tc.cfg, tc.patchesDir, tc.dryrun, os.Stdout) + err := WriteKubeletConfigFiles(tc.cfg, tc.patchesDir, tc.dryrun, os.Stdout) if err != nil && tc.errPattern != "" { if match, _ := regexp.MatchString(tc.errPattern, err.Error()); !match { t.Fatalf("Expected error contains %q, got %v", tc.errPattern, err.Error()) } } if err == nil && len(tc.errPattern) != 0 { - t.Fatalf("writeKubeletConfigFiles didn't return error expected %s", tc.errPattern) + t.Fatalf("WriteKubeletConfigFiles didn't return error expected %s", tc.errPattern) } } }