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

kzys · web-flow · commit d4641e1ce1e0 · 2022-03-11T14:53:54.000-08:00
Handle CRI Device.HostPath on Windows
diff --git a/cmd/ctr/commands/commands.go b/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)",
diff --git a/cmd/ctr/commands/commands_unix.go b/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",
 	})
 }
diff --git a/cmd/ctr/commands/commands_windows.go b/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)",
 	})
 }
diff --git a/cmd/ctr/commands/run/run_windows.go b/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")
diff --git a/integration/main_test.go b/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 {
diff --git a/integration/windows_device_test.go b/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
+
+       https://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"),
+	})
+}
diff --git a/oci/spec_opts.go b/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
+	}
+}
diff --git a/oci/spec_opts_test.go b/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)
+			}
+		})
+	}
+}
diff --git a/pkg/cri/opts/spec_windows.go b/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
+	}
+}
diff --git a/pkg/cri/opts/spec_windows_test.go b/pkg/cri/opts/spec_windows_test.go
diff --git a/pkg/cri/server/container_create_windows.go b/pkg/cri/server/container_create_windows.go

Original file line number	Diff line number	Diff line change
`@@ -40,5 +40,8 @@ func init() {`
`40`	`40`	`}, cli.StringFlag{`
`41`	`41`	`Name: "rootfs-propagation",`
`42`	`42`	`Usage: "set the propagation of the container rootfs",`
	`43`	`+ }, cli.StringSliceFlag{`
	`44`	`+ Name: "device",`
	`45`	`+ 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",`
`43`	`46`	`})`
`44`	`47`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,5 +24,8 @@ func init() {`
`24`	`24`	`ContainerFlags = append(ContainerFlags, cli.Uint64Flag{`
`25`	`25`	`Name: "cpu-count",`
`26`	`26`	`Usage: "number of CPUs available to the container",`
	`27`	`+ }, cli.StringSliceFlag{`
	`28`	`+ Name: "device",`
	`29`	`+ Usage: "identifier of a device to add to the container (e.g. class://5B45201D-F2F2-4F3B-85BB-30FF1F953599)",`
`27`	`30`	`})`
`28`	`31`	`}`