Browse code
c8d/prune: Keep deletion order stable
When untagging multiple images targetting the same digest, delete the
images in lexographic order to be consistent with graphdrivers.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Paweł Gronowski authored on 2024/09/11 21:43:09Showing 2 changed files
| ... | ... |
@@ -2,6 +2,7 @@ package containerd |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 | 4 |
"context" |
| 5 |
+ "sort" |
|
| 5 | 6 |
"strings" |
| 6 | 7 |
|
| 7 | 8 |
containerdimages "github.com/containerd/containerd/images" |
| ... | ... |
@@ -107,8 +108,16 @@ func (i *ImageService) pruneUnused(ctx context.Context, filterFunc imageFilterFu |
| 107 | 107 |
|
| 108 | 108 |
usedDigests := filterImagesUsedByContainers(ctx, i.containers.List(), imagesToPrune) |
| 109 | 109 |
|
| 110 |
+ // Sort images by name to make the behavior deterministic and consistent with graphdrivers. |
|
| 111 |
+ sorted := make([]string, 0, len(imagesToPrune)) |
|
| 112 |
+ for name := range imagesToPrune {
|
|
| 113 |
+ sorted = append(sorted, name) |
|
| 114 |
+ } |
|
| 115 |
+ sort.Strings(sorted) |
|
| 116 |
+ |
|
| 110 | 117 |
// Make sure we don't delete the last image of a particular digest used by any container. |
| 111 |
- for name, img := range imagesToPrune {
|
|
| 118 |
+ for _, name := range sorted {
|
|
| 119 |
+ img := imagesToPrune[name] |
|
| 112 | 120 |
dgst := img.Target.Digest |
| 113 | 121 |
|
| 114 | 122 |
if digestRefCount[dgst] > 1 {
|
| ... | ... |
@@ -53,6 +53,51 @@ func TestPruneDontDeleteUsedDangling(t *testing.T) {
|
| 53 | 53 |
assert.NilError(t, err, "Test dangling image should still exist") |
| 54 | 54 |
} |
| 55 | 55 |
|
| 56 |
+func TestPruneLexographicalOrder(t *testing.T) {
|
|
| 57 |
+ skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot start multiple daemons on windows") |
|
| 58 |
+ skip.If(t, testEnv.IsRemoteDaemon, "cannot run daemon when remote daemon") |
|
| 59 |
+ |
|
| 60 |
+ ctx := setupTest(t) |
|
| 61 |
+ |
|
| 62 |
+ d := daemon.New(t) |
|
| 63 |
+ d.Start(t) |
|
| 64 |
+ defer d.Stop(t) |
|
| 65 |
+ |
|
| 66 |
+ apiClient := d.NewClientT(t) |
|
| 67 |
+ defer apiClient.Close() |
|
| 68 |
+ |
|
| 69 |
+ d.LoadBusybox(ctx, t) |
|
| 70 |
+ |
|
| 71 |
+ inspect, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox:latest") |
|
| 72 |
+ assert.NilError(t, err) |
|
| 73 |
+ |
|
| 74 |
+ id := inspect.ID |
|
| 75 |
+ |
|
| 76 |
+ var tags = []string{"h", "a", "j", "o", "s", "q", "w", "e", "r", "t"}
|
|
| 77 |
+ for _, tag := range tags {
|
|
| 78 |
+ err = apiClient.ImageTag(ctx, id, "busybox:"+tag) |
|
| 79 |
+ assert.NilError(t, err) |
|
| 80 |
+ } |
|
| 81 |
+ err = apiClient.ImageTag(ctx, id, "busybox:z") |
|
| 82 |
+ assert.NilError(t, err) |
|
| 83 |
+ |
|
| 84 |
+ _, err = apiClient.ImageRemove(ctx, "busybox:latest", image.RemoveOptions{Force: true})
|
|
| 85 |
+ assert.NilError(t, err) |
|
| 86 |
+ |
|
| 87 |
+ // run container |
|
| 88 |
+ cid := container.Create(ctx, t, apiClient, container.WithImage(id)) |
|
| 89 |
+ defer container.Remove(ctx, t, apiClient, cid, containertypes.RemoveOptions{Force: true})
|
|
| 90 |
+ |
|
| 91 |
+ pruned, err := apiClient.ImagesPrune(ctx, filters.NewArgs(filters.Arg("dangling", "false")))
|
|
| 92 |
+ assert.NilError(t, err) |
|
| 93 |
+ |
|
| 94 |
+ assert.Check(t, is.Len(pruned.ImagesDeleted, len(tags))) |
|
| 95 |
+ for _, p := range pruned.ImagesDeleted {
|
|
| 96 |
+ assert.Check(t, is.Equal(p.Deleted, "")) |
|
| 97 |
+ assert.Check(t, p.Untagged != "busybox:z") |
|
| 98 |
+ } |
|
| 99 |
+} |
|
| 100 |
+ |
|
| 56 | 101 |
// Regression test for https://github.com/moby/moby/issues/48063 |
| 57 | 102 |
func TestPruneDontDeleteUsedImage(t *testing.T) {
|
| 58 | 103 |
skip.If(t, testEnv.DaemonInfo.OSType == "windows", "cannot start multiple daemons on windows") |
| ... | ... |
@@ -81,18 +126,19 @@ func TestPruneDontDeleteUsedImage(t *testing.T) {
|
| 81 | 81 |
// busybox:other tag pointing to the same image. |
| 82 | 82 |
name: "two tags", |
| 83 | 83 |
prepare: func(t *testing.T, d *daemon.Daemon, apiClient *client.Client) error {
|
| 84 |
- return apiClient.ImageTag(ctx, "busybox:latest", "busybox:other") |
|
| 84 |
+ return apiClient.ImageTag(ctx, "busybox:latest", "busybox:a") |
|
| 85 | 85 |
}, |
| 86 | 86 |
check: func(t *testing.T, apiClient *client.Client, pruned image.PruneReport) {
|
| 87 | 87 |
if assert.Check(t, is.Len(pruned.ImagesDeleted, 1)) {
|
| 88 | 88 |
assert.Check(t, is.Equal(pruned.ImagesDeleted[0].Deleted, "")) |
| 89 |
- t.Log("Untagged", pruned.ImagesDeleted[0].Untagged)
|
|
| 89 |
+ assert.Check(t, is.Equal(pruned.ImagesDeleted[0].Untagged, "busybox:a")) |
|
| 90 | 90 |
} |
| 91 | 91 |
|
| 92 |
- _, _, err1 := apiClient.ImageInspectWithRaw(ctx, "busybox:latest") |
|
| 93 |
- _, _, err2 := apiClient.ImageInspectWithRaw(ctx, "busybox:other") |
|
| 92 |
+ _, _, err := apiClient.ImageInspectWithRaw(ctx, "busybox:a") |
|
| 93 |
+ assert.Check(t, err != nil, "Busybox:a image should be deleted") |
|
| 94 | 94 |
|
| 95 |
- assert.Check(t, err1 != err2 && (err1 == nil || err2 == nil), "One of the images should still exist") |
|
| 95 |
+ _, _, err = apiClient.ImageInspectWithRaw(ctx, "busybox:latest") |
|
| 96 |
+ assert.Check(t, err == nil, "Busybox:latest image should still exist") |
|
| 96 | 97 |
}, |
| 97 | 98 |
}, |
| 98 | 99 |
} {
|