Skip to content

Commit e20f7f4

Browse files
dmcgowandmcgowan
committed
Move CDI device spec out of the OCI package
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 <[email protected]>
1 parent da3d55a commit e20f7f4

File tree

4 files changed

+77
-35
lines changed

4 files changed

+77
-35
lines changed

‎cmd/ctr/commands/run/run_unix.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ import (
2727
"strconv"
2828
"strings"
2929

30+
"github.com/containerd/log"
31+
"github.com/containerd/platforms"
32+
3033
containerd "github.com/containerd/containerd/v2/client"
3134
"github.com/containerd/containerd/v2/cmd/ctr/commands"
3235
"github.com/containerd/containerd/v2/contrib/apparmor"
@@ -35,9 +38,8 @@ import (
3538
"github.com/containerd/containerd/v2/core/containers"
3639
"github.com/containerd/containerd/v2/core/diff"
3740
"github.com/containerd/containerd/v2/core/snapshots"
41+
cdispec "github.com/containerd/containerd/v2/pkg/cdi"
3842
"github.com/containerd/containerd/v2/pkg/oci"
39-
"github.com/containerd/log"
40-
"github.com/containerd/platforms"
4143

4244
"github.com/intel/goresctrl/pkg/blockio"
4345
"github.com/opencontainers/runtime-spec/specs-go"
@@ -359,7 +361,7 @@ func NewContainer(ctx context.Context, client *containerd.Client, cliContext *cl
359361
if len(cdiDeviceIDs) > 0 {
360362
opts = append(opts, withStaticCDIRegistry())
361363
}
362-
opts = append(opts, oci.WithCDIDevices(cdiDeviceIDs...))
364+
opts = append(opts, cdispec.WithCDIDevices(cdiDeviceIDs...))
363365

364366
rootfsPropagation := cliContext.String("rootfs-propagation")
365367
if rootfsPropagation != "" {

‎internal/cri/opts/spec_linux.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ import (
3131
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
3232
"tags.cncf.io/container-device-interface/pkg/cdi"
3333

34+
"github.com/containerd/log"
35+
3436
"github.com/containerd/containerd/v2/core/containers"
37+
cdispec "github.com/containerd/containerd/v2/pkg/cdi"
3538
"github.com/containerd/containerd/v2/pkg/oci"
36-
"github.com/containerd/log"
3739
)
3840

3941
// Linux dependent OCI spec opts.
@@ -172,6 +174,6 @@ func WithCDI(annotations map[string]string, CDIDevices []*runtime.CDIDevice) oci
172174
log.G(ctx).Debug("Passing CDI devices as annotations will be deprecated soon, please use CRI CDIDevices instead")
173175
}
174176

175-
return oci.WithCDIDevices(devices...)(ctx, client, c, s)
177+
return cdispec.WithCDIDevices(devices...)(ctx, client, c, s)
176178
}
177179
}

‎pkg/cdi/oci_opt.go

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
Copyright The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package cdi
18+
19+
import (
20+
"context"
21+
"fmt"
22+
23+
"github.com/containerd/log"
24+
"tags.cncf.io/container-device-interface/pkg/cdi"
25+
26+
"github.com/containerd/containerd/v2/core/containers"
27+
"github.com/containerd/containerd/v2/pkg/oci"
28+
)
29+
30+
// WithCDIDevices injects the requested CDI devices into the OCI specification.
31+
func WithCDIDevices(devices ...string) oci.SpecOpts {
32+
return func(ctx context.Context, _ oci.Client, c *containers.Container, s *oci.Spec) error {
33+
if len(devices) == 0 {
34+
return nil
35+
}
36+
37+
if err := cdi.Refresh(); err != nil {
38+
// We don't consider registry refresh failure a fatal error.
39+
// For instance, a dynamically generated invalid CDI Spec file for
40+
// any particular vendor shouldn't prevent injection of devices of
41+
// different vendors. CDI itself knows better and it will fail the
42+
// injection if necessary.
43+
log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
44+
}
45+
46+
if _, err := cdi.InjectDevices(s, devices...); err != nil {
47+
return fmt.Errorf("CDI device injection failed: %w", err)
48+
}
49+
50+
// One crucial thing to keep in mind is that CDI device injection
51+
// might add OCI Spec environment variables, hooks, and mounts as
52+
// well. Therefore it is important that none of the corresponding
53+
// OCI Spec fields are reset up in the call stack once we return.
54+
return nil
55+
}
56+
}

‎pkg/oci/spec_opts.go

+12-30
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,17 @@ import (
2828
"strconv"
2929
"strings"
3030

31-
"github.com/containerd/containerd/v2/core/containers"
32-
"github.com/containerd/containerd/v2/core/content"
33-
"github.com/containerd/containerd/v2/core/images"
34-
"github.com/containerd/containerd/v2/core/mount"
35-
"github.com/containerd/containerd/v2/pkg/namespaces"
3631
"github.com/containerd/continuity/fs"
37-
"github.com/containerd/log"
3832
"github.com/containerd/platforms"
3933
"github.com/moby/sys/user"
4034
v1 "github.com/opencontainers/image-spec/specs-go/v1"
4135
"github.com/opencontainers/runtime-spec/specs-go"
42-
"tags.cncf.io/container-device-interface/pkg/cdi"
36+
37+
"github.com/containerd/containerd/v2/core/containers"
38+
"github.com/containerd/containerd/v2/core/content"
39+
"github.com/containerd/containerd/v2/core/images"
40+
"github.com/containerd/containerd/v2/core/mount"
41+
"github.com/containerd/containerd/v2/pkg/namespaces"
4342
)
4443

4544
// SpecOpts sets spec specific information to a newly generated OCI spec
@@ -1644,30 +1643,13 @@ func WithWindowsNetworkNamespace(ns string) SpecOpts {
16441643
}
16451644
}
16461645

1647-
// WithCDIDevices injects the requested CDI devices into the OCI specification.
1646+
// WithCDIDevices should be used from the cdi package. This version is used for
1647+
// compatibility to point to the non-deprecated version but will return an error if used.
1648+
// This function will be removed in 2.1.
1649+
//
1650+
// Deprecated: Use [github.com/containerd/containerd/v2/pkg/cdi.WithCDIDevices] instead.
16481651
func WithCDIDevices(devices ...string) SpecOpts {
16491652
return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error {
1650-
if len(devices) == 0 {
1651-
return nil
1652-
}
1653-
1654-
if err := cdi.Refresh(); err != nil {
1655-
// We don't consider registry refresh failure a fatal error.
1656-
// For instance, a dynamically generated invalid CDI Spec file for
1657-
// any particular vendor shouldn't prevent injection of devices of
1658-
// different vendors. CDI itself knows better and it will fail the
1659-
// injection if necessary.
1660-
log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
1661-
}
1662-
1663-
if _, err := cdi.InjectDevices(s, devices...); err != nil {
1664-
return fmt.Errorf("CDI device injection failed: %w", err)
1665-
}
1666-
1667-
// One crucial thing to keep in mind is that CDI device injection
1668-
// might add OCI Spec environment variables, hooks, and mounts as
1669-
// well. Therefore it is important that none of the corresponding
1670-
// OCI Spec fields are reset up in the call stack once we return.
1671-
return nil
1653+
return errors.New("must use cdi package for CDI device injection")
16721654
}
16731655
}

0 commit comments

Comments
 (0)