diff --git a/cmd/minikube/cmd/config/disable_test.go b/cmd/minikube/cmd/config/disable_test.go index 314160597209..d31b4ae5373d 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("Diable 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 007d59516d54..28556dc1d8f9 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 b8becfbfea7f..162665038374 100644 --- a/cmd/minikube/cmd/config/open.go +++ b/cmd/minikube/cmd/config/open.go @@ -21,7 +21,6 @@ import ( "github.com/spf13/cobra" "k8s.io/minikube/pkg/minikube/assets" - "k8s.io/minikube/pkg/minikube/cluster" "k8s.io/minikube/pkg/minikube/exit" "k8s.io/minikube/pkg/minikube/machine" "k8s.io/minikube/pkg/minikube/out" @@ -62,7 +61,6 @@ var addonsOpenCmd = &cobra.Command{ } defer api.Close() - cluster.EnsureMinikubeRunningOrExit(api, 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 d103ba36ea97..e7e5134e4622 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 3cbab76a388c..ca1faa86d417 100644 --- a/cmd/minikube/cmd/config/util_test.go +++ b/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) } } } diff --git a/cmd/minikube/cmd/dashboard.go b/cmd/minikube/cmd/dashboard.go index 4908e319f106..36da2c4b814f 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 d550c736a7ac..2436c1a13920 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 80bd0d20550f..77e516b0bec9 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/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index faa391e3714c..eec3473d03ec 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go @@ -19,6 +19,7 @@ package kubeadm import ( "bytes" "crypto/tls" + "k8s.io/minikube/pkg/minikube/cluster" "fmt" "net" @@ -127,7 +128,7 @@ func NewKubeadmBootstrapper(api libmachine.API) (*Bootstrapper, error) { if err != nil { return nil, errors.Wrap(err, "getting api client") } - runner, err := machine.CommandRunner(h) + runner, err := machine.CommandRunner(h, cluster.IsMinikubeRunning(api)) if err != nil { return nil, errors.Wrap(err, "command runner") } diff --git a/pkg/minikube/cluster/cluster.go b/pkg/minikube/cluster/cluster.go index 96f8e45b7a2f..84e43632faa3 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 }