Allow Windows Devices to be activated for HyperV Isolation

If not using the containerd backend, this will still fail, but later.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>

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 {

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))

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))