Plumb CRI Container Devices through to OCI WindowsDevices

There's two mappings of hostpath to IDType and ID in the wild: - dockershim and dockerd-cri (implicitly via docker) use class/ID -- The only supported IDType in Docker is 'class'. -- https://github.com/aarnaud/k8s-directx-device-plugin generates this form - https://github.com/jterry75/cri (windows_port branch) uses IDType://ID `://` is much more easily distinguishable, so I've gone with that one as the generic separator, with `class/` as a special-case. This also includes support for setting OCI LinuxDevices on LCOW OCI specs, but does not expose a mapping for that from CRI. While https://github.com/jterry75/cri supports a syntax for this, discussion on PR containerd#6618 indicates that this feature may be better-implemented as an OCI WindowsDevice IDType recognised at a lower level, so this syntax is being left unimplemented. Signed-off-by: Paul "TBBle" Hampson <[email protected]>
TBBle · Mar 9, 2022 · 777a77c · 777a77c
1 parent 9f94d2f
commit 777a77c
Show file tree

Hide file tree

Showing 5 changed files with 462 additions and 1 deletion.
diff --git a/oci/spec_opts_windows.go b/oci/spec_opts_windows.go
@@ -100,3 +100,32 @@ func escapeAndCombineArgs(args []string) string {
 	}
 	return strings.Join(escaped, " ")
 }
+
+// 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 len(IDType) == 0 {
+			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
+	}
+}
+
+// WithLCOWUVMDevice adds a device exposed from the LCOW kernel to an LCOW Container
+func WithLCOWUVMDevice(path string) SpecOpts {
+	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+		if len(path) == 0 {
+			return errors.New("missing path")
+		}
+		if s.Linux == nil {
+			return errors.New("spec is not LCOW")
+		}
+		s.Linux.Devices = append(s.Linux.Devices, specs.LinuxDevice{Path: path})
+		return nil
+	}
+
+}
diff --git a/oci/spec_opts_windows_test.go b/oci/spec_opts_windows_test.go
@@ -23,6 +23,8 @@ import (
 
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/namespaces"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -546,3 +548,152 @@ func TestWithImageConfigArgsEscapedWindows(t *testing.T) {
 		})
 	}
 }
+
+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)
+			}
+		})
+	}
+}
+
+func TestWithLCOWUVMDevice(t *testing.T) {
+	testcases := []struct {
+		name   string
+		path   string
+		isLCOW bool
+
+		expectError          bool
+		expectedLinuxDevices []specs.LinuxDevice
+	}{
+		{
+			name:   "empty_lcow",
+			path:   "",
+			isLCOW: true,
+
+			expectError: true,
+		},
+		{
+			name:   "fuse_lcow",
+			path:   "/dev/fuse",
+			isLCOW: true,
+
+			expectError:          false,
+			expectedLinuxDevices: []specs.LinuxDevice{{Path: "/dev/fuse"}},
+		},
+		{
+			name:   "empty_wcow",
+			path:   "",
+			isLCOW: false,
+
+			expectError: true,
+		},
+		{
+			name:   "fuse_wcow",
+			path:   "/dev/fuse",
+			isLCOW: false,
+
+			expectError: true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			spec := Spec{
+				Version: specs.Version,
+				Root:    &specs.Root{},
+				Windows: &specs.Windows{},
+			}
+
+			if tc.isLCOW {
+				spec.Linux = &specs.Linux{}
+			}
+
+			opts := []SpecOpts{
+				WithLCOWUVMDevice(tc.path),
+			}
+
+			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.expectedLinuxDevices) != 0 {
+				require.NotNil(t, spec.Linux)
+				require.NotNil(t, spec.Linux.Devices)
+				assert.ElementsMatch(t, spec.Linux.Devices, tc.expectedLinuxDevices)
+			} else if spec.Linux != nil && spec.Linux.Devices != nil {
+				assert.ElementsMatch(t, spec.Linux.Devices, tc.expectedLinuxDevices)
+			}
+		})
+	}
+}
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", device.ContainerPath)
+			}
+
+			if device.Permissions != "" {
+				return fmt.Errorf("unexpected Permissions %s", device.Permissions)
+			}
+
+			hostPath := device.HostPath
+			if strings.HasPrefix(hostPath, "class/") {
+				hostPath = strings.Replace(hostPath, "class/", "class://", 1)
+			}
+
+			if splitParts := strings.SplitN(hostPath, "://", 2); len(splitParts) == 2 {
+				o := oci.WithWindowsDevice(splitParts[0], splitParts[1])
+				if err := o(ctx, client, c, s); err != nil {
+					return err
+				}
+				return nil
+			}
+
+			return fmt.Errorf("unrecognised hostPath %v", device.HostPath)
+		}
+		return nil
+	}
+}