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>
This commit is contained in:
Brian Goff 2023-03-11 22:34:21 +00:00 committed by Sebastiaan van Stijn
parent 8224c74f08
commit 146df5fbd3
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
5 changed files with 91 additions and 10 deletions

View file

@ -13,7 +13,6 @@ import (
mounttypes "github.com/docker/docker/api/types/mount" mounttypes "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/container" "github.com/docker/docker/container"
"github.com/docker/docker/oci" "github.com/docker/docker/oci"
"github.com/docker/docker/pkg/stringid"
volumeopts "github.com/docker/docker/volume/service/opts" volumeopts "github.com/docker/docker/volume/service/opts"
"github.com/opencontainers/selinux/go-selinux/label" "github.com/opencontainers/selinux/go-selinux/label"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
@ -42,7 +41,6 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
} }
for spec := range config.Volumes { for spec := range config.Volumes {
name := stringid.GenerateRandomID()
destination := filepath.Clean(spec) destination := filepath.Clean(spec)
// Skip volumes for which we already have something mounted on that // 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) 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 { if err != nil {
return err return err
} }

View file

@ -6,7 +6,6 @@ import (
containertypes "github.com/docker/docker/api/types/container" containertypes "github.com/docker/docker/api/types/container"
"github.com/docker/docker/container" "github.com/docker/docker/container"
"github.com/docker/docker/pkg/stringid"
volumemounts "github.com/docker/docker/volume/mounts" volumemounts "github.com/docker/docker/volume/mounts"
volumeopts "github.com/docker/docker/volume/service/opts" 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) 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 // Skip volumes for which we already have something mounted on that
// destination because of a --volume-from. // destination because of a --volume-from.
if container.IsDestinationMounted(mp.Destination) { 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, // Create the volume in the volume driver. If it doesn't exist,
// a new one will be created. // 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 { if err != nil {
return err return err
} }

View file

@ -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
}

View file

@ -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 // 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 { func Create(ctx context.Context, t *testing.T, client client.APIClient, ops ...func(*TestContainerConfig)) string {
t.Helper()
c, err := create(ctx, t, client, ops...) c, err := create(ctx, t, client, ops...)
assert.NilError(t, err) assert.NilError(t, err)

View file

@ -14,8 +14,10 @@ import (
"github.com/docker/docker/api/types/volume" "github.com/docker/docker/api/types/volume"
clientpkg "github.com/docker/docker/client" clientpkg "github.com/docker/docker/client"
"github.com/docker/docker/errdefs" "github.com/docker/docker/errdefs"
"github.com/docker/docker/integration/internal/build"
"github.com/docker/docker/integration/internal/container" "github.com/docker/docker/integration/internal/container"
"github.com/docker/docker/testutil/daemon" "github.com/docker/docker/testutil/daemon"
"github.com/docker/docker/testutil/fakecontext"
"github.com/docker/docker/testutil/request" "github.com/docker/docker/testutil/request"
"github.com/google/go-cmp/cmp/cmpopts" "github.com/google/go-cmp/cmp/cmpopts"
"gotest.tools/v3/assert" "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, v.Name))
assert.Check(t, cmp.Contains(pruneReport.VolumesDeleted, vNamed.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))
}