Changed command runner

Fixed test

Print error message

Change minikube running check to after check if exists

Updates Set and adds test for disabling

Fix action for minikube not running

Fix import order

cmd/minikube/cmd/config/disable_test.go

@@ -16,10 +16,24 @@ limitations under the License.
 
 package config
 
-import "testing"
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	pkgConfig "k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/localpath"
+)
 
 func TestDisableUnknownAddon(t *testing.T) {
 	if err := Set("InvalidAddon", "false"); err == nil {
 		t.Fatalf("Disable did not return error for unknown addon")
 	}
 }
+
+func TestDisableAddon(t *testing.T) {
+	if err := Set("dashboard", "false"); err != nil {
+		t.Fatalf("Disable returned unexpected error: " + err.Error())
+	}
+	config, _ := pkgConfig.ReadConfig(localpath.ConfigFile)
+	assert.Equal(t, config["dashboard"], false)
+}

cmd/minikube/cmd/config/enable_test.go

@@ -16,10 +16,24 @@ limitations under the License.
 
 package config
 
-import "testing"
+import (
+	"testing"
+
+	"gotest.tools/assert"
+	pkgConfig "k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/localpath"
+)
 
 func TestEnableUnknownAddon(t *testing.T) {
 	if err := Set("InvalidAddon", "false"); err == nil {
 		t.Fatalf("Enable did not return error for unknown addon")
 	}
 }
+
+func TestEnableAddon(t *testing.T) {
+	if err := Set("ingress", "true"); err != nil {
+		t.Fatalf("Enable returned unexpected error: " + err.Error())
+	}
+	config, _ := pkgConfig.ReadConfig(localpath.ConfigFile)
+	assert.Equal(t, config["ingress"], true)
+}

cmd/minikube/cmd/config/open.go

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"os"
 	"text/template"
 
 	"github.com/spf13/cobra"
@@ -62,7 +63,9 @@ var addonsOpenCmd = &cobra.Command{
 		}
 		defer api.Close()
 
