GCP-Auth: Add fixes and timeouts

pkg/addons/addons.go

@@ -17,6 +17,7 @@ limitations under the License.
 package addons
 
 import (
+	"context"
 	"fmt"
 	"os/exec"
 	"path"
@@ -433,11 +434,17 @@ func enableOrDisableAddonInternal(cc *config.ClusterConfig, addon *assets.Addon,
 		}
 	}
 
+	// on the first attempt try without force, but on subsequent attempts use force
+	force := false
+
 	// Retry, because sometimes we race against an apiserver restart
 	apply := func() error {
-		_, err := runner.RunCmd(kubectlCommand(cc, deployFiles, enable))
+		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+		defer cancel()
+		_, err := runner.RunCmd(kubectlCommand(ctx, cc, deployFiles, enable, force))
 		if err != nil {
 			klog.Warningf("apply failed, will retry: %v", err)
+			force = true
 		}
 		return err
 	}

pkg/addons/addons_gcpauth.go

@@ -19,6 +19,7 @@ package addons
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"os/exec"
 	"path"
@@ -186,14 +187,15 @@ func patchServiceAccounts(cc *config.ClusterConfig) error {
 }
 
 func refreshExistingPods(cc *config.ClusterConfig) error {
+	klog.Info("refreshing existing pods")
 	client, err := service.K8s.GetCoreClient(cc.Name)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get k8s client: %v", err)
 	}
 
 	namespaces, err := client.Namespaces().List(context.TODO(), metav1.ListOptions{})
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get namespaces: %v", err)
 	}
 	for _, n := range namespaces.Items {
 		// Ignore kube-system and gcp-auth namespaces
@@ -204,7 +206,7 @@ func refreshExistingPods(cc *config.ClusterConfig) error {
 		pods := client.Pods(n.Name)
 		podList, err := pods.List(context.TODO(), metav1.ListOptions{})
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to list pods: %v", err)
 		}
 
 		for _, p := range podList.Items {
@@ -213,23 +215,29 @@ func refreshExistingPods(cc *config.ClusterConfig) error {
 				continue
 			}
 
+			klog.Infof("refreshing pod %q", p.Name)
+
 			// Recreating the pod should pickup the necessary changes
 			err := pods.Delete(context.TODO(), p.Name, metav1.DeleteOptions{})
 			if err != nil {
-				return err
+				return fmt.Errorf("failed to delete pod %q: %v", p.Name, err)
 			}
 
 			p.ResourceVersion = ""
 
-			_, err = pods.Get(context.TODO(), p.Name, metav1.GetOptions{})
+			ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+			defer cancel()
 
 			for err == nil {
-				time.Sleep(time.Second)
+				if ctx.Err() == context.DeadlineExceeded {
+					return fmt.Errorf("pod %q failed to restart", p.Name)
+				}
 				_, err = pods.Get(context.TODO(), p.Name, metav1.GetOptions{})
+				time.Sleep(time.Second)
 			}
 
 			if _, err := pods.Create(context.TODO(), &p, metav1.CreateOptions{}); err != nil {
-				return err
+				return fmt.Errorf("failed to create pod %q: %v", p.Name, err)
 			}
 		}
 	}

pkg/addons/kubectl.go

@@ -17,6 +17,7 @@ limitations under the License.
 package addons
 
 import (
+	"context"
 	"fmt"
 	"os/exec"
 	"path"
@@ -27,7 +28,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/vmpath"
 )
 
-func kubectlCommand(cc *config.ClusterConfig, files []string, enable bool) *exec.Cmd {
+func kubectlCommand(ctx context.Context, cc *config.ClusterConfig, files []string, enable, force bool) *exec.Cmd {
 	v := constants.DefaultKubernetesVersion
 	if cc != nil {
 		v = cc.KubernetesConfig.KubernetesVersion
@@ -41,6 +42,9 @@ func kubectlCommand(cc *config.ClusterConfig, files []string, enable bool) *exec
 	}
 
 	args := []string{fmt.Sprintf("KUBECONFIG=%s", path.Join(vmpath.GuestPersistentDir, "kubeconfig")), kubectlBinary, kubectlAction}
+	if force {
+		args = append(args, "--force")
+	}
 	if !enable {
 		// --ignore-not-found just ignores when we try to delete a resource that is already gone,
 		// like a completed job with a ttlSecondsAfterFinished
@@ -50,5 +54,5 @@ func kubectlCommand(cc *config.ClusterConfig, files []string, enable bool) *exec
 		args = append(args, []string{"-f", f}...)
 	}
 
-	return exec.Command("sudo", args...)
+	return exec.CommandContext(ctx, "sudo", args...)
 }

pkg/addons/kubectl_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package addons
 
 import (
+	"context"
 	"strings"
 	"testing"
 
@@ -28,19 +29,35 @@ func TestKubectlCommand(t *testing.T) {
 		description string
 		files       []string
 		enable      bool
+		force       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 --ignore-not-found -f a -f b",
 		},
+		{
+			description: "enable an addon",
+			files:       []string{"a", "b"},
+			enable:      true,
+			force:       true,
+			expected:    "sudo KUBECONFIG=/var/lib/minikube/kubeconfig /var/lib/minikube/binaries/v1.17.0/kubectl apply --force -f a -f b",
+		},
+		{
+			description: "disable an addon",
+			files:       []string{"a", "b"},
+			enable:      false,
+			force:       true,
+			expected:    "sudo KUBECONFIG=/var/lib/minikube/kubeconfig /var/lib/minikube/binaries/v1.17.0/kubectl delete --force --ignore-not-found -f a -f b",
+		},
 	}
 
 	cc := &config.ClusterConfig{
@@ -51,7 +68,7 @@ func TestKubectlCommand(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			command := kubectlCommand(cc, test.files, test.enable)
+			command := kubectlCommand(context.Background(), cc, test.files, test.enable, test.force)
 			actual := strings.Join(command.Args, " ")
 
 			if actual != test.expected {