From 37b1ae42c001a2f40441f5bc1e22fd5b26069d72 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Tue, 25 Oct 2016 15:36:43 -0300 Subject: [PATCH 1/2] 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 | 8 ++ cmd/kubeadm/app/cmd/reset.go | 36 +++++- cmd/kubeadm/app/cmd/reset_test.go | 168 ++++++++++++++++++++++++++++ cmd/kubeadm/app/preflight/checks.go | 36 ++++-- 4 files changed, 235 insertions(+), 13 deletions(-) create mode 100644 cmd/kubeadm/app/cmd/reset_test.go diff --git a/cmd/kubeadm/app/cmd/BUILD b/cmd/kubeadm/app/cmd/BUILD index db3704fd75c..2edf2c923e4 100644 --- a/cmd/kubeadm/app/cmd/BUILD +++ b/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"], +) diff --git a/cmd/kubeadm/app/cmd/reset.go b/cmd/kubeadm/app/cmd/reset.go index 8706eff9af0..04d9e175b48 100644 --- a/cmd/kubeadm/app/cmd/reset.go +++ b/cmd/kubeadm/app/cmd/reset.go @@ -21,6 +21,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "github.com/spf13/cobra" @@ -66,6 +67,33 @@ func NewReset(skipPreFlight bool) (*Reset, error) { return &Reset{}, nil } +// 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 config directories: %v\n", dirsToClean) + for _, dir := range dirsToClean { + err := os.RemoveAll(dir) + if err != nil { + fmt.Printf("failed to remove directory: [%v]\n", err) + } + } + + 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,9 +109,11 @@ 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"} - fmt.Printf("Deleting the stateful directories: %v\n", dirsToRemove) - for _, dir := range dirsToRemove { + resetConfigDir("/etc/kubernetes/") + + dirsToClean := []string{"/var/lib/kubelet", "/var/lib/etcd"} + fmt.Printf("Deleting stateful directories: %v\n", dirsToClean) + for _, dir := range dirsToClean { err := os.RemoveAll(dir) if err != nil { fmt.Printf("failed to remove directory: [%v]\n", err) diff --git a/cmd/kubeadm/app/cmd/reset_test.go b/cmd/kubeadm/app/cmd/reset_test.go new file mode 100644 index 00000000000..7113cb460af --- /dev/null +++ b/cmd/kubeadm/app/cmd/reset_test.go @@ -0,0 +1,168 @@ +/* +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", + }, + }, + "partial reset": { + setupDirs: []string{ + "pki", + }, + setupFiles: []string{ + "pki/ca.pem", + "kubelet.conf", + }, + }, + "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{ + "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{ + ".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")) + assertNotExists(t, filepath.Join(tmpDir, "manifests")) + assertNotExists(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)) + } + } +} diff --git a/cmd/kubeadm/app/preflight/checks.go b/cmd/kubeadm/app/preflight/checks.go index 0caac9fdd44..b7802af3072 100644 --- a/cmd/kubeadm/app/preflight/checks.go +++ b/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: "/var/lib/etcd"}, - DirAvailableCheck{path: "/var/lib/kubelet"}, + DirAvailableCheck{Path: "/etc/kubernetes/manifests"}, + DirAvailableCheck{Path: "/etc/kubernetes/pki"}, + 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: "/var/lib/kubelet"}, + DirAvailableCheck{Path: "/etc/kubernetes"}, + DirAvailableCheck{Path: "/var/lib/kubelet"}, InPathCheck{executable: "ebtables", mandatory: true}, InPathCheck{executable: "ethtool", mandatory: true}, InPathCheck{executable: "ip", mandatory: true}, From 2ee787c5837561dc35d18bbf9a5f02d9f86e0053 Mon Sep 17 00:00:00 2001 From: Devan Goodwin Date: Thu, 27 Oct 2016 10:26:26 -0300 Subject: [PATCH 2/2] kubeadm: Empty directories during reset, but do not delete them. This will allow packages to maintain ownership of config and data directories, which may carry selinux or other attributes that should be preserved, but we do not wish to manage within kubeadm itself. --- cmd/kubeadm/app/cmd/reset.go | 39 +++++++++++++++++++++++-------- cmd/kubeadm/app/cmd/reset_test.go | 18 ++++++++++++-- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/cmd/kubeadm/app/cmd/reset.go b/cmd/kubeadm/app/cmd/reset.go index 04d9e175b48..a44afbefd9d 100644 --- a/cmd/kubeadm/app/cmd/reset.go +++ b/cmd/kubeadm/app/cmd/reset.go @@ -67,18 +67,40 @@ 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 config directories: %v\n", dirsToClean) + fmt.Printf("Deleting contents of config directories: %v\n", dirsToClean) for _, dir := range dirsToClean { - err := os.RemoveAll(dir) - if err != nil { - fmt.Printf("failed to remove directory: [%v]\n", err) - } + cleanDir(dir) } filesToClean := []string{ @@ -112,12 +134,9 @@ func (r *Reset) Run(out io.Writer) error { resetConfigDir("/etc/kubernetes/") dirsToClean := []string{"/var/lib/kubelet", "/var/lib/etcd"} - fmt.Printf("Deleting stateful directories: %v\n", dirsToClean) + fmt.Printf("Deleting contents of stateful directories: %v\n", dirsToClean) for _, dir := range dirsToClean { - err := os.RemoveAll(dir) - if err != nil { - fmt.Printf("failed to remove directory: [%v]\n", err) - } + cleanDir(dir) } dockerCheck := preflight.ServiceCheck{Service: "docker"} diff --git a/cmd/kubeadm/app/cmd/reset_test.go b/cmd/kubeadm/app/cmd/reset_test.go index 7113cb460af..d9355e51935 100644 --- a/cmd/kubeadm/app/cmd/reset_test.go +++ b/cmd/kubeadm/app/cmd/reset_test.go @@ -66,6 +66,10 @@ func TestConfigDirCleaner(t *testing.T) { "admin.conf", "kubelet.conf", }, + verifyExists: []string{ + "manifests", + "pki", + }, }, "partial reset": { setupDirs: []string{ @@ -75,6 +79,12 @@ func TestConfigDirCleaner(t *testing.T) { "pki/ca.pem", "kubelet.conf", }, + verifyExists: []string{ + "pki", + }, + verifyNotExists: []string{ + "manifests", + }, }, "preserve cloud-config.json": { setupDirs: []string{ @@ -90,6 +100,8 @@ func TestConfigDirCleaner(t *testing.T) { "cloud-config.json", }, verifyExists: []string{ + "manifests", + "pki", "cloud-config.json", }, }, @@ -109,6 +121,8 @@ func TestConfigDirCleaner(t *testing.T) { ".mydir/.myfile", }, verifyExists: []string{ + "manifests", + "pki", ".cloud-config.json", ".mydir", ".mydir/.myfile", @@ -154,8 +168,8 @@ func TestConfigDirCleaner(t *testing.T) { assertExists(t, tmpDir) assertNotExists(t, filepath.Join(tmpDir, "admin.conf")) assertNotExists(t, filepath.Join(tmpDir, "kubelet.conf")) - assertNotExists(t, filepath.Join(tmpDir, "manifests")) - assertNotExists(t, filepath.Join(tmpDir, "pki")) + 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 {