Merge pull request #38414 from thaJeztah/minor_volume_tweaks

Some minor tweaks/refactoring of local volumes

integration-cli/docker_api_containers_test.go

@@ -1828,6 +1828,32 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) {
 		}...)
 	}
 
+	if DaemonIsWindows() {
+		cases = append(cases, []testCase{
+			{
+				config: containertypes.Config{
+					Image: "busybox",
+				},
+				hostConfig: containertypes.HostConfig{
+					Mounts: []mounttypes.Mount{
+						{
+							Type:   "volume",
+							Source: "not-supported-on-windows",
+							Target: destPath,
+							VolumeOptions: &mounttypes.VolumeOptions{
+								DriverConfig: &mounttypes.Driver{
+									Name:    "local",
+									Options: map[string]string{"type": "tmpfs"},
+								},
+							},
+						},
+					},
+				},
+				msg: `options are not supported on this platform`,
+			},
+		}...)
+	}
+
 	if DaemonIsLinux() {
 		cases = append(cases, []testCase{
 			{
@@ -1835,14 +1861,83 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) {
 					Image: "busybox",
 				},
 				hostConfig: containertypes.HostConfig{
-					Mounts: []mounttypes.Mount{{
-						Type:   "volume",
-						Source: "hello3",
-						Target: destPath,
-						VolumeOptions: &mounttypes.VolumeOptions{
-							DriverConfig: &mounttypes.Driver{
-								Name:    "local",
-								Options: map[string]string{"o": "size=1"}}}}}},
+					Mounts: []mounttypes.Mount{
+						{
+							Type:   "volume",
+							Source: "missing-device-opt",
+							Target: destPath,
+							VolumeOptions: &mounttypes.VolumeOptions{
+								DriverConfig: &mounttypes.Driver{
+									Name:    "local",
+									Options: map[string]string{"foobar": "foobaz"},
+								},
+							},
+						},
+					},
+				},
+				msg: `invalid option: "foobar"`,
+			},
+			{
+				config: containertypes.Config{
+					Image: "busybox",
+				},
+				hostConfig: containertypes.HostConfig{
+					Mounts: []mounttypes.Mount{
+						{
+							Type:   "volume",
+							Source: "missing-device-opt",
+							Target: destPath,
+							VolumeOptions: &mounttypes.VolumeOptions{
+								DriverConfig: &mounttypes.Driver{
+									Name:    "local",
+									Options: map[string]string{"type": "tmpfs"},
+								},
+							},
+						},
+					},
+				},
+				msg: `missing required option: "device"`,
+			},
+			{
+				config: containertypes.Config{
+					Image: "busybox",
+				},
+				hostConfig: containertypes.HostConfig{
+					Mounts: []mounttypes.Mount{
+						{
+							Type:   "volume",
+							Source: "missing-type-opt",
+							Target: destPath,
+							VolumeOptions: &mounttypes.VolumeOptions{
+								DriverConfig: &mounttypes.Driver{
+									Name:    "local",
+									Options: map[string]string{"device": "tmpfs"},
+								},
+							},
+						},
+					},
+				},
+				msg: `missing required option: "type"`,
+			},
+			{
+				config: containertypes.Config{
+					Image: "busybox",
+				},
+				hostConfig: containertypes.HostConfig{
+					Mounts: []mounttypes.Mount{
+						{
+							Type:   "volume",
+							Source: "hello4",
+							Target: destPath,
+							VolumeOptions: &mounttypes.VolumeOptions{
+								DriverConfig: &mounttypes.Driver{
+									Name:    "local",
+									Options: map[string]string{"o": "size=1", "type": "tmpfs", "device": "tmpfs"},
+								},
+							},
+						},
+					},
+				},
 				msg: "",
 			},
 			{
@@ -1869,7 +1964,6 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) {
 						}}}},
 				msg: "",
 			},
