Skip to content

inttests: kill test runner containers faster#61

Merged
chmeliik merged 1 commit intokonflux-ci:mainfrom
chmeliik:kill-test-container-faster
Feb 16, 2026
Merged

inttests: kill test runner containers faster#61
chmeliik merged 1 commit intokonflux-ci:mainfrom
chmeliik:kill-test-container-faster

Conversation

@chmeliik
Copy link
Copy Markdown
Contributor

Previously, each test scenario would wait 10 seconds to remove the test container. This is because the test containers use 'sleep' as the entrypoint. Sleep doesn't have SIGTERM handling, so when it runs as PID 1, SIGTERM doesn't do anything and the container engine resorts to SIGKILL after 10 seconds.

Kill test runner containers immediately (without the 10 second wait) if they use --entrypoint sleep. This greatly speeds up integration tests.

@chmeliik chmeliik requested a review from a team as a code owner February 11, 2026 14:48
@chmeliik
Copy link
Copy Markdown
Contributor Author

chmeliik commented Feb 11, 2026

When running

go test ./integration_tests/build_test.go -v

On my machine it takes 105 seconds in main vs 15 seconds with this change

@qodo-code-review
Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. --time breaks docker rm 📘 Rule violation ⛯ Reliability
Description
Delete() appends the --time flag whenever killTimeout is set, but containerTool may be
docker where rm does not support --time. This can cause container deletion failures and
flaky/slow integration tests due to leftover containers.
Code

integration_tests/framework/runner_container.go[R271-276]

func (c *TestRunnerContainer) Delete() error {
-	stdout, stderr, _, err := c.executor.Execute(containerTool, "rm", "-f", c.name)
+	args := []string{"rm", "-f", c.name}
+	if c.killTimeout != nil {
+		args = append(args, "--time", strconv.Itoa(*c.killTimeout))
+	}
+	stdout, stderr, _, err := c.executor.Execute(containerTool, args...)
Evidence
Compliance requires handling edge cases and external dependency differences; the code now
conditionally adds a tool-specific flag without checking which container engine is in use, and the
framework explicitly supports both podman and docker.

Rule 3: Generic: Robust Error Handling and Edge Case Management
integration_tests/framework/runner_container.go[271-276]
integration_tests/framework/runner_container.go[235-240]
integration_tests/framework/common.go[52-59]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Delete()` appends `--time` to `rm` when `killTimeout` is set. The framework supports both `podman` and `docker`, but `docker rm` does not support `--time`, so deletions may fail.

## Issue Context
`Start()` sets `killTimeout` to `0` when `ReplaceEntrypoint` is enabled, meaning the new `--time` behavior is exercised frequently.

## Fix Focus Areas
- integration_tests/framework/runner_container.go[235-240]
- integration_tests/framework/runner_container.go[271-276]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@chmeliik chmeliik force-pushed the kill-test-container-faster branch from a3c0a33 to c0f36d0 Compare February 11, 2026 14:58
@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Feb 11, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Previously, each test scenario would wait 10 seconds to remove the test
container. This is because the test containers use 'sleep' as the
entrypoint. Sleep doesn't have SIGTERM handling, so when it runs as
PID 1, SIGTERM doesn't do anything and the container engine resorts to
SIGKILL after 10 seconds.

Kill test runner containers immediately (without the 10 second wait) to
speed up tests and to make it more consistent between podman and docker
(docker sends SIGKILL immediately).

Signed-off-by: Adam Cmiel <acmiel@redhat.com>
@chmeliik chmeliik force-pushed the kill-test-container-faster branch from c0f36d0 to fcdab01 Compare February 16, 2026 12:30
@chmeliik chmeliik merged commit 2c90cff into konflux-ci:main Feb 16, 2026
7 checks passed
@chmeliik chmeliik deleted the kill-test-container-faster branch February 16, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants