From aed0538dacaa039178c5dfd8ddf13e73d4eefdbc Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Tue, 25 Jan 2022 17:29:40 +0200 Subject: [PATCH] cri: implement CDI device injection Extract the names of requested CDI devices and update the OCI Spec according to the corresponding CDI device specifications. CDI devices are requested using container annotations in the cdi.k8s.io namespace. Once CRI gains dedicated fields for CDI injection the snippet for extracting CDI names will need an update. Signed-off-by: Ed Bartosh --- container_opts.go | 36 +++++ pkg/cri/server/container_create.go | 1 + pkg/cri/server/container_create_linux_test.go | 152 ++++++++++++++++++ 3 files changed, 189 insertions(+) diff --git a/container_opts.go b/container_opts.go index f005fe1c7..83a7a965e 100644 --- a/container_opts.go +++ b/container_opts.go @@ -22,10 +22,12 @@ import ( "errors" "fmt" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/containerd/containerd/containers" "github.com/containerd/containerd/content" "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" + "github.com/containerd/containerd/log" "github.com/containerd/containerd/oci" "github.com/containerd/containerd/protobuf" "github.com/containerd/containerd/snapshots" @@ -324,3 +326,37 @@ func WithSpec(s *oci.Spec, opts ...oci.SpecOpts) NewContainerOpts { func WithoutRefreshedMetadata(i *InfoConfig) { i.Refresh = false } + +// WithCDI updates OCI spec with CDI content +func WithCDI(s *oci.Spec, annotations map[string]string) NewContainerOpts { + return func(ctx context.Context, _ *Client, c *containers.Container) 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() + 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/server/container_create.go b/pkg/cri/server/container_create.go index a03542ea4..58e05a916 100644 --- a/pkg/cri/server/container_create.go +++ b/pkg/cri/server/container_create.go @@ -239,6 +239,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta return nil, fmt.Errorf("failed to get runtime options: %w", err) } opts = append(opts, + containerd.WithCDI(spec, config.Annotations), containerd.WithSpec(spec, specOpts...), containerd.WithRuntime(sandboxInfo.Runtime.Name, runtimeOptions), containerd.WithContainerLabels(containerLabels), diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index b000616ad..7efc42183 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -20,12 +20,15 @@ import ( "context" "errors" "fmt" + "io/ioutil" "os" "path/filepath" "reflect" "strings" "testing" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" + "github.com/containerd/containerd" "github.com/containerd/containerd/containers" "github.com/containerd/containerd/contrib/apparmor" "github.com/containerd/containerd/contrib/seccomp" @@ -1485,3 +1488,152 @@ func TestBaseOCISpec(t *testing.T) { assert.Equal(t, *spec.Linux.Resources.Memory.Limit, containerConfig.Linux.Resources.MemoryLimitInBytes) } + +func writeFilesToTempDir(tmpDirPattern string, content []string) (string, error) { + if len(content) == 0 { + return "", nil + } + + dir, err := ioutil.TempDir("", tmpDirPattern) + if err != nil { + return "", err + } + + for idx, data := range content { + file := filepath.Join(dir, fmt.Sprintf("spec-%d.yaml", idx)) + err := ioutil.WriteFile(file, []byte(data), 0644) + if err != nil { + return "", err + } + } + + return dir, cdi.GetRegistry(cdi.WithSpecDirs(dir)).Refresh() +} + +func TestCDIInjections(t *testing.T) { + testID := "test-id" + testSandboxID := "sandbox-id" + testContainerName := "container-name" + testPid := uint32(1234) + containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() + ociRuntime := config.Runtime{} + c := newTestCRIService() + + for _, test := range []struct { + description string + cdiSpecFiles []string + 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", + annotations: map[string]string{}, + }, + {description: "expect CDI error for invalid CDI device reference in annotations", + annotations: map[string]string{ + cdi.AnnotationPrefix + "devices": "foobar", + }, + expectError: true, + }, + {description: "expect CDI error for unresolvable devices", + annotations: map[string]string{ + cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=no-such-dev", + }, + expectError: true, + }, + {description: "expect properly injected resolvable CDI devices", + cdiSpecFiles: []string{ + ` +cdiVersion: "0.2.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.2.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" +`, + }, + annotations: map[string]string{ + cdi.AnnotationPrefix + "vendor1_devices": "vendor1.com/device=foo", + cdi.AnnotationPrefix + "vendor2_devices": "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", + }, + }, + } { + t.Logf("TestCase %q", test.description) + + spec, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) + require.NoError(t, err) + + specCheck(t, testID, testSandboxID, testPid, spec) + + cdiDir, err := writeFilesToTempDir("containerd-test-CDI-injections-", test.cdiSpecFiles) + if cdiDir != "" { + defer os.RemoveAll(cdiDir) + } + require.NoError(t, err) + + injectFun := containerd.WithCDI(spec, test.annotations) + err = injectFun(nil, nil, nil) + assert.Equal(t, test.expectError, err != nil) + + if err != nil { + if test.expectEnv != nil { + for _, expectedEnv := range test.expectEnv { + assert.Contains(t, spec.Process.Env, expectedEnv) + } + } + if test.expectDevices != nil { + for _, expectedDev := range test.expectDevices { + assert.Contains(t, spec.Linux.Devices, expectedDev) + } + } + } + } +}