Use internal/config structs in hook

This change ensures that the Config structs from internal.Config
are used for the NVIDIA Container Runtime Hook config too.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## v1.14.0-rc.3
 * Added support for generating OCI hook JSON file to `nvidia-ctk runtime configure` command.
 * Remove installation of OCI hook JSON from RPM package.
+* Refactored config for `nvidia-container-runtime-hook`.
 
 ## v1.14.0-rc.2
 

cmd/nvidia-container-runtime-hook/container_config_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/config/image"
 	"github.com/stretchr/testify/require"
 )
@@ -438,10 +439,9 @@ func TestGetNvidiaConfig(t *testing.T) {
 			},
 			privileged: true,
 			hookConfig: &HookConfig{
-				SwarmResource: func() *string {
-					s := "DOCKER_SWARM_RESOURCE"
-					return &s
-				}(),
+				Config: config.Config{
+					SwarmResource: "DOCKER_SWARM_RESOURCE",
+				},
 				SupportedDriverCapabilities: "video,display,utility,compute",
 			},
 			expectedConfig: &nvidiaConfig{
@@ -457,10 +457,9 @@ func TestGetNvidiaConfig(t *testing.T) {
 			},
 			privileged: true,
 			hookConfig: &HookConfig{
-				SwarmResource: func() *string {
-					s := "NOT_DOCKER_SWARM_RESOURCE,DOCKER_SWARM_RESOURCE"
-					return &s
-				}(),
+				Config: config.Config{
+					SwarmResource: "NOT_DOCKER_SWARM_RESOURCE,DOCKER_SWARM_RESOURCE",
+				},
 				SupportedDriverCapabilities: "video,display,utility,compute",
 			},
 			expectedConfig: &nvidiaConfig{

cmd/nvidia-container-runtime-hook/hook_config.go

@@ -22,64 +22,22 @@ var defaultPaths = [...]string{
 	configPath,
 }
 
-// CLIConfig : options for nvidia-container-cli.
-type CLIConfig struct {
-	Root        *string  `toml:"root"`
-	Path        *string  `toml:"path"`
-	Environment []string `toml:"environment"`
-	Debug       *string  `toml:"debug"`
-	Ldcache     *string  `toml:"ldcache"`
-	LoadKmods   bool     `toml:"load-kmods"`
-	NoPivot     bool     `toml:"no-pivot"`
-	NoCgroups   bool     `toml:"no-cgroups"`
-	User        *string  `toml:"user"`
-	Ldconfig    *string  `toml:"ldconfig"`
-}
-
 // HookConfig : options for the nvidia-container-runtime-hook.
 type HookConfig struct {
-	DisableRequire                 bool               `toml:"disable-require"`
-	SwarmResource                  *string            `toml:"swarm-resource"`
-	AcceptEnvvarUnprivileged       bool               `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"`
-	AcceptDeviceListAsVolumeMounts bool               `toml:"accept-nvidia-visible-devices-as-volume-mounts"`
-	SupportedDriverCapabilities    DriverCapabilities `toml:"supported-driver-capabilities"`
-
-	NvidiaContainerCLI         CLIConfig                `toml:"nvidia-container-cli"`
-	NVIDIAContainerRuntime     config.RuntimeConfig     `toml:"nvidia-container-runtime"`
-	NVIDIAContainerRuntimeHook config.RuntimeHookConfig `toml:"nvidia-container-runtime-hook"`
+	config.Config
+	// TODO: We should also migrate the driver capabilities
+	SupportedDriverCapabilities DriverCapabilities `toml:"supported-driver-capabilities"`
 }
 
 func getDefaultHookConfig() (HookConfig, error) {
-	rtConfig, err := config.GetDefaultRuntimeConfig()
-	if err != nil {
-		return HookConfig{}, err
-	}
-
-	rtHookConfig, err := config.GetDefaultRuntimeHookConfig()
+	defaultCfg, err := config.GetDefault()
 	if err != nil {
 		return HookConfig{}, err
 	}
 
 	c := HookConfig{
-		DisableRequire:                 false,
-		SwarmResource:                  nil,
-		AcceptEnvvarUnprivileged:       true,
-		AcceptDeviceListAsVolumeMounts: false,
-		SupportedDriverCapabilities:    allDriverCapabilities,
-		NvidiaContainerCLI: CLIConfig{
-			Root:        nil,
-			Path:        nil,
-			Environment: []string{},
-			Debug:       nil,
-			Ldcache:     nil,
-			LoadKmods:   true,
-			NoPivot:     false,
-			NoCgroups:   false,
-			User:        nil,
-			Ldconfig:    nil,
-		},
-		NVIDIAContainerRuntime:     *rtConfig,
-		NVIDIAContainerRuntimeHook: *rtHookConfig,
+		Config:                      *defaultCfg,
+		SupportedDriverCapabilities: allDriverCapabilities,
 	}
 
 	return c, nil
@@ -142,11 +100,11 @@ func (c HookConfig) getConfigOption(fieldName string) string {
 
 // getSwarmResourceEnvvars returns the swarm resource envvars for the config.
 func (c *HookConfig) getSwarmResourceEnvvars() []string {
-	if c.SwarmResource == nil {
+	if c.SwarmResource == "" {
 		return nil
 	}
 
-	candidates := strings.Split(*c.SwarmResource, ",")
+	candidates := strings.Split(c.SwarmResource, ",")
 
 	var envvars []string
 	for _, c := range candidates {

cmd/nvidia-container-runtime-hook/hook_config_test.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"testing"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 	"github.com/stretchr/testify/require"
 )
 
@@ -110,10 +111,6 @@ func TestGetSwarmResourceEnvvars(t *testing.T) {
 		value    string
 		expected []string
 	}{
-		{
-			value:    "nil",
-			expected: nil,
-		},
 		{
 			value:    "",
 			expected: nil,
@@ -147,12 +144,9 @@ func TestGetSwarmResourceEnvvars(t *testing.T) {
 	for i, tc := range testCases {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 			c := &HookConfig{
-				SwarmResource: func() *string {
-					if tc.value == "nil" {
-						return nil
-					}
-					return &tc.value
-				}(),
+				Config: config.Config{
+					SwarmResource: tc.value,
+				},
 			}
 
 			envvars := c.getSwarmResourceEnvvars()

cmd/nvidia-container-runtime-hook/main.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 	"syscall"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/info"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
@@ -37,16 +38,12 @@ func exit() {
 	os.Exit(0)
 }
 
-func getCLIPath(config CLIConfig) string {
-	if config.Path != nil {
-		return *config.Path
+func getCLIPath(config config.ContainerCLIConfig) string {
+	if config.Path != "" {
+		return config.Path
 	}
 
-	var root string
-	if config.Root != nil {
-		root = *config.Root
-	}
-	if err := os.Setenv("PATH", lookup.GetPath(root)); err != nil {
+	if err := os.Setenv("PATH", lookup.GetPath(config.Root)); err != nil {
 		log.Panicln("couldn't set PATH variable:", err)
 	}
 
@@ -76,7 +73,7 @@ func doPrestart() {
 	if err != nil || hook == nil {
 		log.Panicln("error getting hook config:", err)
 	}
-	cli := hook.NvidiaContainerCLI
+	cli := hook.NVIDIAContainerCLIConfig
 
 	container := getContainerConfig(*hook)
 	nvidia := container.Nvidia
@@ -85,15 +82,15 @@ func doPrestart() {
 		return
 	}
 
-	if !hook.NVIDIAContainerRuntimeHook.SkipModeDetection && info.ResolveAutoMode(&logInterceptor{}, hook.NVIDIAContainerRuntime.Mode, container.Image) != "legacy" {
+	if !hook.NVIDIAContainerRuntimeHookConfig.SkipModeDetection && info.ResolveAutoMode(&logInterceptor{}, hook.NVIDIAContainerRuntimeConfig.Mode, container.Image) != "legacy" {
 		log.Panicln("invoking the NVIDIA Container Runtime Hook directly (e.g. specifying the docker --gpus flag) is not supported. Please use the NVIDIA Container Runtime (e.g. specify the --runtime=nvidia flag) instead.")
 	}
 
 	rootfs := getRootfsPath(container)
 
 	args := []string{getCLIPath(cli)}
-	if cli.Root != nil && *cli.Root != "" {
-		args = append(args, fmt.Sprintf("--root=%s", *cli.Root))
+	if cli.Root != "" {
+		args = append(args, fmt.Sprintf("--root=%s", cli.Root))
 	}
 	if cli.LoadKmods {
 		args = append(args, "--load-kmods")
@@ -103,19 +100,19 @@ func doPrestart() {
 	}
 	if *debugflag {
 		args = append(args, "--debug=/dev/stderr")
-	} else if cli.Debug != nil {
-		args = append(args, fmt.Sprintf("--debug=%s", *cli.Debug))
+	} else if cli.Debug != "" {
+		args = append(args, fmt.Sprintf("--debug=%s", cli.Debug))
 	}
-	if cli.Ldcache != nil {
-		args = append(args, fmt.Sprintf("--ldcache=%s", *cli.Ldcache))
+	if cli.Ldcache != "" {
+		args = append(args, fmt.Sprintf("--ldcache=%s", cli.Ldcache))
 	}
-	if cli.User != nil {
-		args = append(args, fmt.Sprintf("--user=%s", *cli.User))
+	if cli.User != "" {
+		args = append(args, fmt.Sprintf("--user=%s", cli.User))
 	}
 	args = append(args, "configure")
 
-	if cli.Ldconfig != nil {
-		args = append(args, fmt.Sprintf("--ldconfig=%s", *cli.Ldconfig))
+	if cli.Ldconfig != "" {
+		args = append(args, fmt.Sprintf("--ldconfig=%s", cli.Ldconfig))
 	}
 	if cli.NoCgroups {
 		args = append(args, "--no-cgroups")

internal/config/cli.go

@@ -19,7 +19,15 @@ package config
 // ContainerCLIConfig stores the options for the nvidia-container-cli
 type ContainerCLIConfig struct {
 	Root        string   `toml:"root"`
-	LoadKmods   bool     `toml:"load-kmods"`
-	Ldconfig    string   `toml:"ldconfig"`
+	Path        string   `toml:"path"`
 	Environment []string `toml:"environment"`
+	Debug       string   `toml:"debug"`
+	Ldcache     string   `toml:"ldcache"`
+	LoadKmods   bool     `toml:"load-kmods"`
+	// NoPivot disables the pivot root operation in the NVIDIA Container CLI.
+	// This is not exposed in the config if not set.
+	NoPivot   bool   `toml:"no-pivot,omitempty"`
+	NoCgroups bool   `toml:"no-cgroups"`
+	User      string `toml:"user"`
+	Ldconfig  string `toml:"ldconfig"`
 }

internal/config/config.go

@@ -57,8 +57,11 @@ var (
 // Config represents the contents of the config.toml file for the NVIDIA Container Toolkit
 // Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go
 type Config struct {
-	DisableRequire           bool `toml:"disable-require"`
-	AcceptEnvvarUnprivileged bool `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"`
+	DisableRequire                 bool   `toml:"disable-require"`
+	SwarmResource                  string `toml:"swarm-resource"`
+	AcceptEnvvarUnprivileged       bool   `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"`
+	AcceptDeviceListAsVolumeMounts bool   `toml:"accept-nvidia-visible-devices-as-volume-mounts"`
+	// SupportedDriverCapabilities    DriverCapabilities `toml:"supported-driver-capabilities"`
 
 	NVIDIAContainerCLIConfig         ContainerCLIConfig `toml:"nvidia-container-cli"`
 	NVIDIACTKConfig                  CTKConfig          `toml:"nvidia-ctk"`
@@ -290,16 +293,13 @@ func (c Config) asCommentedToml() (*toml.Tree, error) {
 }
 
 func shouldComment(key string, defaultValue interface{}, setTo interface{}) bool {
-	if key == "nvidia-container-cli.root" && setTo == "" {
-		return true
-	}
 	if key == "nvidia-container-cli.user" && !getCommentedUserGroup() {
 		return false
 	}
 	if key == "nvidia-container-runtime.debug" && setTo == "/dev/null" {
 		return true
 	}
-	if setTo == nil || defaultValue == setTo {
+	if setTo == nil || defaultValue == setTo || setTo == "" {
 		return true
 	}