Merge pull request #6618 from TBBle/handle-device-host_path-on-windows

Handle CRI Device.HostPath on Windows

cmd/ctr/commands/commands.go

@@ -165,10 +165,6 @@ var (
 			Name:  "memory-limit",
 			Usage: "memory limit (in bytes) for the container",
 		},
-		cli.StringSliceFlag{
-			Name:  "device",
-			Usage: "file path to a device to add to the container; or a path to a directory tree of devices to add to the container",
-		},
 		cli.StringSliceFlag{
 			Name:  "cap-add",
 			Usage: "add Linux capabilities (Set capabilities with 'CAP_' prefix)",

cmd/ctr/commands/commands_unix.go

@@ -40,5 +40,8 @@ func init() {
 	}, cli.StringFlag{
 		Name:  "rootfs-propagation",
 		Usage: "set the propagation of the container rootfs",
+	}, cli.StringSliceFlag{
+		Name:  "device",
+		Usage: "file path to a device to add to the container; or a path to a directory tree of devices to add to the container",
 	})
 }

cmd/ctr/commands/commands_windows.go

@@ -24,5 +24,8 @@ func init() {
 	ContainerFlags = append(ContainerFlags, cli.Uint64Flag{
 		Name:  "cpu-count",
 		Usage: "number of CPUs available to the container",
+	}, cli.StringSliceFlag{
+		Name:  "device",
+		Usage: "identifier of a device to add to the container  (e.g. class://5B45201D-F2F2-4F3B-85BB-30FF1F953599)",
 	})
 }

cmd/ctr/commands/run/run_windows.go

@@ -19,6 +19,7 @@ package run
 import (
 	gocontext "context"
 	"errors"
+	"strings"
 
 	"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
 	"github.com/containerd/console"
@@ -142,6 +143,16 @@ func NewContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 		if ccount != 0 {
 			opts = append(opts, oci.WithWindowsCPUCount(ccount))
 		}
+		for _, dev := range context.StringSlice("device") {
+			parts := strings.Split(dev, "://")
+			if len(parts) != 2 {
+				return nil, errors.New("devices must be in the format IDType://ID")
+			}
+			if parts[0] == "" {
+				return nil, errors.New("devices must have a non-empty IDType")
+			}
+			opts = append(opts, oci.WithWindowsDevice(parts[0], parts[1]))
+		}
 	}
 
 	runtime := context.String("runtime")

integration/main_test.go

@@ -297,6 +297,15 @@ func WithSupplementalGroups(gids []int64) ContainerOpts { //nolint:unused
 	}
 }
 
+// WithDevice adds a device mount.
+func WithDevice(containerPath, hostPath, permissions string) ContainerOpts { //nolint:unused
+	return func(c *runtime.ContainerConfig) {
+		c.Devices = append(c.Devices, &runtime.Device{
+			ContainerPath: containerPath, HostPath: hostPath, Permissions: permissions,
+		})
+	}
+}
+
 // ContainerConfig creates a container config given a name and image name
 // and additional container config options
 func ContainerConfig(name, image string, opts ...ContainerOpts) *runtime.ContainerConfig {

integration/windows_device_test.go

@@ -0,0 +1,90 @@
+//go:build windows
+// +build windows
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package integration
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
+)
+
+func TestWindowsDevice(t *testing.T) {
+	testPodLogDir := t.TempDir()
+
+	t.Log("Create a sandbox with log directory")
+	sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox", "windows-device",
+		WithPodLogDirectory(testPodLogDir),
+	)
+
+	var (
+		// TODO: An image with the device-dumper
+		testImage     = GetImage(BusyBox)
+		containerName = "test-container"
+	)
+
+	const (
+		// Mount this device class to expose host-GPU-accelerated DirectX inside the container
+		// https://techcommunity.microsoft.com/t5/containers/bringing-gpu-acceleration-to-windows-containers/ba-p/393939
+		GUID_DEVINTERFACE_DISPLAY_ADAPTER = "5B45201D-F2F2-4F3B-85BB-30FF1F953599" //nolint:revive
+	)
+
+	EnsureImageExists(t, testImage)
+
+	t.Log("Create a container to run the device test")
+	cnConfig := ContainerConfig(
+		containerName,
+		testImage,
+		// Per C:\windows\System32\containers\devices.def, enabling GUID_DEVINTERFACE_DISPLAY_ADAPTER
+		// will mount the host driver store into the container at this location.
+		WithCommand("sh", "-c", "ls -d /Windows/System32/HostDriverStore/* | grep /Windows/System32/HostDriverStore/FileRepository"),
+		WithLogPath(containerName),
+		WithDevice("", "class/"+GUID_DEVINTERFACE_DISPLAY_ADAPTER, ""),
+	)
+	cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig)
+	require.NoError(t, err)
+
+	t.Log("Start the container")
+	require.NoError(t, runtimeService.StartContainer(cn))
+
+	t.Log("Wait for container to finish running")
+	require.NoError(t, Eventually(func() (bool, error) {
+		s, err := runtimeService.ContainerStatus(cn)
+		if err != nil {
+			return false, err
+		}
+		if s.GetState() == runtime.ContainerState_CONTAINER_EXITED {
+			return true, nil
+		}
+		return false, nil
+	}, time.Second, 30*time.Second))
+
+	t.Log("Check container log")
+	content, err := os.ReadFile(filepath.Join(testPodLogDir, containerName))
+	assert.NoError(t, err)
+	checkContainerLog(t, string(content), []string{
+		fmt.Sprintf("%s %s %s", runtime.Stdout, runtime.LogTagFull, "/Windows/System32/HostDriverStore/FileRepository"),
+	})
+}

