Merge pull request #11265 from dmcgowan/2.0-fix-cdi-oci-opt

[release/2.0] Move CDI device spec out of the OCI package

cmd/ctr/commands/run/run_unix.go

@@ -27,6 +27,9 @@ 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"
@@ -34,9 +37,8 @@ import (
 	"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 != "" {

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/containerd/v2/core/containers"
-	"github.com/containerd/containerd/v2/pkg/oci"
 	"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"
 )
 
 // 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)
 	}
 }

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
+
+       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 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
+	}
+}

pkg/oci/spec_opts.go

@@ -28,18 +28,17 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/containerd/continuity/fs"
+	"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"
+
 	"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"
 )
 
 // SpecOpts sets spec specific information to a newly generated OCI spec
@@ -1643,31 +1642,3 @@ func WithWindowsNetworkNamespace(ns string) SpecOpts {
 		return nil
 	}
 }
-
-// WithCDIDevices injects the requested CDI devices into the OCI specification.
-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
-	}
-}