Remove a build's throwaway containers with their anonymous volumes, so a build node's disk does not fill (issue 1772)

Makefile

@@ -117,7 +117,7 @@ proxy-image:
 # as the script runs them: a bus per test, of the release the mesh runs, makes the suite the same in any
 # order, and the timeout bounds a hang to a failure with a stack, never a stalled gate.
 check: postgres
-	@sh merge-check.sh ; status=$$? ; docker rm -f $(PG_CONTAINER) >/dev/null 2>&1 || true ; exit $$status
+	@sh merge-check.sh ; status=$$? ; docker rm -f -v $(PG_CONTAINER) >/dev/null 2>&1 || true ; exit $$status
 
 # The unit level alone (untagged: no bus, no store, no other process), which a developer runs continuously. The tests
 # that need a bus or the store carry `//go:build integration` and run in `check`, not here; without a database the
@@ -135,7 +135,7 @@ fmt:
 	if [ -n "$$unformatted" ] ; then echo "not gofmt'd:" ; echo "$$unformatted" ; exit 1 ; fi
 
 postgres:
-	@docker rm -f $(PG_CONTAINER) >/dev/null 2>&1 || true
+	@docker rm -f -v $(PG_CONTAINER) >/dev/null 2>&1 || true
 	@docker run -d --name $(PG_CONTAINER) -e POSTGRES_PASSWORD=check \
 		-p 127.0.0.1:$(PG_PORT):5432 $(PG_IMAGE) >/dev/null
 	@printf 'waiting for postgres'
@@ -147,7 +147,7 @@ postgres:
 	echo ' — never came up' ; docker logs $(PG_CONTAINER) | tail -20 ; exit 1
 
 postgres-stop:
-	@docker rm -f $(PG_CONTAINER) >/dev/null 2>&1 || true
+	@docker rm -f -v $(PG_CONTAINER) >/dev/null 2>&1 || true
 
 clean:
 	rm -rf build/

cmd/mesh-builder/holder_test.go

@@ -91,7 +91,7 @@ func TestKillEndsTheBuildRunningHereAndNoOther(t *testing.T) {
 		t.Errorf("kill said %q", said)
 	}
 	// Removed at the kill and again once the build had ended: a container made in between is caught.