oci/spec_opts.go

@@ -1367,3 +1367,17 @@ func tryReadonlyMounts(mounts []mount.Mount) []mount.Mount {
 	}
 	return mounts
 }
+
+// WithWindowsDevice adds a device exposed to a Windows (WCOW or LCOW) Container
+func WithWindowsDevice(idType, id string) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+		if idType == "" {
+			return errors.New("missing idType")
+		}
+		if s.Windows == nil {
+			s.Windows = &specs.Windows{}
+		}
+		s.Windows.Devices = append(s.Windows.Devices, specs.WindowsDevice{IDType: idType, ID: id})
+		return nil
+	}
+}

oci/spec_opts_test.go

@@ -30,6 +30,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/containerd/containerd/content"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -704,3 +707,75 @@ func TestWithoutMounts(t *testing.T) {
 		t.Fatalf("expected %+v, got %+v", expected, s.Mounts)
 	}
 }
+
+func TestWithWindowsDevice(t *testing.T) {
+	testcases := []struct {
+		name   string
+		idType string
+		id     string
+
+		expectError            bool
+		expectedWindowsDevices []specs.WindowsDevice
+	}{
+		{
+			name:        "empty_idType_and_id",
+			idType:      "",
+			id:          "",
+			expectError: true,
+		},
+		{
+			name:        "empty_idType",
+			idType:      "",
+			id:          "5B45201D-F2F2-4F3B-85BB-30FF1F953599",
+			expectError: true,
+		},
+		{
+			name:   "empty_id",
+			idType: "class",
+			id:     "",
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "", IDType: "class"}},
+		},
+		{
+			name:   "idType_and_id",
+			idType: "class",
+			id:     "5B45201D-F2F2-4F3B-85BB-30FF1F953599",
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "5B45201D-F2F2-4F3B-85BB-30FF1F953599", IDType: "class"}},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			spec := Spec{
+				Version: specs.Version,
+				Root:    &specs.Root{},
+				Windows: &specs.Windows{},
+			}
+
+			opts := []SpecOpts{
+				WithWindowsDevice(tc.idType, tc.id),
+			}
+
+			for _, opt := range opts {
+				if err := opt(nil, nil, nil, &spec); err != nil {
+					if tc.expectError {
+						assert.Error(t, err)
+					} else {
+						require.NoError(t, err)
+					}
+				}
+			}
+
+			if len(tc.expectedWindowsDevices) != 0 {
+				require.NotNil(t, spec.Windows)
+				require.NotNil(t, spec.Windows.Devices)
+				assert.ElementsMatch(t, spec.Windows.Devices, tc.expectedWindowsDevices)
+			} else if spec.Windows != nil && spec.Windows.Devices != nil {
+				assert.ElementsMatch(t, spec.Windows.Devices, tc.expectedWindowsDevices)
+			}
+		})
+	}
+}

pkg/cri/opts/spec_windows.go

@@ -230,3 +230,34 @@ func WithWindowsCredentialSpec(credentialSpec string) oci.SpecOpts {
 		return nil
 	}
 }
