Remove support for config overrides

Signed-off-by: Evan Lezar <elezar@nvidia.com>
This commit is contained in:
Evan Lezar 2024-09-27 13:19:23 +02:00
parent 79c59aeb7f
commit 6c5f4eea63
13 changed files with 28 additions and 271 deletions

View File

@ -17,7 +17,6 @@
package configure
import (
"encoding/json"
"fmt"
"path/filepath"
@ -156,13 +155,6 @@ 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
@ -252,16 +244,10 @@ 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)
@ -314,20 +300,6 @@ 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)

View File

@ -19,7 +19,7 @@ package engine
// Interface defines the API for a runtime config updater.
type Interface interface {
DefaultRuntime() string
AddRuntime(string, string, bool, ...map[string]interface{}) error
AddRuntime(string, string, bool) error
Set(string, interface{})
RemoveRuntime(string) error
Save(string) (int64, error)

View File

@ -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, configOverrides ...map[string]interface{}) error {
func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool) error {
if c == nil || c.Tree == nil {
return fmt.Errorf("config is nil")
}
@ -85,16 +85,6 @@ func (c *ConfigV1) AddRuntime(name string, path string, setAsDefault bool, confi
}
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

View File

@ -27,12 +27,11 @@ import (
func TestAddRuntimeV1(t *testing.T) {
logger, _ := testlog.NewNullLogger()
testCases := []struct {
description string
config string
setAsDefault bool
configOverrides []map[string]interface{}
expectedConfig string
expectedError error
description string
config string
setAsDefault bool
expectedConfig string
expectedError error
}{
{
description: "empty config not default runtime",
@ -53,32 +52,6 @@ func TestAddRuntimeV1(t *testing.T) {
`,
expectedError: nil,
},
{
description: "empty config not default runtime with overrides",
configOverrides: []map[string]interface{}{
{
"options": map[string]interface{}{
"SystemdCgroup": true,
},
},
},
expectedConfig: `
version = 1
[plugins]
[plugins.cri]
[plugins.cri.containerd]
[plugins.cri.containerd.runtimes]
[plugins.cri.containerd.runtimes.test]
privileged_without_host_devices = false
runtime_engine = ""
runtime_root = ""
runtime_type = ""
[plugins.cri.containerd.runtimes.test.options]
BinaryName = "/usr/bin/test"
Runtime = "/usr/bin/test"
SystemdCgroup = true
`,
},
{
description: "options from runc are imported",
config: `
@ -236,7 +209,7 @@ func TestAddRuntimeV1(t *testing.T) {
Tree: config,
}
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...)
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
require.NoError(t, err)
require.EqualValues(t, expectedConfig.String(), config.String())

View File

@ -25,7 +25,7 @@ import (
)
// AddRuntime adds a runtime to the containerd config
func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configOverrides ...map[string]interface{}) error {
func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
if c == nil || c.Tree == nil {
return fmt.Errorf("config is nil")
}
@ -70,11 +70,6 @@ func (c *Config) AddRuntime(name string, path string, setAsDefault bool, configO
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
}

View File

@ -27,12 +27,11 @@ import (
func TestAddRuntime(t *testing.T) {
logger, _ := testlog.NewNullLogger()
testCases := []struct {
description string
config string
setAsDefault bool
configOverrides []map[string]interface{}
expectedConfig string
expectedError error
description string
config string
setAsDefault bool
expectedConfig string
expectedError error
}{
{
description: "empty config not default runtime",
@ -52,31 +51,6 @@ func TestAddRuntime(t *testing.T) {
`,
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
`,
},
{
description: "options from runc are imported",
config: `
@ -234,7 +208,7 @@ func TestAddRuntime(t *testing.T) {
Tree: config,
}
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...)
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
require.NoError(t, err)
require.EqualValues(t, expectedConfig.String(), config.String())

View File

@ -1,56 +0,0 @@
/**
# 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
}

View File

@ -44,7 +44,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, _ ...map[string]interface{}) error {
func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
if c == nil {
return fmt.Errorf("config is nil")
}

View File

@ -27,12 +27,11 @@ import (
func TestAddRuntime(t *testing.T) {
logger, _ := testlog.NewNullLogger()
testCases := []struct {
description string
config string
setAsDefault bool
configOverrides []map[string]interface{}
expectedConfig string
expectedError error
description string
config string
setAsDefault bool
expectedConfig string
expectedError error
}{
{
description: "empty config not default runtime",
@ -137,7 +136,7 @@ func TestAddRuntime(t *testing.T) {
Tree: config,
}
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault, tc.configOverrides...)
err = c.AddRuntime("test", "/usr/bin/test", tc.setAsDefault)
require.NoError(t, err)
require.EqualValues(t, expectedConfig.String(), config.String())

View File

@ -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, _ ...map[string]interface{}) error {
func (c *Config) AddRuntime(name string, path string, setAsDefault bool) error {
if c == nil {
return fmt.Errorf("config is nil")
}

View File

@ -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, configOverrides ...map[string]interface{}) error {
err := o.UpdateConfig(cfg, configOverrides...)
func (o Options) Configure(cfg engine.Interface) error {
err := o.UpdateConfig(cfg)
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, configOverrides ...map[string]interface{}) error {
func (o Options) UpdateConfig(cfg engine.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, configOverrides...)
err := cfg.AddRuntime(name, runtime.Path, runtime.SetAsDefault)
if err != nil {
return fmt.Errorf("failed to update runtime %q: %v", name, err)
}

View File

@ -17,7 +17,6 @@
package main
import (
"encoding/json"
"fmt"
"os"
@ -198,11 +197,7 @@ func Setup(c *cli.Context, o *options) error {
return fmt.Errorf("unable to load config: %v", err)
}
runtimeConfigOverride, err := o.runtimeConfigOverride()
if err != nil {
return fmt.Errorf("unable to parse config overrides: %w", err)
}
err = o.Configure(cfg, runtimeConfigOverride)
err = o.Configure(cfg)
if err != nil {
return fmt.Errorf("unable to configure containerd: %v", err)
}
@ -260,16 +255,3 @@ 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
}

View File

@ -1,72 +0,0 @@
/**
# 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)
})
}
}