Merge pull request #7116 from tstromberg/better-fixhost

Simplify provisioning logic, add fast provisioner detection

cmd/minikube/cmd/start.go

@@ -850,7 +850,7 @@ func validateRegistryMirror() {
 	}
 }
 
-// generateCfgFromFlags generates config.Config based on flags and supplied arguments
+// generateCfgFromFlags generates config.ClusterConfig based on flags and supplied arguments
 func generateCfgFromFlags(cmd *cobra.Command, k8sVersion string, drvName string) (config.ClusterConfig, config.Node, error) {
 	r, err := cruntime.New(cruntime.Config{Type: viper.GetString(containerRuntime)})
 	if err != nil {

go.sum

@@ -61,7 +61,9 @@ github.com/VividCortex/ewma v1.1.1/go.mod h1:2Tkkvm3sRDVXaiyucHiACn4cqf7DpdyLvmx
 github.com/afbjorklund/go-getter v1.4.1-0.20190910175809-eb9f6c26742c h1:18gEt7qzn7CW7qMkfPTFyyotlPbvPQo9o4IDV8jZqP4=
 github.com/afbjorklund/go-getter v1.4.1-0.20190910175809-eb9f6c26742c/go.mod h1:7qxyCd8rBfcShwsvxgIguu4KbS3l8bUCwg2Umn7RjeY=
 github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM=
+github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc h1:cAKDfWh5VpdgMhJosfJnn5/FoN2SRZ4p7fJNX58YPaU=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
+github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf h1:qet1QNfXsQxTZqLG4oE62mJzwPIB8+Tee4RNCL9ulrY=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8=
 github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c=
@@ -972,6 +974,7 @@ google.golang.org/grpc v1.26.0 h1:2dTRdpdFEEhJYQD8EMLB61nnrzSCTbG38PhqdhvOltg=
 google.golang.org/grpc v1.26.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
 gopkg.in/airbrake/gobrake.v2 v2.0.9 h1:7z2uVWwn7oVeeugY1DtlPAy5H+KYgB1KeKTnqjNatLo=
 gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U=
+gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc=
 gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -165,13 +165,6 @@ func (k *Bootstrapper) StartCluster(cfg config.ClusterConfig) error {
 		glog.Infof("StartCluster complete in %s", time.Since(start))
 	}()
 
-	// Remove admin.conf from any previous run
-	c := exec.Command("/bin/bash", "-c", "sudo rm -f /etc/kubernetes/admin.conf")
-	_, err = k.c.RunCmd(c)
-	if err != nil {
-		return errors.Wrap(err, "deleting admin.conf")
-	}
-
 	version, err := util.ParseKubernetesVersion(cfg.KubernetesConfig.KubernetesVersion)
 	if err != nil {
 		return errors.Wrap(err, "parsing kubernetes version")
@@ -209,8 +202,14 @@ func (k *Bootstrapper) StartCluster(cfg config.ClusterConfig) error {
 
 	}
 
+	// Remove the previous kubeadm kubeconfig as the IP may have changed
+	_, err = k.c.RunCmd(exec.Command("sudo", "rm", "-f", "/etc/kubernetes/admin.conf"))
+	if err != nil {
+		return errors.Wrap(err, "deleting admin.conf")
+	}
+
 	conf := bsutil.KubeadmYamlPath
-	c = exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo mv %s.new %s && %s init --config %s %s --ignore-preflight-errors=%s", conf, conf, bsutil.InvokeKubeadm(cfg.KubernetesConfig.KubernetesVersion), conf, extraFlags, strings.Join(ignore, ",")))
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo mv %s.new %s && %s init --config %s %s --ignore-preflight-errors=%s", conf, conf, bsutil.InvokeKubeadm(cfg.KubernetesConfig.KubernetesVersion), conf, extraFlags, strings.Join(ignore, ",")))
 	rr, err := k.c.RunCmd(c)
 	if err != nil {
 		return errors.Wrapf(err, "init failed. output: %q", rr.Output())
@@ -379,6 +378,12 @@ func (k *Bootstrapper) restartCluster(cfg config.ClusterConfig) error {
 		return nil
 	}
 
+	// Remove the previous kubeadm kubeconfig as the IP may have changed
+	_, err = k.c.RunCmd(exec.Command("sudo", "rm", "-f", "/etc/kubernetes/admin.conf"))
+	if err != nil {
+		return errors.Wrap(err, "deleting admin.conf")
+	}
+
 	if _, err := k.c.RunCmd(exec.Command("sudo", "mv", conf+".new", conf)); err != nil {
 		return errors.Wrap(err, "mv")
 	}

pkg/minikube/machine/client.go

@@ -36,11 +36,11 @@ import (
 	"github.com/docker/machine/libmachine/host"
 	"github.com/docker/machine/libmachine/mcnutils"
 	"github.com/docker/machine/libmachine/persist"
-	lib_provision "github.com/docker/machine/libmachine/provision"
 	"github.com/docker/machine/libmachine/ssh"
 	"github.com/docker/machine/libmachine/state"
 	"github.com/docker/machine/libmachine/swarm"
 	"github.com/docker/machine/libmachine/version"
+	"github.com/golang/glog"
 	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/driver"
@@ -49,7 +49,6 @@ import (
 	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/registry"
 	"k8s.io/minikube/pkg/minikube/sshutil"
-	"k8s.io/minikube/pkg/provision"
 )
 
 // NewRPCClient gets a new client.
@@ -167,6 +166,12 @@ func CommandRunner(h *host.Host) (command.Runner, error) {
 
 // Create creates the host
 func (api *LocalClient) Create(h *host.Host) error {
+	glog.Infof("LocalClient.Create starting")
+	start := time.Now()
+	defer func() {
+		glog.Infof("LocalClient.Create took %s", time.Since(start))
+	}()
+
 	def := registry.Driver(h.DriverName)
 	if def.Empty() {
 		return fmt.Errorf("driver %q does not exist", h.DriverName)
@@ -210,21 +215,17 @@ func (api *LocalClient) Create(h *host.Host) error {
 		{
 			"provisioning",
 			func() error {
+				// Skippable because we don't reconfigure Docker?
 				if driver.BareMetal(h.Driver.DriverName()) {
 					return nil
 				}
-				var pv lib_provision.Provisioner
-				if driver.IsKIC(h.Driver.DriverName()) {
-					pv = provision.NewUbuntuProvisioner(h.Driver)
-				} else {
-					pv = provision.NewBuildrootProvisioner(h.Driver)
-				}
-				return pv.Provision(*h.HostOptions.SwarmOptions, *h.HostOptions.AuthOptions, *h.HostOptions.EngineOptions)
+				return provisionDockerMachine(h)
 			},
 		},
 	}
 
 	for _, step := range steps {
+
 		if err := step.f(); err != nil {
 			return errors.Wrap(err, step.name)
 		}

pkg/minikube/machine/cluster_test.go

@@ -140,9 +140,6 @@ func TestStartHostExists(t *testing.T) {
 	if s, _ := h.Driver.GetState(); s != state.Running {
 		t.Fatalf("Machine not started.")
 	}
-	if !md.Provisioner.Provisioned {
-		t.Fatalf("Expected provision to be called")
-	}
 }
 
 func TestStartHostErrMachineNotExist(t *testing.T) {
@@ -188,9 +185,6 @@ func TestStartHostErrMachineNotExist(t *testing.T) {
 	if s, _ := h.Driver.GetState(); s != state.Running {
 		t.Fatalf("Machine not started.")
 	}
-	if !md.Provisioner.Provisioned {
-		t.Fatalf("Expected provision to be called")
-	}
 }
 
 func TestStartStoppedHost(t *testing.T) {
@@ -226,9 +220,6 @@ func TestStartStoppedHost(t *testing.T) {
 		t.Fatalf("Machine must be saved after starting.")
 	}
 
-	if !md.Provisioner.Provisioned {
-		t.Fatalf("Expected provision to be called")
-	}
 }
 
 func TestStartHost(t *testing.T) {

pkg/minikube/machine/fix.go

@@ -27,7 +27,6 @@ import (
 	"github.com/docker/machine/drivers/virtualbox"
 	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/host"
-	"github.com/docker/machine/libmachine/provision"
 	"github.com/docker/machine/libmachine/state"
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
@@ -35,7 +34,6 @@ import (
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/out"
-	"k8s.io/minikube/pkg/util/retry"
 )
 
 // hostRunner is a minimal host.Host based interface for running commands
@@ -77,17 +75,13 @@ func fixHost(api libmachine.API, cc config.ClusterConfig, n config.Node) (*host.
 		return h, err
 	}
 
+	// Technically, we should only have to call provision if Docker has changed,
+	// but who can predict what shape the existing VM is in.
 	e := engineOptions(cc)
-	if len(e.Env) > 0 {
-		h.HostOptions.EngineOptions.Env = e.Env
-		glog.Infof("Detecting provisioner ...")
-		provisioner, err := provision.DetectProvisioner(h.Driver)
-		if err != nil {
-			return h, errors.Wrap(err, "detecting provisioner")
-		}
-		if err := provisioner.Provision(*h.HostOptions.SwarmOptions, *h.HostOptions.AuthOptions, *h.HostOptions.EngineOptions); err != nil {
-			return h, errors.Wrap(err, "provision")
-		}
+	h.HostOptions.EngineOptions.Env = e.Env
+	err = provisionDockerMachine(h)
+	if err != nil {
+		return h, errors.Wrap(err, "provision")
 	}
 
 	if driver.IsMock(h.DriverName) {
@@ -103,10 +97,6 @@ func fixHost(api libmachine.API, cc config.ClusterConfig, n config.Node) (*host.
 		return h, nil
 	}
 
-	glog.Infof("Configuring auth for driver %s ...", h.Driver.DriverName())
-	if err := h.ConfigureAuth(); err != nil {
-		return h, &retry.RetriableError{Err: errors.Wrap(err, "Error configuring auth on host")}
-	}
 	return h, ensureSyncedGuestClock(h, cc.Driver)
 }
 

pkg/minikube/machine/machine.go

@@ -17,8 +17,14 @@ limitations under the License.
 package machine
 
 import (
+	"time"
+
 	"github.com/docker/machine/libmachine/host"
+	libprovision "github.com/docker/machine/libmachine/provision"
+	"github.com/golang/glog"
 	"github.com/pkg/errors"
+	"k8s.io/minikube/pkg/minikube/driver"
+	"k8s.io/minikube/pkg/provision"
 )
 
 // Machine contains information about a machine
@@ -74,3 +80,31 @@ func LoadMachine(name string) (*Machine, error) {
 	}
 	return &mm, nil
 }
+
+// provisionDockerMachine provides fast provisioning of a docker machine
+func provisionDockerMachine(h *host.Host) error {
+	glog.Infof("provisioning docker machine ...")
+	start := time.Now()
+	defer func() {
+		glog.Infof("provisioned docker machine in %s", time.Since(start))
+	}()
+
+	p, err := fastDetectProvisioner(h)
+	if err != nil {
+		return errors.Wrap(err, "fast detect")
+	}
+	return p.Provision(*h.HostOptions.SwarmOptions, *h.HostOptions.AuthOptions, *h.HostOptions.EngineOptions)
+}
+
+// fastDetectProvisioner provides a shortcut for provisioner detection
+func fastDetectProvisioner(h *host.Host) (libprovision.Provisioner, error) {
+	d := h.Driver.DriverName()
+	switch {
+	case driver.IsKIC(d):
+		return provision.NewUbuntuProvisioner(h.Driver), nil
+	case driver.BareMetal(d):
+		return libprovision.DetectProvisioner(h.Driver)
+	default:
+		return provision.NewBuildrootProvisioner(h.Driver), nil
+	}
+}

pkg/minikube/machine/start.go

@@ -32,6 +32,7 @@ import (
 	"github.com/juju/mutex"
 	"github.com/pkg/errors"
 	"github.com/spf13/viper"
+	"golang.org/x/crypto/ssh"
 	"k8s.io/minikube/pkg/drivers/kic/oci"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/config"
@@ -43,6 +44,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/sshutil"
 	"k8s.io/minikube/pkg/minikube/vmpath"
 	"k8s.io/minikube/pkg/util/lock"
+	"k8s.io/minikube/pkg/util/retry"
 )
 
 var (
@@ -195,6 +197,7 @@ func postStartSetup(h *host.Host, mc config.ClusterConfig) error {
 	}
 
 	glog.Infof("creating required directories: %v", requiredDirectories)
+
 	r, err := commandRunner(h)
 	if err != nil {
 		return errors.Wrap(err, "command runner")
@@ -233,11 +236,19 @@ func commandRunner(h *host.Host) (command.Runner, error) {
 	}
 
 	glog.Infof("Creating SSH client and returning SSHRunner for %q driver", d)
-	client, err := sshutil.NewSSHClient(h.Driver)
-	if err != nil {
-		return nil, errors.Wrap(err, "ssh client")
+
+	// Retry in order to survive an ssh restart, which sometimes happens due to provisioning
+	var sc *ssh.Client
+	getSSH := func() (err error) {
+		sc, err = sshutil.NewSSHClient(h.Driver)
+		return err
 	}
-	return command.NewSSHRunner(client), nil
+
+	if err := retry.Expo(getSSH, 250*time.Millisecond, 2*time.Second); err != nil {
+		return nil, err
+	}
+
+	return command.NewSSHRunner(sc), nil
 }
 
 // acquireMachinesLock protects against code that is not parallel-safe (libmachine, cert setup)

pkg/minikube/node/node.go

@@ -17,10 +17,11 @@ limitations under the License.
 package node
 
 import (
-	"errors"
 	"fmt"
 
+	"github.com/pkg/errors"
 	"github.com/spf13/viper"
+
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/machine"
@@ -34,12 +35,11 @@ const (
 
 // Add adds a new node config to an existing cluster.
 func Add(cc *config.ClusterConfig, n config.Node) error {
-
-	err := config.SaveNode(cc, &n)
-	if err != nil {
-		return err
+	if err := config.SaveNode(cc, &n); err != nil {
+		return errors.Wrap(err, "save node")
 	}
 
+	// TODO: Start should return an error rather than calling exit!
 	Start(*cc, n, nil, false)
 	return nil
 }
@@ -48,7 +48,7 @@ func Add(cc *config.ClusterConfig, n config.Node) error {
 func Delete(cc config.ClusterConfig, name string) error {
 	n, index, err := Retrieve(&cc, name)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "retrieve")
 	}
 
 	api, err := machine.NewAPIClient()