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

docker: add ownership regression test #462

Merged
merged 1 commit into from
Oct 24, 2023
Merged

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Oct 9, 2023

This prevent regression like the one reported in flatcar/Flatcar#1203 to occur again.

Testing done

It fails, as expected, on Stable with:

$ kola run --qemu-image ./stable/flatcar_production_qemu_image.img docker.base
=== RUN   docker.base
=== RUN   docker.base/ownership
--- FAIL: docker.base (38.08s)
...
            docker.go:721: ownership should be conserved between run and export of Docker images. Got: 0/42 and 0/0
FAIL, output in _kola_temp/qemu-2023-10-09-1956-136705

Pass on Alpha (and certainly Beta):

$ kola run --qemu-image ./alpha/flatcar_production_qemu_image.img docker.base
=== RUN   docker.base
=== RUN   docker.base/ownership
--- PASS: docker.base (29.75s)
    --- PASS: docker.base/ownership (10.82s)
...
PASS, output in _kola_temp/qemu-2023-10-09-1956-136583

Signed-off-by: Mathieu Tortuyaux <[email protected]>
// that causes ownernship being lost when exporting a Docker
// container.
// https://github.com/flatcar/Flatcar/issues/1203
func dockerOwnership(c cluster.TestCluster) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a large fan of regression tests for mainly upstream issues, specially when they are so specific that they probably won't happen exactly this way again but different. Anyway, if this function won't take long to execute it's ok to have it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get your point. By doing this we can prevent this regression again (even if there is a small probability that it occurs again as you said) but we can also catch other issues caused by this scenario.
The test only runs on qemu as part of the docker base tests, the only "long" action is the pulling of the nginx image, the rest is super fast.

Copy link
Member

@t-lo t-lo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @tormath1!

@tormath1 tormath1 merged commit da11b59 into flatcar-master Oct 24, 2023
2 checks passed
@tormath1 tormath1 deleted the tormath1/docker branch October 24, 2023 08:38
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.

3 participants