From 8ed910c46ae146069a8f85dc10a8eb4de1f84351 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Wed, 12 Oct 2022 13:17:03 +0300 Subject: [PATCH] CDI: configure registry on start Currently CDI registry is reconfigured on every WithCDI call, which is a relatively heavy operation. This happens because cdi.GetRegistry(cdi.WithSpecDirs(cdiSpecDirs...)) unconditionally reconfigures the registry (clears fs notify watch, sets up new watch, rescans directories). Moving configuration to the criService.initPlatform should result in performing registry configuration only once on the service start. Signed-off-by: Ed Bartosh --- pkg/cri/opts/spec_linux.go | 4 ++-- pkg/cri/sbserver/container_create_linux.go | 2 +- pkg/cri/sbserver/container_create_linux_test.go | 6 +++++- pkg/cri/sbserver/service_linux.go | 9 +++++++++ pkg/cri/server/container_create_linux.go | 2 +- pkg/cri/server/container_create_linux_test.go | 6 +++++- pkg/cri/server/service_linux.go | 9 +++++++++ 7 files changed, 32 insertions(+), 6 deletions(-) diff --git a/pkg/cri/opts/spec_linux.go b/pkg/cri/opts/spec_linux.go index c79aa7c3c..8db346cd9 100644 --- a/pkg/cri/opts/spec_linux.go +++ b/pkg/cri/opts/spec_linux.go @@ -732,7 +732,7 @@ func GetPIDNamespace(pid uint32) string { } // WithCDI updates OCI spec with CDI content -func WithCDI(annotations map[string]string, cdiSpecDirs []string) oci.SpecOpts { +func WithCDI(annotations map[string]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) @@ -743,7 +743,7 @@ func WithCDI(annotations map[string]string, cdiSpecDirs []string) oci.SpecOpts { return nil } - registry := cdi.GetRegistry(cdi.WithSpecDirs(cdiSpecDirs...)) + 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 diff --git a/pkg/cri/sbserver/container_create_linux.go b/pkg/cri/sbserver/container_create_linux.go index 6de7c776b..6cb0cb2ef 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, customopts.WithCDI(config.Annotations, c.config.CDISpecDirs)) + specOpts = append(specOpts, customopts.WithCDI(config.Annotations)) } return specOpts, nil } diff --git a/pkg/cri/sbserver/container_create_linux_test.go b/pkg/cri/sbserver/container_create_linux_test.go index 46165e1b8..252f3dd66 100644 --- a/pkg/cri/sbserver/container_create_linux_test.go +++ b/pkg/cri/sbserver/container_create_linux_test.go @@ -1648,7 +1648,11 @@ containerEdits: } require.NoError(t, err) - injectFun := customopts.WithCDI(test.annotations, []string{cdiDir}) + reg := cdi.GetRegistry() + err = reg.Configure(cdi.WithSpecDirs(cdiDir)) + require.NoError(t, err) + + injectFun := customopts.WithCDI(test.annotations) err = injectFun(nil, nil, nil, spec) assert.Equal(t, test.expectError, err != nil) diff --git a/pkg/cri/sbserver/service_linux.go b/pkg/cri/sbserver/service_linux.go index 5c6c32b32..c5da2d46f 100644 --- a/pkg/cri/sbserver/service_linux.go +++ b/pkg/cri/sbserver/service_linux.go @@ -19,6 +19,7 @@ package sbserver import ( "fmt" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/containerd/containerd/pkg/cap" "github.com/containerd/containerd/pkg/userns" "github.com/containerd/go-cni" @@ -87,6 +88,14 @@ func (c *criService) initPlatform() (err error) { } } + if c.config.EnableCDI { + reg := cdi.GetRegistry() + err = reg.Configure(cdi.WithSpecDirs(c.config.CDISpecDirs...)) + if err != nil { + return fmt.Errorf("failed to configure CDI registry") + } + } + return nil } diff --git a/pkg/cri/server/container_create_linux.go b/pkg/cri/server/container_create_linux.go index 4bdbc9435..c995526b9 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, customopts.WithCDI(config.Annotations, c.config.CDISpecDirs)) + specOpts = append(specOpts, customopts.WithCDI(config.Annotations)) } return specOpts, nil } diff --git a/pkg/cri/server/container_create_linux_test.go b/pkg/cri/server/container_create_linux_test.go index eecf788ec..483d728f1 100644 --- a/pkg/cri/server/container_create_linux_test.go +++ b/pkg/cri/server/container_create_linux_test.go @@ -1648,7 +1648,11 @@ containerEdits: } require.NoError(t, err) - injectFun := customopts.WithCDI(test.annotations, []string{cdiDir}) + reg := cdi.GetRegistry() + err = reg.Configure(cdi.WithSpecDirs(cdiDir)) + require.NoError(t, err) + + injectFun := customopts.WithCDI(test.annotations) err = injectFun(nil, nil, nil, spec) assert.Equal(t, test.expectError, err != nil) diff --git a/pkg/cri/server/service_linux.go b/pkg/cri/server/service_linux.go index 020e37397..4b3c0bbe0 100644 --- a/pkg/cri/server/service_linux.go +++ b/pkg/cri/server/service_linux.go @@ -19,6 +19,7 @@ package server import ( "fmt" + "github.com/container-orchestrated-devices/container-device-interface/pkg/cdi" "github.com/containerd/containerd/pkg/cap" "github.com/containerd/containerd/pkg/userns" cni "github.com/containerd/go-cni" @@ -87,6 +88,14 @@ func (c *criService) initPlatform() (err error) { } } + if c.config.EnableCDI { + reg := cdi.GetRegistry() + err = reg.Configure(cdi.WithSpecDirs(c.config.CDISpecDirs...)) + if err != nil { + return fmt.Errorf("failed to configure CDI registry") + } + } + return nil }