Merge pull request #5757 from priyawadhwa/wait_false

Set --wait=false to default but still wait for apiserver

cmd/minikube/cmd/start.go

@@ -171,7 +171,7 @@ func initMinikubeFlags() {
 	startCmd.Flags().String(criSocket, "", "The cri socket path to be used.")
 	startCmd.Flags().String(networkPlugin, "", "The name of the network plugin.")
 	startCmd.Flags().Bool(enableDefaultCNI, false, "Enable the default CNI plugin (/etc/cni/net.d/k8s.conf). Used in conjunction with \"--network-plugin=cni\".")
-	startCmd.Flags().Bool(waitUntilHealthy, true, "Wait until Kubernetes core services are healthy before exiting.")
+	startCmd.Flags().Bool(waitUntilHealthy, false, "Wait until Kubernetes core services are healthy before exiting.")
 	startCmd.Flags().Duration(waitTimeout, 6*time.Minute, "max time to wait per Kubernetes core services to be healthy.")
 	startCmd.Flags().Bool(nativeSSH, true, "Use native Golang SSH client (default true). Set to 'false' to use the command line 'ssh' command when accessing the docker machine. Useful for the machine drivers when they will not start with 'Waiting for SSH'.")
 	startCmd.Flags().Bool(autoUpdate, true, "If set, automatically updates drivers to the latest version. Defaults to true.")
@@ -375,10 +375,15 @@ func runStart(cmd *cobra.Command, args []string) {
 	if driverName == driver.None {
 		prepareNone()
 	}
-	if viper.GetBool(waitUntilHealthy) {
-		if err := bs.WaitCluster(config.KubernetesConfig, viper.GetDuration(waitTimeout)); err != nil {
-			exit.WithError("Wait failed", err)
-		}
+
+	var podsToWaitFor []string
+
+	if !viper.GetBool(waitUntilHealthy) {
+		// only wait for apiserver if wait=false
+		podsToWaitFor = []string{"apiserver"}
+	}
+	if err := bs.WaitForPods(config.KubernetesConfig, viper.GetDuration(waitTimeout), podsToWaitFor); err != nil {
+		exit.WithError("Wait failed", err)
 	}
 	if err := showKubectlInfo(kubeconfig, k8sVersion); err != nil {
 		glog.Errorf("kubectl info: %v", err)

pkg/minikube/bootstrapper/bootstrapper.go

@@ -41,7 +41,7 @@ type Bootstrapper interface {
 	UpdateCluster(config.KubernetesConfig) error
 	RestartCluster(config.KubernetesConfig) error
 	DeleteCluster(config.KubernetesConfig) error
-	WaitCluster(config.KubernetesConfig, time.Duration) error
+	WaitForPods(config.KubernetesConfig, time.Duration, []string) error
 	// LogCommands returns a map of log type to a command which will display that log.
 	LogCommands(LogOptions) map[string]string
 	SetupCerts(cfg config.KubernetesConfig) error

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -36,6 +36,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
@@ -63,6 +64,7 @@ const (
 	defaultCNIConfigPath   = "/etc/cni/net.d/k8s.conf"
 	kubeletServiceFile     = "/lib/systemd/system/kubelet.service"
 	kubeletSystemdConfFile = "/etc/systemd/system/kubelet.service.d/10-kubeadm.conf"
+	AllPods                = "ALL_PODS"
 )
 
 const (
@@ -352,7 +354,7 @@ func addAddons(files *[]assets.CopyableFile, data interface{}) error {
 
 // client returns a Kubernetes client to use to speak to a kubeadm launched apiserver
 func (k *Bootstrapper) client(k8s config.KubernetesConfig) (*kubernetes.Clientset, error) {
-	// Catch case if WaitCluster was called with a stale ~/.kube/config
+	// Catch case if WaitForPods was called with a stale ~/.kube/config
 	config, err := kapi.ClientConfig(k.contextName)
 	if err != nil {
 		return nil, errors.Wrap(err, "client config")
@@ -367,8 +369,8 @@ func (k *Bootstrapper) client(k8s config.KubernetesConfig) (*kubernetes.Clientse
 	return kubernetes.NewForConfig(config)
 }
 
-// WaitCluster blocks until Kubernetes appears to be healthy.
-func (k *Bootstrapper) WaitCluster(k8s config.KubernetesConfig, timeout time.Duration) error {
+// WaitForPods blocks until pods specified in podsToWaitFor appear to be healthy.
+func (k *Bootstrapper) WaitForPods(k8s config.KubernetesConfig, timeout time.Duration, podsToWaitFor []string) error {
 	// Do not wait for "k8s-app" pods in the case of CNI, as they are managed
 	// by a CNI plugin which is usually started after minikube has been brought
 	// up. Otherwise, minikube won't start, as "k8s-app" pods are not ready.
@@ -377,9 +379,12 @@ func (k *Bootstrapper) WaitCluster(k8s config.KubernetesConfig, timeout time.Dur
 
 	// Wait until the apiserver can answer queries properly. We don't care if the apiserver
 	// pod shows up as registered, but need the webserver for all subsequent queries.
-	out.String(" apiserver")
-	if err := k.waitForAPIServer(k8s); err != nil {
-		return errors.Wrap(err, "waiting for apiserver")
+
+	if shouldWaitForPod("apiserver", podsToWaitFor) {
+		out.String(" apiserver")
+		if err := k.waitForAPIServer(k8s); err != nil {
+			return errors.Wrap(err, "waiting for apiserver")
+		}
 	}
 
 	client, err := k.client(k8s)
@@ -391,6 +396,9 @@ func (k *Bootstrapper) WaitCluster(k8s config.KubernetesConfig, timeout time.Dur
 		if componentsOnly && p.key != "component" { // skip component check if network plugin is cni
 			continue
 		}
+		if !shouldWaitForPod(p.name, podsToWaitFor) {
+			continue
+		}
 		out.String(" %s", p.name)
 		selector := labels.SelectorFromSet(labels.Set(map[string]string{p.key: p.value}))
 		if err := kapi.WaitForPodsWithLabelRunning(client, "kube-system", selector, timeout); err != nil {
@@ -401,6 +409,29 @@ func (k *Bootstrapper) WaitCluster(k8s config.KubernetesConfig, timeout time.Dur
 	return nil
 }
 
+// shouldWaitForPod returns true if:
+// 	1. podsToWaitFor is nil
+// 	2. name is in podsToWaitFor
+// 	3. ALL_PODS is in podsToWaitFor
+// else, return false
+func shouldWaitForPod(name string, podsToWaitFor []string) bool {
+	if podsToWaitFor == nil {
+		return true
+	}
+	if len(podsToWaitFor) == 0 {
+		return false
+	}
+	for _, p := range podsToWaitFor {
+		if p == AllPods {
+			return true
+		}
+		if p == name {
+			return true
+		}
+	}
+	return false
+}
+
 // RestartCluster restarts the Kubernetes cluster configured by kubeadm
 func (k *Bootstrapper) RestartCluster(k8s config.KubernetesConfig) error {
 	glog.Infof("RestartCluster start")
@@ -487,11 +518,21 @@ func (k *Bootstrapper) waitForAPIServer(k8s config.KubernetesConfig) error {
 		if status != "Running" {
 			return false, nil
 		}
-		return true, nil
+		// Make sure apiserver pod is retrievable
+		client, err := k.client(k8s)
+		if err != nil {
+			glog.Warningf("get kubernetes client: %v", err)
+			return false, nil
+		}
 
+		_, err = client.CoreV1().Pods("kube-system").Get("kube-apiserver-minikube", metav1.GetOptions{})
+		if err != nil {
+			return false, nil
+		}
+
+		return true, nil
 		// TODO: Check apiserver/kubelet logs for fatal errors so that users don't
 		// need to wait minutes to find out their flag didn't work.
-
 	}
 	err = wait.PollImmediate(kconst.APICallRetryInterval, 2*kconst.DefaultControlPlaneTimeout, f)
 	return err

pkg/minikube/bootstrapper/kubeadm/kubeadm_test.go

@@ -363,3 +363,45 @@ func TestGenerateConfig(t *testing.T) {
 		}
 	}
 }
+
+func TestShouldWaitForPod(t *testing.T) {
+	tests := []struct {
+		description   string
+		pod           string
+		podsToWaitFor []string
+		expected      bool
+	}{
+		{
+			description: "pods to wait for is nil",
+			pod:         "apiserver",
+			expected:    true,
+		}, {
+			description:   "pods to wait for is empty",
+			pod:           "apiserver",
+			podsToWaitFor: []string{},
+		}, {
+			description:   "pod is in podsToWaitFor",
+			pod:           "apiserver",
+			podsToWaitFor: []string{"etcd", "apiserver"},
+			expected:      true,
+		}, {
+			description:   "pod is not in podsToWaitFor",
+			pod:           "apiserver",
+			podsToWaitFor: []string{"etcd", "gvisor"},
+		}, {
+			description:   "wait for all pods",
+			pod:           "apiserver",
+			podsToWaitFor: []string{"ALL_PODS"},
+			expected:      true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			actual := shouldWaitForPod(test.pod, test.podsToWaitFor)
+			if actual != test.expected {
+				t.Fatalf("unexpected diff: got %t, expected %t", actual, test.expected)
+			}
+		})
+	}
+}

test/integration/functional_test.go

@@ -60,6 +60,7 @@ func TestFunctional(t *testing.T) {
 		}{
 			{"StartWithProxy", validateStartWithProxy}, // Set everything else up for success
 			{"KubeContext", validateKubeContext},       // Racy: must come immediately after "minikube start"
+			{"KubectlGetPods", validateKubectlGetPods}, // Make sure apiserver is up
 			{"CacheCmd", validateCacheCmd},             // Caches images needed for subsequent tests because of proxy
 		}
 		for _, tc := range tests {
@@ -142,6 +143,18 @@ func validateKubeContext(ctx context.Context, t *testing.T, profile string) {
 	}
 }
 
+// validateKubectlGetPods asserts that `kubectl get pod -A` returns non-zero content
+func validateKubectlGetPods(ctx context.Context, t *testing.T, profile string) {
+	rr, err := Run(t, exec.CommandContext(ctx, "kubectl", "get", "pod", "-A"))
+	if err != nil {
+		t.Errorf("%s failed: %v", rr.Args, err)
+	}
+	podName := "kube-apiserver-minikube"
+	if !strings.Contains(rr.Stdout.String(), podName) {
+		t.Errorf("%s is not up in running, got: %s\n", podName, rr.Stdout.String())
+	}
+}
+
 // validateAddonManager asserts that the kube-addon-manager pod is deployed properly
 func validateAddonManager(ctx context.Context, t *testing.T, profile string) {
 	// If --wait=false, this may take a couple of minutes