diff --git a/pkg/addons/addons.go b/pkg/addons/addons.go index 184d7952ff..0437793d04 100644 --- a/pkg/addons/addons.go +++ b/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 diff --git a/pkg/addons/kubectl.go b/pkg/addons/kubectl.go new file mode 100644 index 0000000000..5cc96383f8 --- /dev/null +++ b/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") +} diff --git a/pkg/addons/kubectl_test.go b/pkg/addons/kubectl_test.go new file mode 100644 index 0000000000..493ff35412 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/addons/reconcile.go b/pkg/addons/reconcile.go deleted file mode 100644 index f774cb9393..0000000000 --- a/pkg/addons/reconcile.go +++ /dev/null @@ -1,123 +0,0 @@ -/* -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" - - "github.com/blang/semver" - "github.com/golang/glog" - "github.com/pkg/errors" - "k8s.io/minikube/pkg/minikube/bootstrapper/bsutil" - "k8s.io/minikube/pkg/minikube/command" - "k8s.io/minikube/pkg/minikube/config" - "k8s.io/minikube/pkg/minikube/constants" -) - -var ( - // For testing - k8sVersion = kubernetesVersion -) - -// taken from https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/addon-manager/kube-addons.sh -var kubectlPruneWhitelist = []string{ - "core/v1/ConfigMap", - "core/v1/Endpoints", - "core/v1/Namespace", - "core/v1/PersistentVolumeClaim", - "core/v1/PersistentVolume", - "core/v1/Pod", - "core/v1/ReplicationController", - "core/v1/Secret", - "core/v1/Service", - "batch/v1/Job", - "batch/v1beta1/CronJob", - "apps/v1/DaemonSet", - "apps/v1/Deployment", - "apps/v1/ReplicaSet", - "apps/v1/StatefulSet", - "extensions/v1beta1/Ingress", -} - -// reconcile runs kubectl apply -f on the addons directory -// to reconcile addons state in all running profiles -func reconcile(cmd command.Runner, profile string) error { - c, err := kubectlCommand(profile) - if err != nil { - return err - } - if _, err := cmd.RunCmd(c); err != nil { - glog.Warningf("reconciling addons failed: %v", err) - return err - } - return nil -} - -func kubectlCommand(profile string) (*exec.Cmd, error) { - v, err := k8sVersion(profile) - if err != nil { - return nil, err - } - kubectlBinary := kubectlBinaryPath(v) - - // prune will delete any existing objects with the label specified by "-l" which don't appear in /etc/kubernetes/addons - // this is how we delete disabled addons - args := []string{"KUBECONFIG=/var/lib/minikube/kubeconfig", kubectlBinary, "apply", "-f", "/etc/kubernetes/addons", "-l", "kubernetes.io/cluster-service!=true,addonmanager.kubernetes.io/mode=Reconcile", "--prune=true"} - for _, k := range kubectlPruneWhitelist { - args = append(args, []string{"--prune-whitelist", k}...) - } - args = append(args, "--recursive") - - ok, err := shouldAppendNamespaceFlag(v) - if err != nil { - return nil, errors.Wrap(err, "appending namespace flag") - } - if ok { - args = append(args, "--namespace=kube-system") - } - - 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 -} - -// We need to append --namespace=kube-system for Kubernetes versions >=1.17 -// so that prune works as expected. See https://github.com/kubernetes/kubernetes/pull/83084/ -func shouldAppendNamespaceFlag(version string) (bool, error) { - v, err := bsutil.ParseKubernetesVersion(version) - if err != nil { - return false, err - } - return v.GTE(semver.MustParse("1.17.0")), nil -} - -func kubectlBinaryPath(version string) string { - return path.Join("/var/lib/minikube/binaries", version, "kubectl") -} diff --git a/pkg/addons/reconcile_test.go b/pkg/addons/reconcile_test.go deleted file mode 100644 index d53e87e698..0000000000 --- a/pkg/addons/reconcile_test.go +++ /dev/null @@ -1,69 +0,0 @@ -/* -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 ( - "fmt" - "strings" - "testing" -) - -func TestKubectlCommand(t *testing.T) { - expectedCommand := "sudo KUBECONFIG=/var/lib/minikube/kubeconfig /var/lib/minikube/binaries/%s/kubectl apply -f /etc/kubernetes/addons -l kubernetes.io/cluster-service!=true,addonmanager.kubernetes.io/mode=Reconcile --prune=true --prune-whitelist core/v1/ConfigMap --prune-whitelist core/v1/Endpoints --prune-whitelist core/v1/Namespace --prune-whitelist core/v1/PersistentVolumeClaim --prune-whitelist core/v1/PersistentVolume --prune-whitelist core/v1/Pod --prune-whitelist core/v1/ReplicationController --prune-whitelist core/v1/Secret --prune-whitelist core/v1/Service --prune-whitelist batch/v1/Job --prune-whitelist batch/v1beta1/CronJob --prune-whitelist apps/v1/DaemonSet --prune-whitelist apps/v1/Deployment --prune-whitelist apps/v1/ReplicaSet --prune-whitelist apps/v1/StatefulSet --prune-whitelist extensions/v1beta1/Ingress --recursive" - - tests := []struct { - description string - k8sVersion string - expected string - }{ - { - description: "k8s version < 1.17.0", - k8sVersion: "v1.16.0", - expected: expectedCommand, - }, { - description: "k8s version == 1.17.0", - k8sVersion: "v1.17.0", - expected: expectedCommand + " --namespace=kube-system", - }, { - description: "k8s version > 1.17.0", - k8sVersion: "v1.18.0", - expected: expectedCommand + " --namespace=kube-system", - }, - } - - for _, test := range tests { - t.Run(test.description, func(t *testing.T) { - originalK8sVersion := k8sVersion - defer func() { k8sVersion = originalK8sVersion }() - k8sVersion = func(_ string) (string, error) { - return test.k8sVersion, nil - } - - command, err := kubectlCommand("") - if err != nil { - t.Fatalf("error getting kubectl command: %v", err) - } - actual := strings.Join(command.Args, " ") - - expected := fmt.Sprintf(test.expected, test.k8sVersion) - - if actual != expected { - t.Fatalf("expected does not match actual\nExpected: %s\nActual: %s", expected, actual) - } - }) - } -} diff --git a/pkg/minikube/assets/addons.go b/pkg/minikube/assets/addons.go index 1c8a0e0773..67fd6b33df 100644 --- a/pkg/minikube/assets/addons.go +++ b/pkg/minikube/assets/addons.go @@ -69,11 +69,12 @@ func (a *Addon) IsEnabled() (bool, error) { // TODO: Make dynamically loadable: move this data to a .yaml file within each addon directory var Addons = map[string]*Addon{ "dashboard": NewAddon([]*BinAsset{ + // We want to create the kubernetes-dashboard ns first so that every subsequent object can be created + MustBinAsset("deploy/addons/dashboard/dashboard-ns.yaml", vmpath.GuestAddonsDir, "dashboard-ns.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-clusterrole.yaml", vmpath.GuestAddonsDir, "dashboard-clusterrole.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-clusterrolebinding.yaml", vmpath.GuestAddonsDir, "dashboard-clusterrolebinding.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-configmap.yaml", vmpath.GuestAddonsDir, "dashboard-configmap.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-dp.yaml", vmpath.GuestAddonsDir, "dashboard-dp.yaml", "0640", false), - MustBinAsset("deploy/addons/dashboard/dashboard-ns.yaml", vmpath.GuestAddonsDir, "dashboard-ns.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-role.yaml", vmpath.GuestAddonsDir, "dashboard-role.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-rolebinding.yaml", vmpath.GuestAddonsDir, "dashboard-rolebinding.yaml", "0640", false), MustBinAsset("deploy/addons/dashboard/dashboard-sa.yaml", vmpath.GuestAddonsDir, "dashboard-sa.yaml", "0640", false),