Skip to content

Commit

Permalink
volume/service: use local driver as default for anonymous volumes
Browse files Browse the repository at this point in the history
Anonymous volumes get a unique, 64-character name, and intended to be a new
volume (not an existing one). While it's theoretically possible for this name
to exist in other volume drivers, this would be very unlikely, so we should
not need to check other drivers to have this volume.

This patch uses the default ("local") volume-driver for anonymous volumes,
unless the user explicitly asked for a specific driver to use. Setting the
driver skips looking up existing volumes in other drivers.

Before this patch:

    DEBU[2024-10-26T15:51:12.681547126Z] container mounted via snapshotter: /var/lib/docker/rootfs/overlayfs/7e947822249514b6239657a0c54d091d90e0fed4b09da472f3f6258f2b4920bc  container=7e947822249514b6239657a0c54d091d90e0fed4b09da472f3f6258f2b4920bc
    DEBU[2024-10-26T15:51:12.681616084Z] Creating anonymous volume                     volume-name=fd46d688247c3e7d39d9bae4532d6b2dc69e82e354c4a3bf305c50bbfb9ebc6c
    DEBU[2024-10-26T15:51:12.681638959Z] Probing all drivers for volume                volume-name=fd46d688247c3e7d39d9bae4532d6b2dc69e82e354c4a3bf305c50bbfb9ebc6c
    DEBU[2024-10-26T15:51:12.681688917Z] Registering new volume reference              driver=local volume-name=fd46d688247c3e7d39d9bae4532d6b2dc69e82e354c4a3bf305c50bbfb9ebc6c

