Delete: output underlying status failure, refactor wordy code

cmd/minikube/cmd/config/open.go

@@ -78,7 +78,7 @@ var addonsOpenCmd = &cobra.Command{
 		if err != nil {
 			exit.WithError("Error getting control plane", err)
 		}
-		if !machine.IsHostRunning(api, driver.MachineName(*cc, cp)) {
+		if !machine.IsRunning(api, driver.MachineName(*cc, cp)) {
 			os.Exit(1)
 		}
 		addon, ok := assets.Addons[addonName] // validate addon input

cmd/minikube/cmd/config/profile_list.go

@@ -80,7 +80,7 @@ var printProfilesTable = func() {
 		if err != nil {
 			exit.WithError("error getting primary control plane", err)
 		}
-		p.Status, err = machine.GetHostStatus(api, driver.MachineName(*p.Config, cp))
+		p.Status, err = machine.Status(api, driver.MachineName(*p.Config, cp))
 		if err != nil {
 			glog.Warningf("error getting host status for %s: %v", p.Name, err)
 		}
@@ -121,7 +121,7 @@ var printProfilesJSON = func() {
 		if err != nil {
 			exit.WithError("error getting primary control plane", err)
 		}
-		status, err := machine.GetHostStatus(api, driver.MachineName(*v.Config, cp))
+		status, err := machine.Status(api, driver.MachineName(*v.Config, cp))
 		if err != nil {
 			glog.Warningf("error getting host status for %s: %v", v.Name, err)
 		}

cmd/minikube/cmd/dashboard.go

@@ -109,7 +109,7 @@ var dashboardCmd = &cobra.Command{
 			exit.WithCodeT(exit.NoInput, "kubectl not found in PATH, but is required for the dashboard. Installation guide: https://kubernetes.io/docs/tasks/tools/install-kubectl/")
 		}
 
-		if !machine.IsHostRunning(api, machineName) {
+		if !machine.IsRunning(api, machineName) {
 			os.Exit(1)
 		}
 

cmd/minikube/cmd/delete.go

@@ -174,7 +174,7 @@ func DeleteProfiles(profiles []*config.Profile) []error {
 		err := deleteProfile(profile)
 
 		if err != nil {
-			mm, loadErr := machine.Load(profile.Name)
+			mm, loadErr := machine.LoadMachine(profile.Name)
 
 			if !profile.IsValid() || (loadErr != nil || !mm.IsValid()) {
 				invalidProfileDeletionErrs := deleteInvalidProfile(profile)
@@ -208,7 +208,6 @@ func deleteProfileContainersAndVolumes(name string) {
 
 func deleteProfile(profile *config.Profile) error {
 	viper.Set(config.ProfileName, profile.Name)
-
 	deleteProfileContainersAndVolumes(profile.Name)
 
 	api, err := machine.NewAPIClient()
@@ -217,6 +216,7 @@ func deleteProfile(profile *config.Profile) error {
 		return DeletionError{Err: delErr, Errtype: Fatal}
 	}
 	defer api.Close()
+
 	cc, err := config.Load(profile.Name)
 	if err != nil && !config.IsNotExist(err) {
 		delErr := profileDeletionErr(profile.Name, fmt.Sprintf("error loading profile config: %v", err))
@@ -328,7 +328,7 @@ func uninstallKubernetes(api libmachine.API, cc config.ClusterConfig, n config.N
 		return DeletionError{Err: fmt.Errorf("unable to get bootstrapper: %v", err), Errtype: Fatal}
 	}
 
-	host, err := machine.CheckIfHostExistsAndLoad(api, driver.MachineName(cc, n))
+	host, err := machine.LoadHost(api, driver.MachineName(cc, n))
 	if err != nil {
 		exit.WithError("Error getting host", err)
 	}

cmd/minikube/cmd/docker-env.go

@@ -150,7 +150,7 @@ var dockerEnvCmd = &cobra.Command{
 		}
 		for _, n := range cc.Nodes {
 			machineName := driver.MachineName(*cc, n)
-			host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
+			host, err := machine.LoadHost(api, machineName)
 			if err != nil {
 				exit.WithError("Error getting host", err)
 			}
@@ -158,7 +158,7 @@ var dockerEnvCmd = &cobra.Command{
 				exit.UsageT(`'none' driver does not support 'minikube docker-env' command`)
 			}
 
-			hostSt, err := machine.GetHostStatus(api, machineName)
+			hostSt, err := machine.Status(api, machineName)
 			if err != nil {
 				exit.WithError("Error getting host status", err)
 			}

cmd/minikube/cmd/node_start.go

@@ -45,7 +45,7 @@ var nodeStartCmd = &cobra.Command{
 			exit.WithError("creating api client", err)
 		}
 
-		if machine.IsHostRunning(api, name) {
+		if machine.IsRunning(api, name) {
 			out.T(out.Check, "{{.name}} is already running", out.V{"name": name})
 			os.Exit(0)
 		}

cmd/minikube/cmd/pause.go

@@ -66,7 +66,7 @@ func runPause(cmd *cobra.Command, args []string) {
 	glog.Infof("config: %+v", cc)
 
 	for _, n := range cc.Nodes {
-		host, err := machine.CheckIfHostExistsAndLoad(api, driver.MachineName(*cc, n))
+		host, err := machine.LoadHost(api, driver.MachineName(*cc, n))
 		if err != nil {
 			exit.WithError("Error getting host", err)
 		}

cmd/minikube/cmd/podman-env.go

@@ -121,7 +121,7 @@ var podmanEnvCmd = &cobra.Command{
 		}
 		for _, n := range cc.Nodes {
 			machineName := driver.MachineName(*cc, n)
-			host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
+			host, err := machine.LoadHost(api, machineName)
 			if err != nil {
 				exit.WithError("Error getting host", err)
 			}
@@ -129,7 +129,7 @@ var podmanEnvCmd = &cobra.Command{
 				exit.UsageT(`'none' driver does not support 'minikube podman-env' command`)
 			}
 
-			hostSt, err := machine.GetHostStatus(api, machineName)
+			hostSt, err := machine.Status(api, machineName)
 			if err != nil {
 				exit.WithError("Error getting host status", err)
 			}

cmd/minikube/cmd/service.go

@@ -93,7 +93,7 @@ var serviceCmd = &cobra.Command{
 			exit.WithError("Error getting control plane", err)
 		}
 		machineName := driver.MachineName(*cfg, cp)
-		if !machine.IsHostRunning(api, machineName) {
+		if !machine.IsRunning(api, machineName) {
 			os.Exit(1)
 		}
 

cmd/minikube/cmd/service_list.go

@@ -56,7 +56,7 @@ var serviceListCmd = &cobra.Command{
 		if err != nil {
 			exit.WithError("Error getting primary control plane", err)
 		}
-		if !machine.IsHostRunning(api, driver.MachineName(*cfg, cp)) {
+		if !machine.IsRunning(api, driver.MachineName(*cfg, cp)) {
 			exit.WithCodeT(exit.Unavailable, "profile {{.name}} is not running.", out.V{"name": profileName})
 		}
 		serviceURLs, err := service.GetServiceURLs(api, serviceListNamespace, serviceURLTemplate)

cmd/minikube/cmd/ssh.go

@@ -54,7 +54,7 @@ var sshCmd = &cobra.Command{
 		if err != nil {
 			exit.WithError("Error getting primary control plane", err)
 		}
-		host, err := machine.CheckIfHostExistsAndLoad(api, driver.MachineName(*cc, cp))
+		host, err := machine.LoadHost(api, driver.MachineName(*cc, cp))
 		if err != nil {
 			exit.WithError("Error getting host", err)
 		}

cmd/minikube/cmd/status.go

@@ -154,7 +154,7 @@ func status(api libmachine.API, name string) (*Status, error) {
 		Kubeconfig: Nonexistent,
 	}
 
-	hs, err := machine.GetHostStatus(api, name)
+	hs, err := machine.Status(api, name)
 	glog.Infof("%s host status = %q (err=%v)", name, hs, err)
 	if err != nil {
 		return st, errors.Wrap(err, "host")
@@ -196,7 +196,7 @@ func status(api libmachine.API, name string) (*Status, error) {
 		st.Kubeconfig = Configured
 	}
 
-	host, err := machine.CheckIfHostExistsAndLoad(api, name)
+	host, err := machine.LoadHost(api, name)
 	if err != nil {
 		return st, err
 	}

cmd/minikube/cmd/unpause.go

@@ -58,7 +58,7 @@ var unpauseCmd = &cobra.Command{
 
 		for _, n := range cc.Nodes {
 			machineName := driver.MachineName(*cc, n)
-			host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
+			host, err := machine.LoadHost(api, machineName)
 			if err != nil {
 				exit.WithError("Error getting host", err)
 			}

pkg/addons/addons.go

@@ -145,8 +145,8 @@ func enableOrDisableAddon(cc *config.ClusterConfig, name string, val string) err
 	}
 
 	mName := driver.MachineName(*cc, cp)
-	host, err := machine.CheckIfHostExistsAndLoad(api, mName)
+	host, err := machine.LoadHost(api, mName)
-	if err != nil || !machine.IsHostRunning(api, mName) {
+	if err != nil || !machine.IsRunning(api, mName) {
 		glog.Warningf("%q is not running, writing %s=%v to disk and skipping enablement (err=%v)", mName, addon.Name(), enable, err)
 		return nil
 	}
@@ -247,7 +247,7 @@ func enableOrDisableStorageClasses(cc *config.ClusterConfig, name string, val st
 	if err != nil {
 		return errors.Wrap(err, "getting control plane")
 	}
-	if !machine.IsHostRunning(api, driver.MachineName(*cc, cp)) {
+	if !machine.IsRunning(api, driver.MachineName(*cc, cp)) {
 		glog.Warningf("%q is not running, writing %s=%v to disk and skipping enablement", driver.MachineName(*cc, cp), name, val)
 		return enableOrDisableAddon(cc, name, val)
 	}

pkg/drivers/kic/kic.go

@@ -226,7 +226,7 @@ func (d *Driver) GetState() (state.State, error) {
 	}
 	o := strings.TrimSpace(string(out))
 	if err != nil {
-		return state.Error, errors.Wrapf(err, "get container %s status", d.MachineName)
+		return state.Error, errors.Wrapf(err, "%s: %s", cmd.Args, out)
 	}
 	switch o {
 	case "running":

pkg/minikube/cluster/ip.go

@@ -79,7 +79,7 @@ func GetVMHostIP(host *host.Host) (net.IP, error) {
 
 // GetHostDriverIP gets the ip address of the current minikube cluster
 func GetHostDriverIP(api libmachine.API, machineName string) (net.IP, error) {
-	host, err := machine.CheckIfHostExistsAndLoad(api, machineName)
+	host, err := machine.LoadHost(api, machineName)
 	if err != nil {
 		return nil, err
 	}

pkg/minikube/machine/cache_images.go

@@ -152,7 +152,7 @@ func CacheAndLoadImages(images []string) error {
 		}
 		for _, n := range c.Nodes {
 			m := driver.MachineName(*c, n)
-			status, err := GetHostStatus(api, m)
+			status, err := Status(api, m)
 			if err != nil {
 				glog.Warningf("skipping loading cache for profile %s", pName)
 				glog.Errorf("error getting status for %s: %v", pName, err)

pkg/minikube/machine/cluster_test.go

@@ -374,7 +374,7 @@ func TestDeleteHostErrMachineNotExist(t *testing.T) {
 	}
 }
 
-func TestGetHostStatus(t *testing.T) {
+func TestStatus(t *testing.T) {
 	RegisterMockDriver(t)
 	api := tests.NewMockAPI(t)
 
@@ -384,7 +384,7 @@ func TestGetHostStatus(t *testing.T) {
 	m := driver.MachineName(cc, config.Node{Name: "minikube"})
 
 	checkState := func(expected string, machineName string) {
-		s, err := GetHostStatus(api, machineName)
+		s, err := Status(api, machineName)
 		if err != nil {
 			t.Fatalf("Unexpected error getting status: %v", err)
 		}

pkg/minikube/machine/delete.go

@@ -64,10 +64,10 @@ func DeleteHost(api libmachine.API, machineName string) error {
 	}
 
 	// Get the status of the host. Ensure that it exists before proceeding ahead.
-	status, err := GetHostStatus(api, machineName)
+	status, err := Status(api, machineName)
 	if err != nil {
 		// Warn, but proceed
-		out.WarningT("Unable to get the status of the {{.name}} cluster.", out.V{"name": machineName})
+		out.WarningT(`Unable to get host status for "{{.name}}": {{.error}}`, out.V{"name": machineName, "error": err})
 	}
 
 	if status == state.None.String() {

pkg/minikube/machine/host.go

@@ -24,8 +24,8 @@ import (
 	"github.com/pkg/errors"
 )
 
-// GetHostStatus gets the status of the host VM.
+// Status returns the status of a libmachine host
-func GetHostStatus(api libmachine.API, machineName string) (string, error) {
+func Status(api libmachine.API, machineName string) (string, error) {
 	exists, err := api.Exists(machineName)
 	if err != nil {
 		return "", errors.Wrapf(err, "%s exists", machineName)
@@ -46,9 +46,9 @@ func GetHostStatus(api libmachine.API, machineName string) (string, error) {
 	return s.String(), nil
 }
 
-// IsHostRunning asserts that this profile's primary host is in state "Running"
+// IsRunning asserts that a libmachine host is in state "Running"
-func IsHostRunning(api libmachine.API, name string) bool {
+func IsRunning(api libmachine.API, name string) bool {
-	s, err := GetHostStatus(api, name)
+	s, err := Status(api, name)
 	if err != nil {
 		glog.Warningf("host status for %q returned error: %v", name, err)
 		return false
@@ -60,8 +60,8 @@ func IsHostRunning(api libmachine.API, name string) bool {
 	return true
 }
 
-// CheckIfHostExistsAndLoad checks if a host exists, and loads it if it does
+// LoadHost returns a libmachine host by name
-func CheckIfHostExistsAndLoad(api libmachine.API, machineName string) (*host.Host, error) {
+func LoadHost(api libmachine.API, machineName string) (*host.Host, error) {
 	glog.Infof("Checking if %q exists ...", machineName)
 	exists, err := api.Exists(machineName)
 	if err != nil {
@@ -71,9 +71,9 @@ func CheckIfHostExistsAndLoad(api libmachine.API, machineName string) (*host.Hos
 		return nil, errors.Errorf("machine %q does not exist", machineName)
 	}
 
-	host, err := api.Load(machineName)
+	h, err := api.Load(machineName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "loading machine %q", machineName)
 	}
-	return host, nil
+	return h, nil
 }

pkg/minikube/machine/machine.go

@@ -20,13 +20,8 @@ import (
 	"io/ioutil"
 	"path/filepath"
 
-	"github.com/docker/machine/libmachine"
 	"github.com/docker/machine/libmachine/host"
-	"github.com/docker/machine/libmachine/state"
-	"github.com/golang/glog"
 	"github.com/pkg/errors"
-	"k8s.io/minikube/pkg/minikube/config"
-	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/localpath"
 )
 
@@ -63,37 +58,14 @@ func (h *Machine) IsValid() bool {
 	return true
 }
 
-// List return all valid and invalid machines
+// LoadMachine returns a Machine abstracting a libmachine.Host
-// If a machine is valid or invalid is determined by the cluster.IsValid function
+func LoadMachine(name string) (*Machine, error) {
-func List(miniHome ...string) (validMachines []*Machine, inValidMachines []*Machine, err error) {
-	pDirs, err := machineDirs(miniHome...)
-	if err != nil {
-		return nil, nil, err
-	}
-	for _, n := range pDirs {
-		p, err := Load(n)
-		if err != nil {
-			glog.Infof("%s not valid: %v", n, err)
-			inValidMachines = append(inValidMachines, p)
-			continue
-		}
-		if !p.IsValid() {
-			inValidMachines = append(inValidMachines, p)
-			continue
-		}
-		validMachines = append(validMachines, p)
-	}
-	return validMachines, inValidMachines, nil
-}
-
-// Load loads a machine or throws an error if the machine could not be loaded.
-func Load(name string) (*Machine, error) {
 	api, err := NewAPIClient()
 	if err != nil {
 		return nil, err
 	}
 
-	h, err := CheckIfHostExistsAndLoad(api, name)
+	h, err := LoadHost(api, name)
 	if err != nil {
 		return nil, err
 	}
@@ -104,7 +76,6 @@ func Load(name string) (*Machine, error) {
 	} else {
 		return nil, errors.New("host is nil")
 	}
-
 	return &mm, nil
 }
 
@@ -122,27 +93,3 @@ func machineDirs(miniHome ...string) (dirs []string, err error) {
 	}
 	return dirs, err
 }
-
-// CreateSSHShell creates a new SSH shell / client
-func CreateSSHShell(api libmachine.API, cc config.ClusterConfig, n config.Node, args []string) error {
-	machineName := driver.MachineName(cc, n)
-	host, err := CheckIfHostExistsAndLoad(api, machineName)
-	if err != nil {
-		return errors.Wrap(err, "host exists and load")
-	}
-
-	currentState, err := host.Driver.GetState()
-	if err != nil {
-		return errors.Wrap(err, "state")
-	}
-
-	if currentState != state.Running {
-		return errors.Errorf("%q is not running", machineName)
-	}
-
-	client, err := host.CreateSSHClient()
-	if err != nil {
-		return errors.Wrap(err, "Creating ssh client")
-	}
-	return client.Shell(args...)
-}

pkg/minikube/machine/machine_test.go

@@ -15,61 +15,3 @@ limitations under the License.
 */
 
 package machine
-
-import (
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"testing"
-
-	"github.com/spf13/viper"
-	"k8s.io/minikube/pkg/minikube/config"
-	"k8s.io/minikube/pkg/minikube/localpath"
-)
-
-func TestListMachines(t *testing.T) {
-	const (
-		numberOfValidMachines   = 2
-		numberOfInValidMachines = 3
-		totalNumberOfMachines   = numberOfValidMachines + numberOfInValidMachines
-	)
-
-	viper.Set(config.ProfileName, "")
-
-	testMinikubeDir := "./testdata/list-machines/.minikube"
-	miniDir, err := filepath.Abs(testMinikubeDir)
-
-	if err != nil {
-		t.Errorf("error getting dir path for %s : %v", testMinikubeDir, err)
-	}
-
-	err = os.Setenv(localpath.MinikubeHome, miniDir)
-	if err != nil {
-		t.Errorf("error setting up test environment. could not set %s", localpath.MinikubeHome)
-	}
-
-	files, _ := ioutil.ReadDir(filepath.Join(localpath.MiniPath(), "machines"))
-	numberOfMachineDirs := len(files)
-
-	validMachines, inValidMachines, err := List()
-
-	if err != nil {
-		t.Error(err)
-	}
-
-	if numberOfValidMachines != len(validMachines) {
-		t.Errorf("expected %d valid machines, got %d", numberOfValidMachines, len(validMachines))
-	}
-
-	if numberOfInValidMachines != len(inValidMachines) {
-		t.Errorf("expected %d invalid machines, got %d", numberOfInValidMachines, len(inValidMachines))
-	}
-
-	if totalNumberOfMachines != len(validMachines)+len(inValidMachines) {
-		t.Errorf("expected %d total machines, got %d", totalNumberOfMachines, len(validMachines)+len(inValidMachines))
-	}
-
-	if numberOfMachineDirs != len(validMachines)+len(inValidMachines) {
-		t.Error("expected number of machine directories to be equal to the number of total machines")
-	}
-}

pkg/minikube/machine/runner.go

@@ -0,0 +1,43 @@
+/*
+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 machine
+
+import (
+	"sync"
+
+	"github.com/docker/machine/libmachine/host"
+	"github.com/golang/glog"
+	"k8s.io/minikube/pkg/minikube/command"
+	"k8s.io/minikube/pkg/minikube/drvcmd"
+)
+
+var cachedRunner sync.Map
+
+// Runner returns best available command runner for this host
+func Runner(h *host.Host) (command.Runner, error) {
+	result, ok := cachedRunner.Load(h.Name)
+	if ok {
+		glog.Errorf("NOTE: returning cached runner for %q", h.Name)
+		return result.(command.Runner), nil
+	}
+	cr, err := drvcmd.Runner(h.Driver)
+	if err != nil {
+		return nil, err
+	}
+	cachedRunner.Store(h.Name, cr)
+	return cr, nil
+}

pkg/minikube/machine/ssh.go

@@ -0,0 +1,49 @@
+/*
+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 machine
+
+import (
+	"github.com/docker/machine/libmachine"
+	"github.com/docker/machine/libmachine/state"
+	"github.com/pkg/errors"
+	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/driver"
+)
+
+// CreateSSHShell creates a new SSH shell / client
+func CreateSSHShell(api libmachine.API, cc config.ClusterConfig, n config.Node, args []string) error {
+	machineName := driver.MachineName(cc, n)
+	host, err := LoadHost(api, machineName)
+	if err != nil {
+		return errors.Wrap(err, "host exists and load")
+	}
+
+	currentState, err := host.Driver.GetState()
+	if err != nil {
+		return errors.Wrap(err, "state")
+	}
+
+	if currentState != state.Running {
+		return errors.Errorf("%q is not running", machineName)
+	}
+
+	client, err := host.CreateSSHClient()
+	if err != nil {
+		return errors.Wrap(err, "Creating ssh client")
+	}
+	return client.Shell(args...)
+}

pkg/minikube/service/service.go

@@ -119,7 +119,7 @@ type URLs []SvcURL
 // GetServiceURLs returns a SvcURL object for every service in a particular namespace.
 // Accepts a template for formatting
 func GetServiceURLs(api libmachine.API, namespace string, t *template.Template) (URLs, error) {
-	host, err := machine.CheckIfHostExistsAndLoad(api, viper.GetString(config.ProfileName))
+	host, err := machine.LoadHost(api, viper.GetString(config.ProfileName))
 	if err != nil {
 		return nil, err
 	}
@@ -155,7 +155,7 @@ func GetServiceURLs(api libmachine.API, namespace string, t *template.Template)
 
 // GetServiceURLsForService returns a SvcUrl object for a service in a namespace. Supports optional formatting.
 func GetServiceURLsForService(api libmachine.API, namespace, service string, t *template.Template) (SvcURL, error) {
-	host, err := machine.CheckIfHostExistsAndLoad(api, viper.GetString(config.ProfileName))
+	host, err := machine.LoadHost(api, viper.GetString(config.ProfileName))
 	if err != nil {
 		return SvcURL{}, errors.Wrap(err, "Error checking if api exist and loading it")
 	}

pkg/minikube/tunnel/cluster_inspector.go

@@ -37,7 +37,7 @@ type clusterInspector struct {
 
 func (m *clusterInspector) getStateAndHost() (HostState, *host.Host, error) {
 
-	h, err := machine.CheckIfHostExistsAndLoad(m.machineAPI, m.machineName)
+	h, err := machine.LoadHost(m.machineAPI, m.machineName)
 
 	if err != nil {
 		err = errors.Wrapf(err, "error loading docker-machine host for: %s", m.machineName)