Move CDI device spec out of the OCI package

dmcgowan · dmcgowan · commit 3d53430fe14e · 2025-01-14T14:16:18.000-08:00
The CDI device injection spec opt was mistakenly added to the OCI package which brought in an unintended dependency on CDI and its transitive dependencies. Signed-off-by: Derek McGowan <derek@mcg.dev> (cherry picked from commit e20f7f4) Signed-off-by: Derek McGowan <derek@mcg.dev>
diff --git a/cmd/ctr/commands/run/run_unix.go b/cmd/ctr/commands/run/run_unix.go
@@ -27,16 +27,18 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/containerd/log"
+	"github.com/containerd/platforms"
+
 	containerd "github.com/containerd/containerd/v2/client"
 	"github.com/containerd/containerd/v2/cmd/ctr/commands"
 	"github.com/containerd/containerd/v2/contrib/apparmor"
 	"github.com/containerd/containerd/v2/contrib/nvidia"
 	"github.com/containerd/containerd/v2/contrib/seccomp"
 	"github.com/containerd/containerd/v2/core/containers"
 	"github.com/containerd/containerd/v2/core/snapshots"
+	cdispec "github.com/containerd/containerd/v2/pkg/cdi"
 	"github.com/containerd/containerd/v2/pkg/oci"
-	"github.com/containerd/log"
-	"github.com/containerd/platforms"
 
 	"github.com/intel/goresctrl/pkg/blockio"
 	"github.com/opencontainers/runtime-spec/specs-go"
@@ -358,7 +360,7 @@ func NewContainer(ctx context.Context, client *containerd.Client, cliContext *cl
 		if len(cdiDeviceIDs) > 0 {
 			opts = append(opts, withStaticCDIRegistry())
 		}
-		opts = append(opts, oci.WithCDIDevices(cdiDeviceIDs...))
+		opts = append(opts, cdispec.WithCDIDevices(cdiDeviceIDs...))
 
 		rootfsPropagation := cliContext.String("rootfs-propagation")
 		if rootfsPropagation != "" {
diff --git a/internal/cri/opts/spec_linux.go b/internal/cri/opts/spec_linux.go
@@ -31,9 +31,11 @@ import (
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 	"tags.cncf.io/container-device-interface/pkg/cdi"
 
+	"github.com/containerd/log"
+
 	"github.com/containerd/containerd/v2/core/containers"
+	cdispec "github.com/containerd/containerd/v2/pkg/cdi"
 	"github.com/containerd/containerd/v2/pkg/oci"
-	"github.com/containerd/log"
 )
 
 // Linux dependent OCI spec opts.
@@ -172,6 +174,6 @@ func WithCDI(annotations map[string]string, CDIDevices []*runtime.CDIDevice) oci
 			log.G(ctx).Debug("Passing CDI devices as annotations will be deprecated soon, please use CRI CDIDevices instead")
 		}
 
-		return oci.WithCDIDevices(devices...)(ctx, client, c, s)
+		return cdispec.WithCDIDevices(devices...)(ctx, client, c, s)
 	}
 }
