Merge pull request #6376 from priyawadhwa/fix-dashboard

Fix dashboard by using kubectl apply/delete instead of prune for addons

pkg/addons/addons.go

@@ -19,8 +19,10 @@ package addons
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strconv"
 
+	"github.com/golang/glog"
 	"github.com/pkg/errors"
 	"github.com/spf13/viper"
 	"k8s.io/minikube/pkg/minikube/assets"
@@ -174,15 +176,10 @@ func isAddonAlreadySet(addon *assets.Addon, enable bool) (bool, error) {
 }
 
 func enableOrDisableAddonInternal(addon *assets.Addon, cmd command.Runner, data interface{}, enable bool, profile string) error {
-	var err error
-
-	updateFile := cmd.Copy
-	if !enable {
-		updateFile = cmd.Remove
-	}
-
+	files := []string{}
 	for _, addon := range addon.Assets {
 		var addonFile assets.CopyableFile
+		var err error
 		if addon.IsTemplate() {
 			addonFile, err = addon.Evaluate(data)
 			if err != nil {
@@ -192,11 +189,27 @@ func enableOrDisableAddonInternal(addon *assets.Addon, cmd command.Runner, data
 		} else {
 			addonFile = addon
 		}
-		if err := updateFile(addonFile); err != nil {
-			return errors.Wrapf(err, "updating addon %s", addon.AssetName)
+		if enable {
+			if err := cmd.Copy(addonFile); err != nil {
+				return err
+			}
+		} else {
+			defer func() {
+				if err := cmd.Remove(addonFile); err != nil {
+					glog.Warningf("error removing %s; addon should still be disabled as expected", addonFile)
+				}
+			}()
 		}
+		files = append(files, filepath.Join(addonFile.GetTargetDir(), addonFile.GetTargetName()))
 	}
-	return reconcile(cmd, profile)
+	command, err := kubectlCommand(profile, files, enable)
+	if err != nil {
+		return err
+	}
+	if result, err := cmd.RunCmd(command); err != nil {
+		return errors.Wrapf(err, "error updating addon:\n%s", result.Output())
+	}
+	return nil
 }
 
 // enableOrDisableStorageClasses enables or disables storage classes

pkg/addons/kubectl.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package addons
+
+import (
+	"os"
+	"os/exec"
+	"path"
+
+	"k8s.io/minikube/pkg/minikube/config"
+	"k8s.io/minikube/pkg/minikube/constants"
+)
+
+var (
+	// For testing
+	k8sVersion = kubernetesVersion
+)
+
+func kubectlCommand(profile string, files []string, enable bool) (*exec.Cmd, error) {
+	v, err := k8sVersion(profile)
+	if err != nil {
+		return nil, err
+	}
+	kubectlBinary := kubectlBinaryPath(v)
+
+	kubectlAction := "apply"
+	if !enable {
+		kubectlAction = "delete"
+	}
+
+	args := []string{"KUBECONFIG=/var/lib/minikube/kubeconfig", kubectlBinary, kubectlAction}
+	for _, f := range files {
+		args = append(args, []string{"-f", f}...)
+	}
+
+	cmd := exec.Command("sudo", args...)
+	return cmd, nil
+}
+
+func kubernetesVersion(profile string) (string, error) {
+	cc, err := config.Load(profile)
+	if err != nil && !os.IsNotExist(err) {
+		return "", err
+	}
+	version := constants.DefaultKubernetesVersion
+	if cc != nil {
+		version = cc.KubernetesConfig.KubernetesVersion
+	}
+	return version, nil
+}
+
+func kubectlBinaryPath(version string) string {
+	return path.Join("/var/lib/minikube/binaries", version, "kubectl")
+}

pkg/addons/kubectl_test.go

@@ -0,0 +1,63 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package addons
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestKubectlCommand(t *testing.T) {
+	tests := []struct {
+		description string
+		files       []string
+		enable      bool
+		expected    string
+	}{
+		{
+			description: "enable an addon",
+			files:       []string{"a", "b"},
+			enable:      true,
+			expected:    "sudo KUBECONFIG=/var/lib/minikube/kubeconfig /var/lib/minikube/binaries/v1.17.0/kubectl apply -f a -f b",
+		}, {
+			description: "disable an addon",
+			files:       []string{"a", "b"},
+			enable:      false,
+			expected:    "sudo KUBECONFIG=/var/lib/minikube/kubeconfig /var/lib/minikube/binaries/v1.17.0/kubectl delete -f a -f b",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			originalK8sVersion := k8sVersion
+			defer func() { k8sVersion = originalK8sVersion }()
+			k8sVersion = func(_ string) (string, error) {
+				return "v1.17.0", nil
+			}
+
+			command, err := kubectlCommand("", test.files, test.enable)
+			if err != nil {
+				t.Fatalf("error getting kubectl command: %v", err)
+			}
+			actual := strings.Join(command.Args, " ")
+
+			if actual != test.expected {
+				t.Fatalf("expected does not match actual\nExpected: %s\nActual: %s", test.expected, actual)
+			}
+		})
+	}
+}