diff options
author | Brian Goff <cpuguy83@gmail.com> | 2023-03-11 22:34:21 +0000 |
---|---|---|
committer | Sebastiaan van Stijn <github@gone.nl> | 2023-03-14 16:11:09 +0100 |
commit | fd80ca60da47e1c4533c3a66679cf582e7acac66 (patch) | |
tree | e98b439f11aae433b68ac564a31137441d5f67ab | |
parent | f09528b7126f74b9a1ff82f1a55354d0c67b4574 (diff) | |
download | docker-fd80ca60da47e1c4533c3a66679cf582e7acac66.tar.gz |
Fix pruning anon volume created from image config
Volumes created from the image config were not being pruned because the
volume service did not think they were anonymous since the code to
create passes along a generated name instead of letting the volume
service generate it.
This changes the code path to have the volume service generate the name
instead of doing it ahead of time.
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit 146df5fbd369dc2a48180dbbacb4d7071cf18f1a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
-rw-r--r-- | daemon/create_unix.go | 4 | ||||
-rw-r--r-- | daemon/create_windows.go | 8 | ||||
-rw-r--r-- | integration/internal/build/build.go | 52 | ||||
-rw-r--r-- | integration/internal/container/container.go | 1 | ||||
-rw-r--r-- | integration/volume/volume_test.go | 36 |
5 files changed, 91 insertions, 10 deletions
diff --git a/daemon/create_unix.go b/daemon/create_unix.go index f6f9649eb5..118c427aac 100644 --- a/daemon/create_unix.go +++ b/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 } diff --git a/daemon/create_windows.go b/daemon/create_windows.go index f47b732fbd..c7220601f3 100644 --- a/daemon/create_windows.go +++ b/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 } diff --git a/integration/internal/build/build.go b/integration/internal/build/build.go new file mode 100644 index 0000000000..9dbdd1241e --- /dev/null +++ b/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 +} diff --git a/integration/internal/container/container.go b/integration/internal/container/container.go index dadc6b44e4..6559bd4f4a 100644 --- a/integration/internal/container/container.go +++ b/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) diff --git a/integration/volume/volume_test.go b/integration/volume/volume_test.go index f9e1dfcb55..e3d0cd7b96 100644 --- a/integration/volume/volume_test.go +++ b/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)) +} |