diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index b7304e5fb2..0e97fafb45 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -254,7 +254,7 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S return err } - devices, err := setupWindowsDevices(c.HostConfig.Devices, isHyperV) + devices, err := setupWindowsDevices(c.HostConfig.Devices) if err != nil { return err } @@ -452,15 +452,11 @@ func readCredentialSpecFile(id, root, location string) (string, error) { return string(bcontents[:]), nil } -func setupWindowsDevices(devices []containertypes.DeviceMapping, isHyperV bool) (specDevices []specs.WindowsDevice, err error) { +func setupWindowsDevices(devices []containertypes.DeviceMapping) (specDevices []specs.WindowsDevice, err error) { if len(devices) == 0 { return } - if isHyperV { - return nil, errors.New("device assignment is not supported for HyperV containers") - } - for _, deviceMapping := range devices { srcParts := strings.SplitN(deviceMapping.PathOnHost, "/", 2) if len(srcParts) != 2 { diff --git a/daemon/oci_windows_test.go b/daemon/oci_windows_test.go index 29ad7d9d89..a427c58656 100644 --- a/daemon/oci_windows_test.go +++ b/daemon/oci_windows_test.go @@ -313,56 +313,44 @@ func setRegistryOpenKeyFunc(t *testing.T, key *dummyRegistryKey, err ...error) f } func TestSetupWindowsDevices(t *testing.T) { - t.Run("it does nothing if there are no devices and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices(nil, false) + t.Run("it does nothing if there are no devices", func(t *testing.T) { + devices, err := setupWindowsDevices(nil) assert.NilError(t, err) assert.Equal(t, len(devices), 0) }) - t.Run("it does nothing if there are no devices and HyperV is enabled", func(t *testing.T) { - devices, err := setupWindowsDevices(nil, true) - assert.NilError(t, err) - assert.Equal(t, len(devices), 0) - }) - - t.Run("it fails if there are devices and HyperV is enabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}}, true) - assert.ErrorContains(t, err, "device assignment is not supported for HyperV containers") - assert.Equal(t, len(devices), 0) - }) - - t.Run("it fails if any devices are blank and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: ""}}, false) + 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.Equal(t, len(devices), 0) }) - t.Run("it fails if all devices do not contain '/' and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}, {PathOnHost: "goes"}}, false) + t.Run("it fails if all devices do not contain '/'", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "anything"}, {PathOnHost: "goes"}}) assert.ErrorContains(t, err, "invalid device assignment path") assert.Equal(t, len(devices), 0) }) - t.Run("it fails if any devices do not contain '/' and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "goes"}}, false) + t.Run("it fails if any devices do not contain '/'", func(t *testing.T) { + devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "goes"}}) assert.ErrorContains(t, err, "invalid device assignment path") assert.Equal(t, len(devices), 0) }) - t.Run("it fails if all devices do not have IDType 'class' and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "klass/anything"}, {PathOnHost: "klass/goes"}}, false) + t.Run("it fails if all 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.Equal(t, len(devices), 0) }) - t.Run("it fails if any devices do not have IDType 'class' and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "klass/goes"}}, false) + t.Run("it fails if any 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.Equal(t, len(devices), 0) }) - t.Run("it creates devices if all devices have IDType 'class' and HyperV is disabled", func(t *testing.T) { - devices, err := setupWindowsDevices([]containertypes.DeviceMapping{{PathOnHost: "class/anything"}, {PathOnHost: "class/goes"}}, false) + t.Run("it creates devices if all 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) assert.Equal(t, len(devices), len(expectedDevices)) diff --git a/integration/container/devices_windows_test.go b/integration/container/devices_windows_test.go index 7c9c113aaa..35161c94b7 100644 --- a/integration/container/devices_windows_test.go +++ b/integration/container/devices_windows_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/integration/internal/container" "gotest.tools/v3/assert" @@ -22,32 +23,64 @@ func TestWindowsDevices(t *testing.T) { ctx := context.Background() testData := []struct { - doc string - devices []string - expectedExitCode int - expectedStdout string - expectedStderr string + doc string + devices []string + isolation containertypes.Isolation + expectedStartFailure bool + expectedStartFailureMessage string + expectedExitCode int + expectedStdout string + expectedStderr string }{ { - doc: "no device mounted", + doc: "process/no device mounted", + isolation: containertypes.IsolationProcess, expectedExitCode: 1, }, { - doc: "class/5B45201D-F2F2-4F3B-85BB-30FF1F953599 mounted", + 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: "hyperv/no device mounted", + isolation: containertypes.IsolationHyperV, + expectedExitCode: 1, + }, + { + 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", + }, } for _, d := range testData { d := d t.Run(d.doc, func(t *testing.T) { t.Parallel() - deviceOptions := []func(*container.TestContainerConfig){container.WithIsolation(containertypes.IsolationProcess)} + deviceOptions := []func(*container.TestContainerConfig){container.WithIsolation(d.isolation)} for _, deviceName := range d.devices { deviceOptions = append(deviceOptions, container.WithWindowsDevice(deviceName)) } - id := container.Run(ctx, t, client, deviceOptions...) + + id := container.Create(ctx, t, client, deviceOptions...) + + // Hyper-V isolation is failing even with no actual devices added. + // TODO: Once https://github.com/moby/moby/issues/43395 is resolved, + // remove this skip.If and validate the expected behaviour under Hyper-V. + skip.If(t, d.isolation == containertypes.IsolationHyperV && !d.expectedStartFailure, "FIXME. HyperV isolation setup is probably incorrect in the test") + + err := client.ContainerStart(ctx, id, types.ContainerStartOptions{}) + if d.expectedStartFailure { + assert.ErrorContains(t, err, d.expectedStartFailureMessage) + return + } + + assert.NilError(t, err) poll.WaitOn(t, container.IsInState(ctx, client, id, "running"), poll.WithDelay(100*time.Millisecond))