With this patch:

    DEBU[2024-10-27T17:28:28.574956716Z] container mounted via snapshotter: /var/lib/docker/rootfs/overlayfs/7085cb3991b61cbb79edffcb6980ad926f99f6b6b3be617cc3e3b92673cc2eb8  container=7085cb3991b61cbb79edffcb6980ad926f99f6b6b3be617cc3e3b92673cc2eb8
    DEBU[2024-10-27T17:28:28.575002549Z] Creating anonymous volume                     driver=local volume-name=db11c053566362499103213542402af2770a6622fe7a90b9a938a5bed84ca937
    DEBU[2024-10-27T17:28:28.575016299Z] Registering new volume reference              driver=local volume-name=db11c053566362499103213542402af2770a6622fe7a90b9a938a5bed84ca937

Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed Oct 28, 2024
1 parent 56445e1 commit c1652ab
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 16 deletions.
59 changes: 44 additions & 15 deletions integration/container/mounts_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import (
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/client"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/integration/internal/container"
"github.com/docker/docker/pkg/parsers/kernel"
"github.com/docker/docker/testutil"
"github.com/docker/docker/volume"
"github.com/moby/sys/mount"
"github.com/moby/sys/mountinfo"
"gotest.tools/v3/assert"
Expand All @@ -26,6 +28,10 @@ import (
"gotest.tools/v3/skip"
)

// testNonExistingPlugin is a special plugin-name, which overrides defaultTimeOut in tests.
// this is a copy of https://github.com/moby/moby/blob/9e00a63d65434cdedc444e79a2b33a7c202b10d8/pkg/plugins/client.go#L253-L254
const testNonExistingPlugin = "this-plugin-does-not-exist"

func TestContainerNetworkMountsNoChown(t *testing.T) {
// chown only applies to Linux bind mounted volumes; must be same host to verify
skip.If(t, testEnv.IsRemoteDaemon)
Expand Down Expand Up @@ -402,26 +408,49 @@ func TestContainerVolumeAnonymous(t *testing.T) {
skip.If(t, testEnv.IsRemoteDaemon)

ctx := setupTest(t)

mntOpts := mounttypes.Mount{Type: mounttypes.TypeVolume, Target: "/foo"}

apiClient := testEnv.APIClient()
cID := container.Create(ctx, t, apiClient, container.WithMount(mntOpts))

inspect := container.Inspect(ctx, t, apiClient, cID)
assert.Assert(t, is.Len(inspect.HostConfig.Mounts, 1))
assert.Check(t, is.Equal(inspect.HostConfig.Mounts[0], mntOpts))
t.Run("no driver specified", func(t *testing.T) {
mntOpts := mounttypes.Mount{Type: mounttypes.TypeVolume, Target: "/foo"}
cID := container.Create(ctx, t, apiClient, container.WithMount(mntOpts))

assert.Assert(t, is.Len(inspect.Mounts, 1))
volName := inspect.Mounts[0].Name
assert.Check(t, is.Len(volName, 64), "volume name should be 64 bytes (from stringid.GenerateRandomID())")
inspect := container.Inspect(ctx, t, apiClient, cID)
assert.Assert(t, is.Len(inspect.HostConfig.Mounts, 1))
assert.Check(t, is.Equal(inspect.HostConfig.Mounts[0], mntOpts))

volInspect, err := apiClient.VolumeInspect(ctx, volName)
assert.NilError(t, err)
assert.Assert(t, is.Len(inspect.Mounts, 1))
vol := inspect.Mounts[0]
assert.Check(t, is.Len(vol.Name, 64), "volume name should be 64 bytes (from stringid.GenerateRandomID())")
assert.Check(t, is.Equal(vol.Driver, volume.DefaultDriverName))

// see [daemon.AnonymousLabel]; we don't want to import the daemon package here.
const expectedAnonymousLabel = "com.docker.volume.anonymous"
assert.Check(t, is.Contains(volInspect.Labels, expectedAnonymousLabel))
volInspect, err := apiClient.VolumeInspect(ctx, vol.Name)
assert.NilError(t, err)

// see [daemon.AnonymousLabel]; we don't want to import the daemon package here.
const expectedAnonymousLabel = "com.docker.volume.anonymous"
assert.Check(t, is.Contains(volInspect.Labels, expectedAnonymousLabel))
assert.Check(t, is.Equal(volInspect.Driver, volume.DefaultDriverName))
})

// Verify that specifying a custom driver is still taken into account.
t.Run("custom driver", func(t *testing.T) {
config := container.NewTestConfig(container.WithMount(mounttypes.Mount{
Type: mounttypes.TypeVolume,
Target: "/foo",
VolumeOptions: &mounttypes.VolumeOptions{
DriverConfig: &mounttypes.Driver{
Name: testNonExistingPlugin,
},
},
}))
_, err := apiClient.ContainerCreate(ctx, config.Config, config.HostConfig, config.NetworkingConfig, config.Platform, config.Name)
// We use [testNonExistingPlugin] for this, which produces an error
// when used, which we use as indicator that the driver was passed
// through. We should have a cleaner way for this, but that would
// require a custom volume plugin to be installed.
assert.Check(t, is.ErrorType(err, errdefs.IsNotFound))
assert.Check(t, is.ErrorContains(err, fmt.Sprintf(`plugin %q not found`, testNonExistingPlugin)))
})
}

// Regression test for #38995 and #43390.
Expand Down
5 changes: 4 additions & 1 deletion volume/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,11 @@ const AnonymousLabel = "com.docker.volume.anonymous"
func (s *VolumesService) Create(ctx context.Context, name, driverName string, options ...opts.CreateOption) (*volumetypes.Volume, error) {
if name == "" {
name = stringid.GenerateRandomID()
if driverName == "" {
driverName = volume.DefaultDriverName
}
options = append(options, opts.WithCreateLabel(AnonymousLabel, ""))
log.G(ctx).WithField("volume-name", name).Debug("Creating anonymous volume")
log.G(ctx).WithFields(log.Fields{"volume-name": name, "driver": driverName}).Debug("Creating anonymous volume")
} else {
log.G(ctx).WithField("volume-name", name).Debug("Creating named volume")
}
Expand Down

0 comments on commit c1652ab

Please sign in to comment.