-		cluster.EnsureMinikubeRunningOrExit(api, 1)
+		if !cluster.IsMinikubeRunning(api) {
+			os.Exit(1)
+		}
 		addon, ok := assets.Addons[addonName] // validate addon input
 		if !ok {
 			exit.WithCodeT(exit.Data, `addon '{{.name}}' is not a valid addon packaged with minikube.

cmd/minikube/cmd/config/util.go

@@ -111,6 +111,18 @@ func EnableOrDisableAddon(name string, val string) error {
 	if err != nil {
 		return errors.Wrapf(err, "parsing bool: %s", name)
 	}
+	addon := assets.Addons[name]
+
+	// check addon status before enabling/disabling it
+	alreadySet, err := isAddonAlreadySet(addon, enable)
+	if err != nil {
+		out.ErrT(out.Conflict, "{{.error}}", out.V{"error": err})
+		return err
+	}
+	//if addon is already enabled or disabled, do nothing
+	if alreadySet {
+		return nil
+	}
 
 	// TODO(r2d4): config package should not reference API, pull this out
 	api, err := machine.NewAPIClient()
@@ -118,13 +130,18 @@ func EnableOrDisableAddon(name string, val string) error {
 		return errors.Wrap(err, "machine client")
 	}
 	defer api.Close()
-	cluster.EnsureMinikubeRunningOrExit(api, 0)
 
-	addon := assets.Addons[name]
+	//if minikube is not running, we return and simply update the value in the addon
+	//config and rewrite the file
+	if !cluster.IsMinikubeRunning(api) {
+		return nil
+	}
+
 	host, err := cluster.CheckIfHostExistsAndLoad(api, config.GetMachineName())
 	if err != nil {
 		return errors.Wrap(err, "getting host")
 	}
+
 	cmd, err := machine.CommandRunner(host)
 	if err != nil {
 		return errors.Wrap(err, "command runner")
@@ -139,30 +156,24 @@ func EnableOrDisableAddon(name string, val string) error {
 	return enableOrDisableAddonInternal(addon, cmd, data, enable)
 }
 
-func isAddonAlreadySet(addon *assets.Addon, enable bool) error {
-
+func isAddonAlreadySet(addon *assets.Addon, enable bool) (bool, error) {
 	addonStatus, err := addon.IsEnabled()
 
 	if err != nil {
-		return errors.Wrap(err, "get the addon status")
+		return false, errors.Wrap(err, "get the addon status")
 	}
 
 	if addonStatus && enable {
-		return fmt.Errorf("addon %s was already enabled", addon.Name())
+		return true, nil
 	} else if !addonStatus && !enable {
-		return fmt.Errorf("addon %s was already disabled", addon.Name())
+		return true, nil
 	}
 
-	return nil
+	return false, nil
 }
 
 func enableOrDisableAddonInternal(addon *assets.Addon, cmd command.Runner, data interface{}, enable bool) error {
 	var err error
-	// check addon status before enabling/disabling it
-	if err := isAddonAlreadySet(addon, enable); err != nil {
-		out.ErrT(out.Conflict, "{{.error}}", out.V{"error": err})
-		os.Exit(0)
-	}
 
 	if enable {
 		for _, addon := range addon.Assets {

cmd/minikube/cmd/config/util_test.go

@@ -85,15 +85,12 @@ func TestSetBool(t *testing.T) {
 func TestIsAddonAlreadySet(t *testing.T) {
 	testCases := []struct {
 		addonName string
-		expectErr string
 	}{
 		{
 			addonName: "ingress",
-			expectErr: "addon ingress was already ",
 		},
 		{
 			addonName: "heapster",
-			expectErr: "addon heapster was already ",
 		},
 	}
 
@@ -101,13 +98,16 @@ func TestIsAddonAlreadySet(t *testing.T) {
 		addon := assets.Addons[test.addonName]
 		addonStatus, _ := addon.IsEnabled()
 
-		expectMsg := test.expectErr + "enabled"
-		if !addonStatus {
-			expectMsg = test.expectErr + "disabled"
+		alreadySet, err := isAddonAlreadySet(addon, addonStatus)
+		if !alreadySet {
+			if addonStatus {
+				t.Errorf("Did not get expected status, \n\n expected %+v already enabled", test.addonName)
+			} else {
+				t.Errorf("Did not get expected status, \n\n expected %+v already disabled", test.addonName)
+			}
 		}
-		err := isAddonAlreadySet(addon, addonStatus)
-		if err.Error() != expectMsg {
-			t.Errorf("Did not get expected error, \n\n expected: %+v \n\n actual: %+v", expectMsg, err)
+		if err != nil {
+			t.Errorf("Got unexpected error: %+v", err)
 		}
 	}
 }

cmd/minikube/cmd/dashboard.go

@@ -93,7 +93,9 @@ 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/")
 		}
 
-		cluster.EnsureMinikubeRunningOrExit(api, 1)
+		if !cluster.IsMinikubeRunning(api) {
+			os.Exit(1)
+		}
 
 		// Check dashboard status before enabling it
 		dashboardAddon := assets.Addons["dashboard"]

cmd/minikube/cmd/service.go

@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"os"
 	"text/template"
 
 	"github.com/spf13/cobra"
@@ -64,7 +65,9 @@ var serviceCmd = &cobra.Command{
 		}
 		defer api.Close()
 
-		cluster.EnsureMinikubeRunningOrExit(api, 1)
+		if !cluster.IsMinikubeRunning(api) {
+			os.Exit(1)
+		}
 		err = service.WaitAndMaybeOpenService(api, namespace, svc,
 			serviceURLTemplate, serviceURLMode, https, wait, interval)
 		if err != nil {

go.mod

@@ -76,6 +76,7 @@ require (
 	gopkg.in/airbrake/gobrake.v2 v2.0.9 // indirect
 	gopkg.in/gemnasium/logrus-airbrake-hook.v2 v2.1.2 // indirect
 	gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect
+	gotest.tools v2.2.0+incompatible
 	k8s.io/api v0.0.0
 	k8s.io/apimachinery v0.0.0
 	k8s.io/client-go v0.0.0

pkg/minikube/cluster/cluster.go

@@ -604,12 +604,13 @@ func CreateSSHShell(api libmachine.API, args []string) error {
 
 // EnsureMinikubeRunningOrExit checks that minikube has a status available and that
 // the status is `Running`, otherwise it will exit
-func EnsureMinikubeRunningOrExit(api libmachine.API, exitStatus int) {
+func IsMinikubeRunning(api libmachine.API) bool {
 	s, err := GetHostStatus(api)
 	if err != nil {
-		exit.WithError("Error getting machine status", err)
+		return false
 	}
 	if s != state.Running.String() {
-		exit.WithCodeT(exit.Unavailable, "minikube is not running, so the service cannot be accessed")
+		return false
 	}
+	return true
 }