diff --git a/oci/spec_opts_windows.go b/oci/spec_opts_windows.go index 4ddb13d3f7b23..177d7f295c41f 100644 --- 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 index eea6aacb82f6f..4a3e285e49bfe 100644 --- 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 index c534c180c4751..d6dee30a8a69a 100644 --- 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) + } + + splitParts := strings.SplitN(hostPath, "://", 2) + if len(splitParts) != 2 { + return fmt.Errorf("unrecognised hostPath format %v", device.HostPath) + } + + o := oci.WithWindowsDevice(splitParts[0], splitParts[1]) + if err := o(ctx, client, c, s); err != nil { + return err + } + } + return nil + } +} diff --git a/pkg/cri/opts/spec_windows_test.go b/pkg/cri/opts/spec_windows_test.go new file mode 100644 index 0000000000000..5bf93b666e23d --- /dev/null +++ b/pkg/cri/opts/spec_windows_test.go @@ -0,0 +1,237 @@ +/* + 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 + expectedLinuxDevices []specs.LinuxDevice + }{ + { + name: "empty", + isLCOW: false, + + expectError: false, + }, + // The only supported field is HostPath + { + name: "empty fields", + devices: []*runtime.Device{{}}, + isLCOW: false, + + expectError: true, + }, + { + name: "containerPath", + devices: []*runtime.Device{{ContainerPath: "something"}}, + isLCOW: false, + + expectError: true, + }, + { + name: "permissions", + devices: []*runtime.Device{{Permissions: "something"}}, + isLCOW: false, + + expectError: true, + }, + // Produced by https://github.com/aarnaud/k8s-directx-device-plugin/blob/master/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"}}, + isLCOW: false, + + expectError: false, + expectedWindowsDevices: []specs.WindowsDevice{{ID: "5B45201D-F2F2-4F3B-85BB-30FF1F953599", IDType: "class"}}, + }, + // Docker _only_ accepts `class` ID Type, so anything else should fail, even known IDTypes + // See https://github.com/moby/moby/blob/07bb45e23a364f0d27a82d9ac13ab79846bd7add/daemon/oci_windows.go#L262-L283 + { + name: "hostPath_docker_style_non-class_idtype", + devices: []*runtime.Device{{HostPath: "vpci-location-path/5B45201D-F2F2-4F3B-85BB-30FF1F953599"}}, + isLCOW: false, + + expectError: true, + }, + // A bunch of examples from https://github.com/microsoft/hcsshim/blob/master/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)"}}, + isLCOW: false, + + 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"}}, + isLCOW: false, + + 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"}}, + isLCOW: false, + + 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/master/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"}}, + isLCOW: false, + + expectError: true, + }, + { + name: "hostPath_but_no_type", + devices: []*runtime.Device{{HostPath: "://5B45201D-F2F2-4F3B-85BB-30FF1F953599"}}, + isLCOW: false, + + expectError: true, + }, + { + name: "hostPath_but_no_id", + devices: []*runtime.Device{{HostPath: "gpu://"}}, + isLCOW: false, + + expectError: false, + expectedWindowsDevices: []specs.WindowsDevice{{ID: "", IDType: "gpu"}}, + }, + { + name: "hostPath_dockerstyle_with_slashes_in_id", + devices: []*runtime.Device{{HostPath: "class/slashed/id"}}, + isLCOW: false, + + 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"}}, + isLCOW: false, + + 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)"}}, + isLCOW: false, + + 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.Equal(t, spec.Windows.Devices, tc.expectedWindowsDevices) + } + + if len(tc.expectedLinuxDevices) != 0 { + require.NotNil(t, spec.Linux) + require.NotNil(t, spec.Linux.Devices) + assert.Equal(t, spec.Linux.Devices, tc.expectedLinuxDevices) + } else if spec.Linux != nil && spec.Linux.Devices != nil { + assert.Equal(t, spec.Linux.Devices, tc.expectedLinuxDevices) + } + }) + } +} diff --git a/pkg/cri/server/container_create_windows.go b/pkg/cri/server/container_create_windows.go index 0af0ec684f104..2343682f50671 100644 --- a/pkg/cri/server/container_create_windows.go +++ b/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