diff --git a/pkg/cri/opts/spec_linux.go b/pkg/cri/opts/spec_linux.go index 69621e8a9..e268b9dfd 100644 --- a/pkg/cri/opts/spec_linux.go +++ b/pkg/cri/opts/spec_linux.go @@ -30,10 +30,12 @@ import ( "github.com/containerd/cgroups/v3" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" + runtime "k8s.io/cri-api/pkg/apis/runtime/v1" "github.com/containerd/containerd/containers" "github.com/containerd/containerd/log" "github.com/containerd/containerd/oci" + ctrdutil "github.com/containerd/containerd/pkg/cri/util" ) // Linux dependent OCI spec opts. @@ -139,18 +141,40 @@ func IsCgroup2UnifiedMode() bool { } // WithCDI updates OCI spec with CDI content -func WithCDI(annotations map[string]string) oci.SpecOpts { +func WithCDI(annotations map[string]string, CDIDevices []*runtime.CDIDevice) oci.SpecOpts { return func(ctx context.Context, _ oci.Client, c *containers.Container, s *oci.Spec) error { - // TODO: Once CRI is extended with native CDI support this will need to be updated... - _, cdiDevices, err := cdi.ParseAnnotations(annotations) + // Add devices from CDIDevices CRI field + var devices []string + var err error + for _, device := range CDIDevices { + devices = append(devices, device.Name) + } + log.G(ctx).Infof("Container %v: CDI devices from CRI Config.CDIDevices: %v", c.ID, devices) + + // Add devices from CDI annotations + _, devsFromAnnotations, err := cdi.ParseAnnotations(annotations) if err != nil { return fmt.Errorf("failed to parse CDI device annotations: %w", err) } - if cdiDevices == nil { - return nil + + if devsFromAnnotations != nil { + log.G(ctx).Infof("Container %v: CDI devices from annotations: %v", c.ID, devsFromAnnotations) + for _, deviceName := range devsFromAnnotations { + if ctrdutil.InStringSlice(devices, deviceName) { + // TODO: change to Warning when passing CDI devices as annotations is deprecated + log.G(ctx).Debugf("Skipping duplicated CDI device %s", deviceName) + continue + } + devices = append(devices, deviceName) + } + // TODO: change to Warning when passing CDI devices as annotations is deprecated + log.G(ctx).Debug("Passing CDI devices as annotations will be deprecated soon, please use CRI CDIDevices instead") } - log.G(ctx).Infof("container %v: CDI devices: %v", c.ID, cdiDevices) + if len(devices) == 0 { + // No devices found, skip device injection + return nil + } registry := cdi.GetRegistry() if err = registry.Refresh(); err != nil { @@ -162,7 +186,7 @@ func WithCDI(annotations map[string]string) oci.SpecOpts { log.G(ctx).Warnf("CDI registry refresh failed: %v", err) } - if _, err := registry.InjectDevices(s, cdiDevices...); err != nil { + if _, err := registry.InjectDevices(s, devices...); err != nil { return fmt.Errorf("CDI device injection failed: %w", err) } diff --git a/pkg/cri/sbserver/container_create_linux.go b/pkg/cri/sbserver/container_create_linux.go index 79639b3dd..49b5e79bf 100644 --- a/pkg/cri/sbserver/container_create_linux.go +++ b/pkg/cri/sbserver/container_create_linux.go @@ -122,7 +122,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, seccompSpecOpts) } if c.config.EnableCDI { - specOpts = append(specOpts, customopts.WithCDI(config.Annotations)) + specOpts = append(specOpts, customopts.WithCDI(config.Annotations, config.CDIDevices)) } return specOpts, nil } diff --git a/pkg/cri/sbserver/container_create_linux_test.go b/pkg/cri/sbserver/container_create_linux_test.go index 01b7f5611..2638acf63 100644 --- a/pkg/cri/sbserver/container_create_linux_test.go +++ b/pkg/cri/sbserver/container_create_linux_test.go @@ -1634,13 +1634,20 @@ func TestCDIInjections(t *testing.T) { for _, test := range []struct { description string cdiSpecFiles []string + cdiDevices []*runtime.CDIDevice annotations map[string]string expectError bool expectDevices []runtimespec.LinuxDevice expectEnv []string }{ - {description: "expect no CDI error for nil annotations"}, - {description: "expect no CDI error for empty annotations", + {description: "expect no CDI error for nil annotations", + cdiDevices: []*runtime.CDIDevice{}, + }, + {description: "expect no CDI error for nil CDIDevices", + annotations: map[string]string{}, + }, + {description: "expect no CDI error for empty CDI devices and annotations", + cdiDevices: []*runtime.CDIDevice{}, annotations: map[string]string{}, }, {description: "expect CDI error for invalid CDI device reference in annotations", @@ -1649,13 +1656,25 @@ func TestCDIInjections(t *testing.T) { }, expectError: true, }, - {description: "expect CDI error for unresolvable devices", + {description: "expect CDI error for invalid CDI device reference in CDIDevices", + cdiDevices: []*runtime.CDIDevice{ + {Name: "foobar"}, + }, + expectError: true, + }, + {description: "expect CDI error for unresolvable devices in annotations", annotations: map[string]string{ cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=no-such-dev", }, expectError: true, }, - {description: "expect properly injected resolvable CDI devices", + {description: "expect CDI error for unresolvable devices in CDIDevices", + cdiDevices: []*runtime.CDIDevice{ + {Name: "vendor1.com/device=no-such-dev"}, + }, + expectError: true, + }, + {description: "expect properly injected resolvable CDI devices from annotations", cdiSpecFiles: []string{ ` cdiVersion: "0.3.0" @@ -1717,6 +1736,185 @@ containerEdits: "VENDOR2=present", }, }, + {description: "expect properly injected resolvable CDI devices from CDIDevices", + cdiSpecFiles: []string{ + ` +cdiVersion: "0.3.0" +kind: "vendor1.com/device" +devices: + - name: foo + containerEdits: + deviceNodes: + - path: /dev/loop8 + type: b + major: 7 + minor: 8 + env: + - FOO=injected +containerEdits: + env: + - "VENDOR1=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor2.com/device" +devices: + - name: bar + containerEdits: + deviceNodes: + - path: /dev/loop9 + type: b + major: 7 + minor: 9 + env: + - BAR=injected +containerEdits: + env: + - "VENDOR2=present" +`, + }, + cdiDevices: []*runtime.CDIDevice{ + {Name: "vendor1.com/device=foo"}, + {Name: "vendor2.com/device=bar"}, + }, + expectDevices: []runtimespec.LinuxDevice{ + { + Path: "/dev/loop8", + Type: "b", + Major: 7, + Minor: 8, + }, + { + Path: "/dev/loop9", + Type: "b", + Major: 7, + Minor: 9, + }, + }, + expectEnv: []string{ + "FOO=injected", + "VENDOR1=present", + "BAR=injected", + "VENDOR2=present", + }, + }, + {description: "expect CDI devices from CDIDevices and annotations", + cdiSpecFiles: []string{ + ` +cdiVersion: "0.3.0" +kind: "vendor1.com/device" +devices: + - name: foo + containerEdits: + deviceNodes: + - path: /dev/loop8 + type: b + major: 7 + minor: 8 + env: + - FOO=injected +containerEdits: + env: + - "VENDOR1=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor2.com/device" +devices: + - name: bar + containerEdits: + deviceNodes: + - path: /dev/loop9 + type: b + major: 7 + minor: 9 + env: + - BAR=injected +containerEdits: + env: + - "VENDOR2=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor3.com/device" +devices: + - name: foo3 + containerEdits: + deviceNodes: + - path: /dev/loop10 + type: b + major: 7 + minor: 10 + env: + - FOO3=injected +containerEdits: + env: + - "VENDOR3=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor4.com/device" +devices: + - name: bar4 + containerEdits: + deviceNodes: + - path: /dev/loop11 + type: b + major: 7 + minor: 11 + env: + - BAR4=injected +containerEdits: + env: + - "VENDOR4=present" +`, + }, + cdiDevices: []*runtime.CDIDevice{ + {Name: "vendor1.com/device=foo"}, + {Name: "vendor2.com/device=bar"}, + {Name: "vendor3.com/device=foo3"}, + }, + annotations: map[string]string{ + cdi.AnnotationPrefix + "vendor3_devices": "vendor3.com/device=foo3", // Duplicated device, should be ignored + cdi.AnnotationPrefix + "vendor4_devices": "vendor4.com/device=bar4", + }, + expectDevices: []runtimespec.LinuxDevice{ + { + Path: "/dev/loop8", + Type: "b", + Major: 7, + Minor: 8, + }, + { + Path: "/dev/loop9", + Type: "b", + Major: 7, + Minor: 9, + }, + { + Path: "/dev/loop10", + Type: "b", + Major: 7, + Minor: 10, + }, + { + Path: "/dev/loop11", + Type: "b", + Major: 7, + Minor: 11, + }, + }, + expectEnv: []string{ + "FOO=injected", + "VENDOR1=present", + "BAR=injected", + "VENDOR2=present", + "FOO3=injected", + "VENDOR3=present", + "BAR4=injected", + "VENDOR4=present", + }, + }, } { t.Run(test.description, func(t *testing.T) { spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) @@ -1734,7 +1932,7 @@ containerEdits: err = reg.Configure(cdi.WithSpecDirs(cdiDir)) require.NoError(t, err) - injectFun := customopts.WithCDI(test.annotations) + injectFun := customopts.WithCDI(test.annotations, test.cdiDevices) err = injectFun(ctx, nil, testContainer, spec) assert.Equal(t, test.expectError, err != nil) diff --git a/pkg/cri/server/container_create_linux.go b/pkg/cri/server/container_create_linux.go index d4206c654..d01c2c2e6 100644 --- a/pkg/cri/server/container_create_linux.go +++ b/pkg/cri/server/container_create_linux.go @@ -415,7 +415,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, seccompSpecOpts) } if c.config.EnableCDI { - specOpts = append(specOpts, customopts.WithCDI(config.Annotations)) + specOpts = append(specOpts, customopts.WithCDI(config.Annotations, config.CDIDevices)) } return specOpts, nil } diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index e0a7ea998..3afb7e77e 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -1814,13 +1814,20 @@ func TestCDIInjections(t *testing.T) { for _, test := range []struct { description string cdiSpecFiles []string + cdiDevices []*runtime.CDIDevice annotations map[string]string expectError bool expectDevices []runtimespec.LinuxDevice expectEnv []string }{ - {description: "expect no CDI error for nil annotations"}, - {description: "expect no CDI error for empty annotations", + {description: "expect no CDI error for nil annotations", + cdiDevices: []*runtime.CDIDevice{}, + }, + {description: "expect no CDI error for nil CDIDevices", + annotations: map[string]string{}, + }, + {description: "expect no CDI error for empty CDI devices and annotations", + cdiDevices: []*runtime.CDIDevice{}, annotations: map[string]string{}, }, {description: "expect CDI error for invalid CDI device reference in annotations", @@ -1829,13 +1836,25 @@ func TestCDIInjections(t *testing.T) { }, expectError: true, }, - {description: "expect CDI error for unresolvable devices", + {description: "expect CDI error for invalid CDI device reference in CDIDevices", + cdiDevices: []*runtime.CDIDevice{ + {Name: "foobar"}, + }, + expectError: true, + }, + {description: "expect CDI error for unresolvable devices in annotations", annotations: map[string]string{ cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=no-such-dev", }, expectError: true, }, - {description: "expect properly injected resolvable CDI devices", + {description: "expect CDI error for unresolvable devices in CDIDevices", + cdiDevices: []*runtime.CDIDevice{ + {Name: "vendor1.com/device=no-such-dev"}, + }, + expectError: true, + }, + {description: "expect properly injected resolvable CDI devices from annotations", cdiSpecFiles: []string{ ` cdiVersion: "0.3.0" @@ -1897,6 +1916,185 @@ containerEdits: "VENDOR2=present", }, }, + {description: "expect properly injected resolvable CDI devices from CDIDevices", + cdiSpecFiles: []string{ + ` +cdiVersion: "0.3.0" +kind: "vendor1.com/device" +devices: + - name: foo + containerEdits: + deviceNodes: + - path: /dev/loop8 + type: b + major: 7 + minor: 8 + env: + - FOO=injected +containerEdits: + env: + - "VENDOR1=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor2.com/device" +devices: + - name: bar + containerEdits: + deviceNodes: + - path: /dev/loop9 + type: b + major: 7 + minor: 9 + env: + - BAR=injected +containerEdits: + env: + - "VENDOR2=present" +`, + }, + cdiDevices: []*runtime.CDIDevice{ + {Name: "vendor1.com/device=foo"}, + {Name: "vendor2.com/device=bar"}, + }, + expectDevices: []runtimespec.LinuxDevice{ + { + Path: "/dev/loop8", + Type: "b", + Major: 7, + Minor: 8, + }, + { + Path: "/dev/loop9", + Type: "b", + Major: 7, + Minor: 9, + }, + }, + expectEnv: []string{ + "FOO=injected", + "VENDOR1=present", + "BAR=injected", + "VENDOR2=present", + }, + }, + {description: "expect CDI devices from CDIDevices and annotations", + cdiSpecFiles: []string{ + ` +cdiVersion: "0.3.0" +kind: "vendor1.com/device" +devices: + - name: foo + containerEdits: + deviceNodes: + - path: /dev/loop8 + type: b + major: 7 + minor: 8 + env: + - FOO=injected +containerEdits: + env: + - "VENDOR1=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor2.com/device" +devices: + - name: bar + containerEdits: + deviceNodes: + - path: /dev/loop9 + type: b + major: 7 + minor: 9 + env: + - BAR=injected +containerEdits: + env: + - "VENDOR2=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor3.com/device" +devices: + - name: foo3 + containerEdits: + deviceNodes: + - path: /dev/loop10 + type: b + major: 7 + minor: 10 + env: + - FOO3=injected +containerEdits: + env: + - "VENDOR3=present" +`, + ` +cdiVersion: "0.3.0" +kind: "vendor4.com/device" +devices: + - name: bar4 + containerEdits: + deviceNodes: + - path: /dev/loop11 + type: b + major: 7 + minor: 11 + env: + - BAR4=injected +containerEdits: + env: + - "VENDOR4=present" +`, + }, + cdiDevices: []*runtime.CDIDevice{ + {Name: "vendor1.com/device=foo"}, + {Name: "vendor2.com/device=bar"}, + {Name: "vendor3.com/device=foo3"}, + }, + annotations: map[string]string{ + cdi.AnnotationPrefix + "vendor3_devices": "vendor3.com/device=foo3", // Duplicated device, should be ignored + cdi.AnnotationPrefix + "vendor4_devices": "vendor4.com/device=bar4", + }, + expectDevices: []runtimespec.LinuxDevice{ + { + Path: "/dev/loop8", + Type: "b", + Major: 7, + Minor: 8, + }, + { + Path: "/dev/loop9", + Type: "b", + Major: 7, + Minor: 9, + }, + { + Path: "/dev/loop10", + Type: "b", + Major: 7, + Minor: 10, + }, + { + Path: "/dev/loop11", + Type: "b", + Major: 7, + Minor: 11, + }, + }, + expectEnv: []string{ + "FOO=injected", + "VENDOR1=present", + "BAR=injected", + "VENDOR2=present", + "FOO3=injected", + "VENDOR3=present", + "BAR4=injected", + "VENDOR4=present", + }, + }, } { t.Run(test.description, func(t *testing.T) { spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) @@ -1914,7 +2112,7 @@ containerEdits: err = reg.Configure(cdi.WithSpecDirs(cdiDir)) require.NoError(t, err) - injectFun := customopts.WithCDI(test.annotations) + injectFun := customopts.WithCDI(test.annotations, test.cdiDevices) err = injectFun(ctx, nil, testContainer, spec) assert.Equal(t, test.expectError, err != nil)