Fix minikube start in order to be able to start VM even if machine …

…does not exist (kubernetes#5730)

* fix startHost in order to be able to  start VM even if machine does not exist

* Update func StartHost() and its unit test no to use config.GetMachineName()

* Add handle case when the kic image isn't pulled

cmd/minikube/cmd/start.go

@@ -1091,6 +1091,13 @@ func startHost(api libmachine.API, mc config.MachineConfig) (*host.Host, bool) {
 
 	host, err := cluster.StartHost(api, mc)
 	if err != nil {
+		// If virtual machine does not exist due to user interrupt cancel(i.e. Ctrl + C), initialize exists flag
+		if err == cluster.ErrorMachineNotExist {
+			// If Machine does not exist, of course the machine does not have kubeadm config files
+			// In order not to determine the machine has kubeadm config files, initialize exists flag
+			// ※ If exists flag is true, minikube determines the machine has kubeadm config files
+			return host, false
+		}
 		exit.WithError("Unable to start VM. Please investigate and run 'minikube delete' if possible", err)
 	}
 	return host, exists

pkg/minikube/cluster/cluster_test.go

@@ -146,6 +146,49 @@ func TestStartHostExists(t *testing.T) {
 	}
 }
 
+func TestStartHostErrMachineNotExist(t *testing.T) {
+	RegisterMockDriver(t)
+	api := tests.NewMockAPI(t)
+	// Create an incomplete host with machine does not exist error(i.e. User Interrupt Cancel)
+	api.NotExistError = true
+	h, err := createHost(api, defaultMachineConfig)
+	if err != nil {
+		t.Fatalf("Error creating host: %v", err)
+	}
+
+	md := &tests.MockDetector{Provisioner: &tests.MockProvisioner{}}
+	provision.SetDetector(md)
+
+	mc := defaultMachineConfig
+	mc.Name = h.Name
+
+	// This should pass with creating host, while machine does not exist.
+	h, err = StartHost(api, mc)
+	if err != nil {
+		if err != ErrorMachineNotExist {
+			t.Fatalf("Error starting host: %v", err)
+		}
+	}
+
+	mc.Name = h.Name
+
+	// Second call. This should pass without calling Create because the host exists already.
+	h, err = StartHost(api, mc)
+	if err != nil {
+		t.Fatalf("Error starting host: %v", err)
+	}
+
+	if h.Name != viper.GetString("profile") {
+		t.Fatalf("GetMachineName()=%q, want %q", viper.GetString("profile"), h.Name)
+	}
+	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) {
 	RegisterMockDriver(t)
 	api := tests.NewMockAPI(t)
@@ -309,6 +352,21 @@ func TestDeleteHostErrorDeletingFiles(t *testing.T) {
 	}
 }
 
+func TestDeleteHostErrMachineNotExist(t *testing.T) {
+	RegisterMockDriver(t)
+	api := tests.NewMockAPI(t)
+	// Create an incomplete host with machine does not exist error(i.e. User Interrupt Cancel)
+	api.NotExistError = true
+	_, err := createHost(api, defaultMachineConfig)
+	if err != nil {
+		t.Errorf("createHost failed: %v", err)
+	}
+
+	if err := DeleteHost(api, viper.GetString("profile")); err == nil {
+		t.Fatal("Expected error deleting host.")
+	}
+}
+
 func TestGetHostStatus(t *testing.T) {
 	RegisterMockDriver(t)
 	api := tests.NewMockAPI(t)

pkg/minikube/cluster/fix.go

@@ -23,6 +23,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/docker/machine/drivers/virtualbox"
+
 	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/host"
 	"github.com/docker/machine/libmachine/provision"
@@ -46,6 +48,11 @@ const (
 	maxClockDesyncSeconds = 2.1
 )
 
+var (
+	// ErrorMachineNotExist is returned when virtual machine does not exist due to user interrupt cancel(i.e. Ctrl + C)
+	ErrorMachineNotExist = errors.New("machine does not exist")
+)
+
 // fixHost fixes up a previously configured VM so that it is ready to run Kubernetes
 func fixHost(api libmachine.API, mc config.MachineConfig) (*host.Host, error) {
 	out.T(out.Waiting, "Reconfiguring existing host ...")
@@ -62,8 +69,35 @@ func fixHost(api libmachine.API, mc config.MachineConfig) (*host.Host, error) {
 	}
 
 	s, err := h.Driver.GetState()
-	if err != nil {
-		return h, errors.Wrap(err, "Error getting state for host")
+	if err != nil || s == state.Stopped || s == state.None {
+		// If virtual machine does not exist due to user interrupt cancel(i.e. Ctrl + C), recreate virtual machine
+		me, err := machineExists(h.Driver.DriverName(), s, err)
+		if !me {
+			// If the error is that virtual machine does not exist error, handle error(recreate virtual machine)
+			if err == ErrorMachineNotExist {
+				// remove virtual machine
+				if err := h.Driver.Remove(); err != nil {
+					// skip returning error since it may be before docker image pulling(so, no host exist)
+					if h.Driver.DriverName() != driver.Docker {
+						return nil, errors.Wrap(err, "host remove")
+					}
+				}
+				// remove machine config directory
+				if err := api.Remove(mc.Name); err != nil {
+					return nil, errors.Wrap(err, "api remove")
+				}
+				// recreate virtual machine
+				out.T(out.Meh, "machine '{{.name}}' does not exist. Proceeding ahead with recreating VM.", out.V{"name": mc.Name})
+				h, err = createHost(api, mc)
+				if err != nil {
+					return nil, errors.Wrap(err, "Error recreating VM")
+				}
+				// return ErrMachineNotExist err to initialize preExists flag
+				return h, ErrorMachineNotExist
+			}
+			// If the error is not that virtual machine does not exist error, return error
+			return nil, errors.Wrap(err, "Error getting state for host")
+		}
 	}
 
 	if s == state.Running {
@@ -161,3 +195,66 @@ func adjustGuestClock(h hostRunner, t time.Time) error {
 	glog.Infof("clock set: %s (err=%v)", out, err)
 	return err
 }
+
+// machineExists checks if virtual machine does not exist
+// if the virtual machine exists, return true
+func machineExists(vmDriver string, s state.State, err error) (bool, error) {
+	switch vmDriver {
+	case driver.HyperKit:
+		if s == state.Stopped || err.Error() == "connection is shut down" {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.HyperV:
+		if s == state.None {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.KVM2:
+		if s == state.None || s == state.Stopped {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.None:
+		if s == state.Stopped {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.Parallels:
+		if err.Error() == "machine does not exist" {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.VirtualBox:
+		if err == virtualbox.ErrMachineNotExist {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.VMware:
+		if s == state.None || s == state.Stopped {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.VMwareFusion:
+		if s == state.Stopped {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.Docker:
+		if s == state.Error {
+			// if the kic image is not present on the host machine, when user cancel `minikube start`, state.Error will be return
+			return false, ErrorMachineNotExist
+		} else if s == state.None {
+			// if the kic image is present on the host machine, when user cancel `minikube start`, state.None will be return
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	case driver.Mock:
+		if s == state.Error {
+			return false, ErrorMachineNotExist
+		}
+		return true, err
+	default:
+		return true, err
+	}
+}

pkg/minikube/tests/api_mock.go

@@ -33,10 +33,11 @@ import (
 // MockAPI is a struct used to mock out libmachine.API
 type MockAPI struct {
 	FakeStore
-	CreateError bool
-	RemoveError bool
-	SaveCalled  bool
-	t           *testing.T
+	CreateError   bool
+	RemoveError   bool
+	NotExistError bool
+	SaveCalled    bool
+	t             *testing.T
 }
 
 // NewMockAPI returns a new MockAPI
@@ -109,6 +110,12 @@ func (api *MockAPI) Create(h *host.Host) error {
 	if ok {
 		drv.T = api.t
 	}
+	if api.NotExistError {
+		// initialize api.NotExistError
+		api.NotExistError = false
+		// reproduce ErrMachineNotExist
+		drv.NotExistError = true
+	}
 	return h.Driver.Create()
 }
 

pkg/minikube/tests/driver_mock.go

@@ -29,12 +29,13 @@ import (
 // MockDriver is a struct used to mock out libmachine.Driver
 type MockDriver struct {
 	drivers.BaseDriver
-	CurrentState state.State
-	RemoveError  bool
-	HostError    bool
-	Port         int
-	IP           string
-	T            *testing.T
+	CurrentState  state.State
+	RemoveError   bool
+	NotExistError bool
+	HostError     bool
+	Port          int
+	IP            string
+	T             *testing.T
 }
 
 // Logf logs mock interactions
@@ -49,6 +50,11 @@ func (d *MockDriver) Logf(format string, args ...interface{}) {
 // Create creates a MockDriver instance
 func (d *MockDriver) Create() error {
 	d.Logf("MockDriver.Create")
+	if d.NotExistError {
+		d.Logf("MockDriver.Create but machine does not exist")
+		d.CurrentState = state.Error
+		return nil
+	}
 	d.CurrentState = state.Running
 	return nil
 }
@@ -91,6 +97,11 @@ func (d *MockDriver) GetSSHKeyPath() string {
 // GetState returns the state of the driver
 func (d *MockDriver) GetState() (state.State, error) {
 	d.Logf("MockDriver.GetState: %v", d.CurrentState)
+	if d.NotExistError {
+		d.CurrentState = state.Error
+		// don't use cluster.ErrorMachineNotExist to avoid import cycle
+		return d.CurrentState, errors.New("machine does not exist")
+	}
 	return d.CurrentState, nil
 }