-	if len(removed) != 4 || !strings.Contains(removed[0], "label=mesh.build=build-1") || !strings.Contains(removed[2], "label=mesh.build=build-1") {
+	if len(removed) != 6 || !strings.Contains(removed[0], "label=mesh.build=build-1") || !strings.Contains(removed[3], "label=mesh.build=build-1") {
 		t.Errorf("removed %v", removed)
 	}
 	if c := h.current(); c.Running != nil {
@@ -160,7 +160,7 @@ func TestAHolderStartingRemovesWhatEarlierBuildsLeftHere(t *testing.T) {
 		}
 		return "", nil
 	}
-	if n := h.removeLeftBehind(); n != 2 || len(removed) != 2 || removed[1] != "docker rm -f c1 c2" {
+	if n := h.removeLeftBehind(); n != 2 || len(removed) != 3 || removed[2] != "docker rm -f -v c1 c2" {
 		t.Fatalf("removed %d: %v", n, removed)
 	}
 	h.remove = func(context.Context, string, string, ...string) (string, error) {

internal/builder/kill.go

@@ -3,6 +3,7 @@ package builder
 import (
 	"context"
 	"os/exec"
+	"regexp"
 	"strings"
 	"syscall"
 	"time"
@@ -71,12 +72,45 @@ func RemoveContainersOf(ctx context.Context, run Runner, id string) (int, error)
 	if len(ids) == 0 {
 		return 0, nil
 	}
-	if _, err := run(ctx, "", "docker", append([]string{"rm", "-f"}, ids...)...); err != nil {
+	if err := removeWithVolumes(ctx, run, ids); err != nil {
 		return 0, err
 	}
 	return len(ids), nil
 }
 
+// MountedVolumes is the template `docker inspect --format` reads the names of a container's volumes with.
+const MountedVolumes = `{{range .Mounts}}{{if eq .Type "volume"}}{{println .Name}}{{end}}{{end}}`
+
+// anonymousVolume is the name docker gives a volume it makes for an image's VOLUME line: 64 hex digits.
+var anonymousVolume = regexp.MustCompile(`^[0-9a-f]{64}$`)
+
+// removeWithVolumes removes containers together with their anonymous volumes (novox/hq issue 1772). A throwaway
+// store declares its data directory a VOLUME, docker makes an anonymous volume for it, and `docker rm -f`
+// without -v leaves that volume for ever: 1,629 of them, 107 GB, on one build node. So the volumes the
+// containers mount are read first, the containers are removed with -v, and an anonymous volume still there
+// afterwards is removed by name. Only a volume one of these containers mounted, and only one with an anonymous
+// volume's name, is touched: a named volume — an operator's own data — never is. A volume already gone is
+// no failure.
+func removeWithVolumes(ctx context.Context, run Runner, ids []string) error {
+	var volumes []string
+	if out, err := run(ctx, "", "docker", append([]string{"inspect", "--format", MountedVolumes}, ids...)...); err == nil {
+		seen := map[string]bool{}
+		for _, name := range strings.Fields(out) {
+			if anonymousVolume.MatchString(name) && !seen[name] {
+				seen[name] = true
+				volumes = append(volumes, name)
+			}
+		}
+	}
+	if _, err := run(ctx, "", "docker", append([]string{"rm", "-f", "-v"}, ids...)...); err != nil {
+		return err
+	}
+	if len(volumes) > 0 {
+		_, _ = run(ctx, "", "docker", append([]string{"volume", "rm"}, volumes...)...)
+	}
+	return nil
+}
+
 // A holder that stops mid-build leaves its containers running (novox/hq issue 306).
 //
 // A build's containers are removed by the build itself, by a kill, or by the next delivery of the same
@@ -106,7 +140,7 @@ func RemoveLeftBehind(ctx context.Context, run Runner) (int, error) {
 	if len(ids) == 0 {
 		return 0, nil
 	}
-	if _, err := run(ctx, "", "docker", append([]string{"rm", "-f"}, ids...)...); err != nil {
+	if err := removeWithVolumes(ctx, run, ids); err != nil {
 		return 0, err
 	}
 	return len(ids), nil

internal/builder/kill_test.go

@@ -2,6 +2,7 @@ package builder
 
 import (
 	"context"
+	"errors"
 	"reflect"
 	"strings"
 	"testing"
@@ -49,7 +50,8 @@ func TestAKillRemovesTheContainersLabelledWithTheBuild(t *testing.T) {
 	if err != nil || n != 2 {
 		t.Fatalf("%d %v", n, err)
 	}
-	if want := []string{"docker ps -aq --filter label=mesh.build=build-3", "docker rm -f c1 c2"}; !reflect.DeepEqual(ran, want) {
+	if want := []string{"docker ps -aq --filter label=mesh.build=build-3", "docker inspect --format " + MountedVolumes + " c1 c2",
+		"docker rm -f -v c1 c2"}; !reflect.DeepEqual(ran, want) {
 		t.Errorf("ran %v", ran)
 	}
 }
@@ -73,7 +75,7 @@ func TestAStartingHolderRemovesWhatEarlierHoldersLeftAndNothingElse(t *testing.T
 	if n != 3 {
 		t.Errorf("removed %d", n)
 	}
-	if len(ran) != 2 || !reflect.DeepEqual(ran[1], []string{"docker", "rm", "-f", "c1", "c2", "c7"}) {
+	if len(ran) != 3 || !reflect.DeepEqual(ran[2], []string{"docker", "rm", "-f", "-v", "c1", "c2", "c7"}) {
 		t.Fatalf("ran %v", ran)
 	}
 	if !strings.Contains(strings.Join(ran[0], " "), "--filter label=mesh.build") {
@@ -90,3 +92,54 @@ func TestAStartingHolderRemovesWhatEarlierHoldersLeftAndNothingElse(t *testing.T
 		t.Errorf("with nothing of the seat's: %d %v %v", n, err, ran)
 	}
 }
+
+// **Issue 1772**: a throwaway store declares its data directory a VOLUME, so docker makes an anonymous volume for
+// it, and `docker rm -f` without -v leaves that volume behind (1,629 of them, 107 GB, on one build node). Every
+// container a build or check removes is removed with its anonymous volumes (-v), and any volume the container
+// mounted that is still there afterwards is removed by name — those, and only those, never a named volume.
+func TestARemovedContainerTakesItsAnonymousVolumesWithIt(t *testing.T) {
+	anon := strings.Repeat("a", 64)
+	other := strings.Repeat("b", 64)
+	var ran []string
+	run := func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		ran = append(ran, name+" "+strings.Join(args, " "))
+		switch args[0] {
+		case "ps":
+			return "c1\n", nil
+		case "inspect":
+			return anon + "\n" + "compose_data\n" + other + "\n", nil
+		}
+		return "", nil
+	}
+	if n, err := RemoveContainersOf(context.Background(), run, "build-4"); err != nil || n != 1 {
+		t.Fatalf("%d %v", n, err)
+	}
+	last := ran[len(ran)-1]
+	if want := "docker volume rm " + anon + " " + other; last != want {
+		t.Errorf("last command %q, want %q", last, want)
+	}
+	if !strings.Contains(strings.Join(ran, "\n"), "docker rm -f -v c1") {
+		t.Errorf("ran %v", ran)
+	}
+	if strings.Contains(last, "compose_data") {
+		t.Errorf("a named volume was removed: %q", last)
+	}
+}
+
+// A volume already gone when the sweep looks (rm -v took it) is no failure.
+func TestAVolumeAlreadyGoneIsNoFailure(t *testing.T) {
+	run := func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		switch args[0] {
+		case "ps":
+			return "c1\n", nil
+		case "inspect":
+			return strings.Repeat("c", 64) + "\n", nil
+		case "volume":
+			return "", errors.New("no such volume")
+		}
+		return "", nil
+	}
+	if n, err := RemoveContainersOf(context.Background(), run, "build-5"); err != nil || n != 1 {
+		t.Fatalf("%d %v", n, err)
+	}
+}

internal/builder/same_program.go

@@ -105,7 +105,7 @@ func imageContent(ctx context.Context, run Runner, dir, ref string) (string, err
 	if id == "" {
 		return "", errors.New("the container engine named no container for " + ref)
 	}
-	defer func() { _, _ = run(context.WithoutCancel(ctx), dir, "docker", "rm", "--force", id) }()
+	defer func() { _, _ = run(context.WithoutCancel(ctx), dir, "docker", "rm", "--force", "--volumes", id) }()
 	if err := os.MkdirAll(dir, 0o700); err != nil {
 		return "", err
 	}