Skip to content

Commit d69cc70

Browse files
crazy-maxcrazy-max
committed
cdi: use worker cdi manager when generating devices oci spec
Signed-off-by: CrazyMax <[email protected]>
1 parent f7f1774 commit d69cc70

File tree

16 files changed

+82
-66
lines changed

16 files changed

+82
-66
lines changed

‎cmd/buildkitd/main.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import (
7474
"google.golang.org/grpc/health"
7575
healthv1 "google.golang.org/grpc/health/grpc_health_v1"
7676
"google.golang.org/grpc/reflection"
77+
"tags.cncf.io/container-device-interface/pkg/cdi"
7778
)
7879

7980
func init() {
@@ -1044,3 +1045,30 @@ func newMeterProvider(ctx context.Context) (*sdkmetric.MeterProvider, error) {
10441045
}
10451046
return sdkmetric.NewMeterProvider(opts...), nil
10461047
}
1048+
1049+
// getCDIManager returns a new CDI registry with disabled auto-refresh.
1050+
func getCDIManager(disabled *bool, specDirs []string) (*cdi.Cache, error) {
1051+
if disabled != nil && *disabled {
1052+
return nil, nil
1053+
}
1054+
if len(specDirs) == 0 {
1055+
return nil, errors.New("No CDI specification directories specified")
1056+
}
1057+
cdiCache, err := func() (*cdi.Cache, error) {
1058+
cdiCache, err := cdi.NewCache(
1059+
cdi.WithSpecDirs(specDirs...),
1060+
cdi.WithAutoRefresh(false),
1061+
)
1062+
if err != nil {
1063+
return nil, err
1064+
}
1065+
if err := cdiCache.Refresh(); err != nil {
1066+
return nil, err
1067+
}
1068+
return cdiCache, nil
1069+
}()
1070+
if err != nil {
1071+
return nil, errors.Wrapf(err, "CDI registry initialization failure")
1072+
}
1073+
return cdiCache, nil
1074+
}