diff --git a/pkg/cdi/oci_opt.go b/pkg/cdi/oci_opt.go
@@ -0,0 +1,56 @@
+/*
+   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
+
+       https://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 cdi
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/containerd/log"
+	"tags.cncf.io/container-device-interface/pkg/cdi"
+
+	"github.com/containerd/containerd/v2/core/containers"
+	"github.com/containerd/containerd/v2/pkg/oci"
+)
+
+// WithCDIDevices injects the requested CDI devices into the OCI specification.
+func WithCDIDevices(devices ...string) oci.SpecOpts {
+	return func(ctx context.Context, _ oci.Client, c *containers.Container, s *oci.Spec) error {
+		if len(devices) == 0 {
+			return nil
+		}
+
+		if err := cdi.Refresh(); err != nil {
+			// We don't consider registry refresh failure a fatal error.
+			// For instance, a dynamically generated invalid CDI Spec file for
+			// any particular vendor shouldn't prevent injection of devices of
+			// different vendors. CDI itself knows better and it will fail the
+			// injection if necessary.
+			log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
+		}
+
+		if _, err := cdi.InjectDevices(s, devices...); err != nil {
+			return fmt.Errorf("CDI device injection failed: %w", err)
+		}
+
+		// One crucial thing to keep in mind is that CDI device injection
+		// might add OCI Spec environment variables, hooks, and mounts as
+		// well. Therefore it is important that none of the corresponding
+		// OCI Spec fields are reset up in the call stack once we return.
+		return nil
+	}
+}
diff --git a/pkg/oci/spec_opts.go b/pkg/oci/spec_opts.go
@@ -28,18 +28,17 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/containerd/containerd/v2/core/containers"
-	"github.com/containerd/containerd/v2/core/content"
-	"github.com/containerd/containerd/v2/core/images"
-	"github.com/containerd/containerd/v2/core/mount"
-	"github.com/containerd/containerd/v2/pkg/namespaces"
 	"github.com/containerd/continuity/fs"
-	"github.com/containerd/log"
 	"github.com/containerd/platforms"
 	"github.com/moby/sys/user"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/runtime-spec/specs-go"
-	"tags.cncf.io/container-device-interface/pkg/cdi"
+
+	"github.com/containerd/containerd/v2/core/containers"
+	"github.com/containerd/containerd/v2/core/content"
+	"github.com/containerd/containerd/v2/core/images"
+	"github.com/containerd/containerd/v2/core/mount"
+	"github.com/containerd/containerd/v2/pkg/namespaces"
 )
 
 // SpecOpts sets spec specific information to a newly generated OCI spec
@@ -1644,30 +1643,13 @@ func WithWindowsNetworkNamespace(ns string) SpecOpts {
 	}
 }
 
-// WithCDIDevices injects the requested CDI devices into the OCI specification.
+// WithCDIDevices should be used from the cdi package. This version is used for
+// compatibility to point to the non-deprecated version but will return an error if used.
+// This function will be removed in 2.1.
+//
+// Deprecated: Use [github.com/containerd/containerd/v2/pkg/cdi.WithCDIDevices] instead.
 func WithCDIDevices(devices ...string) SpecOpts {
 	return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error {
-		if len(devices) == 0 {
-			return nil
-		}
-
-		if err := cdi.Refresh(); err != nil {
-			// We don't consider registry refresh failure a fatal error.
-			// For instance, a dynamically generated invalid CDI Spec file for
-			// any particular vendor shouldn't prevent injection of devices of
-			// different vendors. CDI itself knows better and it will fail the
-			// injection if necessary.
-			log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
-		}
-
-		if _, err := cdi.InjectDevices(s, devices...); err != nil {
-			return fmt.Errorf("CDI device injection failed: %w", err)
-		}
-
-		// One crucial thing to keep in mind is that CDI device injection
-		// might add OCI Spec environment variables, hooks, and mounts as
-		// well. Therefore it is important that none of the corresponding
-		// OCI Spec fields are reset up in the call stack once we return.
-		return nil
+		return errors.New("must use cdi package for CDI device injection")
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -31,9 +31,11 @@ import (`
`31`	`31`	`runtime "k8s.io/cri-api/pkg/apis/runtime/v1"`
`32`	`32`	`"tags.cncf.io/container-device-interface/pkg/cdi"`
`33`	`33`
	`34`	`+ "github.com/containerd/log"`
	`35`	`+`
`34`	`36`	`"github.com/containerd/containerd/v2/core/containers"`
	`37`	`+ cdispec "github.com/containerd/containerd/v2/pkg/cdi"`
`35`	`38`	`"github.com/containerd/containerd/v2/pkg/oci"`
`36`		`- "github.com/containerd/log"`
`37`	`39`	`)`
`38`	`40`
`39`	`41`	`// Linux dependent OCI spec opts.`
`@@ -172,6 +174,6 @@ func WithCDI(annotations map[string]string, CDIDevices []*runtime.CDIDevice) oci`
`172`	`174`	`log.G(ctx).Debug("Passing CDI devices as annotations will be deprecated soon, please use CRI CDIDevices instead")`
`173`	`175`	`}`
`174`	`176`
`175`		`- return oci.WithCDIDevices(devices...)(ctx, client, c, s)`
	`177`	`+ return cdispec.WithCDIDevices(devices...)(ctx, client, c, s)`
`176`	`178`	`}`
`177`	`179`	`}`