+
+// WithDevices sets the provided devices onto the container spec
+func WithDevices(config *runtime.ContainerConfig) oci.SpecOpts {
+	return func(ctx context.Context, client oci.Client, c *containers.Container, s *runtimespec.Spec) (err error) {
+		for _, device := range config.GetDevices() {
+			if device.ContainerPath != "" {
+				return fmt.Errorf("unexpected ContainerPath %s, must be empty", device.ContainerPath)
+			}
+
+			if device.Permissions != "" {
+				return fmt.Errorf("unexpected Permissions %s, must be empty", device.Permissions)
+			}
+
+			hostPath := device.HostPath
+			if strings.HasPrefix(hostPath, "class/") {
+				hostPath = strings.Replace(hostPath, "class/", "class://", 1)
+			}
+
+			splitParts := strings.SplitN(hostPath, "://", 2)
+			if len(splitParts) != 2 {
+				return fmt.Errorf("unrecognised HostPath format %v, must match IDType://ID", device.HostPath)
+			}
+
+			o := oci.WithWindowsDevice(splitParts[0], splitParts[1])
+			if err := o(ctx, client, c, s); err != nil {
+				return fmt.Errorf("failed adding device with HostPath %v: %w", device.HostPath, err)
+			}
+		}
+		return nil
+	}
+}

pkg/cri/opts/spec_windows_test.go

