diff --git a/oci/spec_opts_linux.go b/oci/spec_opts_linux.go index 54bb0151f..9ff23519c 100644 --- a/oci/spec_opts_linux.go +++ b/oci/spec_opts_linux.go @@ -18,11 +18,8 @@ package oci import ( "context" - "fmt" - "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/containerd/containerd/containers" - "github.com/containerd/containerd/log" "github.com/containerd/containerd/pkg/cap" specs "github.com/opencontainers/runtime-spec/specs-go" ) @@ -170,40 +167,6 @@ func escapeAndCombineArgs(args []string) string { panic("not supported") } -// WithCDI updates OCI spec with CDI content -func WithCDI(annotations map[string]string, cdiSpecDirs []string) SpecOpts { - return func(ctx context.Context, _ Client, c *containers.Container, s *Spec) error { - // TODO: Once CRI is extended with native CDI support this will need to be updated... - _, cdiDevices, err := cdi.ParseAnnotations(annotations) - if err != nil { - return fmt.Errorf("failed to parse CDI device annotations: %w", err) - } - if cdiDevices == nil { - return nil - } - - registry := cdi.GetRegistry(cdi.WithSpecDirs(cdiSpecDirs...)) - if err = registry.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 := registry.InjectDevices(s, cdiDevices...); 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 - } -} - func appendOSMounts(s *Spec, os string) error { return nil } diff --git a/pkg/cri/opts/spec_linux.go b/pkg/cri/opts/spec_linux.go index 9306d42b6..c79aa7c3c 100644 --- a/pkg/cri/opts/spec_linux.go +++ b/pkg/cri/opts/spec_linux.go @@ -28,6 +28,7 @@ import ( "sync" "syscall" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/containerd/containerd/containers" "github.com/containerd/containerd/log" "github.com/containerd/containerd/mount" @@ -729,3 +730,37 @@ func GetUTSNamespace(pid uint32) string { func GetPIDNamespace(pid uint32) string { return fmt.Sprintf(pidNSFormat, pid) } + +// WithCDI updates OCI spec with CDI content +func WithCDI(annotations map[string]string, cdiSpecDirs []string) oci.SpecOpts { + return func(ctx context.Context, _ oci.Client, c *containers.Container, s *oci.Spec) error { + // TODO: Once CRI is extended with native CDI support this will need to be updated... + _, cdiDevices, err := cdi.ParseAnnotations(annotations) + if err != nil { + return fmt.Errorf("failed to parse CDI device annotations: %w", err) + } + if cdiDevices == nil { + return nil + } + + registry := cdi.GetRegistry(cdi.WithSpecDirs(cdiSpecDirs...)) + if err = registry.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 := registry.InjectDevices(s, cdiDevices...); 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/cri/sbserver/container_create_linux.go b/pkg/cri/sbserver/container_create_linux.go index d5de6d78d..6de7c776b 100644 --- a/pkg/cri/sbserver/container_create_linux.go +++ b/pkg/cri/sbserver/container_create_linux.go @@ -404,7 +404,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, seccompSpecOpts) } if c.config.EnableCDI { - specOpts = append(specOpts, oci.WithCDI(config.Annotations, c.config.CDISpecDirs)) + specOpts = append(specOpts, customopts.WithCDI(config.Annotations, c.config.CDISpecDirs)) } return specOpts, nil } diff --git a/pkg/cri/sbserver/container_create_linux_test.go b/pkg/cri/sbserver/container_create_linux_test.go index b6edf77cd..46165e1b8 100644 --- a/pkg/cri/sbserver/container_create_linux_test.go +++ b/pkg/cri/sbserver/container_create_linux_test.go @@ -43,6 +43,7 @@ import ( "github.com/containerd/containerd/pkg/cri/annotations" "github.com/containerd/containerd/pkg/cri/config" "github.com/containerd/containerd/pkg/cri/opts" + customopts "github.com/containerd/containerd/pkg/cri/opts" "github.com/containerd/containerd/pkg/cri/util" ctrdutil "github.com/containerd/containerd/pkg/cri/util" ostesting "github.com/containerd/containerd/pkg/os/testing" @@ -1647,7 +1648,7 @@ containerEdits: } require.NoError(t, err) - injectFun := oci.WithCDI(test.annotations, []string{cdiDir}) + injectFun := customopts.WithCDI(test.annotations, []string{cdiDir}) err = injectFun(nil, nil, nil, spec) assert.Equal(t, test.expectError, err != nil) diff --git a/pkg/cri/server/container_create_linux.go b/pkg/cri/server/container_create_linux.go index c719d1d41..4bdbc9435 100644 --- a/pkg/cri/server/container_create_linux.go +++ b/pkg/cri/server/container_create_linux.go @@ -404,7 +404,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon specOpts = append(specOpts, seccompSpecOpts) } if c.config.EnableCDI { - specOpts = append(specOpts, oci.WithCDI(config.Annotations, c.config.CDISpecDirs)) + specOpts = append(specOpts, customopts.WithCDI(config.Annotations, c.config.CDISpecDirs)) } return specOpts, nil } diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index b277423dd..eecf788ec 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -43,6 +43,7 @@ import ( "github.com/containerd/containerd/pkg/cri/annotations" "github.com/containerd/containerd/pkg/cri/config" "github.com/containerd/containerd/pkg/cri/opts" + customopts "github.com/containerd/containerd/pkg/cri/opts" "github.com/containerd/containerd/pkg/cri/util" ctrdutil "github.com/containerd/containerd/pkg/cri/util" ostesting "github.com/containerd/containerd/pkg/os/testing" @@ -1647,7 +1648,7 @@ containerEdits: } require.NoError(t, err) - injectFun := oci.WithCDI(test.annotations, []string{cdiDir}) + injectFun := customopts.WithCDI(test.annotations, []string{cdiDir}) err = injectFun(nil, nil, nil, spec) assert.Equal(t, test.expectError, err != nil)