Reduce integration test retries (#1546)

Reduce integration test retries - Exit Retry fn early if container doesn't exist on rm, stop, and kill - Shorten collector health check tick (30s -> 5s) - Small fixes for collector and jsonlabel cleanup
stackrox · Feb 12, 2024 · 2db86d1 · 2db86d1
1 parent c60e7f9
commit 2db86d1
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 34 deletions.
diff --git a/integration-tests/suites/base.go b/integration-tests/suites/base.go
@@ -30,7 +30,7 @@ const (
 
 	containerStatsName = "container-stats"
 
-	defaultWaitTickSeconds = 30 * time.Second
+	defaultWaitTickSeconds = 5 * time.Second
 )
 
 type IntegrationTestSuiteBase struct {
@@ -261,9 +261,7 @@ func (s *IntegrationTestSuiteBase) launchContainer(name string, args ...string)
 	cmd := []string{common.RuntimeCommand, "run", "-d", "--name", name}
 	cmd = append(cmd, args...)
 
-	output, err := common.Retry(func() (string, error) {
-		return s.Executor().Exec(cmd...)
-	})
+	output, err := s.Executor().Exec(cmd...)
 
 	outLines := strings.Split(output, "\n")
 	return outLines[len(outLines)-1], err
@@ -390,20 +388,20 @@ func (s *IntegrationTestSuiteBase) execContainerShellScript(containerName string
 
 func (s *IntegrationTestSuiteBase) cleanupContainers(containers ...string) {
 	for _, container := range containers {
-		s.Executor().Exec(common.RuntimeCommand, "kill", container)
-		s.Executor().Exec(common.RuntimeCommand, "rm", container)
+		s.Executor().KillContainer(container)
+		s.Executor().RemoveContainer(container)
 	}
 }
 
 func (s *IntegrationTestSuiteBase) stopContainers(containers ...string) {
 	for _, container := range containers {
-		s.Executor().Exec(common.RuntimeCommand, "stop", "-t", config.StopTimeout(), container)
+		s.Executor().StopContainer(container)
 	}
 }
 
 func (s *IntegrationTestSuiteBase) removeContainers(containers ...string) {
 	for _, container := range containers {
-		s.Executor().Exec(common.RuntimeCommand, "rm", container)
+		s.Executor().RemoveContainer(container)
 	}
 }
 

diff --git a/integration-tests/suites/common/collector_manager.go b/integration-tests/suites/common/collector_manager.go
@@ -220,8 +220,8 @@ func (c *CollectorManager) captureLogs(containerName string) (string, error) {
 }
 
 func (c *CollectorManager) killContainer(name string) error {
-	_, err1 := c.executor.Exec(RuntimeCommand, "kill", name)
-	_, err2 := c.executor.Exec(RuntimeCommand, "rm", "-fv", name)
+	_, err1 := c.executor.KillContainer(name)
+	_, err2 := c.executor.RemoveContainer(name)
 
 	var result error
 	if err1 != nil {
@@ -235,7 +235,7 @@ func (c *CollectorManager) killContainer(name string) error {
 }
 
 func (c *CollectorManager) stopContainer(name string) error {
-	_, err := c.executor.Exec(RuntimeCommand, "stop", name)
+	_, err := c.executor.StopContainer(name)
 	return err
 }
 

diff --git a/integration-tests/suites/common/executor.go b/integration-tests/suites/common/executor.go
@@ -27,8 +27,12 @@ type Executor interface {
 	ContainerExists(container string) (bool, error)
 	ExitCode(container string) (int, error)
 	Exec(args ...string) (string, error)
+	ExecWithErrorCheck(errCheckFn func(string, error) error, args ...string) (string, error)
 	ExecWithStdin(pipedContent string, args ...string) (string, error)
-	ExecRetry(args ...string) (string, error)
+	ExecWithoutRetry(args ...string) (string, error)
+	KillContainer(name string) (string, error)
+	RemoveContainer(name string) (string, error)
+	StopContainer(name string) (string, error)
 }
 
 type CommandBuilder interface {
@@ -96,7 +100,7 @@ func NewExecutor() Executor {
 	return &e
 }
 
-// Execute provided command with retries on error.
+// Exec executes the provided command with retries on non-zero error from the command.
 func (e *executor) Exec(args ...string) (string, error) {
 	if args[0] == RuntimeCommand && RuntimeAsRoot {
 		args = append([]string{"sudo"}, args...)
@@ -106,25 +110,23 @@ func (e *executor) Exec(args ...string) (string, error) {
 	})
 }
 
-func (e *executor) ExecRetry(args ...string) (res string, err error) {
-	maxAttempts := 3
-	attempt := 0
-
+// ExecWithErrorCheck executes the provided command, retrying if an error occurs
+// and the command's output does not contain any of the accepted output contents.
+func (e *executor) ExecWithErrorCheck(errCheckFn func(string, error) error, args ...string) (string, error) {
 	if args[0] == RuntimeCommand && RuntimeAsRoot {
 		args = append([]string{"sudo"}, args...)
 	}
+	return RetryWithErrorCheck(errCheckFn, func() (string, error) {
+		return e.RunCommand(e.builder.ExecCommand(args...))
+	})
+}
 
-	for attempt < maxAttempts {
-		if attempt > 0 {
-			fmt.Printf("Retrying (%v) (%d of %d) Error: %v\n", args, attempt, maxAttempts, err)
-		}
-		attempt++
-		res, err = e.RunCommand(e.builder.ExecCommand(args...))
-		if err == nil {
-			break
-		}
+// ExecWithoutRetry executes provided command once, without retries.
+func (e *executor) ExecWithoutRetry(args ...string) (string, error) {
+	if args[0] == RuntimeCommand && RuntimeAsRoot {
+		args = append([]string{"sudo"}, args...)
 	}
-	return res, err
+	return e.RunCommand(e.builder.ExecCommand(args...))
 }
 
 func (e *executor) RunCommand(cmd *exec.Cmd) (string, error) {
@@ -189,12 +191,12 @@ func (e *executor) PullImage(image string) error {
 	if err == nil {
 		return nil
 	}
-	_, err = e.ExecRetry(RuntimeCommand, "pull", image)
+	_, err = e.Exec(RuntimeCommand, "pull", image)
 	return err
 }
 
 func (e *executor) IsContainerRunning(containerID string) (bool, error) {
-	result, err := e.Exec(RuntimeCommand, "inspect", containerID, "--format='{{.State.Running}}'")
+	result, err := e.ExecWithoutRetry(RuntimeCommand, "inspect", containerID, "--format='{{.State.Running}}'")
 	if err != nil {
 		return false, err
 	}
@@ -217,6 +219,42 @@ func (e *executor) ExitCode(containerID string) (int, error) {
 	return strconv.Atoi(strings.Trim(result, "\"'"))
 }
 
+// checkContainerCommandError returns nil if the output of the container
+// command indicates retries are not needed.
+func checkContainerCommandError(name string, cmd string, output string, err error) error {
+	for _, str := range []string{
+		"no such container",
+		"cannot " + cmd + " container",
+		"can only " + cmd + " running containers",
+	} {
+		if strings.Contains(strings.ToLower(output), strings.ToLower(str)) {
+			return nil
+		}
+	}
+	return err
+}
+
+func containerErrorCheckFunction(name string, cmd string) func(string, error) error {
+	return func(stdout string, err error) error {
+		return checkContainerCommandError(name, cmd, stdout, err)
+	}
+}
+
+// KillContainer runs the kill operation on the provided container name
+func (e *executor) KillContainer(name string) (string, error) {
+	return e.ExecWithErrorCheck(containerErrorCheckFunction(name, "kill"), RuntimeCommand, "kill", name)
+}
+
+// RemoveContainer runs the remove operation on the provided container name
+func (e *executor) RemoveContainer(name string) (string, error) {
+	return e.ExecWithErrorCheck(containerErrorCheckFunction(name, "remove"), RuntimeCommand, "rm", name)
+}
+
+// StopContainer runs the stop operation on the provided container name
+func (e *executor) StopContainer(name string) (string, error) {
+	return e.ExecWithErrorCheck(containerErrorCheckFunction(name, "stop"), RuntimeCommand, "stop", name)
+}
+
 func (e *localCommandBuilder) ExecCommand(execArgs ...string) *exec.Cmd {
 	return exec.Command(execArgs[0], execArgs[1:]...)
 }

diff --git a/integration-tests/suites/common/retry.go b/integration-tests/suites/common/retry.go
@@ -10,12 +10,18 @@ const (
 )
 
 type retryable = func() (string, error)
+type errorchecker = func(string, error) error
 
 // Simple retry in loop for commands produce only one string output and error
 func Retry(f retryable) (output string, err error) {
+	return RetryWithErrorCheck(func(s string, e error) error { return e }, f)
+}
+
+// Simple retry with error checker
+func RetryWithErrorCheck(ec errorchecker, f retryable) (output string, err error) {
 	for i := 0; i < max_retries; i++ {
 		output, err = f()
-		if err == nil {
+		if ec(output, err) == nil {
 			return output, nil
 		} else if i != max_retries-1 {
 			time.Sleep(retry_wait_time)

diff --git a/integration-tests/suites/image_json.go b/integration-tests/suites/image_json.go
@@ -28,6 +28,5 @@ func (s *ImageLabelJSONTestSuite) TestRunImageWithJSONLabel() {
 }
 
 func (s *ImageLabelJSONTestSuite) TearDownSuite() {
-	s.StopCollector()
-	s.cleanupContainers("json-label")
+	s.cleanupContainers("jsonlabel")
 }
diff --git a/integration-tests/suites/process_network.go b/integration-tests/suites/process_network.go
@@ -69,8 +69,6 @@ func (s *ProcessNetworkTestSuite) SetupSuite() {
 }
 
 func (s *ProcessNetworkTestSuite) TearDownSuite() {
-	s.StopCollector()
-	s.cleanupContainers("nginx", "nginx-curl")
 	s.WritePerfResults()
 }