@@ -0,0 +1,217 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package opts
+
+import (
+	"context"
+	"testing"
+
+	"github.com/containerd/containerd/containers"
+	"github.com/containerd/containerd/namespaces"
+	"github.com/containerd/containerd/oci"
+	"github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
+)
+
+func TestWithDevices(t *testing.T) {
+	testcases := []struct {
+		name    string
+		devices []*runtime.Device
+		isLCOW  bool
+
+		expectError            bool
+		expectedWindowsDevices []specs.WindowsDevice
+	}{
+		{
+			name: "empty",
+
+			expectError: false,
+		},
+		// The only supported field is HostPath
+		{
+			name:    "empty fields",
+			devices: []*runtime.Device{{}},
+
+			expectError: true,
+		},
+		{
+			name:    "containerPath",
+			devices: []*runtime.Device{{ContainerPath: "something"}},
+
+			expectError: true,
+		},
+		{
+			name:    "permissions",
+			devices: []*runtime.Device{{Permissions: "something"}},
+
+			expectError: true,
+		},
+		// Produced by https://github.com/aarnaud/k8s-directx-device-plugin/blob/0f3db32622daa577c85621941682bee6f9080954/cmd/k8s-device-plugin/main.go
+		// This is also the syntax dockershim and cri-dockerd support (or rather, pass through to docker, which parses this syntax)
+		{
+			name:    "hostPath_docker_style",
+			devices: []*runtime.Device{{HostPath: "class/5B45201D-F2F2-4F3B-85BB-30FF1F953599"}},
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "5B45201D-F2F2-4F3B-85BB-30FF1F953599", IDType: "class"}},
+		},
+		// Docker _only_ accepts `class` ID Type, so anything else should fail.
+		// See https://github.com/moby/moby/blob/v20.10.13/daemon/oci_windows.go#L283-L294
+		{
+			name:    "hostPath_docker_style_non-class_idtype",
+			devices: []*runtime.Device{{HostPath: "vpci-location-path/5B45201D-F2F2-4F3B-85BB-30FF1F953599"}},
+
+			expectError: true,
+		},
+		// A bunch of examples from https://github.com/microsoft/hcsshim/blob/v0.9.2/test/cri-containerd/container_virtual_device_test.go
+		{
+			name: "hostPath_hcsshim_lcow_gpu",
+			// Not actually a GPU PCIP instance, but my personal machine doesn't have any PCIP devices, so I found one on the 'net.
+			devices: []*runtime.Device{{HostPath: `gpu://PCIP\VEN_8086&DEV_43A2&SUBSYS_72708086&REV_00\3&11583659&0&F5`}},
+			isLCOW:  true,
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: `PCIP\VEN_8086&DEV_43A2&SUBSYS_72708086&REV_00\3&11583659&0&F5`, IDType: "gpu"}},
+		},
+		{
+			name:    "hostPath_hcsshim_wcow_location_path",
+			devices: []*runtime.Device{{HostPath: "vpci-location-path://PCIROOT(0)#PCI(0100)#PCI(0000)#PCI(0000)#PCI(0001)"}},
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "PCIROOT(0)#PCI(0100)#PCI(0000)#PCI(0000)#PCI(0001)", IDType: "vpci-location-path"}},
+		},
+		{
+			name:    "hostPath_hcsshim_wcow_class_guid",
+			devices: []*runtime.Device{{HostPath: "class://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}},
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "5B45201D-F2F2-4F3B-85BB-30FF1F953599", IDType: "class"}},
+		},
+		{
+			name: "hostPath_hcsshim_wcow_gpu_hyper-v",
+			// Not actually a GPU PCIP instance, but my personal machine doesn't have any PCIP devices, so I found one on the 'net.
+			devices: []*runtime.Device{{HostPath: `vpci://PCIP\VEN_8086&DEV_43A2&SUBSYS_72708086&REV_00\3&11583659&0&F5`}},
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: `PCIP\VEN_8086&DEV_43A2&SUBSYS_72708086&REV_00\3&11583659&0&F5`, IDType: "vpci"}},
+		},
+		// Example from https://github.com/microsoft/hcsshim/blob/v0.9.2/test/cri-containerd/container_test.go
+		// According to https://github.com/jterry75/cri/blob/f8e83e63cc027d0e9c0c984f9db3cba58d3672d4/pkg/server/container_create_windows.go#L625-L649
+		// this is intended to generate LinuxDevice entries that the GCS shim in a LCOW container host will remap
+		// into device mounts from its own in-UVM kernel.
+		// From discussion on https://github.com/containerd/containerd/pull/6618, we reject this syntax for now.
+		{
+			name:    "hostPath_hcsshim_lcow_sandbox_device",
+			devices: []*runtime.Device{{HostPath: "/dev/fuse"}},
+			isLCOW:  true,
+
+			expectError: true,
+		},
+		// Some edge cases suggested by the above real-world examples
+		{
+			name:    "hostPath_no_slash",
+			devices: []*runtime.Device{{HostPath: "no_slash"}},
+
+			expectError: true,
+		},
+		{
+			name:    "hostPath_but_no_type",
+			devices: []*runtime.Device{{HostPath: "://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}},
+
+			expectError: true,
+		},
+		{
+			name:    "hostPath_but_no_id",
+			devices: []*runtime.Device{{HostPath: "gpu://"}},
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "", IDType: "gpu"}},
+		},
+		{
+			name:    "hostPath_dockerstyle_with_slashes_in_id",
+			devices: []*runtime.Device{{HostPath: "class/slashed/id"}},
+
+			expectError:            false,
+			expectedWindowsDevices: []specs.WindowsDevice{{ID: "slashed/id", IDType: "class"}},
+		},
+		{
+			name:    "hostPath_docker_style_non-class_idtypewith_slashes_in_id",
+			devices: []*runtime.Device{{HostPath: "vpci-location-path/slashed/id"}},
+
+			expectError: true,
+		},
+		{
+			name: "hostPath_hcsshim_wcow_location_path_twice",
+			devices: []*runtime.Device{
+				{HostPath: "vpci-location-path://PCIROOT(0)#PCI(0100)#PCI(0000)#PCI(0000)#PCI(0001)"},
+				{HostPath: "vpci-location-path://PCIROOT(0)#PCI(0100)#PCI(0000)#PCI(0000)#PCI(0002)"}},
+
+			expectError: false,
+			expectedWindowsDevices: []specs.WindowsDevice{
+				{ID: "PCIROOT(0)#PCI(0100)#PCI(0000)#PCI(0000)#PCI(0001)", IDType: "vpci-location-path"},
+				{ID: "PCIROOT(0)#PCI(0100)#PCI(0000)#PCI(0000)#PCI(0002)", IDType: "vpci-location-path"},
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			var (
+				ctx = namespaces.WithNamespace(context.Background(), "testing")
+				c   = &containers.Container{ID: t.Name()}
+			)
+
+			config := runtime.ContainerConfig{}
+			config.Devices = tc.devices
+
+			specOpts := []oci.SpecOpts{WithDevices(&config)}
+
+			platform := "windows"
+			if tc.isLCOW {
+				platform = "linux"
+			}
+
+			spec, err := oci.GenerateSpecWithPlatform(ctx, nil, platform, c, specOpts...)
+			if tc.expectError {
+				assert.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			// Ensure we got the right LCOWness in the spec
+			if tc.isLCOW {
+				assert.NotNil(t, spec.Linux)
+			} else {
+				assert.Nil(t, spec.Linux)
+			}
+
+			if len(tc.expectedWindowsDevices) != 0 {
+				require.NotNil(t, spec.Windows)
+				require.NotNil(t, spec.Windows.Devices)
+				assert.Equal(t, spec.Windows.Devices, tc.expectedWindowsDevices)
+			} else if spec.Windows != nil && spec.Windows.Devices != nil {
+				assert.Empty(t, spec.Windows.Devices)
+			}
+
+			if spec.Linux != nil && spec.Linux.Devices != nil {
+				assert.Empty(t, spec.Linux.Devices)
+			}
+		})
+	}
+}

pkg/cri/server/container_create_windows.go

@@ -76,7 +76,7 @@ func (c *criService) containerSpec(
 		oci.WithHostname(sandboxConfig.GetHostname()),
 	)
 
-	specOpts = append(specOpts, customopts.WithWindowsMounts(c.os, config, extraMounts))
+	specOpts = append(specOpts, customopts.WithWindowsMounts(c.os, config, extraMounts), customopts.WithDevices(config))
 
 	// Start with the image config user and override below if RunAsUsername is not "".
 	username := imageConfig.User