Merge pull request #34632 from Random-Liu/fix-wait-for-success

Automatic merge from submit-queue

Fix the wait for pod success in test framework.

Fixes https://github.com/kubernetes/kubernetes/issues/34623.
Addresses https://github.com/kubernetes/kubernetes/issues/33189#issuecomment-253282725.
Related to #34630.

This PR:
1) Changes `WaitForPodSuccessInNamespace` to use pod phase instead of container status because of https://github.com/kubernetes/kubernetes/issues/33189#issuecomment-253287397. The code was introduced because of https://github.com/kubernetes/kubernetes/issues/2632, which is never true now.
2) Fixes the cluster logging test to set the pod as `RestartOnFailure`.

@yujuhong @Crassirostris

test/e2e/cluster_logging_es.go

@@ -53,7 +53,7 @@ var _ = framework.KubeDescribe("Cluster level logging using Elasticsearch [Featu
 		By("Running synthetic logger")
 		createSynthLogger(f, expectedLinesCount)
 		defer f.PodClient().Delete(synthLoggerPodName, &api.DeleteOptions{})
-		err = framework.WaitForPodSuccessInNamespace(f.Client, synthLoggerPodName, synthLoggerPodName, f.Namespace.Name)
+		err = framework.WaitForPodSuccessInNamespace(f.Client, synthLoggerPodName, f.Namespace.Name)
 		framework.ExpectNoError(err, fmt.Sprintf("Should've successfully waited for pod %s to succeed", synthLoggerPodName))
 
 		By("Waiting for logs to ingest")

test/e2e/cluster_logging_gcl.go

@@ -43,7 +43,7 @@ var _ = framework.KubeDescribe("Cluster level logging using GCL", func() {
 		By("Running synthetic logger")
 		createSynthLogger(f, expectedLinesCount)
 		defer f.PodClient().Delete(synthLoggerPodName, &api.DeleteOptions{})
-		err := framework.WaitForPodSuccessInNamespace(f.Client, synthLoggerPodName, synthLoggerPodName, f.Namespace.Name)
+		err := framework.WaitForPodSuccessInNamespace(f.Client, synthLoggerPodName, f.Namespace.Name)
 		framework.ExpectNoError(err, fmt.Sprintf("Should've successfully waited for pod %s to succeed", synthLoggerPodName))
 
 		By("Waiting for logs to ingest")

test/e2e/cluster_logging_utils.go

@@ -45,6 +45,7 @@ func createSynthLogger(f *framework.Framework, linesCount int) {
 			Namespace: f.Namespace.Name,
 		},
 		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicyOnFailure,
 			Containers: []api.Container{
 				{
 					Name:  synthLoggerPodName,

test/e2e/framework/util.go

@@ -1465,35 +1465,31 @@ func waitForPodTerminatedInNamespace(c *client.Client, podName, reason, namespac
 }
 
 // waitForPodSuccessInNamespaceTimeout returns nil if the pod reached state success, or an error if it reached failure or ran too long.
-func waitForPodSuccessInNamespaceTimeout(c *client.Client, podName string, contName string, namespace string, timeout time.Duration) error {
+func waitForPodSuccessInNamespaceTimeout(c *client.Client, podName string, namespace string, timeout time.Duration) error {
 	return waitForPodCondition(c, namespace, podName, "success or failure", timeout, func(pod *api.Pod) (bool, error) {
-		// Cannot use pod.Status.Phase == api.PodSucceeded/api.PodFailed due to #2632
-		// TODO: This was not true from long time ago. We can use api.PodSucceeded now.
-		ci, ok := api.GetContainerStatus(pod.Status.ContainerStatuses, contName)
-		if !ok {
-			Logf("No Status.Info for container '%s' in pod '%s' yet", contName, podName)
-		} else {
-			if ci.State.Terminated != nil {
-				if ci.State.Terminated.ExitCode == 0 {
+		if pod.Spec.RestartPolicy == api.RestartPolicyAlways {
+			return false, fmt.Errorf("pod %q will never terminate with a succeeded state since its restart policy is Always", podName)
+		}
+		switch pod.Status.Phase {
+		case api.PodSucceeded:
 			By("Saw pod success")
 			return true, nil
-				}
-				return true, fmt.Errorf("pod '%s' terminated with failure: %+v", podName, ci.State.Terminated)
-			}
-			Logf("Nil State.Terminated for container '%s' in pod '%s' in namespace '%s' so far", contName, podName, namespace)
-		}
+		case api.PodFailed:
+			return true, fmt.Errorf("pod %q failed with status: %+v", podName, pod.Status)
+		default:
 			return false, nil
+		}
 	})
 }
 
 // WaitForPodSuccessInNamespace returns nil if the pod reached state success, or an error if it reached failure or until podStartupTimeout.
-func WaitForPodSuccessInNamespace(c *client.Client, podName string, contName string, namespace string) error {
-	return waitForPodSuccessInNamespaceTimeout(c, podName, contName, namespace, PodStartTimeout)
+func WaitForPodSuccessInNamespace(c *client.Client, podName string, namespace string) error {
+	return waitForPodSuccessInNamespaceTimeout(c, podName, namespace, PodStartTimeout)
 }
 
 // WaitForPodSuccessInNamespaceSlow returns nil if the pod reached state success, or an error if it reached failure or until slowPodStartupTimeout.
-func WaitForPodSuccessInNamespaceSlow(c *client.Client, podName string, contName string, namespace string) error {
-	return waitForPodSuccessInNamespaceTimeout(c, podName, contName, namespace, slowPodStartTimeout)
+func WaitForPodSuccessInNamespaceSlow(c *client.Client, podName string, namespace string) error {
+	return waitForPodSuccessInNamespaceTimeout(c, podName, namespace, slowPodStartTimeout)
 }
 
 // waitForRCPodOnNode returns the pod from the given replication controller (described by rcName) which is scheduled on the given node.
@@ -2307,11 +2303,9 @@ func (f *Framework) MatchContainerOutput(
 	defer podClient.Delete(pod.Name, api.NewDeleteOptions(0))
 	podClient.Create(pod)
 
-	// Wait for client pod to complete. All containers should succeed.
-	for _, container := range pod.Spec.Containers {
-		if err := WaitForPodSuccessInNamespace(f.Client, pod.Name, container.Name, ns); err != nil {
-			return fmt.Errorf("expected container %s success: %v", container.Name, err)
-		}
+	// Wait for client pod to complete.
+	if err := WaitForPodSuccessInNamespace(f.Client, pod.Name, ns); err != nil {
+		return fmt.Errorf("expected pod %q success: %v", pod.Name, err)
 	}
 
 	// Grab its logs.  Get host first.
@@ -4986,7 +4980,7 @@ func CheckConnectivityToHost(f *Framework, nodeName, podName, host string, timeo
 		return err
 	}
 	defer podClient.Delete(podName, nil)
-	err = WaitForPodSuccessInNamespace(f.Client, podName, contName, f.Namespace.Name)
+	err = WaitForPodSuccessInNamespace(f.Client, podName, f.Namespace.Name)
 
 	if err != nil {
 		logs, logErr := GetPodLogs(f.Client, f.Namespace.Name, pod.Name, contName)

test/e2e/persistent_volumes.go

@@ -277,7 +277,7 @@ func testPodSuccessOrFail(f *framework.Framework, c *client.Client, ns string, p
 
 	By("Pod should terminate with exitcode 0 (success)")
 
-	err := framework.WaitForPodSuccessInNamespace(c, pod.Name, pod.Spec.Containers[0].Name, ns)
+	err := framework.WaitForPodSuccessInNamespace(c, pod.Name, ns)
 	if err != nil {
 		return fmt.Errorf("Pod %v returned non-zero exitcode: %+v", pod.Name, err)
 	}

test/e2e/volume_provisioning.go

@@ -228,7 +228,7 @@ func runInPodWithVolume(c *client.Client, ns, claimName, command string) {
 		framework.ExpectNoError(c.Pods(ns).Delete(pod.Name, nil))
 	}()
 	framework.ExpectNoError(err, "Failed to create pod: %v", err)
-	framework.ExpectNoError(framework.WaitForPodSuccessInNamespaceSlow(c, pod.Name, pod.Spec.Containers[0].Name, pod.Namespace))
+	framework.ExpectNoError(framework.WaitForPodSuccessInNamespaceSlow(c, pod.Name, pod.Namespace))
 }
 
 func newStorageClass() *storage.StorageClass {

test/e2e/volumes.go

@@ -319,7 +319,7 @@ func injectHtml(client *client.Client, config VolumeTestConfig, volume api.Volum
 
 	injectPod, err := podClient.Create(injectPod)
 	framework.ExpectNoError(err, "Failed to create injector pod: %v", err)
-	err = framework.WaitForPodSuccessInNamespace(client, injectPod.Name, injectPod.Spec.Containers[0].Name, injectPod.Namespace)
+	err = framework.WaitForPodSuccessInNamespace(client, injectPod.Name, injectPod.Namespace)
 	Expect(err).NotTo(HaveOccurred())
 }
 

test/e2e_node/cgroup_manager_test.go

@@ -68,7 +68,7 @@ var _ = framework.KubeDescribe("Kubelet Cgroup Manager [Skip]", func() {
 				}
 				podClient := f.PodClient()
 				podClient.Create(pod)
-				err := framework.WaitForPodSuccessInNamespace(f.Client, podName, contName, f.Namespace.Name)
+				err := framework.WaitForPodSuccessInNamespace(f.Client, podName, f.Namespace.Name)
 				Expect(err).NotTo(HaveOccurred())
 			})
 		})