Skip to content

Commit

Permalink
Fix pruning anon volume created from image config
Browse files Browse the repository at this point in the history
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 146df5f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
cpuguy83 authored and thaJeztah committed Mar 14, 2023
1 parent f09528b commit fd80ca6
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 10 deletions.
4 changes: 1 addition & 3 deletions daemon/create_unix.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
8 changes: 1 addition & 7 deletions daemon/create_windows.go
Expand Up @@ -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"
)
Expand All @@ -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) {
Expand All @@ -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
}
Expand Down
52 changes: 52 additions & 0 deletions 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
}
1 change: 1 addition & 0 deletions integration/internal/container/container.go
Expand Up @@ -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)

Expand Down
36 changes: 36 additions & 0 deletions integration/volume/volume_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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))
}

0 comments on commit fd80ca6

Please sign in to comment.