Skip to content

Commit

Permalink
Changed command runner
Browse files Browse the repository at this point in the history
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
  • Loading branch information
RickyRajinder committed Oct 11, 2019
1 parent 03c241d commit a229ef2
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 31 deletions.
16 changes: 15 additions & 1 deletion cmd/minikube/cmd/config/disable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
16 changes: 15 additions & 1 deletion cmd/minikube/cmd/config/enable_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 6 additions & 2 deletions cmd/minikube/cmd/config/open.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ limitations under the License.
package config

import (
"os"
"text/template"

"k8s.io/minikube/pkg/minikube/cluster"

"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"
Expand Down Expand Up @@ -62,7 +64,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.
Expand Down
37 changes: 24 additions & 13 deletions cmd/minikube/cmd/config/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,20 +111,37 @@ 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()
if err != nil {
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")
Expand All @@ -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 {
Expand Down
18 changes: 9 additions & 9 deletions cmd/minikube/cmd/config/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,29 +85,29 @@ 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 ",
},
}

for _, test := range testCases {
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)
}
}
}
4 changes: 3 additions & 1 deletion cmd/minikube/cmd/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
5 changes: 4 additions & 1 deletion cmd/minikube/cmd/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package cmd

import (
"os"
"text/template"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions pkg/minikube/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit a229ef2

Please sign in to comment.