Merge pull request #1606 from jhowardmsft/device

Add --device support for Windows

cli/command/container/create.go

@@ -72,7 +72,7 @@ func runCreate(dockerCli command.Cli, flags *pflag.FlagSet, options *createOptio
 		}
 	}
 	copts.env = *opts.NewListOptsRef(&newEnv, nil)
-	containerConfig, err := parse(flags, copts)
+	containerConfig, err := parse(flags, copts, dockerCli.ServerInfo().OSType)
 	if err != nil {
 		reportError(dockerCli.Err(), "create", err.Error(), true)
 		return cli.StatusError{StatusCode: 125}

cli/command/container/opts.go

@@ -141,7 +141,7 @@ func addFlags(flags *pflag.FlagSet) *containerOptions {
 		deviceReadIOps:    opts.NewThrottledeviceOpt(opts.ValidateThrottleIOpsDevice),
 		deviceWriteBps:    opts.NewThrottledeviceOpt(opts.ValidateThrottleBpsDevice),
 		deviceWriteIOps:   opts.NewThrottledeviceOpt(opts.ValidateThrottleIOpsDevice),
-		devices:           opts.NewListOpts(validateDevice),
+		devices:           opts.NewListOpts(nil), // devices can only be validated after we know the server OS
 		env:               opts.NewListOpts(opts.ValidateEnv),
 		envFile:           opts.NewListOpts(nil),
 		expose:            opts.NewListOpts(nil),
@@ -299,7 +299,7 @@ type containerConfig struct {
 // a HostConfig and returns them with the specified command.
 // If the specified args are not valid, it will return an error.
 // nolint: gocyclo
-func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, error) {
+func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) {
 	var (
 		attachStdin  = copts.attach.Get("stdin")
 		attachStdout = copts.attach.Get("stdout")
@@ -417,10 +417,22 @@ func parse(flags *pflag.FlagSet, copts *containerOptions) (*containerConfig, err
 		}
 	}
 
-	// parse device mappings
+	// validate and parse device mappings. Note we do late validation of the
+	// device path (as opposed to during flag parsing), as at the time we are
+	// parsing flags, we haven't yet sent a _ping to the daemon to determine
+	// what operating system it is.
 	deviceMappings := []container.DeviceMapping{}
 	for _, device := range copts.devices.GetAll() {
-		deviceMapping, err := parseDevice(device)
+		var (
+			validated     string
+			deviceMapping container.DeviceMapping
+			err           error
+		)
+		validated, err = validateDevice(device, serverOS)
+		if err != nil {
+			return nil, err
+		}
+		deviceMapping, err = parseDevice(validated, serverOS)
 		if err != nil {
 			return nil, err
 		}
@@ -747,7 +759,19 @@ func parseStorageOpts(storageOpts []string) (map[string]string, error) {
 }
 
 // parseDevice parses a device mapping string to a container.DeviceMapping struct
-func parseDevice(device string) (container.DeviceMapping, error) {
+func parseDevice(device, serverOS string) (container.DeviceMapping, error) {
+	switch serverOS {
+	case "linux":
+		return parseLinuxDevice(device)
+	case "windows":
+		return parseWindowsDevice(device)
+	}
+	return container.DeviceMapping{}, errors.Errorf("unknown server OS: %s", serverOS)
+}
+
+// parseLinuxDevice parses a device mapping string to a container.DeviceMapping struct
+// knowing that the target is a Linux daemon
+func parseLinuxDevice(device string) (container.DeviceMapping, error) {
 	src := ""
 	dst := ""
 	permissions := "rwm"
@@ -781,6 +805,12 @@ func parseDevice(device string) (container.DeviceMapping, error) {
 	return deviceMapping, nil
 }
 
+// parseWindowsDevice parses a device mapping string to a container.DeviceMapping struct
+// knowing that the target is a Windows daemon
+func parseWindowsDevice(device string) (container.DeviceMapping, error) {
+	return container.DeviceMapping{PathOnHost: device}, nil
+}
+
 // validateDeviceCgroupRule validates a device cgroup rule string format
 // It will make sure 'val' is in the form:
 //    'type major:minor mode'
@@ -813,14 +843,23 @@ func validDeviceMode(mode string) bool {
 }
 
 // validateDevice validates a path for devices
+func validateDevice(val string, serverOS string) (string, error) {
+	switch serverOS {
+	case "linux":
+		return validateLinuxPath(val, validDeviceMode)
+	case "windows":
+		// Windows does validation entirely server-side
+		return val, nil
+	}
+	return "", errors.Errorf("unknown server OS: %s", serverOS)
+}
+
+// validateLinuxPath is the implementation of validateDevice knowing that the
+// target server operating system is a Linux daemon.
 // It will make sure 'val' is in the form:
 //    [host-dir:]container-path[:mode]
 // It also validates the device mode.
-func validateDevice(val string) (string, error) {
+func validateLinuxPath(val string, validator func(string) bool) (string, error) {
-	return validatePath(val, validDeviceMode)
-}
-
-func validatePath(val string, validator func(string) bool) (string, error) {
 	var containerPath string
 	var mode string
 

cli/command/container/opts_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/spf13/pflag"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
+	"gotest.tools/skip"
 )
 
 func TestValidateAttach(t *testing.T) {
@@ -48,7 +49,7 @@ func parseRun(args []string) (*container.Config, *container.HostConfig, *network
 		return nil, nil, nil, err
 	}
 	// TODO: fix tests to accept ContainerConfig
-	containerConfig, err := parse(flags, copts)
+	containerConfig, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -351,6 +352,7 @@ func TestParseWithExpose(t *testing.T) {
 }
 
 func TestParseDevice(t *testing.T) {
+	skip.If(t, runtime.GOOS == "windows") // Windows validates server-side
 	valids := map[string]container.DeviceMapping{
 		"/dev/snd": {
 			PathOnHost:        "/dev/snd",
@@ -393,7 +395,7 @@ func TestParseModes(t *testing.T) {
 	flags, copts := setupRunFlags()
 	args := []string{"--pid=container:", "img", "cmd"}
 	assert.NilError(t, flags.Parse(args))
-	_, err := parse(flags, copts)
+	_, err := parse(flags, copts, runtime.GOOS)
 	assert.ErrorContains(t, err, "--pid: invalid PID mode")
 
 	// pid ok
@@ -615,6 +617,7 @@ func TestParseEntryPoint(t *testing.T) {
 }
 
 func TestValidateDevice(t *testing.T) {
+	skip.If(t, runtime.GOOS == "windows") // Windows validates server-side
 	valid := []string{
 		"/home",
 		"/home:/home",
@@ -649,13 +652,13 @@ func TestValidateDevice(t *testing.T) {
 	}
 
 	for _, path := range valid {
-		if _, err := validateDevice(path); err != nil {
+		if _, err := validateDevice(path, runtime.GOOS); err != nil {
 			t.Fatalf("ValidateDevice(`%q`) should succeed: error %q", path, err)
 		}
 	}
 
 	for path, expectedError := range invalid {
-		if _, err := validateDevice(path); err == nil {
+		if _, err := validateDevice(path, runtime.GOOS); err == nil {
 			t.Fatalf("ValidateDevice(`%q`) should have failed validation", path)
 		} else {
 			if err.Error() != expectedError {

cli/command/container/run.go

@@ -78,7 +78,7 @@ func runRun(dockerCli command.Cli, flags *pflag.FlagSet, ropts *runOptions, copt
 		}
 	}
 	copts.env = *opts.NewListOptsRef(&newEnv, nil)
-	containerConfig, err := parse(flags, copts)
+	containerConfig, err := parse(flags, copts, dockerCli.ServerInfo().OSType)
 	// just in case the parse does not exit
 	if err != nil {
 		reportError(dockerCli.Err(), "run", err.Error(), true)