Merge pull request #35632 from dgoodwin/preflight-conf-fixes

Automatic merge from submit-queue

kubeadm: Stop assuming full ownership of /etc/kubernetes.

Packages may auto-create directories in /etc/kubernetes, and users also
need files such as cloud-config.json to be present and preserved at
their default locations in /etc/kubernetes. As such this modifies
pre-flight checks to only require the absence of the files and
directories we explicitly create in kubeadm.

Reset is similarly modified to not wipe out /etc/kubernetes entirely.
When resetting directories we also now preserve the directory itself,
but delete it's contents.

Also adds tests for reset command logic specifically for /etc/kubernetes
cleanup, to ensure user files are not inadvertently wiped out.

cmd/kubeadm/app/cmd/BUILD

@@ -39,3 +39,11 @@ go_library(
         "//vendor:github.com/spf13/cobra",
     ],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["reset_test.go"],
+    library = "go_default_library",
+    tags = ["automanaged"],
+    deps = ["//cmd/kubeadm/app/preflight:go_default_library"],
+)

cmd/kubeadm/app/cmd/reset.go

@@ -21,6 +21,7 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"path/filepath"
 
 	"github.com/spf13/cobra"
 
@@ -66,6 +67,55 @@ func NewReset(skipPreFlight bool) (*Reset, error) {
 	return &Reset{}, nil
 }
 
