Merge pull request #37325 from ivan4th/fix-e2e-with-complete-pods-in-kube-system-ns

Automatic merge from submit-queue (batch tested with PRs 37325, 38313, 38141, 38321, 38333)

Fix running e2e with 'Completed' kube-system pods

As of now, e2e runner keeps waiting for pods in `kube-system` namespace to be "Running and Ready" if there are any pods in `Completed` state in that namespace.
This for example happens after following [Kubernetes Hosted Installation](http://docs.projectcalico.org/v2.0/getting-started/kubernetes/installation/#kubernetes-hosted-installation) instructions for Calico, making it impossible to run conformance tests against the cluster. It's also to possible to reproduce the problem like that:
```
$ cat testjob.yaml
apiVersion: batch/v1
kind: Job
metadata:
  name: tst
  namespace: kube-system
spec:
  template:
    metadata:
      name: tst
    spec:
      containers:
      - name: tst
        image: busybox
        command: ["echo",  "test"]
      restartPolicy: Never
$ kubectl create -f testjob.yaml
$ go run hack/e2e.go -v --test --test_args='--ginkgo.focus=existing\s+RC'
```

test/e2e/e2e.go

@@ -128,7 +128,7 @@ var _ = ginkgo.SynchronizedBeforeSuite(func() []byte {
 	// test pods from running, and tests that ensure all pods are running and
 	// ready will fail).
 	podStartupTimeout := framework.TestContext.SystemPodsStartupTimeout
-	if err := framework.WaitForPodsRunningReady(c, api.NamespaceSystem, int32(framework.TestContext.MinStartupPods), podStartupTimeout, framework.ImagePullerLabels); err != nil {
+	if err := framework.WaitForPodsRunningReady(c, api.NamespaceSystem, int32(framework.TestContext.MinStartupPods), podStartupTimeout, framework.ImagePullerLabels, true); err != nil {
 		framework.DumpAllNamespaceInfo(c, api.NamespaceSystem)
 		framework.LogFailedContainers(c, api.NamespaceSystem, framework.Logf)
 		runKubernetesServiceTestContainer(c, v1.NamespaceDefault)

test/e2e/framework/util.go

@@ -516,8 +516,8 @@ var ReadyReplicaVersion = version.MustParse("v1.4.0")
 // even if there are minPods pods, some of which are in Running/Ready
 // and some in Success. This is to allow the client to decide if "Success"
 // means "Ready" or not.
-func WaitForPodsRunningReady(c clientset.Interface, ns string, minPods int32, timeout time.Duration, ignoreLabels map[string]string) error {
-
+// If skipSucceeded is true, any pods that are Succeeded are not counted.
+func WaitForPodsRunningReady(c clientset.Interface, ns string, minPods int32, timeout time.Duration, ignoreLabels map[string]string, skipSucceeded bool) error {
 	// This can be removed when we no longer have 1.3 servers running with upgrade tests.
 	hasReadyReplicas, err := ServerVersionGTE(ReadyReplicaVersion, c.Discovery())
 	if err != nil {
@@ -581,13 +581,22 @@ func WaitForPodsRunningReady(c clientset.Interface, ns string, minPods int32, ti
 				Logf("%v in state %v, ignoring", pod.Name, pod.Status.Phase)
 				continue
 			}
-			if res, err := testutils.PodRunningReady(&pod); res && err == nil {
+			res, err := testutils.PodRunningReady(&pod)
+			switch {
+			case res && err == nil:
 				nOk++
-			} else {
-				if pod.Status.Phase != v1.PodFailed {
-					Logf("The status of Pod %s is %s (Ready = false), waiting for it to be either Running (with Ready = true) or Failed", pod.ObjectMeta.Name, pod.Status.Phase)
-					badPods = append(badPods, pod)
-				} else if _, ok := pod.Annotations[v1.CreatedByAnnotation]; !ok {
+			case pod.Status.Phase == v1.PodSucceeded && skipSucceeded:
+				continue
+			case pod.Status.Phase == v1.PodSucceeded:
+				Logf("The status of Pod %s is Succeeded which is unexpected", pod.ObjectMeta.Name)
+				badPods = append(badPods, pod)
+				// it doesn't make sense to wait for this pod
+				return false, errors.New("unexpected Succeeded pod state")
+			case pod.Status.Phase != v1.PodFailed:
+				Logf("The status of Pod %s is %s (Ready = false), waiting for it to be either Running (with Ready = true) or Failed", pod.ObjectMeta.Name, pod.Status.Phase)
+				badPods = append(badPods, pod)
+			default:
+				if _, ok := pod.Annotations[v1.CreatedByAnnotation]; !ok {
 					Logf("Pod %s is Failed, but it's not controlled by a controller", pod.ObjectMeta.Name)
 					badPods = append(badPods, pod)
 				}

test/e2e/mesos.go

@@ -68,7 +68,7 @@ var _ = framework.KubeDescribe("Mesos", func() {
 		nodelist := framework.GetReadySchedulableNodesOrDie(client)
 		const ns = "static-pods"
 		numpods := int32(len(nodelist.Items))
-		framework.ExpectNoError(framework.WaitForPodsRunningReady(client, ns, numpods, wait.ForeverTestTimeout, map[string]string{}),
+		framework.ExpectNoError(framework.WaitForPodsRunningReady(client, ns, numpods, wait.ForeverTestTimeout, map[string]string{}, false),
 			fmt.Sprintf("number of static pods in namespace %s is %d", ns, numpods))
 	})
 

test/e2e/resize_nodes.go

@@ -285,7 +285,7 @@ var _ = framework.KubeDescribe("Nodes [Disruptive]", func() {
 			// Many e2e tests assume that the cluster is fully healthy before they start.  Wait until
 			// the cluster is restored to health.
 			By("waiting for system pods to successfully restart")
-			err := framework.WaitForPodsRunningReady(c, api.NamespaceSystem, systemPodsNo, framework.PodReadyBeforeTimeout, ignoreLabels)
+			err := framework.WaitForPodsRunningReady(c, api.NamespaceSystem, systemPodsNo, framework.PodReadyBeforeTimeout, ignoreLabels, true)
 			Expect(err).NotTo(HaveOccurred())
 			By("waiting for image prepulling pods to complete")
 			framework.WaitForPodsSuccess(c, api.NamespaceSystem, framework.ImagePullerLabels, imagePrePullingTimeout)

test/e2e/scheduler_predicates.go

@@ -90,7 +90,7 @@ var _ = framework.KubeDescribe("SchedulerPredicates [Serial]", func() {
 			}
 		}
 
-		err = framework.WaitForPodsRunningReady(cs, api.NamespaceSystem, int32(systemPodsNo), framework.PodReadyBeforeTimeout, ignoreLabels)
+		err = framework.WaitForPodsRunningReady(cs, api.NamespaceSystem, int32(systemPodsNo), framework.PodReadyBeforeTimeout, ignoreLabels, true)
 		Expect(err).NotTo(HaveOccurred())
 
 		for _, node := range nodeList.Items {