diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index 0e97fafb45..886be9ae25 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -458,12 +458,17 @@ func setupWindowsDevices(devices []containertypes.DeviceMapping) (specDevices [] } for _, deviceMapping := range devices { - srcParts := strings.SplitN(deviceMapping.PathOnHost, "/", 2) - if len(srcParts) != 2 { - return nil, errors.New("invalid device assignment path") + devicePath := deviceMapping.PathOnHost + if strings.HasPrefix(devicePath, "class/") { + devicePath = strings.Replace(devicePath, "class/", "class://", 1) } - if srcParts[0] != "class" { - return nil, errors.Errorf("invalid device assignment type: '%s' should be 'class'", srcParts[0]) + + srcParts := strings.SplitN(devicePath, "://", 2) + if len(srcParts) != 2 { + return nil, errors.Errorf("invalid device assignment path: '%s', must be 'class/ID' or 'IDType://ID'", deviceMapping.PathOnHost) + } + if srcParts[0] == "" { + return nil, errors.Errorf("invalid device assignment path: '%s', IDType cannot be empty", deviceMapping.PathOnHost) } wd := specs.WindowsDevice{ ID: srcParts[1], diff --git a/daemon/oci_windows_test.go b/daemon/oci_windows_test.go index a427c58656..d9695a58eb 100644 --- a/daemon/oci_windows_test.go +++ b/daemon/oci_windows_test.go @@ -322,34 +322,46 @@ func TestSetupWindowsDevices(t *testing.T) { t.Run("it fails if any devices are blank", func(t *testing.T) { devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: ""}}) assert.ErrorContains(t, err, "invalid device assignment path") + assert.ErrorContains(t, err, "''") assert.Equal(t, len(devices), 0) }) - t.Run("it fails if all devices do not contain '/'", func(t *testing.T) { + t.Run("it fails if all devices do not contain '/' or '://'", func(t *testing.T) { devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}, {PathOnHost: "goes"}}) assert.ErrorContains(t, err, "invalid device assignment path") + assert.ErrorContains(t, err, "'anything'") assert.Equal(t, len(devices), 0) }) - t.Run("it fails if any devices do not contain '/'", func(t *testing.T) { + t.Run("it fails if any devices do not contain '/' or '://'", func(t *testing.T) { devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "goes"}}) assert.ErrorContains(t, err, "invalid device assignment path") + assert.ErrorContains(t, err, "'goes'") assert.Equal(t, len(devices), 0) }) - t.Run("it fails if all devices do not have IDType 'class'", func(t *testing.T) { + t.Run("it fails if all '/'-separated devices do not have IDType 'class'", func(t *testing.T) { devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "klass/anything"}, {PathOnHost: "klass/goes"}}) - assert.ErrorContains(t, err, "invalid device assignment type: 'klass' should be 'class'") + assert.ErrorContains(t, err, "invalid device assignment path") + assert.ErrorContains(t, err, "'klass/anything'") assert.Equal(t, len(devices), 0) }) - t.Run("it fails if any devices do not have IDType 'class'", func(t *testing.T) { + t.Run("it fails if any '/'-separated devices do not have IDType 'class'", func(t *testing.T) { devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "klass/goes"}}) - assert.ErrorContains(t, err, "invalid device assignment type: 'klass' should be 'class'") + assert.ErrorContains(t, err, "invalid device assignment path") + assert.ErrorContains(t, err, "'klass/goes'") assert.Equal(t, len(devices), 0) }) - t.Run("it creates devices if all devices have IDType 'class'", func(t *testing.T) { + t.Run("it fails if any '://'-separated devices have IDType ''", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "://goes"}}) + assert.ErrorContains(t, err, "invalid device assignment path") + assert.ErrorContains(t, err, "'://goes'") + assert.Equal(t, len(devices), 0) + }) + + t.Run("it creates devices if all '/'-separated devices have IDType 'class'", func(t *testing.T) { devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "class/goes"}}) expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "class", ID: "goes"}} assert.NilError(t, err) @@ -358,4 +370,24 @@ func TestSetupWindowsDevices(t *testing.T) { assert.Equal(t, devices[i], expectedDevices[i]) } }) + + t.Run("it creates devices if all '://'-separated devices have non-blank IDType", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class://anything"}, {PathOnHost: "klass://goes"}}) + expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "klass", ID: "goes"}} + assert.NilError(t, err) + assert.Equal(t, len(devices), len(expectedDevices)) + for i := range expectedDevices { + assert.Equal(t, devices[i], expectedDevices[i]) + } + }) + + t.Run("it creates devices when given a mix of '/'-separated and '://'-separated devices", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "klass://goes"}}) + expectedDevices := []specs.WindowsDevice{{IDType: "class", ID: "anything"}, {IDType: "klass", ID: "goes"}} + assert.NilError(t, err) + assert.Equal(t, len(devices), len(expectedDevices)) + for i := range expectedDevices { + assert.Equal(t, devices[i], expectedDevices[i]) + } + }) } diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 9440dabd1f..1f627a196f 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -65,6 +65,11 @@ keywords: "API, Docker, rcli, REST, documentation" - "locked" - "active/worker" - "active/manager" +* `POST /containers/create` for Windows containers now accepts a new syntax in + `HostConfig.Resources.Devices.PathOnHost`. As well as the existing `class/` + syntax, `://` is now recognised. Support for specific `` values + depends on the underlying implementation and Windows version. This change is not + versioned, and affects all API versions if the daemon has this patch. ## v1.41 API changes diff --git a/integration/container/devices_windows_test.go b/integration/container/devices_windows_test.go index 35161c94b7..759bfc940c 100644 --- a/integration/container/devices_windows_test.go +++ b/integration/container/devices_windows_test.go @@ -43,6 +43,20 @@ func TestWindowsDevices(t *testing.T) { isolation: containertypes.IsolationProcess, expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", }, + { + doc: "process/class://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted", + devices: []string{"class://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}, + isolation: containertypes.IsolationProcess, + expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", + }, + { + doc: "process/vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted", + devices: []string{"vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}, + isolation: containertypes.IsolationProcess, + expectedStartFailure: !testEnv.RuntimeIsWindowsContainerd(), + expectedStartFailureMessage: "device assignment of type 'vpci-class-guid' is not supported", + expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", + }, { doc: "hyperv/no device mounted", isolation: containertypes.IsolationHyperV, @@ -56,6 +70,22 @@ func TestWindowsDevices(t *testing.T) { expectedStartFailureMessage: "device assignment is not supported for HyperV containers", expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", }, + { + doc: "hyperv/class://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted", + devices: []string{"class://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}, + isolation: containertypes.IsolationHyperV, + expectedStartFailure: !testEnv.RuntimeIsWindowsContainerd(), + expectedStartFailureMessage: "device assignment is not supported for HyperV containers", + expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", + }, + { + doc: "hyperv/vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted", + devices: []string{"vpci-class-guid://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}, + isolation: containertypes.IsolationHyperV, + expectedStartFailure: !testEnv.RuntimeIsWindowsContainerd(), + expectedStartFailureMessage: "device assignment is not supported for HyperV containers", + expectedStdout: "/Windows/System32/HostDriverStore/FileRepository", + }, } for _, d := range testData { diff --git a/libcontainerd/local/local_windows.go b/libcontainerd/local/local_windows.go index 065e7d385a..d9c7b7a6c7 100644 --- a/libcontainerd/local/local_windows.go +++ b/libcontainerd/local/local_windows.go @@ -312,6 +312,11 @@ func (c *client) createWindows(id string, spec *specs.Spec, runtimeOptions inter return errors.New("device assignment is not supported for HyperV containers") } for _, d := range spec.Windows.Devices { + // Per https://github.com/microsoft/hcsshim/blob/v0.9.2/internal/uvm/virtual_device.go#L17-L18, + // this represents an Interface Class GUID. + if d.IDType != "class" { + return errors.Errorf("device assignment of type '%s' is not supported", d.IDType) + } configuration.AssignedDevices = append(configuration.AssignedDevices, hcsshim.AssignedDevice{InterfaceClassGUID: d.ID}) } }