From c86c3aeeaf15914dbb3b79d8fdc81f9d689a2735 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 15 May 2024 22:19:18 +0200 Subject: [PATCH 1/2] Allow per-runtime config overrides Signed-off-by: Evan Lezar --- pkg/config/engine/api.go | 2 +- pkg/config/engine/containerd/config_v1.go | 12 ++- pkg/config/engine/containerd/config_v2.go | 7 +- .../engine/containerd/config_v2_test.go | 97 +++++++++++++++++++ pkg/config/engine/containerd/toml.go | 56 +++++++++++ pkg/config/engine/crio/crio.go | 2 +- pkg/config/engine/docker/docker.go | 2 +- 7 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 pkg/config/engine/containerd/config_v2_test.go create mode 100644 pkg/config/engine/containerd/toml.go diff --git a/pkg/config/engine/api.go b/pkg/config/engine/api.go index b074dadf..78b1549c 100644 --- a/pkg/config/engine/api.go +++ b/pkg/config/engine/api.go @@ -19,7 +19,7 @@ package engine // Interface defines the API for a runtime config updater. type Interface interface { DefaultRuntime() string - AddRuntime(string, string, bool) error + AddRuntime(string, string, bool, ...map[string]interface{}) error Set(string, interface{}) RemoveRuntime(string) error Save(string) (int64, error) diff --git a/pkg/config/engine/containerd/config_v1.go b/pkg/config/engine/containerd/config_v1.go index e9afda95..10e80b58 100644 --- a/pkg/config/engine/containerd/config_v1.go +++ b/pkg/config/engine/containerd/config_v1.go @@ -30,7 +30,7 @@ type ConfigV1 Config var _ engine.Interface = (*ConfigV1)(nil) // AddRuntime adds a runtime to the containerd config -func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error { +func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool, configOverrides ...map[string]interface{}) error { if c == nil || c.Tree == nil { return fmt.Errorf("config is nil") } @@ -75,6 +75,16 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error } config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "BinaryName"}, path) config.SetPath([]string{"plugins", "cri", "containerd", "default_runtime", "options", "Runtime"}, path) + + defaultRuntimeSubtree := subtreeAtPath(config, "plugins", "cri", "containerd", "default_runtime") + if err := defaultRuntimeSubtree.applyOverrides(configOverrides...); err != nil { + return fmt.Errorf("failed to apply config overrides to default_runtime: %w", err) + } + } + + runtimeSubtree := subtreeAtPath(config, "plugins", "cri", "containerd", "runtimes", name) + if err := runtimeSubtree.applyOverrides(configOverrides...); err != nil { + return fmt.Errorf("failed to apply config overrides: %w", err) } *c.Tree = config diff --git a/pkg/config/engine/containerd/config_v2.go b/pkg/config/engine/containerd/config_v2.go index 52ea54e2..e8818988 100644 --- a/pkg/config/engine/containerd/config_v2.go +++ b/pkg/config/engine/containerd/config_v2.go @@ -25,7 +25,7 @@ import ( ) // AddRuntime adds a runtime to the containerd config -func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { +func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configOverrides ...map[string]interface{}) error { if c == nil || c.Tree == nil { return fmt.Errorf("config is nil") } @@ -60,6 +60,11 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { config.SetPath([]string{"plugins", "io.containerd.grpc.v1.cri", "containerd", "default_runtime_name"}, name) } + runtimeSubtree := subtreeAtPath(config, "plugins", "io.containerd.grpc.v1.cri", "containerd", "runtimes", name) + if err := runtimeSubtree.applyOverrides(configOverrides...); err != nil { + return fmt.Errorf("failed to apply config overrides: %w", err) + } + *c.Tree = config return nil } diff --git a/pkg/config/engine/containerd/config_v2_test.go b/pkg/config/engine/containerd/config_v2_test.go new file mode 100644 index 00000000..1ab6cc1e --- /dev/null +++ b/pkg/config/engine/containerd/config_v2_test.go @@ -0,0 +1,97 @@ +/** +# Copyright 2024 NVIDIA CORPORATION +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package containerd + +import ( + "testing" + + "github.com/pelletier/go-toml" + "github.com/stretchr/testify/require" +) + +func TestAddRuntime(t *testing.T) { + testCases := []struct { + description string + config string + setAsDefault bool + configOverrides []map[string]interface{} + expectedConfig string + expectedError error + }{ + { + description: "empty config not default runtime", + expectedConfig: ` + version = 2 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test] + privileged_without_host_devices = false + runtime_engine = "" + runtime_root = "" + runtime_type = "" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + `, + expectedError: nil, + }, + { + description: "empty config not default runtime with overrides", + configOverrides: []map[string]interface{}{ + { + "options": map[string]interface{}{ + "SystemdCgroup": true, + }, + }, + }, + expectedConfig: ` + version = 2 + [plugins] + [plugins."io.containerd.grpc.v1.cri"] + [plugins."io.containerd.grpc.v1.cri".containerd] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes] + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test] + privileged_without_host_devices = false + runtime_engine = "" + runtime_root = "" + runtime_type = "" + [plugins."io.containerd.grpc.v1.cri".containerd.runtimes.test.options] + BinaryName = "/usr/bin/test" + SystemdCgroup = true + `, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + config, err := toml.Load(tc.config) + require.NoError(t, err) + expectedConfig, err := toml.Load(tc.expectedConfig) + require.NoError(t, err) + + c := &Config{ + Tree: config, + } + + err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...) + require.NoError(t, err) + + require.EqualValues(t, expectedConfig.String(), config.String()) + }) + } +} diff --git a/pkg/config/engine/containerd/toml.go b/pkg/config/engine/containerd/toml.go new file mode 100644 index 00000000..85c20d0e --- /dev/null +++ b/pkg/config/engine/containerd/toml.go @@ -0,0 +1,56 @@ +/** +# Copyright 2024 NVIDIA CORPORATION +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package containerd + +import ( + "fmt" + + "github.com/pelletier/go-toml" +) + +// tomlTree is an alias for toml.Tree that allows for extensions. +type tomlTree toml.Tree + +func subtreeAtPath(c toml.Tree, path ...string) *tomlTree { + tree := c.GetPath(path).(*toml.Tree) + return (*tomlTree)(tree) +} + +func (t *tomlTree) insert(other map[string]interface{}) error { + + for key, value := range other { + if insertsubtree, ok := value.(map[string]interface{}); ok { + subtree := (*toml.Tree)(t).Get(key).(*toml.Tree) + return (*tomlTree)(subtree).insert(insertsubtree) + } + (*toml.Tree)(t).Set(key, value) + } + return nil +} + +func (t *tomlTree) applyOverrides(overrides ...map[string]interface{}) error { + for _, override := range overrides { + subconfig, err := toml.TreeFromMap(override) + if err != nil { + return fmt.Errorf("invalid toml config: %w", err) + } + if err := t.insert(subconfig.ToMap()); err != nil { + return err + } + } + return nil +} diff --git a/pkg/config/engine/crio/crio.go b/pkg/config/engine/crio/crio.go index 46ef72b7..4fa9795e 100644 --- a/pkg/config/engine/crio/crio.go +++ b/pkg/config/engine/crio/crio.go @@ -40,7 +40,7 @@ func New(opts ...Option) (engine.Interface, error) { } // AddRuntime adds a new runtime to the crio config -func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { +func (c *Config) AddRuntime(name string, path string, setAsDefault bool, _ ...map[string]interface{}) error { if c == nil { return fmt.Errorf("config is nil") } diff --git a/pkg/config/engine/docker/docker.go b/pkg/config/engine/docker/docker.go index 831360f6..e6edbe81 100644 --- a/pkg/config/engine/docker/docker.go +++ b/pkg/config/engine/docker/docker.go @@ -49,7 +49,7 @@ func New(opts ...Option) (engine.Interface, error) { } // AddRuntime adds a new runtime to the docker config -func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error { +func (c *Config) AddRuntime(name string, path string, setAsDefault bool, _ ...map[string]interface{}) error { if c == nil { return fmt.Errorf("config is nil") } From b435b797af077aa510108c57ecb071182d8525a3 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 15 May 2024 22:21:25 +0200 Subject: [PATCH 2/2] Add support for adding additional containerd configs This allow for options such as SystemdCgroup to be optionally set. Signed-off-by: Evan Lezar --- cmd/nvidia-ctk/runtime/configure/configure.go | 35 +++++++++ tools/container/container.go | 8 +-- tools/container/containerd/containerd.go | 31 +++++++- tools/container/containerd/containerd_test.go | 72 +++++++++++++++++++ 4 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 tools/container/containerd/containerd_test.go diff --git a/cmd/nvidia-ctk/runtime/configure/configure.go b/cmd/nvidia-ctk/runtime/configure/configure.go index b4b53ed3..6f8f42f3 100644 --- a/cmd/nvidia-ctk/runtime/configure/configure.go +++ b/cmd/nvidia-ctk/runtime/configure/configure.go @@ -17,6 +17,7 @@ package configure import ( + "encoding/json" "fmt" "path/filepath" @@ -66,6 +67,8 @@ type config struct { mode string hookFilePath string + runtimeConfigOverrideJSON string + nvidiaRuntime struct { name string path string @@ -153,6 +156,13 @@ func (m command) build() *cli.Command { Usage: "Enable CDI in the configured runtime", Destination: &config.cdi.enabled, }, + &cli.StringFlag{ + Name: "runtime-config-override", + Destination: &config.runtimeConfigOverrideJSON, + Usage: "specify additional runtime options as a JSON string. The paths are relative to the runtime config.", + Value: "{}", + EnvVars: []string{"RUNTIME_CONFIG_OVERRIDE"}, + }, } return &configure @@ -194,6 +204,11 @@ func (m command) validateFlags(c *cli.Context, config *config) error { config.cdi.enabled = false } + if config.runtimeConfigOverrideJSON != "" && config.runtime != "containerd" { + m.logger.Warningf("Ignoring runtime-config-override flag for %v", config.runtime) + config.runtimeConfigOverrideJSON = "" + } + return nil } @@ -237,10 +252,16 @@ func (m command) configureConfigFile(c *cli.Context, config *config) error { return fmt.Errorf("unable to load config for runtime %v: %v", config.runtime, err) } + runtimeConfigOverride, err := config.runtimeConfigOverride() + if err != nil { + return fmt.Errorf("unable to parse config overrides: %w", err) + } + err = cfg.AddRuntime( config.nvidiaRuntime.name, config.nvidiaRuntime.path, config.nvidiaRuntime.setAsDefault, + runtimeConfigOverride, ) if err != nil { return fmt.Errorf("unable to update config: %v", err) @@ -293,6 +314,20 @@ func (c *config) getOuputConfigPath() string { return c.resolveConfigFilePath() } +// runtimeConfigOverride converts the specified runtimeConfigOverride JSON string to a map. +func (o *config) runtimeConfigOverride() (map[string]interface{}, error) { + if o.runtimeConfigOverrideJSON == "" { + return nil, nil + } + + runtimeOptions := make(map[string]interface{}) + if err := json.Unmarshal([]byte(o.runtimeConfigOverrideJSON), &runtimeOptions); err != nil { + return nil, fmt.Errorf("failed to read %v as JSON: %w", o.runtimeConfigOverrideJSON, err) + } + + return runtimeOptions, nil +} + // configureOCIHook creates and configures the OCI hook for the NVIDIA runtime func (m *command) configureOCIHook(c *cli.Context, config *config) error { err := ocihook.CreateHook(config.hookFilePath, config.nvidiaRuntime.hookPath) diff --git a/tools/container/container.go b/tools/container/container.go index c2c50c5b..8d09e734 100644 --- a/tools/container/container.go +++ b/tools/container/container.go @@ -67,8 +67,8 @@ func ParseArgs(c *cli.Context, o *Options) error { } // Configure applies the options to the specified config -func (o Options) Configure(cfg engine.Interface) error { - err := o.UpdateConfig(cfg) +func (o Options) Configure(cfg engine.Interface, configOverrides ...map[string]interface{}) error { + err := o.UpdateConfig(cfg, configOverrides...) if err != nil { return fmt.Errorf("unable to update config: %v", err) } @@ -98,14 +98,14 @@ func (o Options) flush(cfg engine.Interface) error { } // UpdateConfig updates the specified config to include the nvidia runtimes -func (o Options) UpdateConfig(cfg engine.Interface) error { +func (o Options) UpdateConfig(cfg engine.Interface, configOverrides ...map[string]interface{}) error { runtimes := operator.GetRuntimes( operator.WithNvidiaRuntimeName(o.RuntimeName), operator.WithSetAsDefault(o.SetAsDefault), operator.WithRoot(o.RuntimeDir), ) for name, runtime := range runtimes { - err := cfg.AddRuntime(name, runtime.Path, runtime.SetAsDefault) + err := cfg.AddRuntime(name, runtime.Path, runtime.SetAsDefault, configOverrides...) if err != nil { return fmt.Errorf("failed to update runtime %q: %v", name, err) } diff --git a/tools/container/containerd/containerd.go b/tools/container/containerd/containerd.go index bfe055e5..b5dff882 100644 --- a/tools/container/containerd/containerd.go +++ b/tools/container/containerd/containerd.go @@ -17,6 +17,7 @@ package main import ( + "encoding/json" "fmt" "os" @@ -47,6 +48,8 @@ type options struct { runtimeType string ContainerRuntimeModesCDIAnnotationPrefixes cli.StringSlice + + runtimeConfigOverrideJSON string } func main() { @@ -162,6 +165,13 @@ func main() { Destination: &options.ContainerRuntimeModesCDIAnnotationPrefixes, EnvVars: []string{"NVIDIA_CONTAINER_RUNTIME_MODES_CDI_ANNOTATION_PREFIXES"}, }, + &cli.StringFlag{ + Name: "runtime-config-override", + Destination: &options.runtimeConfigOverrideJSON, + Usage: "specify additional runtime options as a JSON string. The paths are relative to the runtime config.", + Value: "{}", + EnvVars: []string{"RUNTIME_CONFIG_OVERRIDE", "CONTAINERD_RUNTIME_CONFIG_OVERRIDE"}, + }, } // Update the subcommand flags with the common subcommand flags @@ -170,7 +180,7 @@ func main() { // Run the top-level CLI if err := c.Run(os.Args); err != nil { - log.Fatal(fmt.Errorf("Error: %v", err)) + log.Fatal(fmt.Errorf("error: %v", err)) } } @@ -188,7 +198,11 @@ func Setup(c *cli.Context, o *options) error { return fmt.Errorf("unable to load config: %v", err) } - err = o.Configure(cfg) + runtimeConfigOverride, err := o.runtimeConfigOverride() + if err != nil { + return fmt.Errorf("unable to parse config overrides: %w", err) + } + err = o.Configure(cfg, runtimeConfigOverride) if err != nil { return fmt.Errorf("unable to configure containerd: %v", err) } @@ -246,3 +260,16 @@ func (o *options) containerAnnotationsFromCDIPrefixes() []string { return annotations } + +func (o *options) runtimeConfigOverride() (map[string]interface{}, error) { + if o.runtimeConfigOverrideJSON == "" { + return nil, nil + } + + runtimeOptions := make(map[string]interface{}) + if err := json.Unmarshal([]byte(o.runtimeConfigOverrideJSON), &runtimeOptions); err != nil { + return nil, fmt.Errorf("failed to read %v as JSON: %w", o.runtimeConfigOverrideJSON, err) + } + + return runtimeOptions, nil +} diff --git a/tools/container/containerd/containerd_test.go b/tools/container/containerd/containerd_test.go new file mode 100644 index 00000000..7240a4dc --- /dev/null +++ b/tools/container/containerd/containerd_test.go @@ -0,0 +1,72 @@ +/** +# Copyright 2024 NVIDIA CORPORATION +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +**/ + +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRuntimeOptions(t *testing.T) { + testCases := []struct { + description string + options options + expected map[string]interface{} + expectedError error + }{ + { + description: "empty is nil", + }, + { + description: "empty json", + options: options{ + runtimeConfigOverrideJSON: "{}", + }, + expected: map[string]interface{}{}, + expectedError: nil, + }, + { + description: "SystemdCgroup is true", + options: options{ + runtimeConfigOverrideJSON: "{\"SystemdCgroup\": true}", + }, + expected: map[string]interface{}{ + "SystemdCgroup": true, + }, + expectedError: nil, + }, + { + description: "SystemdCgroup is false", + options: options{ + runtimeConfigOverrideJSON: "{\"SystemdCgroup\": false}", + }, + expected: map[string]interface{}{ + "SystemdCgroup": false, + }, + expectedError: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + runtimeOptions, err := tc.options.runtimeConfigOverride() + require.ErrorIs(t, tc.expectedError, err) + require.EqualValues(t, tc.expected, runtimeOptions) + }) + } +}