fix: container stat test flakiness (runfinch#303)

Signed-off-by: Shubhranshu Mahapatra <shubhum@amazon.com>

Author: Shubhranshu153

e2e/tests/container_stats.go

@@ -21,6 +21,39 @@ import (
 	"github.com/runfinch/finch-daemon/e2e/client"
 )
 
+// waitForContainerRunning waits for a container to reach the running state.
+func waitForContainerRunning(uClient *http.Client, version string, containerID string, maxRetries int) bool {
+	const retryInterval = 500 * time.Millisecond
+
+	for i := 0; i < maxRetries; i++ {
+		res, err := uClient.Get(client.ConvertToFinchUrl(version, fmt.Sprintf("/containers/%s/json", containerID)))
+		if err != nil {
+			time.Sleep(retryInterval)
+			continue
+		}
+
+		if res.StatusCode != http.StatusOK {
+			time.Sleep(retryInterval)
+			continue
+		}
+
+		var container types.Container
+		err = json.NewDecoder(res.Body).Decode(&container)
+		res.Body.Close()
+		if err != nil {
+			time.Sleep(retryInterval)
+			continue
+		}
+
+		if container.State.Status == "running" {
+			return true
+		}
+
+		time.Sleep(retryInterval)
+	}
+	return false
+}
+
 // ContainerStats tests the `GET containers/{id}/stats` API.
 func ContainerStats(opt *option.Option) {
 	Describe("get container stats", func() {
@@ -30,9 +63,7 @@ func ContainerStats(opt *option.Option) {
 			wantContainerName string
 		)
 		BeforeEach(func() {
-			// create a custom client to use http over unix sockets
 			uClient = client.NewClient(GetDockerHostUrl())
-			// get the docker api version that will be tested
 			version = GetDockerApiVersion()
 			wantContainerName = fmt.Sprintf("/%s", testContainerName)
 		})
@@ -55,13 +86,14 @@ func ContainerStats(opt *option.Option) {
 				opt, "run", "-d", "--name", testContainerName, defaultImage, "sleep", "Infinity",
 			)
 
-			// get container stats
+			isRunning := waitForContainerRunning(uClient, version, cid, 10)
+			Expect(isRunning).Should(BeTrue(), "Container should be in running state before checking stats")
+
 			relativeUrl := fmt.Sprintf("/containers/%s/stats?stream=false", testContainerName)
 			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			var statsJSON types.StatsJSON
 			err = json.NewDecoder(res.Body).Decode(&statsJSON)
 			Expect(err).Should(BeNil())
@@ -72,13 +104,14 @@ func ContainerStats(opt *option.Option) {
 				opt, "run", "-d", "--name", testContainerName, defaultImage, "sleep", "Infinity",
 			)
 
-			// get container stats
+			isRunning := waitForContainerRunning(uClient, version, cid, 10)
+			Expect(isRunning).Should(BeTrue(), "Container should be in running state before checking stats")
+
 			relativeUrl := fmt.Sprintf("/containers/%s/stats?stream=false", cid)
 			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			var statsJSON types.StatsJSON
 			err = json.NewDecoder(res.Body).Decode(&statsJSON)
 			Expect(err).Should(BeNil())
@@ -89,13 +122,14 @@ func ContainerStats(opt *option.Option) {
 				opt, "run", "-d", "--name", testContainerName, defaultImage, "sleep", "Infinity",
 			)
 
-			// get container stats
+			isRunning := waitForContainerRunning(uClient, version, cid, 10)
+			Expect(isRunning).Should(BeTrue(), "Container should be in running state before checking stats")
+
 			relativeUrl := fmt.Sprintf("/containers/%s/stats?stream=false", cid[:12])
 			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			var statsJSON types.StatsJSON
 			err = json.NewDecoder(res.Body).Decode(&statsJSON)
 			Expect(err).Should(BeNil())
@@ -106,51 +140,62 @@ func ContainerStats(opt *option.Option) {
 				opt, "run", "-d", "--name", testContainerName, defaultImage, "sleep", "Infinity",
 			)
 
-			// start a routine to remove the container in 3 seconds
+			isRunning := waitForContainerRunning(uClient, version, cid, 10)
+			Expect(isRunning).Should(BeTrue(), "Container should be in running state before checking stats")
+
 			go func() {
-				time.Sleep(time.Second * 3)
+				time.Sleep(time.Second * 5)
 				command.Run(opt, "rm", "-f", testContainerName)
 			}()
 
-			// get container stats
 			relativeUrl := fmt.Sprintf("/containers/%s/stats", testContainerName)
 			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			scanner := bufio.NewScanner(res.Body)
 			num := 0
 			for scanner.Scan() {
 				var statsJSON types.StatsJSON
 				err = json.Unmarshal(scanner.Bytes(), &statsJSON)
 				Expect(err).Should(BeNil())
+				isRunning := waitForContainerRunning(uClient, version, cid, 1)
+				// confirm need to check the stats were obtained for container running state
+				if !isRunning {
+					break
+				}
 				expectValidStats(&statsJSON, wantContainerName, cid, 1)
 				num += 1
 			}
-			// should tick at least twice in 3 seconds
+
 			Expect(num).Should(BeNumerically(">", 1))
-			Expect(num).Should(BeNumerically("<", 5))
+			Expect(num).Should(BeNumerically("<", 10))
 		})
 		It("should stream stats for a stopped container", func() {
 			cid := command.StdoutStr(
 				opt, "run", "-d", "--name", testContainerName, defaultImage, "echo", "hello",
 			)
 			command.Run(opt, "wait", testContainerName)
 
-			// start a routine to remove the container in 3 seconds
+			res, err := uClient.Get(client.ConvertToFinchUrl(version, fmt.Sprintf("/containers/%s/json", cid)))
+			Expect(err).Should(BeNil())
+			Expect(res.StatusCode).Should(Equal(http.StatusOK))
+			var container types.Container
+			err = json.NewDecoder(res.Body).Decode(&container)
+			res.Body.Close()
+			Expect(err).Should(BeNil())
+			Expect(container.State.Status).ShouldNot(Equal("running"))
+
 			go func() {
-				time.Sleep(time.Second * 3)
+				time.Sleep(time.Second * 5)
 				command.Run(opt, "rm", "-f", testContainerName)
 			}()
 
-			// get container stats
 			relativeUrl := fmt.Sprintf("/containers/%s/stats", testContainerName)
-			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
+			res, err = uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			scanner := bufio.NewScanner(res.Body)
 			num := 0
 			for scanner.Scan() {
@@ -160,9 +205,9 @@ func ContainerStats(opt *option.Option) {
 				expectEmptyStats(&statsJSON, wantContainerName, cid)
 				num += 1
 			}
-			// should tick at least twice in 3 seconds
+
 			Expect(num).Should(BeNumerically(">", 1))
-			Expect(num).Should(BeNumerically("<", 5))
+			Expect(num).Should(BeNumerically("<", 10))
 		})
 		It("should stream stats when no network interface is created", func() {
 			cid := command.StdoutStr(
@@ -175,34 +220,38 @@ func ContainerStats(opt *option.Option) {
 				"sleep", "Infinity",
 			)
 
-			// start a routine to remove the container in 3 seconds
+			isRunning := waitForContainerRunning(uClient, version, cid, 10)
+			Expect(isRunning).Should(BeTrue(), "Container should be in running state before checking stats")
+
 			go func() {
-				time.Sleep(time.Second * 3)
+				time.Sleep(time.Second * 5)
 				command.Run(opt, "rm", "-f", testContainerName)
 			}()
 
-			// get container stats
 			relativeUrl := fmt.Sprintf("/containers/%s/stats", testContainerName)
 			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			scanner := bufio.NewScanner(res.Body)
 			num := 0
 			for scanner.Scan() {
 				var statsJSON types.StatsJSON
 				err = json.Unmarshal(scanner.Bytes(), &statsJSON)
 				Expect(err).Should(BeNil())
+				isRunning := waitForContainerRunning(uClient, version, cid, 1)
+				// confirm need to check the stats were obtained for container running state
+				if !isRunning {
+					break
+				}
 				expectValidStats(&statsJSON, wantContainerName, cid, 0)
 				num += 1
 			}
-			// should tick at least twice in 3 seconds
+
 			Expect(num).Should(BeNumerically(">", 1))
-			Expect(num).Should(BeNumerically("<", 5))
+			Expect(num).Should(BeNumerically("<", 10))
 		})
 		It("should stream stats with multiple network interfaces", func() {
-			// create networks and run a container with multiple networks
 			command.Run(opt, "network", "create", "net1")
 			command.Run(opt, "network", "create", "net2")
 			cid := command.StdoutStr(
@@ -216,31 +265,36 @@ func ContainerStats(opt *option.Option) {
 				"sleep", "Infinity",
 			)
 
-			// start a routine to remove the container in 3 seconds
+			isRunning := waitForContainerRunning(uClient, version, cid, 10)
+			Expect(isRunning).Should(BeTrue(), "Container should be in running state before checking stats")
+
 			go func() {
-				time.Sleep(time.Second * 3)
+				time.Sleep(time.Second * 5)
 				command.Run(opt, "rm", "-f", testContainerName)
 			}()
 
-			// get container stats
 			relativeUrl := fmt.Sprintf("/containers/%s/stats", testContainerName)
 			res, err := uClient.Get(client.ConvertToFinchUrl(version, relativeUrl))
 			Expect(err).Should(BeNil())
 			Expect(res.StatusCode).Should(Equal(http.StatusOK))
 
-			// check json contents
 			scanner := bufio.NewScanner(res.Body)
 			num := 0
 			for scanner.Scan() {
 				var statsJSON types.StatsJSON
 				err = json.Unmarshal(scanner.Bytes(), &statsJSON)
 				Expect(err).Should(BeNil())
+				isRunning := waitForContainerRunning(uClient, version, cid, 1)
+				// confirm need to check the stats were obtained for container running state
+				if !isRunning {
+					break
+				}
 				expectValidStats(&statsJSON, wantContainerName, cid, 2)
 				num += 1
 			}
-			// should tick at least twice in 3 seconds
+
 			Expect(num).Should(BeNumerically(">", 1))
-			Expect(num).Should(BeNumerically("<", 5))
+			Expect(num).Should(BeNumerically("<", 10))
 		})
 	})
 }
@@ -258,14 +312,10 @@ func expectValidStats(st *types.StatsJSON, name, id string, numNetworks int) {
 		Expect(st.Read).Should(BeTemporally("~", st.PreRead.Add(time.Second), time.Millisecond*100))
 	}
 
-	// check that number of current PIDs is > 0
 	Expect(st.PidsStats.Current).ShouldNot(BeZero())
-
-	// check that number of CPUs and system usage is > 0
 	Expect(st.CPUStats.OnlineCPUs).ShouldNot(BeZero())
 	Expect(st.CPUStats.SystemUsage).ShouldNot(BeZero())
 
-	// check that object contains networks data
 	if numNetworks == 0 {
 		Expect(st.Networks).Should(BeNil())
 	} else {
@@ -277,10 +327,8 @@ func expectValidStats(st *types.StatsJSON, name, id string, numNetworks int) {
 // expectEmptyStats ensures that the data contained in the stats object is empty
 // which is the case with containers that are not running.
 func expectEmptyStats(st *types.StatsJSON, name, id string) {
-	// verify container name and ID
 	Expect(st.Name).Should(Equal(name))
 	Expect(st.ID).Should(Equal(id))
-
 	Expect(st.Read).Should(Equal(time.Time{}))
 	Expect(st.PreRead).Should(Equal(time.Time{}))
 	Expect(st.PidsStats).Should(Equal(dockertypes.PidsStats{}))