Skip to content

Commit

Permalink
Merge pull request moby#48775 from thaJeztah/anonymous_use_default_dr…
Browse files Browse the repository at this point in the history
…iver

volume/service: use local driver as default for anonymous volumes
  • Loading branch information
thaJeztah authored Oct 28, 2024
2 parents cfe4b4d + c1652ab commit 4ade1b1
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 4ade1b1

Please sign in to comment.