Get CDI devices from CRI Config.CDIDevices field

Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>

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...
+		// Add devices from CDIDevices CRI field
-		_, cdiDevices, err := cdi.ParseAnnotations(annotations)
+		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)
 		}
 

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
 }

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 nil annotations",
-		{description: "expect no CDI error for empty 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)
 

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
 }

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 nil annotations",
-		{description: "expect no CDI error for empty 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)