From dc144a4e7dd72f96bf1f705611fec69900b04ef1 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 00:23:21 +0200 Subject: [PATCH] Count volumes a half-done batch already removed as removed, and test the batching (issue 1796) --- modules/docker/cmd/docker-tools/docker.go | 6 +++ .../docker/cmd/docker-tools/docker_test.go | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/modules/docker/cmd/docker-tools/docker.go b/modules/docker/cmd/docker-tools/docker.go index 31e6791f..ea17f624 100644 --- a/modules/docker/cmd/docker-tools/docker.go +++ b/modules/docker/cmd/docker-tools/docker.go @@ -1313,6 +1313,12 @@ func (c *Client) RemoveAnonymousVolumes(ctx context.Context, a RemoveVolumesAsk) } for _, name := range chunk { if _, err := c.docker(ctx, "volume", "rm", name); err != nil { + // The batch took it before it refused another (or before its bound ran out): + // a volume that is no longer there is a removed one. + if strings.Contains(strings.ToLower(err.Error()), "no such volume") { + removed = append(removed, name) + continue + } skip(name, "docker refused: "+err.Error()) continue } diff --git a/modules/docker/cmd/docker-tools/docker_test.go b/modules/docker/cmd/docker-tools/docker_test.go index c4d2cc03..d8f8a58b 100644 --- a/modules/docker/cmd/docker-tools/docker_test.go +++ b/modules/docker/cmd/docker-tools/docker_test.go @@ -525,3 +525,51 @@ func TestRemoveAnonymousVolumesAnswersWhenTheSizeListingDoesNot(t *testing.T) { t.Fatalf("removal: %v %v %+v", got, err, f.calls) } } + +// 1796/AC4: a batch docker half-did is settled by name: what is already gone counts as removed, what is in use stays with why. +func TestRemoveAnonymousVolumesCountsWhatAFailedBatchAlreadyRemoved(t *testing.T) { + f := (&fake{}). + on("docker ps --all --quiet --no-trunc", Ran{}). + on("docker volume ls --quiet", Ran{Stdout: "a1\na2\na3\n"}). + on("docker volume inspect", Ran{Stdout: `[{"Name":"a1","Labels":{"com.docker.volume.anonymous":""}},{"Name":"a2","Labels":{"com.docker.volume.anonymous":""}},{"Name":"a3","Labels":{"com.docker.volume.anonymous":""}}]`}). + on("docker volume rm a1 a2 a3", Ran{Status: 1, Stdout: "a1\n", Stderr: "Error response from daemon: remove a2: volume is in use - [c1]\n"}). + on("docker volume rm a1", Ran{Status: 1, Stderr: "Error response from daemon: get a1: no such volume\n"}). + on("docker volume rm a2", Ran{Status: 1, Stderr: "Error response from daemon: remove a2: volume is in use - [c1]\n"}). + on("docker volume rm a3", Ran{Stdout: "a3\n"}) + got, err := client(f, 1000).RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{Why: "w"}) + if err != nil { + t.Fatal(err) + } + sk := got["skipped"].([]map[string]any) + if !reflect.DeepEqual(got["removed"], []string{"a1", "a3"}) || got["count"] != 2 || len(sk) != 1 || sk[0]["name"] != "a2" || !strings.Contains(sk[0]["why"].(string), "in use") { + t.Fatalf("%v", got) + } +} + +// 1796/AC5: removal goes in batches of a hundred names. +func TestRemoveAnonymousVolumesRemovesInBatchesOfAHundred(t *testing.T) { + names, entries := []string{}, []string{} + for i := 0; i < 250; i++ { + n := fmt.Sprintf("v%03d", i) + names = append(names, n) + entries = append(entries, `{"Name":"`+n+`","Labels":{"com.docker.volume.anonymous":""}}`) + } + f := (&fake{}). + on("docker ps --all --quiet --no-trunc", Ran{}). + on("docker volume ls --quiet", Ran{Stdout: strings.Join(names, "\n")}). + on("docker volume inspect", Ran{Stdout: "[" + strings.Join(entries, ",") + "]"}). + on("docker volume rm", Ran{Stdout: "ok\n"}) + got, err := client(f, 1000).RemoveAnonymousVolumes(context.Background(), RemoveVolumesAsk{Why: "w", Limit: 250}) + if err != nil || got["count"] != 250 { + t.Fatalf("%v %v", got, err) + } + sizes := []int{} + for _, c := range f.calls { + if len(c.args) > 1 && c.args[0] == "volume" && c.args[1] == "rm" { + sizes = append(sizes, len(c.args)-2) + } + } + if !reflect.DeepEqual(sizes, []int{100, 100, 50}) { + t.Fatalf("batches %v", sizes) + } +}