-
 			{
 				config: containertypes.Config{
 					Image: "busybox",
@@ -1888,6 +1982,7 @@ func (s *DockerSuite) TestContainersAPICreateMountsValidation(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	defer cli.Close()
 
+	// TODO add checks for statuscode returned by API
 	for i, x := range cases {
 		c.Logf("case %d", i)
 		_, err = cli.ContainerCreate(context.Background(), &x.config, &x.hostConfig, &networktypes.NetworkingConfig{}, "")

volume/local/local.go

@@ -248,20 +248,12 @@ func (r *Root) Scope() string {
 	return volume.LocalScope
 }
 
-type validationError string
-
-func (e validationError) Error() string {
-	return string(e)
-}
-
-func (e validationError) InvalidParameter() {}
-
 func (r *Root) validateName(name string) error {
 	if len(name) == 1 {
-		return validationError("volume name is too short, names should be at least two alphanumeric characters")
+		return errdefs.InvalidParameter(errors.New("volume name is too short, names should be at least two alphanumeric characters"))
 	}
 	if !volumeNameRegex.MatchString(name) {
-		return validationError(fmt.Sprintf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, names.RestrictedNameChars))
+		return errdefs.InvalidParameter(errors.Errorf("%q includes invalid characters for a local volume name, only %q are allowed. If you intended to pass a host directory, use absolute path", name, names.RestrictedNameChars))
 	}
 	return nil
 }
@@ -352,15 +344,6 @@ func (v *localVolume) unmount() error {
 	return nil
 }
 
-func validateOpts(opts map[string]string) error {
-	for opt := range opts {
-		if !validOpts[opt] {
-			return validationError(fmt.Sprintf("invalid option key: %q", opt))
-		}
-	}
-	return nil
-}
-
 func (v *localVolume) Status() map[string]interface{} {
 	return nil
 }

volume/local/local_unix.go

@@ -14,18 +14,22 @@ import (
 	"syscall"
 	"time"
 
-	"github.com/pkg/errors"
-
+	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/pkg/mount"
+	"github.com/pkg/errors"
 )
 
 var (
 	oldVfsDir = filepath.Join("vfs", "dir")
 
-	validOpts = map[string]bool{
-		"type":   true, // specify the filesystem type for mount, e.g. nfs
-		"o":      true, // generic mount options
-		"device": true, // device to mount from
+	validOpts = map[string]struct{}{
+		"type":   {}, // specify the filesystem type for mount, e.g. nfs
+		"o":      {}, // generic mount options
+		"device": {}, // device to mount from
+	}
+	mandatoryOpts = map[string]struct{}{
+		"device": {},
+		"type":   {},
 	}
 )
 
@@ -71,6 +75,23 @@ func setOpts(v *localVolume, opts map[string]string) error {
 	return nil
 }
 
+func validateOpts(opts map[string]string) error {
+	if len(opts) == 0 {
+		return nil
+	}
+	for opt := range opts {
+		if _, ok := validOpts[opt]; !ok {
+			return errdefs.InvalidParameter(errors.Errorf("invalid option: %q", opt))
+		}
+	}
+	for opt := range mandatoryOpts {
+		if _, ok := opts[opt]; !ok {
+			return errdefs.InvalidParameter(errors.Errorf("missing required option: %q", opt))
+		}
+	}
+	return nil
+}
+
 func (v *localVolume) mount() error {
 	if v.opts.MountDevice == "" {
 		return fmt.Errorf("missing device in volume options")

volume/local/local_windows.go

@@ -4,18 +4,18 @@
 package local // import "github.com/docker/docker/volume/local"
 
 import (
-	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"syscall"
 	"time"
+
+	"github.com/docker/docker/errdefs"
+	"github.com/pkg/errors"
 )
 
 type optsConfig struct{}
 
-var validOpts map[string]bool
-
 // scopedPath verifies that the path where the volume is located
 // is under Docker's root and the valid local paths.
 func (r *Root) scopedPath(realPath string) bool {
@@ -27,7 +27,7 @@ func (r *Root) scopedPath(realPath string) bool {
 
 func setOpts(v *localVolume, opts map[string]string) error {
 	if len(opts) > 0 {
-		return fmt.Errorf("options are not supported on this platform")
+		return errdefs.InvalidParameter(errors.New("options are not supported on this platform"))
 	}
 	return nil
 }