Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests reuse cached container even if container build for test group failed #557

Closed
jackorp opened this issue Aug 1, 2024 · 2 comments · Fixed by #560
Closed

Tests reuse cached container even if container build for test group failed #557

jackorp opened this issue Aug 1, 2024 · 2 comments · Fixed by #560
Assignees

Comments

@jackorp
Copy link
Contributor

jackorp commented Aug 1, 2024

Container platform

Podman/Docker

Version

All in regular container CI, OCP tests maybe also, but I haven't investigated how the tests look there.

OS version of the container image

RHEL 7, RHEL 8, RHEL 9, CentOS 7, CentOS Stream 8, CentOS Stream 9, Fedora

Bugzilla, Jira

No response

Description

I have put all OS version to the container image, since this behavior probably applies to any and all where the CI code is run.

When running tests the main test loop builds container image and then runs tests on that image. AFAICT the resulting image has constant name and it is not deleted after running all related tests, therefore there can be a situation of container re-use.

This would mean that a container is build for test set A, those might pass,
an attempt at build for test set B fails, but since the container is still in the
local image registry it might re-use container from test set A.

This might be one of the reasons of a silent failure as observed in #554

1 idea is using non-colliding container names/tags for use in the relevant tests. Or after-test cleanup of such resources. Or a combination.

Reproducer

Run tests, but make the build of some container fail, but probably make sure the first container build passes to make sure there is some cached image that will be used in place of the used container.

Similarly as in issue #554

@jackorp
Copy link
Contributor Author

jackorp commented Aug 1, 2024

Discussed on the PR of PR 556 that fixes issue #554 : #556 (comment)

@SlouchyButton SlouchyButton self-assigned this Aug 1, 2024
@SlouchyButton
Copy link
Contributor

SlouchyButton commented Aug 9, 2024

The images from builds across different test apps aren't cleared as we thought between the individual test apps.
When build fails for one of the test apps there will be a new untagged image present (from the failed build it seems? But that also is weird and needs further investigation) and unmodified tagged image from previous test app build.

The tag 0-testapp is used across all test apps like rack, puma and db, so when the build for specific test app fails, the previous built image is not overwritten like in case of a successful build and started for tests. Since it seems we are not testing specific functionality of the individual test apps, all tests will pass.

When doing a fix for #554 and getting the code from python containers, I noticed some code in tests that should do clearing between tests. I am not sure yet whether that could also be copied, but will investigate there further.

Other possibility for a fix is using different images for each test app, so not clearing them wouldn't be an issue as they couldn't be reused then.

It would also be worth checking whether this issue is also present in other containers.

SlouchyButton added a commit to SlouchyButton/s2i-ruby-container that referenced this issue Aug 28, 2024
SlouchyButton added a commit to SlouchyButton/s2i-ruby-container that referenced this issue Aug 30, 2024
Cleanup function wasn't properly cleaning all the folders after test run.
This resulted in unstaged changes after running tests.
One of the cleaning parts were not in the for each loop by mistake.

Adds clearing code for container images that normally would stay between
test runs and would result in the possibility of tests reusing the image
from previous test.

Fixes: sclorg#557
SlouchyButton added a commit to SlouchyButton/s2i-ruby-container that referenced this issue Aug 30, 2024
Cleanup function wasn't properly cleaning all the folders after test run.
This resulted in unstaged changes after running tests.
One of the cleaning parts were conditional despite creation of deleted
folder was not. Folder is unconditionally created on each test run, but
would have been cleaned (if the cleaning code worked in a first place)
only in certain conditions. Cleanup was put at the end of the test loop
without additional condition.

Adds clearing code for container images that normally would stay between
test runs and would result in the possibility of tests reusing the image
from previous test.

Fixes: sclorg#557
@phracek phracek closed this as completed in adfd752 Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants