From 0e3d2bc3a1d225937ed18e2dd6e74344f5d8aa27 Mon Sep 17 00:00:00 2001 From: go_vargo <44902466+govargo@users.noreply.github.com> Date: Tue, 11 Feb 2020 03:37:30 +0900 Subject: [PATCH] Fix `minikube start` in order to be able to start VM even if machine does not exist (#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 | 7 ++ pkg/minikube/cluster/cluster_test.go | 58 +++++++++++++++ pkg/minikube/cluster/fix.go | 101 ++++++++++++++++++++++++++- pkg/minikube/tests/api_mock.go | 15 ++-- pkg/minikube/tests/driver_mock.go | 23 ++++-- 5 files changed, 192 insertions(+), 12 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 1b9f72e78db6..fa6d539148b6 100644 --- a/cmd/minikube/cmd/start.go +++ b/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 diff --git a/pkg/minikube/cluster/cluster_test.go b/pkg/minikube/cluster/cluster_test.go index 332f768eba06..4c81ed58b6a6 100644 --- a/pkg/minikube/cluster/cluster_test.go +++ b/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) diff --git a/pkg/minikube/cluster/fix.go b/pkg/minikube/cluster/fix.go index 8ba2dd83c777..061be57c40fe 100644 --- a/pkg/minikube/cluster/fix.go +++ b/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 + } +} diff --git a/pkg/minikube/tests/api_mock.go b/pkg/minikube/tests/api_mock.go index 976d3cdd0ec6..d0d0fdd17e37 100644 --- a/pkg/minikube/tests/api_mock.go +++ b/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() } diff --git a/pkg/minikube/tests/driver_mock.go b/pkg/minikube/tests/driver_mock.go index 1ed8c7d1117f..14d5b2f59de5 100644 --- a/pkg/minikube/tests/driver_mock.go +++ b/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 }