diff --git a/cmd/minikube/cmd/config/disable_test.go b/cmd/minikube/cmd/config/disable_test.go index 3141605972..dbde3850c7 100644 --- a/cmd/minikube/cmd/config/disable_test.go +++ b/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) +} diff --git a/cmd/minikube/cmd/config/enable_test.go b/cmd/minikube/cmd/config/enable_test.go index 007d59516d..28556dc1d8 100644 --- a/cmd/minikube/cmd/config/enable_test.go +++ b/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) +} diff --git a/cmd/minikube/cmd/config/open.go b/cmd/minikube/cmd/config/open.go index b8becfbfea..8a852729ce 100644 --- a/cmd/minikube/cmd/config/open.go +++ b/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. diff --git a/cmd/minikube/cmd/config/util.go b/cmd/minikube/cmd/config/util.go index 10be06c4f5..cb26c7349c 100644 --- a/cmd/minikube/cmd/config/util.go +++ b/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 { diff --git a/cmd/minikube/cmd/config/util_test.go b/cmd/minikube/cmd/config/util_test.go index 11845ba6c4..46bd62d8de 100644 --- a/cmd/minikube/cmd/config/util_test.go +++ b/cmd/minikube/cmd/config/util_test.go @@ -86,15 +86,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 ", }, } @@ -102,13 +99,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) } } } diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 4908e319f1..27a472d80b 100644 --- a/cmd/minikube/cmd/dashboard.go +++ b/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"] diff --git a/cmd/minikube/cmd/service.go b/cmd/minikube/cmd/service.go index d550c736a7..d0068a23da 100644 --- a/cmd/minikube/cmd/service.go +++ b/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 { diff --git a/go.mod b/go.mod index 80bd0d2055..77e516b0be 100644 --- a/go.mod +++ b/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 diff --git a/pkg/minikube/cluster/cluster.go b/pkg/minikube/cluster/cluster.go index 96f8e45b7a..84e43632fa 100644 --- a/pkg/minikube/cluster/cluster.go +++ b/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 }