Merge pull request #8035 from priyawadhwa/embed-certs

Wait to add aliases to /etc/hosts before starting kubelet

Author: medyagh

pkg/drivers/kic/kic.go

@@ -295,7 +295,6 @@ func (d *Driver) Restart() error {
 		return fmt.Errorf("start during restart %v", err)
 	}
 	return nil
-
 }
 
 // Start an already created kic container

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -744,8 +744,8 @@ func (k *Bootstrapper) UpdateNode(cfg config.ClusterConfig, n config.Node, r cru
 
 	files := []assets.CopyableFile{
 		assets.NewMemoryAssetTarget(kubeadmCfg, bsutil.KubeadmYamlPath+".new", "0640"),
-		assets.NewMemoryAssetTarget(kubeletCfg, bsutil.KubeletSystemdConfFile+".new", "0644"),
-		assets.NewMemoryAssetTarget(kubeletService, bsutil.KubeletServiceFile+".new", "0644"),
+		assets.NewMemoryAssetTarget(kubeletCfg, bsutil.KubeletSystemdConfFile, "0644"),
+		assets.NewMemoryAssetTarget(kubeletService, bsutil.KubeletServiceFile, "0644"),
 	}
 	// Copy the default CNI config (k8s.conf), so that kubelet can successfully
 	// start a Pod in the case a user hasn't manually installed any CNI plugin
@@ -775,11 +775,7 @@ func (k *Bootstrapper) UpdateNode(cfg config.ClusterConfig, n config.Node, r cru
 		return errors.Wrap(err, "host alias")
 	}
 
-	if err := startKubeletIfRequired(k.c, sm); err != nil {
-		return errors.Wrap(err, "reload")
-	}
-
-	return nil
+	return sm.Start("kubelet")
 }
 
 func copyFiles(runner command.Runner, files []assets.CopyableFile) error {
@@ -801,32 +797,6 @@ func copyFiles(runner command.Runner, files []assets.CopyableFile) error {
 	return nil
 }
 
-func startKubeletIfRequired(runner command.Runner, sm sysinit.Manager) error {
-	now := time.Now()
-	defer func() {
-		glog.Infof("reloadKubelet took %s", time.Since(now))
-	}()
-
-	svc := bsutil.KubeletServiceFile
-	conf := bsutil.KubeletSystemdConfFile
-
-	checkCmd := exec.Command("/bin/bash", "-c", fmt.Sprintf("pgrep kubelet && diff -u %s %s.new && diff -u %s %s.new", svc, svc, conf, conf))
-	if _, err := runner.RunCmd(checkCmd); err == nil {
-		glog.Infof("kubelet is already running with the right configs")
-		return nil
-	}
-
-	startCmd := exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo cp %s.new %s && sudo cp %s.new %s", svc, svc, conf, conf))
-	if _, err := runner.RunCmd(startCmd); err != nil {
-		return errors.Wrap(err, "starting kubelet")
-	}
-
-	if err := sm.Enable("kubelet"); err != nil {
-		return err
-	}
-	return sm.Start("kubelet")
-}
-
 // kubectlPath returns the path to the kubelet
 func kubectlPath(cfg config.ClusterConfig) string {
 	return path.Join(vmpath.GuestPersistentDir, "binaries", cfg.KubernetesConfig.KubernetesVersion, "kubectl")

pkg/minikube/cluster/pause.go

@@ -96,9 +96,6 @@ func unpause(cr cruntime.Manager, r command.Runner, namespaces []string) ([]stri
 	}
 
 	sm := sysinit.New(r)
-	if err := sm.Enable("kubelet"); err != nil {
-		return ids, errors.Wrap(err, "kubelet enable")
-	}
 
 	if err := sm.Start("kubelet"); err != nil {
 		return ids, errors.Wrap(err, "kubelet start")

pkg/minikube/sysinit/systemd.go

@@ -18,6 +18,7 @@ limitations under the License.
 package sysinit
 
 import (
+	"errors"
 	"os/exec"
 
 	"k8s.io/minikube/pkg/minikube/assets"
@@ -53,6 +54,9 @@ func (s *Systemd) Disable(svc string) error {
 
 // Enable enables a service
 func (s *Systemd) Enable(svc string) error {
+	if svc == "kubelet" {
+		return errors.New("please don't enable kubelet as it creates a race condition; if it starts on systemd boot it will pick up /etc/hosts before we have time to configure /etc/hosts")
+	}
 	_, err := s.r.RunCmd(exec.Command("sudo", "systemctl", "enable", svc))
 	return err
 }

pkg/minikube/sysinit/systemd_test.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2020 The Kubernetes Authors All rights reserved.
+
+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 sysinit provides an abstraction over init systems like systemctl
+package sysinit
+
+import (
+	"testing"
+
+	"k8s.io/minikube/pkg/minikube/command"
+)
+
+func TestEnable(t *testing.T) {
+
+	tests := []struct {
+		service   string
+		shouldErr bool
+	}{
+		{
+			service: "docker",
+		}, {
+			service:   "kubelet",
+			shouldErr: true,
+		},
+	}
+	cr := command.NewFakeCommandRunner()
+	cr.SetCommandToOutput(map[string]string{
+		"sudo systemctl enable docker": "",
+	})
+	sd := &Systemd{
+		r: cr,
+	}
+	for _, test := range tests {
+		t.Run(test.service, func(t *testing.T) {
+			err := sd.Enable(test.service)
+			if err == nil && test.shouldErr {
+				t.Fatalf("expected %s service to error, but it did not", test.service)
+			}
+			if err != nil && !test.shouldErr {
+				t.Fatalf("expected %s service to pass, but it did not", test.service)
+			}
+		})
+	}
+}