github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

move WithCDI to pkg/cri/opts

Browse Source 
As WithCDI is CRI-only API it makes sense to move it
out of oci module.

This move can also fix possible issues with this API when
CRI plugin is disabled.

Signed-off-by: Ed Bartosh <eduard.bartosh@intel.com>
This commit is contained in:
Ed Bartosh 2022-10-12 12:21:19 +03:00
parent e6b5311508
commit eec7a76ecd
6 changed files with 41 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
oci/spec_opts_linux.go
View File

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

35
pkg/cri/opts/spec_linux.go
View File

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

2
pkg/cri/sbserver/container_create_linux.go
View File

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

3
pkg/cri/sbserver/container_create_linux_test.go
View File

@ -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)

2
pkg/cri/server/container_create_linux.go
View File

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

3
pkg/cri/server/container_create_linux_test.go
View File

@ -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)