Merge pull request #45147 from cpuguy83/fix_volume_anon_from_image

Fix pruning anon volume created from image config

daemon/create_unix.go

@@ -13,7 +13,6 @@ import (
 	mounttypes "github.com/docker/docker/api/types/mount"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/oci"
-	"github.com/docker/docker/pkg/stringid"
 	volumeopts "github.com/docker/docker/volume/service/opts"
 	"github.com/opencontainers/selinux/go-selinux/label"
 	"github.com/sirupsen/logrus"
@@ -42,7 +41,6 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 	}
 
 	for spec := range config.Volumes {
-		name := stringid.GenerateRandomID()
 		destination := filepath.Clean(spec)
 
 		// Skip volumes for which we already have something mounted on that
@@ -62,7 +60,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 			return fmt.Errorf("cannot mount volume over existing file, file exists %s", path)
 		}
 
-		v, err := daemon.volumes.Create(context.TODO(), name, hostConfig.VolumeDriver, volumeopts.WithCreateReference(container.ID))
+		v, err := daemon.volumes.Create(context.TODO(), "", hostConfig.VolumeDriver, volumeopts.WithCreateReference(container.ID))
 		if err != nil {
 			return err
 		}

daemon/create_windows.go

@@ -6,7 +6,6 @@ import (
 
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
-	"github.com/docker/docker/pkg/stringid"
 	volumemounts "github.com/docker/docker/volume/mounts"
 	volumeopts "github.com/docker/docker/volume/service/opts"
 )
@@ -25,11 +24,6 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 			return fmt.Errorf("Unrecognised volume spec: %v", err)
 		}
 
-		// If the mountpoint doesn't have a name, generate one.
-		if len(mp.Name) == 0 {
-			mp.Name = stringid.GenerateRandomID()
-		}
-
 		// Skip volumes for which we already have something mounted on that
 		// destination because of a --volume-from.
 		if container.IsDestinationMounted(mp.Destination) {
@@ -40,7 +34,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 
 		// Create the volume in the volume driver. If it doesn't exist,
 		// a new one will be created.
-		v, err := daemon.volumes.Create(context.TODO(), mp.Name, volumeDriver, volumeopts.WithCreateReference(container.ID))
+		v, err := daemon.volumes.Create(context.TODO(), "", volumeDriver, volumeopts.WithCreateReference(container.ID))
 		if err != nil {
 			return err
 		}

integration/internal/build/build.go

@@ -0,0 +1,52 @@
+package build
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"testing"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/client"
+	"github.com/docker/docker/pkg/jsonmessage"
+	"github.com/docker/docker/testutil/fakecontext"
+	"gotest.tools/v3/assert"
+)
+
+// Do builds an image from the given context and returns the image ID.
+func Do(ctx context.Context, t *testing.T, client client.APIClient, buildCtx *fakecontext.Fake) string {
+	resp, err := client.ImageBuild(ctx, buildCtx.AsTarReader(t), types.ImageBuildOptions{})
+	if resp.Body != nil {
+		defer resp.Body.Close()
+	}
+	assert.NilError(t, err)
+	img := GetImageIDFromBody(t, resp.Body)
+	t.Cleanup(func() {
+		client.ImageRemove(ctx, img, types.ImageRemoveOptions{Force: true})
+	})
+	return img
+}
+
+// GetImageIDFRommBody reads the image ID from the build response body.
+func GetImageIDFromBody(t *testing.T, body io.Reader) string {
+	var (
+		jm  jsonmessage.JSONMessage
+		br  types.BuildResult
+		dec = json.NewDecoder(body)
+	)
+	for {
+		err := dec.Decode(&jm)
+		if err == io.EOF {
+			break
+		}
+		assert.NilError(t, err)
+		if jm.Aux == nil {
+			continue
+		}
+		assert.NilError(t, json.Unmarshal(*jm.Aux, &br))
+		assert.Assert(t, br.ID != "", "could not read image ID from build output")
+		break
+	}
+	io.Copy(io.Discard, body)
+	return br.ID
+}

integration/internal/container/container.go

@@ -48,6 +48,7 @@ func create(ctx context.Context, t *testing.T, client client.APIClient, ops ...f
 
 // Create creates a container with the specified options, asserting that there was no error
 func Create(ctx context.Context, t *testing.T, client client.APIClient, ops ...func(*TestContainerConfig)) string {
+	t.Helper()
 	c, err := create(ctx, t, client, ops...)
 	assert.NilError(t, err)
 

integration/volume/volume_test.go

@@ -14,8 +14,10 @@ import (
 	"github.com/docker/docker/api/types/volume"
 	clientpkg "github.com/docker/docker/client"
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/integration/internal/build"
 	"github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/testutil/daemon"
+	"github.com/docker/docker/testutil/fakecontext"
 	"github.com/docker/docker/testutil/request"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
@@ -304,3 +306,37 @@ func TestVolumePruneAnonymous(t *testing.T) {
 	assert.Check(t, cmp.Contains(pruneReport.VolumesDeleted, v.Name))
 	assert.Check(t, cmp.Contains(pruneReport.VolumesDeleted, vNamed.Name))
 }
+
+func TestVolumePruneAnonFromImage(t *testing.T) {
+	defer setupTest(t)()
+	client := testEnv.APIClient()
+
+	volDest := "/foo"
+	if testEnv.OSType == "windows" {
+		volDest = `c:\\foo`
+	}
+
+	dockerfile := `FROM busybox
+VOLUME ` + volDest
+
+	ctx := context.Background()
+	img := build.Do(ctx, t, client, fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile)))
+
+	id := container.Create(ctx, t, client, container.WithImage(img))
+	defer client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{})
+
+	inspect, err := client.ContainerInspect(ctx, id)
+	assert.NilError(t, err)
+
+	assert.Assert(t, cmp.Len(inspect.Mounts, 1))
+
+	volumeName := inspect.Mounts[0].Name
+	assert.Assert(t, volumeName != "")
+
+	err = client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{})
+	assert.NilError(t, err)
+
+	pruneReport, err := client.VolumesPrune(ctx, filters.Args{})
+	assert.NilError(t, err)
+	assert.Assert(t, cmp.Contains(pruneReport.VolumesDeleted, volumeName))
+}