‎cmd/buildkitd/main_containerd_worker.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,11 @@ func containerdWorkerInitializer(c *cli.Context, common workerInitializerOpt) ([
282282

283283
dns := getDNSConfig(common.config.DNS)
284284

285+
cdiManager, err := getCDIManager(common.config.CDI.Disabled, common.config.CDI.SpecDirs)
286+
if err != nil {
287+
return nil, err
288+
}
289+
285290
nc := netproviders.Opt{
286291
Mode: common.config.Workers.Containerd.NetworkConfig.Mode,
287292
CNI: cniprovider.Opt{
@@ -339,6 +344,7 @@ func containerdWorkerInitializer(c *cli.Context, common workerInitializerOpt) ([
339344
ParallelismSem: parallelismSem,
340345
TraceSocket: common.traceSocket,
341346
Runtime: runtime,
347+
CDIManager: cdiManager,
342348
}
343349

344350
opt, err := containerd.NewWorkerOpt(workerOpts, ctd.WithTimeout(60*time.Second))

‎cmd/buildkitd/main_oci_worker.go

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ import (
4848
"google.golang.org/grpc"
4949
"google.golang.org/grpc/backoff"
5050
"google.golang.org/grpc/credentials/insecure"
51-
"tags.cncf.io/container-device-interface/pkg/cdi"
5251
)
5352

5453
func init() {
@@ -299,6 +298,11 @@ func ociWorkerInitializer(c *cli.Context, common workerInitializerOpt) ([]worker
299298

300299
dns := getDNSConfig(common.config.DNS)
301300

301+
cdiManager, err := getCDIManager(common.config.CDI.Disabled, common.config.CDI.SpecDirs)
302+
if err != nil {
303+
return nil, err
304+
}
305+
302306
nc := netproviders.Opt{
303307
Mode: common.config.Workers.OCI.NetworkConfig.Mode,
304308
CNI: cniprovider.Opt{
@@ -316,28 +320,14 @@ func ociWorkerInitializer(c *cli.Context, common workerInitializerOpt) ([]worker
316320
parallelismSem = semaphore.NewWeighted(int64(cfg.MaxParallelism))
317321
}
318322

319-
opt, err := runc.NewWorkerOpt(common.config.Root, snFactory, cfg.Rootless, processMode, cfg.Labels, idmapping, nc, dns, cfg.Binary, cfg.ApparmorProfile, cfg.SELinux, parallelismSem, common.traceSocket, cfg.DefaultCgroupParent)
323+
opt, err := runc.NewWorkerOpt(common.config.Root, snFactory, cfg.Rootless, processMode, cfg.Labels, idmapping, nc, dns, cfg.Binary, cfg.ApparmorProfile, cfg.SELinux, parallelismSem, common.traceSocket, cfg.DefaultCgroupParent, cdiManager)
320324
if err != nil {
321325
return nil, err
322326
}
323327
opt.GCPolicy = getGCPolicy(cfg.GCConfig, common.config.Root)
324328
opt.BuildkitVersion = getBuildkitVersion()
325329
opt.RegistryHosts = hosts
326330

327-
if common.config.CDI.Disabled == nil || !*common.config.CDI.Disabled {
328-
if len(common.config.CDI.SpecDirs) == 0 {
329-
return nil, errors.New("No CDI specification directories specified")
330-
}
331-
cdiCache, err := cdi.NewCache(cdi.WithSpecDirs(common.config.CDI.SpecDirs...))
332-
if err != nil {
333-
return nil, errors.Wrapf(err, "CDI registry initialization failure")
334-
}
335-
if err := cdiCache.Refresh(); err != nil {
336-
return nil, errors.Wrapf(err, "CDI registry initialization failure")
337-
}
338-
opt.CDIManager = cdiCache
339-
}
340-
341331
if platformsStr := cfg.Platforms; len(platformsStr) != 0 {
342332
platforms, err := parsePlatforms(platformsStr)
343333
if err != nil {

‎executor/containerdexecutor/executor.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/moby/buildkit/util/bklog"
1414
"go.opentelemetry.io/otel/attribute"
1515
"go.opentelemetry.io/otel/trace"
16+
"tags.cncf.io/container-device-interface/pkg/cdi"
1617

1718
ctd "github.com/containerd/containerd/v2/client"
1819
"github.com/containerd/containerd/v2/core/mount"
@@ -40,6 +41,7 @@ type containerdExecutor struct {
4041
traceSocket string
4142
rootless bool
4243
runtime *RuntimeInfo
44+
cdiManager *cdi.Cache
4345
}
4446

4547
// OnCreateRuntimer provides an alternative to OCI hooks for applying network
@@ -72,6 +74,7 @@ type ExecutorOptions struct {
7274
TraceSocket string
7375
Rootless bool
7476
Runtime *RuntimeInfo
77+
CDIManager *cdi.Cache
7578
}
7679

7780
// New creates a new executor backed by connection to containerd API
@@ -92,6 +95,7 @@ func New(executorOpts ExecutorOptions) executor.Executor {
9295
traceSocket: executorOpts.TraceSocket,
9396
rootless: executorOpts.Rootless,
9497
runtime: executorOpts.Runtime,
98+
cdiManager: executorOpts.CDIManager,
9599
}
96100
}
97101

‎executor/containerdexecutor/executor_unix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func (w *containerdExecutor) createOCISpec(ctx context.Context, id, resolvConf,
145145
}
146146

147147
processMode := oci.ProcessSandbox // FIXME(AkihiroSuda)
148-
spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, namespace, w.cgroupParent, processMode, nil, w.apparmorProfile, w.selinux, w.traceSocket, opts...)
148+
spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, resolvConf, hostsFile, namespace, w.cgroupParent, processMode, nil, w.apparmorProfile, w.selinux, w.traceSocket, w.cdiManager, opts...)
149149
if err != nil {
150150
releaseAll()
151151
return nil, nil, err

‎executor/containerdexecutor/executor_windows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func (w *containerdExecutor) createOCISpec(ctx context.Context, id, _, _ string,
8888
}
8989

9090
processMode := oci.ProcessSandbox // FIXME(AkihiroSuda)
91-
spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, "", "", namespace, "", processMode, nil, "", false, w.traceSocket, opts...)
91+
spec, cleanup, err := oci.GenerateSpec(ctx, meta, mounts, id, "", "", namespace, "", processMode, nil, "", false, w.traceSocket, nil, opts...)
9292
if err != nil {
9393
releaseAll()
9494
return nil, nil, err

‎executor/oci/spec.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
specs "github.com/opencontainers/runtime-spec/specs-go"
2525
"github.com/opencontainers/selinux/go-selinux"
2626
"github.com/pkg/errors"
27+
"tags.cncf.io/container-device-interface/pkg/cdi"
2728
)
2829

2930
// ProcessMode configures PID namespaces
@@ -59,7 +60,7 @@ func (pm ProcessMode) String() string {
5960

6061
// GenerateSpec generates spec using containerd functionality.
6162
// opts are ignored for s.Process, s.Hostname, and s.Mounts .
62-
func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mount, id, resolvConf, hostsFile string, namespace network.Namespace, cgroupParent string, processMode ProcessMode, idmap *idtools.IdentityMapping, apparmorProfile string, selinuxB bool, tracingSocket string, opts ...oci.SpecOpts) (*specs.Spec, func(), error) {
63+
func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mount, id, resolvConf, hostsFile string, namespace network.Namespace, cgroupParent string, processMode ProcessMode, idmap *idtools.IdentityMapping, apparmorProfile string, selinuxB bool, tracingSocket string, cdiManager *cdi.Cache, opts ...oci.SpecOpts) (*specs.Spec, func(), error) {
6364
c := &containers.Container{
6465
ID: id,
6566
}
@@ -110,12 +111,6 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
110111
return nil, nil, err
111112
}
112113

113-
if cdiOpts, err := generateCDIOpts(ctx, meta.CDIDevices); err == nil {
114-
opts = append(opts, cdiOpts...)
115-
} else {
116-
return nil, nil, err
117-
}
118-
119114
hostname := defaultHostname
120115
if meta.Hostname != "" {
121116
hostname = meta.Hostname
@@ -135,6 +130,14 @@ func GenerateSpec(ctx context.Context, meta executor.Meta, mounts []executor.Mou
135130
oci.WithHostname(hostname),
136131
)
137132

133+
if cdiManager != nil {
134+
if cdiOpts, err := generateCDIOpts(cdiManager, meta.CDIDevices); err == nil {
135+
opts = append(opts, cdiOpts...)
136+
} else {
137+
return nil, nil, err
138+
}
139+
}
140+
138141
s, err := oci.GenerateSpec(ctx, nil, c, opts...)
139142
if err != nil {
140143
return nil, nil, errors.WithStack(err)

‎executor/oci/spec_darwin.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
package oci
22

33
import (
4-
"context"
5-
64
"github.com/containerd/containerd/v2/core/mount"
75
"github.com/containerd/containerd/v2/pkg/oci"
86
"github.com/containerd/continuity/fs"
97
"github.com/docker/docker/pkg/idtools"
108
"github.com/moby/buildkit/solver/pb"
119
"github.com/opencontainers/runtime-spec/specs-go"
1210
"github.com/pkg/errors"
11+
"tags.cncf.io/container-device-interface/pkg/cdi"
1312
)
1413

1514
func withProcessArgs(args ...string) oci.SpecOpts {
@@ -65,7 +64,7 @@ func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) {
6564
return m, func() error { return nil }, nil
6665
}
6766

68-
func generateCDIOpts(ctx context.Context, devices []*pb.CDIDevice) ([]oci.SpecOpts, error) {
67+
func generateCDIOpts(_ *cdi.Cache, devices []*pb.CDIDevice) ([]oci.SpecOpts, error) {
6968
if len(devices) == 0 {
7069
return nil, nil
7170
}

‎executor/oci/spec_freebsd.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
package oci
22

33
import (
4-
"context"
5-
64
"github.com/containerd/containerd/v2/core/mount"
75
"github.com/containerd/containerd/v2/pkg/oci"
86
"github.com/containerd/continuity/fs"
97
"github.com/docker/docker/pkg/idtools"
108
"github.com/moby/buildkit/solver/pb"
119
specs "github.com/opencontainers/runtime-spec/specs-go"
1210
"github.com/pkg/errors"
11+
"tags.cncf.io/container-device-interface/pkg/cdi"
1312
)
1413

1514
func withProcessArgs(args ...string) oci.SpecOpts {
@@ -73,7 +72,7 @@ func sub(m mount.Mount, subPath string) (mount.Mount, func() error, error) {
7372
return m, func() error { return nil }, nil
7473
}
7574

76-
func generateCDIOpts(ctx context.Context, devices []*pb.CDIDevice) ([]oci.SpecOpts, error) {
75+
func generateCDIOpts(_ *cdi.Cache, devices []*pb.CDIDevice) ([]oci.SpecOpts, error) {
7776
if len(devices) == 0 {
7877
return nil, nil
7978
}

‎executor/oci/spec_linux.go

Lines changed: 5 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/containerd/containerd/v2/pkg/oci"
1515
cdseccomp "github.com/containerd/containerd/v2/pkg/seccomp"
1616
"github.com/containerd/continuity/fs"
17-
"github.com/containerd/log"
1817
"github.com/docker/docker/pkg/idtools"
1918
"github.com/docker/docker/profiles/seccomp"
2019
"github.com/moby/buildkit/snapshot"
@@ -153,10 +152,8 @@ func generateRlimitOpts(ulimits []*pb.Ulimit) ([]oci.SpecOpts, error) {
153152
}
154153

155154
// genereateCDIOptions creates the OCI runtime spec options for injecting CDI
156-
// devices. Two options are returned: The first ensures that the CDI registry
157-
// is initialized with refresh disabled, and the second injects the devices
158-
// into the container.
159-
func generateCDIOpts(ctx context.Context, devices []*pb.CDIDevice) ([]oci.SpecOpts, error) {
155+
// devices.
156+
func generateCDIOpts(manager *cdi.Cache, devices []*pb.CDIDevice) ([]oci.SpecOpts, error) {
160157
if len(devices) == 0 {
161158
return nil, nil
162159
}
@@ -171,38 +168,16 @@ func generateCDIOpts(ctx context.Context, devices []*pb.CDIDevice) ([]oci.SpecOp
171168
dd = append(dd, d.Name)
172169
}
173170

174-
// withStaticCDIRegistry inits the CDI registry and disables auto-refresh.
175-
// This is used from the `run` command to avoid creating a registry with
176-
// auto-refresh enabled. It also provides a way to override the CDI spec
177-
// file paths if required.
178-
withStaticCDIRegistry := func() oci.SpecOpts {
179-
return func(ctx context.Context, _ oci.Client, _ *containers.Container, s *oci.Spec) error {
180-
_ = cdi.Configure(cdi.WithAutoRefresh(false))
181-
// TODO: this should use worker CDIManager
182-
if err := cdi.Refresh(); err != nil {
183-
// We don't consider registry refresh failure a fatal error.
184-
// For instance, a dynamically generated invalid CDI Spec file
185-
// for any particular vendor shouldn't prevent injection of
186-
// devices of different vendors. CDI itself knows better, and
187-
// it will fail the injection if necessary.
188-
bklog.G(ctx).Warnf("CDI registry refresh failed: %v", err)
189-
}
190-
return nil
191-
}
192-
}
193-
194-
// withCDIDevices injects the requested CDI devices into the OCI specification.
195-
// FIXME: Use oci.WithCDIDevices once we switch to containerd 2.0.
196171
withCDIDevices := func(devices ...string) oci.SpecOpts {
197172
return func(ctx context.Context, _ oci.Client, c *containers.Container, s *specs.Spec) error {
198173
if len(devices) == 0 {
199174
return nil
200175
}
201-
if err := cdi.Refresh(); err != nil {
202-
log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
176+
if err := manager.Refresh(); err != nil {
177+
bklog.G(ctx).Warnf("CDI registry refresh failed: %v", err)
203178
}
204179
bklog.G(ctx).Debugf("Injecting CDI devices %v", devices)
205-
if _, err := cdi.InjectDevices(s, devices...); err != nil {
180+
if _, err := manager.InjectDevices(s, devices...); err != nil {
206181
return errors.Wrapf(err, "CDI device injection failed")
207182
}
208183
// One crucial thing to keep in mind is that CDI device injection
@@ -214,7 +189,6 @@ func generateCDIOpts(ctx context.Context, devices []*pb.CDIDevice) ([]oci.SpecOp
214189
}
215190

216191
return []oci.SpecOpts{
217-
withStaticCDIRegistry(),
218192
withCDIDevices(dd...),
219193
}, nil
220194
}

0 commit comments

Comments
 (0)