Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare changes for v1.15.1 release #495

Closed
wants to merge 11 commits into from
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
# NVIDIA Container Toolkit Changelog

## v1.15.1

* Use : as a config --set list separator. This fixes a bug in modifying lists in the config file with the `nvidia-ctk config` command.
* Add `RUNTIME_CONFIG_OVERRIDE` (`--runtime-config-override`) to the `nvidia-ctk runtime configure` command and the toolkit container to allow for containerd runtime options to be set directly. This can be used to override the `SystemdCroup` option explicitly, for example.
* Ensure consistent construction of libraries for CDI spec generation.
* Ensure that `nvidia-ctk cdi transform` creates specs with world-readable permissions.
* Remove provenance information from published images.
* Fix bug in processing of `--log=` argument of `nvidia-container-runtime`.
* Reduce verbosity of logging in the NVIDIA Container Runtime especially for non-`create` commands.
* Extract runtime options from default runtime if runc is not present in config.

## v1.15.0

* Remove `nvidia-container-runtime` and `nvidia-docker2` packages.
Expand Down
34 changes: 28 additions & 6 deletions cmd/nvidia-ctk/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ type command struct {
// options stores the subcommand options
type options struct {
flags.Options
sets cli.StringSlice
setListSeparator string
sets cli.StringSlice
}

// NewCommand constructs an config command with the specified logger
Expand All @@ -57,6 +58,9 @@ func (m command) build() *cli.Command {
c := cli.Command{
Name: "config",
Usage: "Interact with the NVIDIA Container Toolkit configuration",
Before: func(ctx *cli.Context) error {
return validateFlags(ctx, &opts)
},
Action: func(ctx *cli.Context) error {
return run(ctx, &opts)
},
Expand All @@ -71,10 +75,21 @@ func (m command) build() *cli.Command {
Destination: &opts.Config,
},
&cli.StringSliceFlag{
Name: "set",
Usage: "Set a config value using the pattern key=value. If value is empty, this is equivalent to specifying the same key in unset. This flag can be specified multiple times",
Name: "set",
Usage: "Set a config value using the pattern 'key[=value]'. " +
"Specifying only 'key' is equivalent to 'key=true' for boolean settings. " +
"This flag can be specified multiple times, but only the last value for a specific " +
"config option is applied. " +
"If the setting represents a list, the elements are colon-separated.",
Destination: &opts.sets,
},
&cli.StringFlag{
Name: "set-list-separator",
Usage: "Specify a separator for lists applied using the set command.",
Hidden: true,
Value: ":",
Destination: &opts.setListSeparator,
},
&cli.BoolFlag{
Name: "in-place",
Aliases: []string{"i"},
Expand All @@ -96,6 +111,13 @@ func (m command) build() *cli.Command {
return &c
}

func validateFlags(c *cli.Context, opts *options) error {
if opts.setListSeparator == "" {
return fmt.Errorf("set-list-separator must be set")
}
return nil
}

func run(c *cli.Context, opts *options) error {
cfgToml, err := config.New(
config.WithConfigFile(opts.Config),
Expand All @@ -105,7 +127,7 @@ func run(c *cli.Context, opts *options) error {
}

for _, set := range opts.sets.Value() {
key, value, err := setFlagToKeyValue(set)
key, value, err := setFlagToKeyValue(set, opts.setListSeparator)
if err != nil {
return fmt.Errorf("invalid --set option %v: %w", set, err)
}
Expand Down Expand Up @@ -139,7 +161,7 @@ var errInvalidFormat = errors.New("invalid format")
// setFlagToKeyValue converts a --set flag to a key-value pair.
// The set flag is of the form key[=value], with the value being optional if key refers to a
// boolean config option.
func setFlagToKeyValue(setFlag string) (string, interface{}, error) {
func setFlagToKeyValue(setFlag string, setListSeparator string) (string, interface{}, error) {
setParts := strings.SplitN(setFlag, "=", 2)
key := setParts[0]

Expand Down Expand Up @@ -172,7 +194,7 @@ func setFlagToKeyValue(setFlag string) (string, interface{}, error) {
case reflect.String:
return key, value, nil
case reflect.Slice:
valueParts := strings.Split(value, ",")
valueParts := strings.Split(value, setListSeparator)
switch field.Elem().Kind() {
case reflect.String:
return key, valueParts, nil
Expand Down
41 changes: 27 additions & 14 deletions cmd/nvidia-ctk/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ import (
func TestSetFlagToKeyValue(t *testing.T) {
// TODO: We need to enable this test again since switching to reflect.
testCases := []struct {
description string
setFlag string
expectedKey string
expectedValue interface{}
expectedError error
description string
setFlag string
setListSeparator string
expectedKey string
expectedValue interface{}
expectedError error
}{
{
description: "option not present returns an error",
Expand Down Expand Up @@ -106,22 +107,34 @@ func TestSetFlagToKeyValue(t *testing.T) {
expectedValue: []string{"string-value"},
},
{
description: "[]string option returns multiple values",
setFlag: "nvidia-container-cli.environment=first,second",
expectedKey: "nvidia-container-cli.environment",
expectedValue: []string{"first", "second"},
description: "[]string option returns multiple values",
setFlag: "nvidia-container-cli.environment=first,second",
setListSeparator: ",",
expectedKey: "nvidia-container-cli.environment",
expectedValue: []string{"first", "second"},
},
{
description: "[]string option returns values with equals",
setFlag: "nvidia-container-cli.environment=first=1,second=2",
expectedKey: "nvidia-container-cli.environment",
expectedValue: []string{"first=1", "second=2"},
description: "[]string option returns values with equals",
setFlag: "nvidia-container-cli.environment=first=1,second=2",
setListSeparator: ",",
expectedKey: "nvidia-container-cli.environment",
expectedValue: []string{"first=1", "second=2"},
},
{
description: "[]string option returns multiple values semi-colon",
setFlag: "nvidia-container-cli.environment=first;second",
setListSeparator: ";",
expectedKey: "nvidia-container-cli.environment",
expectedValue: []string{"first", "second"},
},
}

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
k, v, err := setFlagToKeyValue(tc.setFlag)
if tc.setListSeparator == "" {
tc.setListSeparator = ","
}
k, v, err := setFlagToKeyValue(tc.setFlag, tc.setListSeparator)
require.ErrorIs(t, err, tc.expectedError)
require.EqualValues(t, tc.expectedKey, k)
require.EqualValues(t, tc.expectedValue, v)
Expand Down
35 changes: 35 additions & 0 deletions cmd/nvidia-ctk/runtime/configure/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package configure

import (
"encoding/json"
"fmt"
"path/filepath"

Expand Down Expand Up @@ -66,6 +67,8 @@ type config struct {
mode string
hookFilePath string

runtimeConfigOverrideJSON string

nvidiaRuntime struct {
name string
path string
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions deployments/container/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ ARTIFACTS_ROOT ?= $(shell realpath --relative-to=$(CURDIR) $(DIST_DIR))
$(BUILD_TARGETS): build-%: $(ARTIFACTS_ROOT)
DOCKER_BUILDKIT=1 \
$(DOCKER) $(BUILDX) build --pull \
--provenance=false --sbom=false \
$(DOCKER_BUILD_OPTIONS) \
$(DOCKER_BUILD_PLATFORM_OPTIONS) \
--tag $(IMAGE) \
Expand Down
50 changes: 12 additions & 38 deletions internal/info/auto.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,66 +17,40 @@
package info

import (
"github.com/NVIDIA/go-nvlib/pkg/nvlib/device"
"github.com/NVIDIA/go-nvlib/pkg/nvlib/info"
"github.com/NVIDIA/go-nvml/pkg/nvml"

"github.com/NVIDIA/nvidia-container-toolkit/internal/config/image"
"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
)

type resolver struct {
logger logger.Interface
info info.PropertyExtractor
}

// ResolveAutoMode determines the correct mode for the platform if set to "auto"
func ResolveAutoMode(logger logger.Interface, mode string, image image.CUDA) (rmode string) {
nvinfo := info.New()
nvmllib := nvml.New()
devicelib := device.New(
device.WithNvml(nvmllib),
)

info := additionalInfo{
Interface: nvinfo,
nvmllib: nvmllib,
devicelib: devicelib,
}

r := resolver{
logger: logger,
info: info,
}
return r.resolveMode(mode, image)
return resolveMode(logger, mode, image, nil)
}

// resolveMode determines the correct mode for the platform if set to "auto"
func (r resolver) resolveMode(mode string, image image.CUDA) (rmode string) {
func resolveMode(logger logger.Interface, mode string, image image.CUDA, propertyExtractor info.PropertyExtractor) (rmode string) {
if mode != "auto" {
r.logger.Infof("Using requested mode '%s'", mode)
logger.Infof("Using requested mode '%s'", mode)
return mode
}
defer func() {
r.logger.Infof("Auto-detected mode as '%v'", rmode)
logger.Infof("Auto-detected mode as '%v'", rmode)
}()

if image.OnlyFullyQualifiedCDIDevices() {
return "cdi"
}

isTegra, reason := r.info.HasTegraFiles()
r.logger.Debugf("Is Tegra-based system? %v: %v", isTegra, reason)

hasNVML, reason := r.info.HasNvml()
r.logger.Debugf("Has NVML? %v: %v", hasNVML, reason)

usesNVGPUModule, reason := r.info.UsesOnlyNVGPUModule()
r.logger.Debugf("Uses nvgpu kernel module? %v: %v", usesNVGPUModule, reason)
nvinfo := info.New(
info.WithLogger(logger),
info.WithPropertyExtractor(propertyExtractor),
)

if (isTegra && !hasNVML) || usesNVGPUModule {
switch nvinfo.ResolvePlatform() {
case info.PlatformNVML, info.PlatformWSL:
return "legacy"
case info.PlatformTegra:
return "csv"
}

return "legacy"
}
15 changes: 8 additions & 7 deletions internal/info/auto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,16 @@ func TestResolveAutoMode(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
info := &info.PropertyExtractorMock{
properties := &info.PropertyExtractorMock{
HasNvmlFunc: func() (bool, string) {
return tc.info["nvml"], "nvml"
},
HasDXCoreFunc: func() (bool, string) {
return tc.info["dxcore"], "dxcore"
},
IsTegraSystemFunc: func() (bool, string) {
return tc.info["tegra"], "tegra"
},
HasTegraFilesFunc: func() (bool, string) {
return tc.info["tegra"], "tegra"
},
Expand All @@ -215,11 +221,6 @@ func TestResolveAutoMode(t *testing.T) {
},
}

r := resolver{
logger: logger,
info: info,
}

var mounts []specs.Mount
for _, d := range tc.mounts {
mount := specs.Mount{
Expand All @@ -232,7 +233,7 @@ func TestResolveAutoMode(t *testing.T) {
image.WithEnvMap(tc.envmap),
image.WithMounts(mounts),
)
mode := r.resolveMode(tc.mode, image)
mode := resolveMode(logger, tc.mode, image, properties)
require.EqualValues(t, tc.expectedMode, mode)
})
}
Expand Down
1 change: 1 addition & 0 deletions internal/logger/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ type Interface interface {
Infof(string, ...interface{})
Warning(...interface{})
Warningf(string, ...interface{})
Tracef(string, ...interface{})
}
3 changes: 3 additions & 0 deletions internal/logger/lib.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ func (l *NullLogger) Warning(...interface{}) {}

// Warningf is a no-op for the null logger
func (l *NullLogger) Warningf(string, ...interface{}) {}

// Tracef is a no-op for the null logger
func (l *NullLogger) Tracef(string, ...interface{}) {}
5 changes: 3 additions & 2 deletions internal/oci/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@

package oci

//go:generate moq -stub -out runtime_mock.go . Runtime

// Runtime is an interface for a runtime shim. The Exec method accepts a list
// of command line arguments, and returns an error / nil.
//
//go:generate moq -rm -stub -out runtime_mock.go . Runtime
type Runtime interface {
Exec([]string) error
String() string
}
Loading