Skip to content

Commit

Permalink
Merge pull request #17324 from spowelljr/fixEnablingAddon
Browse files Browse the repository at this point in the history
Fix enabling & disabling addons with non-existing cluster
  • Loading branch information
medyagh authored Oct 4, 2023
2 parents daf0e66 + 8c80640 commit 6dd875c
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 6 deletions.
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/config/disable.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ var addonsDisableCmd = &cobra.Command{
if len(args) != 1 {
exit.Message(reason.Usage, "usage: minikube addons disable ADDON_NAME")
}
_, cc := mustload.Partial(ClusterFlagValue())
err := addons.VerifyNotPaused(ClusterFlagValue(), false)
if err != nil {
exit.Error(reason.InternalAddonDisablePaused, "disable failed", err)
Expand All @@ -43,7 +44,6 @@ var addonsDisableCmd = &cobra.Command{
if addon == "heapster" {
exit.Message(reason.AddonUnsupported, "The heapster addon is depreciated. please try to disable metrics-server instead")
}
_, cc := mustload.Partial(ClusterFlagValue())
validAddon, ok := assets.Addons[addon]
if !ok {
exit.Message(reason.AddonUnsupported, `"'{{.minikube_addon}}' is not a valid minikube addon`, out.V{"minikube_addon": addon})
Expand Down
8 changes: 3 additions & 5 deletions cmd/minikube/cmd/config/enable.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/constants"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/mustload"
"k8s.io/minikube/pkg/minikube/out"
"k8s.io/minikube/pkg/minikube/reason"
"k8s.io/minikube/pkg/minikube/style"
Expand All @@ -40,15 +41,12 @@ var addonsEnableCmd = &cobra.Command{
if len(args) != 1 {
exit.Message(reason.Usage, "usage: minikube addons enable ADDON_NAME")
}
cc, err := config.Load(ClusterFlagValue())
if err != nil && !config.IsNotExist(err) {
out.ErrT(style.Sad, `Unable to load config: {{.error}}`, out.V{"error": err})
}
_, cc := mustload.Partial(ClusterFlagValue())
if cc.KubernetesConfig.KubernetesVersion == constants.NoKubernetesVersion {
exit.Message(reason.Usage, "You cannot enable addons on a cluster without Kubernetes, to enable Kubernetes on your cluster, run: minikube start --kubernetes-version=stable")
}

err = addons.VerifyNotPaused(ClusterFlagValue(), true)
err := addons.VerifyNotPaused(ClusterFlagValue(), true)
if err != nil {
exit.Error(reason.InternalAddonEnablePaused, "enabled failed", err)
}
Expand Down
42 changes: 42 additions & 0 deletions test/integration/addons_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,26 @@ func TestAddons(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), Minutes(40))
defer Cleanup(t, profile, cancel)

t.Run("PreSetup", func(t *testing.T) {
tests := []struct {
name string
validator validateFunc
}{
{"EnablingAddonOnNonExistingCluster", validateEnablingAddonOnNonExistingCluster},
{"DisablingAddonOnNonExistingCluster", validateDisablingAddonOnNonExistingCluster},
}
for _, tc := range tests {
tc := tc
if ctx.Err() == context.DeadlineExceeded {
t.Fatalf("Unable to run more tests (deadline exceeded)")
}
t.Run(tc.name, func(t *testing.T) {
MaybeParallel(t)
tc.validator(ctx, t, profile)
})
}
})

setupSucceeded := t.Run("Setup", func(t *testing.T) {
// Set an env var to point to our dummy credentials file
// don't use t.Setenv because we sometimes manually unset the env var later manually
Expand Down Expand Up @@ -900,3 +920,25 @@ func validateLocalPathAddon(ctx context.Context, t *testing.T, profile string) {
t.Errorf("failed to disable storage-provisioner-rancher addon: args %q: %v", rr.Command(), err)
}
}

// validateEnablingAddonOnNonExistingCluster tests enabling an addon on a non-existing cluster
func validateEnablingAddonOnNonExistingCluster(ctx context.Context, t *testing.T, profile string) {
rr, err := Run(t, exec.CommandContext(ctx, Target(), "addons", "enable", "dashboard", "-p", profile))
if err == nil {
t.Fatalf("enabling addon succeeded when it shouldn't have: %s", rr.Output())
}
if !strings.Contains(rr.Output(), "To start a cluster, run") {
t.Fatalf("unexpected error was returned: %v", err)
}
}

// validateDisablingAddonOnNonExistingCluster tests disabling an addon on a non-existing cluster
func validateDisablingAddonOnNonExistingCluster(ctx context.Context, t *testing.T, profile string) {
rr, err := Run(t, exec.CommandContext(ctx, Target(), "addons", "disable", "dashboard", "-p", profile))
if err == nil {
t.Fatalf("disabling addon succeeded when it shouldn't have: %s", rr.Output())
}
if !strings.Contains(rr.Output(), "To start a cluster, run") {
t.Fatalf("unexpected error was returned: %v", err)
}
}

0 comments on commit 6dd875c

Please sign in to comment.