From 04c6d9324ea098a7bed79d20879f8f30593b86ea Mon Sep 17 00:00:00 2001 From: Tsubasa Nagasawa Date: Sat, 12 Oct 2024 14:47:37 +0900 Subject: [PATCH] Check for restarts without being affected by container startup order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test for checking container restarts in a Pod with restartable-init-1 and regular-1 is flaky. Right now, when we check if restartable-init-1 has restarted, we see if it hasn’t written the "Started" log after regular-1 has written its "Started" log. But even though the startup sequence starts with restartable-init-1 and then regular-1, there’s no guarantee they’ll finish starting up in that order. Sometimes regular-1 finishes first and writes its "Started" log before restartable-init-1. 1. restartable-init-1 Starting 2. regular-1 Starting 3. regular-1 Started 4. restartable-init-1 Started In this test, the startup order doesn’t really matter; all we need to check is if restartable-init-1 restarted. So I changed the test to simply look for more than one "Starting" log in restartable-init-1's logs. There were other places that used the same helper function DoesntStartAfter, so replaced those as well and deleted the helper function. --- .../container_lifecycle_pod_construction.go | 18 ------------------ test/e2e_node/container_lifecycle_test.go | 6 +++--- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/test/e2e_node/container_lifecycle_pod_construction.go b/test/e2e_node/container_lifecycle_pod_construction.go index b8220919c49..9c69d7254ba 100644 --- a/test/e2e_node/container_lifecycle_pod_construction.go +++ b/test/e2e_node/container_lifecycle_pod_construction.go @@ -179,24 +179,6 @@ func (o containerOutputList) StartsBefore(lhs, rhs string) error { return nil } -// DoesntStartAfter returns an error if lhs started after rhs -func (o containerOutputList) DoesntStartAfter(lhs, rhs string) error { - rhsStart := o.findIndex(rhs, "Starting", 0) - - if rhsStart == -1 { - return fmt.Errorf("couldn't find that %s ever started, got\n%v", rhs, o) - } - - // this works even for the same names (restart case) - lhsStart := o.findIndex(lhs, "Started", rhsStart+1) - - if lhsStart != -1 { - return fmt.Errorf("expected %s to not start after %s, got\n%v", lhs, rhs, o) - } - - return nil -} - // ExitsBefore returns an error if lhs did not end before rhs func (o containerOutputList) ExitsBefore(lhs, rhs string) error { lhsExit := o.findIndex(lhs, "Exiting", 0) diff --git a/test/e2e_node/container_lifecycle_test.go b/test/e2e_node/container_lifecycle_test.go index 0c6150f9c61..5ed99eabb25 100644 --- a/test/e2e_node/container_lifecycle_test.go +++ b/test/e2e_node/container_lifecycle_test.go @@ -1631,7 +1631,7 @@ var _ = SIGDescribe(nodefeature.SidecarContainers, "Containers Lifecycle", func( results = parseOutput(context.TODO(), f, podSpec) }) ginkgo.It("should not restart a restartable init container", func() { - framework.ExpectNoError(results.DoesntStartAfter(restartableInit1, regular1)) + framework.ExpectNoError(results.HasNotRestarted(restartableInit1)) }) ginkgo.It("should run a regular container to completion", func() { framework.ExpectNoError(results.Exits(regular1)) @@ -2033,7 +2033,7 @@ var _ = SIGDescribe(nodefeature.SidecarContainers, "Containers Lifecycle", func( results = parseOutput(context.TODO(), f, podSpec) }) ginkgo.It("should not restart a restartable init container", func() { - framework.ExpectNoError(results.DoesntStartAfter(restartableInit1, regular1)) + framework.ExpectNoError(results.HasNotRestarted(restartableInit1)) }) ginkgo.It("should run a regular container to completion", func() { framework.ExpectNoError(results.Exits(regular1)) @@ -2454,7 +2454,7 @@ var _ = SIGDescribe(nodefeature.SidecarContainers, "Containers Lifecycle", func( }) ginkgo.It("should not restart a restartable init container", func() { - framework.ExpectNoError(results.DoesntStartAfter(restartableInit1, regular1)) + framework.ExpectNoError(results.HasNotRestarted(restartableInit1)) }) // this test case is different from restartPolicy=Never ginkgo.It("should start a regular container", func() {