Skip to content

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Dec 5, 2023

Isolate the prune effects by running the test in a separate daemon.
This minimizes the impact of/on other integration tests.

- How to verify it

- Description for the changelog
TestPruneDontDeleteUsedDangling in CI

- A picture of a cute animal (not mandatory but encouraged)

@vvoland vvoland added status/2-code-review area/testing kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Dec 5, 2023
@vvoland vvoland added this to the 25.0.0 milestone Dec 5, 2023
@vvoland vvoland self-assigned this Dec 5, 2023
@vvoland vvoland force-pushed the TestPruneDontDeleteUsedDangling-separate-daemon branch 3 times, most recently from 2ed437b to 150963b Compare December 7, 2023 10:31
@vvoland vvoland marked this pull request as ready for review December 7, 2023 10:31
@vvoland vvoland force-pushed the TestPruneDontDeleteUsedDangling-separate-daemon branch from 150963b to a9e0daf Compare December 7, 2023 11:15
@vvoland vvoland changed the title integration/prune: Run separate daemon integration/prune: Run in a separate daemon Dec 7, 2023
@vvoland vvoland force-pushed the TestPruneDontDeleteUsedDangling-separate-daemon branch from a9e0daf to 036735b Compare December 7, 2023 11:19

for _, deleted := range pruned.ImagesDeleted {
if strings.Contains(deleted.Deleted, danglingID) || strings.Contains(deleted.Untagged, danglingID) {
t.Fatalf("used dangling image %s shouldn't be deleted", danglingID)
Copy link
Member

Choose a reason for hiding this comment

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

This one could probably be t.Errorf (as we're looping over a range?) or do we want to fail fast always?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Either is fine I think. Changed to Errorf. Thanks!

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, but left one minor comment

Isolate the prune effects by running the test in a separate daemon.
This minimizes the impact of/on other integration tests.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the TestPruneDontDeleteUsedDangling-separate-daemon branch from 036735b to eaaf1ea Compare December 7, 2023 16:33
@thaJeztah thaJeztah merged commit 659e7b5 into moby:master Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs status/4-merge
Projects
Development

Successfully merging this pull request may close these issues.

2 participants