Merge pull request #9295 from klihub/fixes/release-1.33/delay-cdi-device-injection

openshift-merge-bot[bot] · web-flow · commit 4a121aa3c99f · 2025-07-17T16:43:45.000Z
[release-1.33] server,factory/container: delay CDI device injection later.
diff --git a/internal/factory/container/container.go b/internal/factory/container/container.go
@@ -111,6 +111,9 @@ type Container interface {
 	// SpecAddDevices adds devices from the server config, and container CRI config
 	SpecAddDevices([]device.Device, []device.Device, bool, bool) error
 
+	// SpecInjectCDIDevices injects any requested CDI devices to the container's Spec.
+	SpecInjectCDIDevices() error
+
 	// AddUnifiedResourcesFromAnnotations adds the cgroup-v2 resources specified in the io.kubernetes.cri-o.UnifiedCgroup annotation
 	AddUnifiedResourcesFromAnnotations(annotationsMap map[string]string) error
 
diff --git a/internal/factory/container/device_freebsd.go b/internal/factory/container/device_freebsd.go
@@ -1,9 +1,19 @@
 package container
 
 import (
+	"fmt"
+	"runtime"
+
 	devicecfg "github.com/cri-o/cri-o/internal/config/device"
 )
 
 func (c *container) SpecAddDevices(configuredDevices, annotationDevices []devicecfg.Device, privilegedWithoutHostDevices, enableDeviceOwnershipFromSecurityContext bool) error {
 	return nil
 }
+
+func (c *container) SpecInjectCDIDevices() error {
+	if len(c.Config().CDIDevices) > 0 {
+		return fmt.Errorf("(*container).SpecInjectCDIDevices not supported on %s", runtime.GOOS)
+	}
+	return nil
+}
diff --git a/internal/factory/container/device_linux.go b/internal/factory/container/device_linux.go
@@ -49,8 +49,7 @@ func (c *container) SpecAddDevices(configuredDevices, annotationDevices []device
 		return err
 	}
 
-	// Finally, inject CDI devices
-	return c.specInjectCDIDevices()
+	return nil
 }
 
 func (c *container) specAddHostDevicesIfPrivileged(privilegedWithoutHostDevices bool) error {
@@ -185,7 +184,7 @@ func (c *container) specAddContainerConfigDevices(enableDeviceOwnershipFromSecur
 	return nil
 }
 
-func (c *container) specInjectCDIDevices() error {
+func (c *container) SpecInjectCDIDevices() error {
 	var (
 		cdiDevices = c.Config().CDIDevices
 		fromCRI    = map[string]struct{}{}
diff --git a/internal/factory/container/device_test.go b/internal/factory/container/device_test.go
@@ -186,7 +186,7 @@ var _ = t.Describe("Container", func() {
 		}
 	})
 
-	t.Describe("SpecAdd(CDI)Devices", func() {
+	t.Describe("SpecInjectCDIDevices", func() {
 		writeCDISpecFiles := func(content []string) error {
 			if len(content) == 0 {
 				return nil
@@ -421,7 +421,7 @@ containerEdits:
 				Expect(writeCDISpecFiles(test.cdiSpecFiles)).To(Succeed())
 
 				// When
-				err := sut.SpecAddDevices(nil, nil, false, false)
+				err := sut.SpecInjectCDIDevices()
 
 				// Then
 				Expect(err != nil).To(Equal(test.expectError))
diff --git a/internal/factory/container/device_unsupported.go b/internal/factory/container/device_unsupported.go
@@ -12,3 +12,10 @@ import (
 func (c *container) SpecAddDevices(configuredDevices, annotationDevices []devicecfg.Device, privilegedWithoutHostDevices, enableDeviceOwnershipFromSecurityContext bool) error {
 	return fmt.Errorf("(*container).SpecAddDevices not supported on %s", runtime.GOOS)
 }
+
+func (c *container) SpecInjectCDIDevices() error {
+	if len(c.Config().CDIDevices) > 0 {
+		return fmt.Errorf("(*container).SpecInjectCDIDevices not supported on %s", runtime.GOOS)
+	}
+	return nil
+}
diff --git a/server/container_create.go b/server/container_create.go
@@ -1199,6 +1199,10 @@ func (s *Server) createSandboxContainer(ctx context.Context, ctr container.Conta
 		}
 	}
 
+	if err := ctr.SpecInjectCDIDevices(); err != nil {
+		return nil, err
+	}
+
 	// Set up pids limit if pids cgroup is mounted
 	if node.CgroupHasPid() {
 		specgen.SetLinuxResourcesPidsLimit(s.config.PidsLimit)
diff --git a/test/cdi.bats b/test/cdi.bats
@@ -111,7 +111,7 @@ function annotate_ctr_with_unknown_cdidev {
 }
 
 function prepare_ctr_with_cdidev {
-	jq ".CDI_Devices |= . + [ { \"Name\": \"vendor0.com/device=loop8\" }, { \"Name\": \"vendor0.com/device=loop9\" } ]" \
+	jq ".CDI_Devices |= . + [ { \"Name\": \"vendor0.com/device=loop8\" }, { \"Name\": \"vendor0.com/device=loop9\" } ] | .envs |= . + [ { \"key\": \"VENDOR0\", \"value\": \"unset\" }, { \"key\": \"LOOP8\", \"value\": \"unset\" } ]" \
 		"$TESTDATA/container_sleep.json" > "$ctr_config"
 }
 

Original file line number	Diff line number	Diff line change
`@@ -49,8 +49,7 @@ func (c *container) SpecAddDevices(configuredDevices, annotationDevices []device`
`49`	`49`	`return err`
`50`	`50`	`}`
`51`	`51`
`52`		`- // Finally, inject CDI devices`
`53`		`- return c.specInjectCDIDevices()`
	`52`	`+ return nil`
`54`	`53`	`}`
`55`	`54`
`56`	`55`	`func (c *container) specAddHostDevicesIfPrivileged(privilegedWithoutHostDevices bool) error {`
`@@ -185,7 +184,7 @@ func (c *container) specAddContainerConfigDevices(enableDeviceOwnershipFromSecur`
`185`	`184`	`return nil`
`186`	`185`	`}`
`187`	`186`
`188`		`-func (c *container) specInjectCDIDevices() error {`
	`187`	`+func (c *container) SpecInjectCDIDevices() error {`
`189`	`188`	`var (`
`190`	`189`	`cdiDevices = c.Config().CDIDevices`
`191`	`190`	`fromCRI = map[string]struct{}{}`
Original file line number	Diff line number	Diff line change
`@@ -1199,6 +1199,10 @@ func (s *Server) createSandboxContainer(ctx context.Context, ctr container.Conta`
`1199`	`1199`	`}`
`1200`	`1200`	`}`
`1201`	`1201`
	`1202`	`+ if err := ctr.SpecInjectCDIDevices(); err != nil {`
	`1203`	`+ return nil, err`
	`1204`	`+ }`
	`1205`	`+`
`1202`	`1206`	`// Set up pids limit if pids cgroup is mounted`
`1203`	`1207`	`if node.CgroupHasPid() {`
`1204`	`1208`	`specgen.SetLinuxResourcesPidsLimit(s.config.PidsLimit)`
Original file line number	Diff line number	Diff line change
`@@ -111,7 +111,7 @@ function annotate_ctr_with_unknown_cdidev {`
`111`	`111`	`}`
`112`	`112`
`113`	`113`	`function prepare_ctr_with_cdidev {`
`114`		`- jq ".CDI_Devices \|= . + [ { \"Name\": \"vendor0.com/device=loop8\" }, { \"Name\": \"vendor0.com/device=loop9\" } ]" \`
	`114`	`+ jq ".CDI_Devices \|= . + [ { \"Name\": \"vendor0.com/device=loop8\" }, { \"Name\": \"vendor0.com/device=loop9\" } ] \| .envs \|= . + [ { \"key\": \"VENDOR0\", \"value\": \"unset\" }, { \"key\": \"LOOP8\", \"value\": \"unset\" } ]" \`
`115`	`115`	`"$TESTDATA/container_sleep.json" > "$ctr_config"`
`116`	`116`	`}`
`117`	`117`