diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index d1c66dfd02c..b13cb77906d 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go @@ -18,18 +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" - dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun" ) var ( @@ -65,41 +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 } - // 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 4422b47a9c2..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 - 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 e766fc60f48..5754136be77 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(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 +}