Merge pull request #8252 from bart0sh/PR008-CDI-use-CRI-field

CDI: Use CRI Config.CDIDevices field for CDI injection
This commit is contained in:
Fu Wei 2023-05-10 21:16:49 +08:00 committed by GitHub
commit dc60137467
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 439 additions and 19 deletions

View File

@ -29,10 +29,12 @@ import (
"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi"
"github.com/containerd/cgroups/v3"
"golang.org/x/sys/unix"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
"github.com/containerd/containerd/containers"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/oci"
ctrdutil "github.com/containerd/containerd/pkg/cri/util"
)
// Linux dependent OCI spec opts.
@ -138,18 +140,40 @@ func IsCgroup2UnifiedMode() bool {
}
// WithCDI updates OCI spec with CDI content
func WithCDI(annotations map[string]string) oci.SpecOpts {
func WithCDI(annotations map[string]string, CDIDevices []*runtime.CDIDevice) 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)
// Add devices from CDIDevices CRI field
var devices []string
var err error
for _, device := range CDIDevices {
devices = append(devices, device.Name)
}
log.G(ctx).Infof("Container %v: CDI devices from CRI Config.CDIDevices: %v", c.ID, devices)
// Add devices from CDI annotations
_, devsFromAnnotations, err := cdi.ParseAnnotations(annotations)
if err != nil {
return fmt.Errorf("failed to parse CDI device annotations: %w", err)
}
if cdiDevices == nil {
return nil
if devsFromAnnotations != nil {
log.G(ctx).Infof("Container %v: CDI devices from annotations: %v", c.ID, devsFromAnnotations)
for _, deviceName := range devsFromAnnotations {
if ctrdutil.InStringSlice(devices, deviceName) {
// TODO: change to Warning when passing CDI devices as annotations is deprecated
log.G(ctx).Debugf("Skipping duplicated CDI device %s", deviceName)
continue
}
devices = append(devices, deviceName)
}
// TODO: change to Warning when passing CDI devices as annotations is deprecated
log.G(ctx).Debug("Passing CDI devices as annotations will be deprecated soon, please use CRI CDIDevices instead")
}
log.G(ctx).Infof("container %v: CDI devices: %v", c.ID, cdiDevices)
if len(devices) == 0 {
// No devices found, skip device injection
return nil
}
registry := cdi.GetRegistry()
if err = registry.Refresh(); err != nil {
@ -161,7 +185,7 @@ func WithCDI(annotations map[string]string) oci.SpecOpts {
log.G(ctx).Warnf("CDI registry refresh failed: %v", err)
}
if _, err := registry.InjectDevices(s, cdiDevices...); err != nil {
if _, err := registry.InjectDevices(s, devices...); err != nil {
return fmt.Errorf("CDI device injection failed: %w", err)
}

View File

@ -122,7 +122,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
specOpts = append(specOpts, seccompSpecOpts)
}
if c.config.EnableCDI {
specOpts = append(specOpts, customopts.WithCDI(config.Annotations))
specOpts = append(specOpts, customopts.WithCDI(config.Annotations, config.CDIDevices))
}
return specOpts, nil
}

View File

@ -1755,13 +1755,20 @@ func TestCDIInjections(t *testing.T) {
for _, test := range []struct {
description string
cdiSpecFiles []string
cdiDevices []*runtime.CDIDevice
annotations map[string]string
expectError bool
expectDevices []runtimespec.LinuxDevice
expectEnv []string
}{
{description: "expect no CDI error for nil annotations"},
{description: "expect no CDI error for empty annotations",
{description: "expect no CDI error for nil annotations",
cdiDevices: []*runtime.CDIDevice{},
},
{description: "expect no CDI error for nil CDIDevices",
annotations: map[string]string{},
},
{description: "expect no CDI error for empty CDI devices and annotations",
cdiDevices: []*runtime.CDIDevice{},
annotations: map[string]string{},
},
{description: "expect CDI error for invalid CDI device reference in annotations",
@ -1770,13 +1777,25 @@ func TestCDIInjections(t *testing.T) {
},
expectError: true,
},
{description: "expect CDI error for unresolvable devices",
{description: "expect CDI error for invalid CDI device reference in CDIDevices",
cdiDevices: []*runtime.CDIDevice{
{Name: "foobar"},
},
expectError: true,
},
{description: "expect CDI error for unresolvable devices in annotations",
annotations: map[string]string{
cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=no-such-dev",
},
expectError: true,
},
{description: "expect properly injected resolvable CDI devices",
{description: "expect CDI error for unresolvable devices in CDIDevices",
cdiDevices: []*runtime.CDIDevice{
{Name: "vendor1.com/device=no-such-dev"},
},
expectError: true,
},
{description: "expect properly injected resolvable CDI devices from annotations",
cdiSpecFiles: []string{
`
cdiVersion: "0.3.0"
@ -1838,6 +1857,185 @@ containerEdits:
"VENDOR2=present",
},
},
{description: "expect properly injected resolvable CDI devices from CDIDevices",
cdiSpecFiles: []string{
`
cdiVersion: "0.3.0"
kind: "vendor1.com/device"
devices:
- name: foo
containerEdits:
deviceNodes:
- path: /dev/loop8
type: b
major: 7
minor: 8
env:
- FOO=injected
containerEdits:
env:
- "VENDOR1=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor2.com/device"
devices:
- name: bar
containerEdits:
deviceNodes:
- path: /dev/loop9
type: b
major: 7
minor: 9
env:
- BAR=injected
containerEdits:
env:
- "VENDOR2=present"
`,
},
cdiDevices: []*runtime.CDIDevice{
{Name: "vendor1.com/device=foo"},
{Name: "vendor2.com/device=bar"},
},
expectDevices: []runtimespec.LinuxDevice{
{
Path: "/dev/loop8",
Type: "b",
Major: 7,
Minor: 8,
},
{
Path: "/dev/loop9",
Type: "b",
Major: 7,
Minor: 9,
},
},
expectEnv: []string{
"FOO=injected",
"VENDOR1=present",
"BAR=injected",
"VENDOR2=present",
},
},
{description: "expect CDI devices from CDIDevices and annotations",
cdiSpecFiles: []string{
`
cdiVersion: "0.3.0"
kind: "vendor1.com/device"
devices:
- name: foo
containerEdits:
deviceNodes:
- path: /dev/loop8
type: b
major: 7
minor: 8
env:
- FOO=injected
containerEdits:
env:
- "VENDOR1=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor2.com/device"
devices:
- name: bar
containerEdits:
deviceNodes:
- path: /dev/loop9
type: b
major: 7
minor: 9
env:
- BAR=injected
containerEdits:
env:
- "VENDOR2=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor3.com/device"
devices:
- name: foo3
containerEdits:
deviceNodes:
- path: /dev/loop10
type: b
major: 7
minor: 10
env:
- FOO3=injected
containerEdits:
env:
- "VENDOR3=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor4.com/device"
devices:
- name: bar4
containerEdits:
deviceNodes:
- path: /dev/loop11
type: b
major: 7
minor: 11
env:
- BAR4=injected
containerEdits:
env:
- "VENDOR4=present"
`,
},
cdiDevices: []*runtime.CDIDevice{
{Name: "vendor1.com/device=foo"},
{Name: "vendor2.com/device=bar"},
{Name: "vendor3.com/device=foo3"},
},
annotations: map[string]string{
cdi.AnnotationPrefix + "vendor3_devices": "vendor3.com/device=foo3", // Duplicated device, should be ignored
cdi.AnnotationPrefix + "vendor4_devices": "vendor4.com/device=bar4",
},
expectDevices: []runtimespec.LinuxDevice{
{
Path: "/dev/loop8",
Type: "b",
Major: 7,
Minor: 8,
},
{
Path: "/dev/loop9",
Type: "b",
Major: 7,
Minor: 9,
},
{
Path: "/dev/loop10",
Type: "b",
Major: 7,
Minor: 10,
},
{
Path: "/dev/loop11",
Type: "b",
Major: 7,
Minor: 11,
},
},
expectEnv: []string{
"FOO=injected",
"VENDOR1=present",
"BAR=injected",
"VENDOR2=present",
"FOO3=injected",
"VENDOR3=present",
"BAR4=injected",
"VENDOR4=present",
},
},
} {
t.Run(test.description, func(t *testing.T) {
spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
@ -1855,7 +2053,7 @@ containerEdits:
err = reg.Configure(cdi.WithSpecDirs(cdiDir))
require.NoError(t, err)
injectFun := customopts.WithCDI(test.annotations)
injectFun := customopts.WithCDI(test.annotations, test.cdiDevices)
err = injectFun(ctx, nil, testContainer, spec)
assert.Equal(t, test.expectError, err != nil)

View File

@ -415,7 +415,7 @@ func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageCon
specOpts = append(specOpts, seccompSpecOpts)
}
if c.config.EnableCDI {
specOpts = append(specOpts, customopts.WithCDI(config.Annotations))
specOpts = append(specOpts, customopts.WithCDI(config.Annotations, config.CDIDevices))
}
return specOpts, nil
}

View File

@ -1941,13 +1941,20 @@ func TestCDIInjections(t *testing.T) {
for _, test := range []struct {
description string
cdiSpecFiles []string
cdiDevices []*runtime.CDIDevice
annotations map[string]string
expectError bool
expectDevices []runtimespec.LinuxDevice
expectEnv []string
}{
{description: "expect no CDI error for nil annotations"},
{description: "expect no CDI error for empty annotations",
{description: "expect no CDI error for nil annotations",
cdiDevices: []*runtime.CDIDevice{},
},
{description: "expect no CDI error for nil CDIDevices",
annotations: map[string]string{},
},
{description: "expect no CDI error for empty CDI devices and annotations",
cdiDevices: []*runtime.CDIDevice{},
annotations: map[string]string{},
},
{description: "expect CDI error for invalid CDI device reference in annotations",
@ -1956,13 +1963,25 @@ func TestCDIInjections(t *testing.T) {
},
expectError: true,
},
{description: "expect CDI error for unresolvable devices",
{description: "expect CDI error for invalid CDI device reference in CDIDevices",
cdiDevices: []*runtime.CDIDevice{
{Name: "foobar"},
},
expectError: true,
},
{description: "expect CDI error for unresolvable devices in annotations",
annotations: map[string]string{
cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=no-such-dev",
},
expectError: true,
},
{description: "expect properly injected resolvable CDI devices",
{description: "expect CDI error for unresolvable devices in CDIDevices",
cdiDevices: []*runtime.CDIDevice{
{Name: "vendor1.com/device=no-such-dev"},
},
expectError: true,
},
{description: "expect properly injected resolvable CDI devices from annotations",
cdiSpecFiles: []string{
`
cdiVersion: "0.3.0"
@ -2024,6 +2043,185 @@ containerEdits:
"VENDOR2=present",
},
},
{description: "expect properly injected resolvable CDI devices from CDIDevices",
cdiSpecFiles: []string{
`
cdiVersion: "0.3.0"
kind: "vendor1.com/device"
devices:
- name: foo
containerEdits:
deviceNodes:
- path: /dev/loop8
type: b
major: 7
minor: 8
env:
- FOO=injected
containerEdits:
env:
- "VENDOR1=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor2.com/device"
devices:
- name: bar
containerEdits:
deviceNodes:
- path: /dev/loop9
type: b
major: 7
minor: 9
env:
- BAR=injected
containerEdits:
env:
- "VENDOR2=present"
`,
},
cdiDevices: []*runtime.CDIDevice{
{Name: "vendor1.com/device=foo"},
{Name: "vendor2.com/device=bar"},
},
expectDevices: []runtimespec.LinuxDevice{
{
Path: "/dev/loop8",
Type: "b",
Major: 7,
Minor: 8,
},
{
Path: "/dev/loop9",
Type: "b",
Major: 7,
Minor: 9,
},
},
expectEnv: []string{
"FOO=injected",
"VENDOR1=present",
"BAR=injected",
"VENDOR2=present",
},
},
{description: "expect CDI devices from CDIDevices and annotations",
cdiSpecFiles: []string{
`
cdiVersion: "0.3.0"
kind: "vendor1.com/device"
devices:
- name: foo
containerEdits:
deviceNodes:
- path: /dev/loop8
type: b
major: 7
minor: 8
env:
- FOO=injected
containerEdits:
env:
- "VENDOR1=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor2.com/device"
devices:
- name: bar
containerEdits:
deviceNodes:
- path: /dev/loop9
type: b
major: 7
minor: 9
env:
- BAR=injected
containerEdits:
env:
- "VENDOR2=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor3.com/device"
devices:
- name: foo3
containerEdits:
deviceNodes:
- path: /dev/loop10
type: b
major: 7
minor: 10
env:
- FOO3=injected
containerEdits:
env:
- "VENDOR3=present"
`,
`
cdiVersion: "0.3.0"
kind: "vendor4.com/device"
devices:
- name: bar4
containerEdits:
deviceNodes:
- path: /dev/loop11
type: b
major: 7
minor: 11
env:
- BAR4=injected
containerEdits:
env:
- "VENDOR4=present"
`,
},
cdiDevices: []*runtime.CDIDevice{
{Name: "vendor1.com/device=foo"},
{Name: "vendor2.com/device=bar"},
{Name: "vendor3.com/device=foo3"},
},
annotations: map[string]string{
cdi.AnnotationPrefix + "vendor3_devices": "vendor3.com/device=foo3", // Duplicated device, should be ignored
cdi.AnnotationPrefix + "vendor4_devices": "vendor4.com/device=bar4",
},
expectDevices: []runtimespec.LinuxDevice{
{
Path: "/dev/loop8",
Type: "b",
Major: 7,
Minor: 8,
},
{
Path: "/dev/loop9",
Type: "b",
Major: 7,
Minor: 9,
},
{
Path: "/dev/loop10",
Type: "b",
Major: 7,
Minor: 10,
},
{
Path: "/dev/loop11",
Type: "b",
Major: 7,
Minor: 11,
},
},
expectEnv: []string{
"FOO=injected",
"VENDOR1=present",
"BAR=injected",
"VENDOR2=present",
"FOO3=injected",
"VENDOR3=present",
"BAR4=injected",
"VENDOR4=present",
},
},
} {
test := test
t.Run(test.description, func(t *testing.T) {
@ -2042,7 +2240,7 @@ containerEdits:
err = reg.Configure(cdi.WithSpecDirs(cdiDir))
require.NoError(t, err)
injectFun := customopts.WithCDI(test.annotations)
injectFun := customopts.WithCDI(test.annotations, test.cdiDevices)
err = injectFun(ctx, nil, testContainer, spec)
assert.Equal(t, test.expectError, err != nil)