Merge pull request #26856 from Random-Liu/flakeready

Automatic merge from submit-queue

shorten e2e_node test, fix pod ready test issue

Fix #25703, #25812.
I fixed https://github.com/kubernetes/kubernetes/pull/26070 and open this PR.

@liangchenye 

[![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/.github/PULL_REQUEST_TEMPLATE.md?pixel)]()

test/e2e_node/container.go

@@ -110,17 +110,32 @@ func (cc *ConformanceContainer) Get() (ConformanceContainer, error) {
 	return ConformanceContainer{containers[0], cc.Client, pod.Spec.RestartPolicy, pod.Spec.Volumes, pod.Spec.NodeName, cc.Namespace, cc.podName}, nil
 }
 
-func (cc *ConformanceContainer) GetStatus() (api.ContainerStatus, api.PodPhase, error) {
+func (cc *ConformanceContainer) IsReady() (bool, error) {
 	pod, err := cc.Client.Pods(cc.Namespace).Get(cc.podName)
 	if err != nil {
-		return api.ContainerStatus{}, api.PodUnknown, err
+		return false, err
 	}
+	return api.IsPodReady(pod), nil
+}
 
-	statuses := pod.Status.ContainerStatuses
+func (cc *ConformanceContainer) GetPhase() (api.PodPhase, error) {
-	if len(statuses) != 1 {
+	pod, err := cc.Client.Pods(cc.Namespace).Get(cc.podName)
-		return api.ContainerStatus{}, api.PodUnknown, errors.New("Failed to get container status")
+	if err != nil {
+		return api.PodUnknown, err
 	}
-	return statuses[0], pod.Status.Phase, nil
+	return pod.Status.Phase, nil
+}
+
+func (cc *ConformanceContainer) GetStatus() (api.ContainerStatus, error) {
+	pod, err := cc.Client.Pods(cc.Namespace).Get(cc.podName)
+	if err != nil {
+		return api.ContainerStatus{}, err
+	}
+	statuses := pod.Status.ContainerStatuses
+	if len(statuses) != 1 || statuses[0].Name != cc.Container.Name {
+		return api.ContainerStatus{}, fmt.Errorf("unexpected container statuses %v", statuses)
+	}
+	return statuses[0], nil
 }
 
 func (cc *ConformanceContainer) Present() (bool, error) {
@@ -134,10 +149,10 @@ func (cc *ConformanceContainer) Present() (bool, error) {
 	return false, err
 }
 
-type ContainerState uint32
+type ContainerState int
 
 const (
-	ContainerStateWaiting ContainerState = 1 << iota
+	ContainerStateWaiting ContainerState = iota
 	ContainerStateRunning
 	ContainerStateTerminated
 	ContainerStateUnknown

test/e2e_node/runtime_conformance_test.go

@@ -37,7 +37,7 @@ const (
 	pollInterval           = time.Second * 5
 )
 
-type testStatus struct {
+type testCase struct {
 	Name             string
 	RestartPolicy    api.RestartPolicy
 	Phase            api.PodPhase
@@ -47,7 +47,7 @@ type testStatus struct {
 	Ready            bool
 }
 
-var _ = Describe("[FLAKY] Container runtime Conformance Test", func() {
+var _ = Describe("Container Runtime Conformance Test", func() {
 	var cl *client.Client
 
 	BeforeEach(func() {
@@ -56,28 +56,24 @@ var _ = Describe("[FLAKY] Container runtime Conformance Test", func() {
 	})
 
 	Describe("container runtime conformance blackbox test", func() {
-		var testCContainers []ConformanceContainer
 		namespace := "runtime-conformance"
 
-		BeforeEach(func() {
+		Context("when starting a container that exits", func() {
-			testCContainers = []ConformanceContainer{}
-		})
-
-		Context("when start a container that exits successfully", func() {
 			It("it should run with the expected status [Conformance]", func() {
+				restartCountVolumeName := "restart-count"
+				restartCountVolumePath := "/restart-count"
 				testContainer := api.Container{
 					Image: ImageRegistry[busyBoxImage],
 					VolumeMounts: []api.VolumeMount{
 						{
-							MountPath: "/restart-count",
+							MountPath: restartCountVolumePath,
-							Name:      "restart-count",
+							Name:      restartCountVolumeName,
 						},
 					},
-					ImagePullPolicy: api.PullIfNotPresent,
 				}
 				testVolumes := []api.Volume{
 					{
-						Name: "restart-count",
+						Name: restartCountVolumeName,
 						VolumeSource: api.VolumeSource{
 							HostPath: &api.HostPathVolumeSource{
 								Path: os.TempDir(),
@@ -85,171 +81,63 @@ var _ = Describe("[FLAKY] Container runtime Conformance Test", func() {
 						},
 					},
 				}
-				testCount := int32(3)
+				testCases := []testCase{
-				testStatuses := []testStatus{
+					{"terminate-cmd-rpa", api.RestartPolicyAlways, api.PodRunning, ContainerStateRunning, "==", 2, true},
-					{"terminate-cmd-rpa", api.RestartPolicyAlways, api.PodRunning, ContainerStateWaiting | ContainerStateRunning | ContainerStateTerminated, ">", testCount, false},
+					{"terminate-cmd-rpof", api.RestartPolicyOnFailure, api.PodSucceeded, ContainerStateTerminated, "==", 1, false},
-					{"terminate-cmd-rpof", api.RestartPolicyOnFailure, api.PodSucceeded, ContainerStateTerminated, "==", testCount, false},
+					{"terminate-cmd-rpn", api.RestartPolicyNever, api.PodFailed, ContainerStateTerminated, "==", 0, false},
-					{"terminate-cmd-rpn", api.RestartPolicyNever, api.PodSucceeded, ContainerStateTerminated, "==", 0, false},
 				}
-
+				for _, testCase := range testCases {
-				for _, testStatus := range testStatuses {
 					tmpFile, err := ioutil.TempFile("", "restartCount")
 					Expect(err).NotTo(HaveOccurred())
 					defer os.Remove(tmpFile.Name())
 
-					// It fails in the first three runs and succeeds after that.
+					// It failed at the 1st run, then succeeded at 2nd run, then run forever
-					tmpCmd := fmt.Sprintf("echo 'hello' >> /restart-count/%s ; test $(wc -l /restart-count/%s| awk {'print $1'}) -ge %d", path.Base(tmpFile.Name()), path.Base(tmpFile.Name()), testCount+1)
+					cmdScripts := `
-					testContainer.Name = testStatus.Name
+f=%s
+count=$(echo 'hello' >> $f ; wc -l $f | awk {'print $1'})
+if [ $count -eq 1 ]; then
+	exit 1
+fi
+if [ $count -eq 2 ]; then
+	exit 0
+fi
+while true; do sleep 1; done
+`
+					tmpCmd := fmt.Sprintf(cmdScripts, path.Join(restartCountVolumePath, path.Base(tmpFile.Name())))
+					testContainer.Name = testCase.Name
 					testContainer.Command = []string{"sh", "-c", tmpCmd}
 					terminateContainer := ConformanceContainer{
 						Container:     testContainer,
 						Client:        cl,
-						RestartPolicy: testStatus.RestartPolicy,
+						RestartPolicy: testCase.RestartPolicy,
 						Volumes:       testVolumes,
 						NodeName:      *nodeName,
 						Namespace:     namespace,
 					}
-					err = terminateContainer.Create()
+					Expect(terminateContainer.Create()).To(Succeed())
-					Expect(err).NotTo(HaveOccurred())
+					defer terminateContainer.Delete()
-					testCContainers = append(testCContainers, terminateContainer)
-
-					Eventually(func() api.PodPhase {
-						_, phase, _ := terminateContainer.GetStatus()
-						return phase
-					}, retryTimeout, pollInterval).ShouldNot(Equal(api.PodPending))
-
-					var status api.ContainerStatus
-					By("it should get the expected 'RestartCount'")
-					Eventually(func() int32 {
-						status, _, _ = terminateContainer.GetStatus()
-						return status.RestartCount
-					}, retryTimeout, pollInterval).Should(BeNumerically(testStatus.RestartCountOper, testStatus.RestartCount))
-
-					By("it should get the expected 'Ready' status")
-					Expect(status.Ready).To(Equal(testStatus.Ready))
-
-					By("it should get the expected 'State'")
-					Expect(GetContainerState(status.State) & testStatus.State).NotTo(Equal(0))
-
-					By("it should be possible to delete [Conformance]")
-					err = terminateContainer.Delete()
-					Expect(err).NotTo(HaveOccurred())
-					Eventually(func() bool {
-						isPresent, err := terminateContainer.Present()
-						return err == nil && !isPresent
-					}, retryTimeout, pollInterval).Should(BeTrue())
-				}
-			})
-		})
-
-		Context("when start a container that keeps running", func() {
-			It("it should run with the expected status [Conformance]", func() {
-				testContainer := api.Container{
-					Image:           ImageRegistry[busyBoxImage],
-					Command:         []string{"sh", "-c", "while true; do echo hello; sleep 1; done"},
-					ImagePullPolicy: api.PullIfNotPresent,
-				}
-				testStatuses := []testStatus{
-					{"loop-cmd-rpa", api.RestartPolicyAlways, api.PodRunning, ContainerStateRunning, "==", 0, true},
-					{"loop-cmd-rpof", api.RestartPolicyOnFailure, api.PodRunning, ContainerStateRunning, "==", 0, true},
-					{"loop-cmd-rpn", api.RestartPolicyNever, api.PodRunning, ContainerStateRunning, "==", 0, true},
-				}
-				for _, testStatus := range testStatuses {
-					testContainer.Name = testStatus.Name
-					runningContainer := ConformanceContainer{
-						Container:     testContainer,
-						Client:        cl,
-						RestartPolicy: testStatus.RestartPolicy,
-						NodeName:      *nodeName,
-						Namespace:     namespace,
-					}
-					err := runningContainer.Create()
-					Expect(err).NotTo(HaveOccurred())
-					testCContainers = append(testCContainers, runningContainer)
-
-					Eventually(func() api.PodPhase {
-						_, phase, _ := runningContainer.GetStatus()
-						return phase
-					}, retryTimeout, pollInterval).Should(Equal(api.PodRunning))
-
-					var status api.ContainerStatus
-					var phase api.PodPhase
-					Consistently(func() api.PodPhase {
-						status, phase, err = runningContainer.GetStatus()
-						return phase
-					}, consistentCheckTimeout, pollInterval).Should(Equal(testStatus.Phase))
-					Expect(err).NotTo(HaveOccurred())
 
 					By("it should get the expected 'RestartCount'")
-					Expect(status.RestartCount).To(BeNumerically(testStatus.RestartCountOper, testStatus.RestartCount))
+					Eventually(func() (int32, error) {
+						status, err := terminateContainer.GetStatus()
+						return status.RestartCount, err
+					}, retryTimeout, pollInterval).Should(BeNumerically(testCase.RestartCountOper, testCase.RestartCount))
 
-					By("it should get the expected 'Ready' status")
+					By("it should get the expected 'Phase'")
-					Expect(status.Ready).To(Equal(testStatus.Ready))
+					Eventually(terminateContainer.GetPhase, retryTimeout, pollInterval).Should(Equal(testCase.Phase))
+
+					By("it should get the expected 'Ready' condition")
+					Expect(terminateContainer.IsReady()).Should(Equal(testCase.Ready))
+
+					status, err := terminateContainer.GetStatus()
+					Expect(err).ShouldNot(HaveOccurred())
 
 					By("it should get the expected 'State'")
-					Expect(GetContainerState(status.State) & testStatus.State).NotTo(Equal(0))
+					Expect(GetContainerState(status.State)).To(Equal(testCase.State))
 
 					By("it should be possible to delete [Conformance]")
-					err = runningContainer.Delete()
+					Expect(terminateContainer.Delete()).To(Succeed())
-					Expect(err).NotTo(HaveOccurred())
+					Eventually(terminateContainer.Present, retryTimeout, pollInterval).Should(BeFalse())
-					Eventually(func() bool {
-						isPresent, err := runningContainer.Present()
-						return err == nil && !isPresent
-					}, retryTimeout, pollInterval).Should(BeTrue())
-				}
-			})
-		})
-
-		Context("when start a container that exits failure", func() {
-			It("it should run with the expected status [Conformance]", func() {
-				testContainer := api.Container{
-					Image:           ImageRegistry[busyBoxImage],
-					Command:         []string{"false"},
-					ImagePullPolicy: api.PullIfNotPresent,
-				}
-				testStatuses := []testStatus{
-					{"fail-cmd-rpa", api.RestartPolicyAlways, api.PodRunning, ContainerStateWaiting | ContainerStateRunning | ContainerStateTerminated, ">", 0, false},
-					{"fail-cmd-rpof", api.RestartPolicyOnFailure, api.PodRunning, ContainerStateTerminated, ">", 0, false},
-					{"fail-cmd-rpn", api.RestartPolicyNever, api.PodFailed, ContainerStateTerminated, "==", 0, false},
-				}
-				for _, testStatus := range testStatuses {
-					testContainer.Name = testStatus.Name
-					failureContainer := ConformanceContainer{
-						Container:     testContainer,
-						Client:        cl,
-						RestartPolicy: testStatus.RestartPolicy,
-						NodeName:      *nodeName,
-						Namespace:     namespace,
-					}
-					err := failureContainer.Create()
-					Expect(err).NotTo(HaveOccurred())
-					testCContainers = append(testCContainers, failureContainer)
-
-					Eventually(func() api.PodPhase {
-						_, phase, _ := failureContainer.GetStatus()
-						return phase
-					}, retryTimeout, pollInterval).ShouldNot(Equal(api.PodPending))
-
-					var status api.ContainerStatus
-					By("it should get the expected 'RestartCount'")
-					Eventually(func() int32 {
-						status, _, _ = failureContainer.GetStatus()
-						return status.RestartCount
-					}, retryTimeout, pollInterval).Should(BeNumerically(testStatus.RestartCountOper, testStatus.RestartCount))
-
-					By("it should get the expected 'Ready' status")
-					Expect(status.Ready).To(Equal(testStatus.Ready))
-
-					By("it should get the expected 'State'")
-					Expect(GetContainerState(status.State) & testStatus.State).NotTo(Equal(0))
-
-					By("it should be possible to delete [Conformance]")
-					err = failureContainer.Delete()
-					Expect(err).NotTo(HaveOccurred())
-					Eventually(func() bool {
-						isPresent, err := failureContainer.Present()
-						return err == nil && !isPresent
-					}, retryTimeout, pollInterval).Should(BeTrue())
 				}
 			})
 		})
@@ -257,58 +145,40 @@ var _ = Describe("[FLAKY] Container runtime Conformance Test", func() {
 		Context("when running a container with invalid image", func() {
 			It("it should run with the expected status [Conformance]", func() {
 				testContainer := api.Container{
-					Image:           "foo.com/foo/foo",
+					Image:   "foo.com/foo/foo",
-					Command:         []string{"false"},
+					Command: []string{"false"},
-					ImagePullPolicy: api.PullIfNotPresent,
 				}
-				testStatus := testStatus{"invalid-image-rpa", api.RestartPolicyAlways, api.PodPending, ContainerStateWaiting, "==", 0, false}
+				testCase := testCase{"invalid-image-rpa", api.RestartPolicyAlways, api.PodPending, ContainerStateWaiting, "==", 0, false}
-				testContainer.Name = testStatus.Name
+				testContainer.Name = testCase.Name
 				invalidImageContainer := ConformanceContainer{
 					Container:     testContainer,
 					Client:        cl,
-					RestartPolicy: testStatus.RestartPolicy,
+					RestartPolicy: testCase.RestartPolicy,
 					NodeName:      *nodeName,
 					Namespace:     namespace,
 				}
-				err := invalidImageContainer.Create()
+				Expect(invalidImageContainer.Create()).To(Succeed())
-				Expect(err).NotTo(HaveOccurred())
+				defer invalidImageContainer.Delete()
-				testCContainers = append(testCContainers, invalidImageContainer)
 
-				var status api.ContainerStatus
+				Eventually(invalidImageContainer.GetPhase, retryTimeout, pollInterval).Should(Equal(testCase.Phase))
-				var phase api.PodPhase
+				Consistently(invalidImageContainer.GetPhase, consistentCheckTimeout, pollInterval).Should(Equal(testCase.Phase))
 
-				Consistently(func() api.PodPhase {
+				status, err := invalidImageContainer.GetStatus()
-					if status, phase, err = invalidImageContainer.GetStatus(); err != nil {
-						return api.PodPending
-					} else {
-						return phase
-					}
-				}, consistentCheckTimeout, pollInterval).Should(Equal(testStatus.Phase))
 				Expect(err).NotTo(HaveOccurred())
 
 				By("it should get the expected 'RestartCount'")
-				Expect(status.RestartCount).To(BeNumerically(testStatus.RestartCountOper, testStatus.RestartCount))
+				Expect(status.RestartCount).To(BeNumerically(testCase.RestartCountOper, testCase.RestartCount))
 
 				By("it should get the expected 'Ready' status")
-				Expect(status.Ready).To(Equal(testStatus.Ready))
+				Expect(status.Ready).To(Equal(testCase.Ready))
 
 				By("it should get the expected 'State'")
-				Expect(GetContainerState(status.State) & testStatus.State).NotTo(Equal(0))
+				Expect(GetContainerState(status.State)).To(Equal(testCase.State))
 
 				By("it should be possible to delete [Conformance]")
-				err = invalidImageContainer.Delete()
+				Expect(invalidImageContainer.Delete()).To(Succeed())
-				Expect(err).NotTo(HaveOccurred())
+				Eventually(invalidImageContainer.Present, retryTimeout, pollInterval).Should(BeFalse())
-				Eventually(func() bool {
-					isPresent, err := invalidImageContainer.Present()
-					return err == nil && !isPresent
-				}, retryTimeout, pollInterval).Should(BeTrue())
 			})
 		})
-
-		AfterEach(func() {
-			for _, cc := range testCContainers {
-				cc.Delete()
-			}
-		})
 	})
 })