+// cleanDir removes everything in a directory, but not the directory itself:
+func cleanDir(path string) {
+	// If the directory doesn't even exist there's nothing to do, and we do
+	// not consider this an error:
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		return
+	}
+
+	d, err := os.Open(path)
+	if err != nil {
+		fmt.Printf("failed to remove directory: [%v]\n", err)
+	}
+	defer d.Close()
+	names, err := d.Readdirnames(-1)
+	if err != nil {
+		fmt.Printf("failed to remove directory: [%v]\n", err)
+	}
+	for _, name := range names {
+		err = os.RemoveAll(filepath.Join(path, name))
+		if err != nil {
+			fmt.Printf("failed to remove directory: [%v]\n", err)
+		}
+	}
+}
+
+// resetConfigDir is used to cleanup the files kubeadm writes in /etc/kubernetes/.
+func resetConfigDir(configDirPath string) {
+	dirsToClean := []string{
+		filepath.Join(configDirPath, "manifests"),
+		filepath.Join(configDirPath, "pki"),
+	}
+	fmt.Printf("Deleting contents of config directories: %v\n", dirsToClean)
+	for _, dir := range dirsToClean {
+		cleanDir(dir)
+	}
+
+	filesToClean := []string{
+		filepath.Join(configDirPath, "admin.conf"),
+		filepath.Join(configDirPath, "kubelet.conf"),
+	}
+	fmt.Printf("Deleting files: %v\n", filesToClean)
+	for _, path := range filesToClean {
+		err := os.RemoveAll(path)
+		if err != nil {
+			fmt.Printf("failed to remove file: [%v]\n", err)
+		}
+	}
+}
+
 // Run reverts any changes made to this host by "kubeadm init" or "kubeadm join".
 func (r *Reset) Run(out io.Writer) error {
 	serviceToStop := "kubelet"
@@ -81,13 +131,12 @@ func (r *Reset) Run(out io.Writer) error {
 	// Don't check for errors here, since umount will return a non-zero exit code if there is no directories to umount
 	exec.Command("sh", "-c", "cat /proc/mounts | awk '{print $2}' | grep '/var/lib/kubelet' | xargs umount").Run()
 
-	dirsToRemove := []string{"/var/lib/kubelet", "/var/lib/etcd", "/etc/kubernetes"}
+	resetConfigDir("/etc/kubernetes/")
-	fmt.Printf("Deleting the stateful directories: %v\n", dirsToRemove)
+
-	for _, dir := range dirsToRemove {
+	dirsToClean := []string{"/var/lib/kubelet", "/var/lib/etcd"}
-		err := os.RemoveAll(dir)
+	fmt.Printf("Deleting contents of stateful directories: %v\n", dirsToClean)
-		if err != nil {
+	for _, dir := range dirsToClean {
-			fmt.Printf("failed to remove directory: [%v]\n", err)
+		cleanDir(dir)
-		}
 	}
 
 	dockerCheck := preflight.ServiceCheck{Service: "docker"}

cmd/kubeadm/app/cmd/reset_test.go

@@ -0,0 +1,182 @@
+/*
+Copyright 2014 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cmd
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
+)
+
+func assertExists(t *testing.T, path string) {
+	if _, err := os.Stat(path); os.IsNotExist(err) {
+		t.Errorf("file/dir does not exist error: %s", err)
+		t.Errorf("file/dir does not exist: %s", path)
+	}
+}
+
+func assertNotExists(t *testing.T, path string) {
+	if _, err := os.Stat(path); err == nil {
+		t.Errorf("file/dir exists: %s", path)
+	}
+}
+
+// assertDirEmpty verifies a directory either does not exist, or is empty.
+func assertDirEmpty(t *testing.T, path string) {
+	dac := preflight.DirAvailableCheck{Path: path}
+	_, errors := dac.Check()
+	if len(errors) != 0 {
+		t.Errorf("directory not empty: [%v]", errors)
+	}
+}
+
+func TestConfigDirCleaner(t *testing.T) {
+	tests := map[string]struct {
+		setupDirs       []string
+		setupFiles      []string
+		verifyExists    []string
+		verifyNotExists []string
+	}{
+		"simple reset": {
+			setupDirs: []string{
+				"manifests",
+				"pki",
+			},
+			setupFiles: []string{
+				"manifests/etcd.json",
+				"manifests/kube-apiserver.json",
+				"pki/ca.pem",
+				"admin.conf",
+				"kubelet.conf",
+			},
+			verifyExists: []string{
+				"manifests",
+				"pki",
+			},
+		},
+		"partial reset": {
+			setupDirs: []string{
+				"pki",
+			},
+			setupFiles: []string{
+				"pki/ca.pem",
+				"kubelet.conf",
+			},
+			verifyExists: []string{
+				"pki",
+			},
+			verifyNotExists: []string{
+				"manifests",
+			},
+		},
+		"preserve cloud-config.json": {
+			setupDirs: []string{
+				"manifests",
+				"pki",
+			},
+			setupFiles: []string{
+				"manifests/etcd.json",
+				"manifests/kube-apiserver.json",
+				"pki/ca.pem",
+				"admin.conf",
+				"kubelet.conf",
+				"cloud-config.json",
+			},
+			verifyExists: []string{
+				"manifests",
+				"pki",
+				"cloud-config.json",
+			},
+		},
+		"preserve hidden files and directories": {
+			setupDirs: []string{
+				"manifests",
+				"pki",
+				".mydir",
+			},
+			setupFiles: []string{
+				"manifests/etcd.json",
+				"manifests/kube-apiserver.json",
+				"pki/ca.pem",
+				"admin.conf",
+				"kubelet.conf",
+				".cloud-config.json",
+				".mydir/.myfile",
+			},
+			verifyExists: []string{
+				"manifests",
+				"pki",
+				".cloud-config.json",
+				".mydir",
+				".mydir/.myfile",
+			},
+		},
+		"no-op reset": {
+			verifyNotExists: []string{
+				"pki",
+				"manifests",
+			},
+		},
+	}
+
+	for name, test := range tests {
+		t.Logf("Running test: %s", name)
+
+		// Create a temporary directory for our fake config dir:
+		tmpDir, err := ioutil.TempDir("", "kubeadm-reset-test")
+		if err != nil {
+			t.Errorf("Unable to create temp directory: %s", err)
+		}
+		defer os.RemoveAll(tmpDir)
+
+		for _, createDir := range test.setupDirs {
+			err := os.Mkdir(filepath.Join(tmpDir, createDir), 0700)
+			if err != nil {
+				t.Errorf("Unable to setup test config directory: %s", err)
+			}
+		}
+
+		for _, createFile := range test.setupFiles {
+			fullPath := filepath.Join(tmpDir, createFile)
+			f, err := os.Create(fullPath)
+			defer f.Close()
+			if err != nil {
+				t.Errorf("Unable to create test file: %s", err)
+			}
+		}
+
+		resetConfigDir(tmpDir)
+
+		// Verify the files we cleanup implicitly in every test:
+		assertExists(t, tmpDir)
+		assertNotExists(t, filepath.Join(tmpDir, "admin.conf"))
+		assertNotExists(t, filepath.Join(tmpDir, "kubelet.conf"))
+		assertDirEmpty(t, filepath.Join(tmpDir, "manifests"))
+		assertDirEmpty(t, filepath.Join(tmpDir, "pki"))
+
+		// Verify the files as requested by the test:
+		for _, path := range test.verifyExists {
+			assertExists(t, filepath.Join(tmpDir, path))
+		}
+		for _, path := range test.verifyNotExists {
+			assertNotExists(t, filepath.Join(tmpDir, path))
+		}
+	}
+}

cmd/kubeadm/app/preflight/checks.go

@@ -113,31 +113,44 @@ func (irc IsRootCheck) Check() (warnings, errors []error) {
 // DirAvailableCheck checks if the given directory either does not exist, or
 // is empty.
 type DirAvailableCheck struct {
-	path string
+	Path string
 }
 
 func (dac DirAvailableCheck) Check() (warnings, errors []error) {
 	errors = []error{}
 	// If it doesn't exist we are good:
-	if _, err := os.Stat(dac.path); os.IsNotExist(err) {
+	if _, err := os.Stat(dac.Path); os.IsNotExist(err) {
 		return nil, nil
 	}
 
-	f, err := os.Open(dac.path)
+	f, err := os.Open(dac.Path)
 	if err != nil {
-		errors = append(errors, fmt.Errorf("unable to check if %s is empty: %s", dac.path, err))
+		errors = append(errors, fmt.Errorf("unable to check if %s is empty: %s", dac.Path, err))
 		return nil, errors
 	}
 	defer f.Close()
 
 	_, err = f.Readdirnames(1)
 	if err != io.EOF {
-		errors = append(errors, fmt.Errorf("%s is not empty", dac.path))
+		errors = append(errors, fmt.Errorf("%s is not empty", dac.Path))
 	}
 
 	return nil, errors
 }
 
+// FileAvailableCheck checks that the given file does not already exist.
+type FileAvailableCheck struct {
+	Path string
+}
+
+func (fac FileAvailableCheck) Check() (warnings, errors []error) {
+	errors = []error{}
+	if _, err := os.Stat(fac.Path); err == nil {
+		errors = append(errors, fmt.Errorf("%s already exists", fac.Path))
+	}
+	return nil, errors
+}
+
 // InPathChecks checks if the given executable is present in the path.
 type InPathCheck struct {
 	executable string
@@ -170,9 +183,12 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 		PortOpenCheck{port: 10250},
 		PortOpenCheck{port: 10251},
 		PortOpenCheck{port: 10252},
-		DirAvailableCheck{path: "/etc/kubernetes"},
+		DirAvailableCheck{Path: "/etc/kubernetes/manifests"},
-		DirAvailableCheck{path: "/var/lib/etcd"},
+		DirAvailableCheck{Path: "/etc/kubernetes/pki"},
-		DirAvailableCheck{path: "/var/lib/kubelet"},
+		DirAvailableCheck{Path: "/var/lib/etcd"},
+		DirAvailableCheck{Path: "/var/lib/kubelet"},
+		FileAvailableCheck{Path: "/etc/kubernetes/admin.conf"},
+		FileAvailableCheck{Path: "/etc/kubernetes/kubelet.conf"},
 		InPathCheck{executable: "ebtables", mandatory: true},
 		InPathCheck{executable: "ethtool", mandatory: true},
 		InPathCheck{executable: "ip", mandatory: true},
@@ -194,8 +210,8 @@ func RunJoinNodeChecks() error {
 		ServiceCheck{Service: "docker"},
 		ServiceCheck{Service: "kubelet"},
 		PortOpenCheck{port: 10250},
-		DirAvailableCheck{path: "/etc/kubernetes"},
+		DirAvailableCheck{Path: "/etc/kubernetes"},
-		DirAvailableCheck{path: "/var/lib/kubelet"},
+		DirAvailableCheck{Path: "/var/lib/kubelet"},
 		InPathCheck{executable: "ebtables", mandatory: true},
 		InPathCheck{executable: "ethtool", mandatory: true},
 		InPathCheck{executable: "ip", mandatory: true},