[no-relnote] Minor comments

Signed-off-by: Evan Lezar <[email protected]>

Author: elezar

cmd/nvidia-ctk/runtime/configure/configure.go

@@ -52,6 +52,10 @@ const (
 	defaultConfigSource = configSourceFile
 	configSourceCommand = "command"
 	configSourceFile    = "file"
+
+	// TODO: We may want to spend some time unifying the handling of config
+	// files here with the Setup-Cleanup logic in nvidia-ctk-installer.
+	runtimeSpecificDefault = "RUNTIME_SPECIFIC_DEFAULT"
 )
 
 type command struct {
@@ -125,6 +129,7 @@ func (m command) build() *cli.Command {
 			&cli.StringFlag{
 				Name:        "drop-in-config",
 				Usage:       "path to the NVIDIA-specific config file to create. When specified, runtime configurations are saved to this file instead of modifying the main config file",
+				Value:       runtimeSpecificDefault,
 				Destination: &config.dropInConfigPath,
 			},
 			&cli.StringFlag{
@@ -250,23 +255,25 @@ func (m command) validateFlags(config *config) error {
 		}
 	}
 
-	if config.dropInConfigPath == "" {
+	if config.dropInConfigPath == runtimeSpecificDefault {
 		switch config.runtime {
 		case "containerd":
 			config.dropInConfigPath = defaultContainerdDropInConfigFilePath
 		case "crio":
 			config.dropInConfigPath = defaultCrioDropInConfigFilePath
+		case "docker":
+			config.dropInConfigPath = ""
 		}
 	}
 
-	if config.dropInConfigPath != "" && !filepath.IsAbs(config.dropInConfigPath) {
-		return fmt.Errorf("the drop-in-config path %q is not an absolute path", config.dropInConfigPath)
-	}
-
 	if config.dropInConfigPath != "" && config.runtime == "docker" {
 		return fmt.Errorf("runtime %v does not support drop-in configs", config.runtime)
 	}
 
+	if config.dropInConfigPath != "" && !filepath.IsAbs(config.dropInConfigPath) {
+		return fmt.Errorf("the drop-in-config path %q is not an absolute path", config.dropInConfigPath)
+	}
+
 	return nil
 }
 
@@ -371,16 +378,12 @@ func (c *config) getCommandConfigSource() toml.Loader {
 // getOutputConfigPath returns the configured config path or "" if dry-run is enabled
 func (c *config) getOutputConfigPath() string {
 	if c.dryRun {
-		return ""
+		return engine.SaveToSTDOUT
 	}
-	var configFilePath string
-	if c.runtime == "containerd" || c.runtime == "crio" {
-		configFilePath = c.dropInConfigPath
-	} else {
-		configFilePath = c.configFilePath
+	if c.dropInConfigPath != "" {
+		return c.dropInConfigPath
 	}
-
-	return configFilePath
+	return c.configFilePath
 }
 
 // configureOCIHook creates and configures the OCI hook for the NVIDIA runtime

pkg/config/engine/api.go

@@ -16,6 +16,12 @@
 
 package engine
 
+const (
+	// SaveToSTDOUT is used to write the specified config to stdout instead of
+	// to a file on disk.
+	SaveToSTDOUT = ""
+)
+
 // Interface defines the API for a runtime config updater.
 type Interface interface {
 	AddRuntime(string, string, bool) error

pkg/config/engine/containerd/config_drop_in.go

@@ -21,13 +21,15 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/config/engine"
 )
 
 // A ConfigWithDropIn represents a pair of containerd configs.
 // The first is the top-level config and the second is an in-memory drop-in config
 // that only contains modifications made to the config.
 type ConfigWithDropIn struct {
+	logger         logger.Interface
 	topLevelConfig *topLevelConfig
 	engine.Interface
 }
@@ -37,14 +39,17 @@ var _ engine.Interface = (*ConfigWithDropIn)(nil)
 // A topLevelConfig stores the original on-disk top-level config.
 // The path to the config is also stored to allow it to be modified if required.
 type topLevelConfig struct {
+	logger                 logger.Interface
 	path                   string
 	containerToHostPathMap map[string]string
 	config                 *Config
 }
 
-func NewConfigWithDropIn(topLevelConfigPath string, containerToHostPathMap map[string]string, tlConfig *Config, dropInConfig engine.Interface) *ConfigWithDropIn {
+func NewConfigWithDropIn(logger logger.Interface, topLevelConfigPath string, containerToHostPathMap map[string]string, tlConfig *Config, dropInConfig engine.Interface) *ConfigWithDropIn {
 	return &ConfigWithDropIn{
+		logger: logger,
 		topLevelConfig: &topLevelConfig{
+			logger:                 logger,
 			path:                   topLevelConfigPath,
 			containerToHostPathMap: containerToHostPathMap,
 			config:                 tlConfig,
@@ -57,6 +62,9 @@ func NewConfigWithDropIn(topLevelConfigPath string, containerToHostPathMap map[s
 // The top-level config is optionally updated to include the required imports
 // to allow the drop-in-file to be created.
 func (c *ConfigWithDropIn) Save(dropInPath string) (int64, error) {
+	if dropInPath == engine.SaveToSTDOUT {
+		c.logger.Infof("Drop-in config:")
+	}
 	bytesWritten, err := c.Interface.Save(dropInPath)
 	if err != nil {
 		return 0, err
@@ -92,11 +100,10 @@ func (c *ConfigWithDropIn) RemoveRuntime(name string) error {
 // If the config is empty, the file will be deleted.
 func (c *topLevelConfig) Save(dropInPath string) (int64, error) {
 	saveToPath := c.path
-	// If dropInPath is empty, we write to STDOUT
-	if dropInPath == "" {
-		saveToPath = ""
+	if dropInPath == engine.SaveToSTDOUT {
+		saveToPath = engine.SaveToSTDOUT
+		c.logger.Infof("Top-level config:")
 	}
-
 	return c.config.Save(saveToPath)
 }
 
@@ -129,6 +136,11 @@ func (c *topLevelConfig) removeImports(dropInFilename string) {
 }
 
 func (c *topLevelConfig) importPattern(dropInFilename string) string {
+	// TODO: If we make output to STDOUT a property of the config itself, then
+	// we can actually generate the correct import statement.
+	if dropInFilename == engine.SaveToSTDOUT {
+		return "/etc/containerd/config.d/*.toml"
+	}
 	return c.asHostPath(filepath.Dir(dropInFilename)) + "/*.toml"
 }
 

pkg/config/engine/containerd/containerd.go

@@ -141,7 +141,7 @@ func New(opts ...Option) (engine.Interface, error) {
 			},
 		}
 
-		cfg := NewConfigWithDropIn(b.topLevelConfigPath, b.containerToHostPathMap, topLevelConfig, dropInConfig)
+		cfg := NewConfigWithDropIn(b.logger, b.topLevelConfigPath, b.containerToHostPathMap, topLevelConfig, dropInConfig)
 		return cfg, nil
 	}
 }