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 <dave.chen@arm.com>

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)
-}

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)
 }

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)
 		}
 	}
 }