[no-relnote] devel commit

Signed-off-by: Carlos Eduardo Arango Gutierrez <[email protected]>

Author: ArangoGutierrez

cmd/nvidia-ctk/cdi/generate/generate.go

@@ -62,7 +62,7 @@ type options struct {
 	configSearchPaths  []string
 	librarySearchPaths []string
 	disabledHooks      []string
-	enableChmodHook    bool
+	enabledHooks       []string
 
 	csv struct {
 		files          []string
@@ -215,11 +215,12 @@ func (m command) build() *cli.Command {
 				Destination: &opts.disabledHooks,
 				Sources:     cli.EnvVars("NVIDIA_CTK_CDI_GENERATE_DISABLED_HOOKS"),
 			},
-			&cli.BoolFlag{
-				Name:        "enable-chmod-hook",
-				Usage:       "Enable the chmod hook for device folder permissions. This hook is disabled by default.",
-				Destination: &opts.enableChmodHook,
-				Sources:     cli.EnvVars("NVIDIA_CTK_CDI_GENERATE_ENABLE_CHMOD_HOOK"),
+			&cli.StringSliceFlag{
+				Name:        "enable-hook",
+				Aliases:     []string{"enable-hooks", "force-hook", "force-hooks"},
+				Usage:       "Explicitly enable a hook in the generated CDI specification. This can be specified multiple times.",
+				Destination: &opts.enabledHooks,
+				Sources:     cli.EnvVars("NVIDIA_CTK_CDI_GENERATE_ENABLED_HOOKS", "NVIDIA_CTK_CDI_GENERATE_FORCE_HOOKS"),
 			},
 		},
 	}
@@ -299,6 +300,33 @@ func formatFromFilename(filename string) string {
 }
 
 func (m command) generateSpec(opts *options) (spec.Interface, error) {
+	// Load hooks configuration from config file if not specified via CLI
+	if m.config != nil {
+		if len(opts.disabledHooks) == 0 {
+			if disabledHooks := m.config.Get("nvidia-container-runtime-hook.disabled"); disabledHooks != nil {
+				if hooks, ok := disabledHooks.([]interface{}); ok {
+					for _, hook := range hooks {
+						if h, ok := hook.(string); ok {
+							opts.disabledHooks = append(opts.disabledHooks, h)
+						}
+					}
+				}
+			}
+		}
+
+		if len(opts.enabledHooks) == 0 {
+			if enabledHooks := m.config.Get("nvidia-container-runtime-hook.enabled"); enabledHooks != nil {
+				if hooks, ok := enabledHooks.([]interface{}); ok {
+					for _, hook := range hooks {
+						if h, ok := hook.(string); ok {
+							opts.enabledHooks = append(opts.enabledHooks, h)
+						}
+					}
+				}
+			}
+		}
+	}
+
 	var deviceNamers []nvcdi.DeviceNamer
 	for _, strategy := range opts.deviceNameStrategies {
 		deviceNamer, err := nvcdi.NewDeviceNamer(strategy)
@@ -328,8 +356,8 @@ func (m command) generateSpec(opts *options) (spec.Interface, error) {
 		cdiOptions = append(cdiOptions, nvcdi.WithDisabledHook(hook))
 	}
 
-	if opts.enableChmodHook {
-		cdiOptions = append(cdiOptions, nvcdi.WithEnableChmodHook(true))
+	for _, hook := range opts.enabledHooks {
+		cdiOptions = append(cdiOptions, nvcdi.WithEnabledHook(hook))
 	}
 
 	cdilib, err := nvcdi.New(cdiOptions...)

cmd/nvidia-ctk/cdi/generate/generate_test.go

@@ -328,12 +328,12 @@ containerEdits:
 		{
 			description: "enableChmodHook",
 			options: options{
-				format:          "yaml",
-				mode:            "nvml",
-				vendor:          "example.com",
-				class:           "device",
-				driverRoot:      driverRoot,
-				enableChmodHook: true,
+				format:       "yaml",
+				mode:         "nvml",
+				vendor:       "example.com",
+				class:        "device",
+				driverRoot:   driverRoot,
+				enabledHooks: []string{"enable-cuda-compat"},
 			},
 			expectedOptions: options{
 				format:            "yaml",
@@ -342,7 +342,7 @@ containerEdits:
 				class:             "device",
 				nvidiaCDIHookPath: "/usr/bin/nvidia-cdi-hook",
 				driverRoot:        driverRoot,
-				enableChmodHook:   true,
+				enabledHooks:      []string{"enable-cuda-compat"},
 			},
 			expectedSpec: `---
 cdiVersion: 0.5.0

internal/config/features.go

@@ -42,12 +42,6 @@ type features struct {
 	// possibly bypassing other checks by an orchestration system such as
 	// kubernetes.
 	IgnoreImexChannelRequests *feature `toml:"ignore-imex-channel-requests,omitempty"`
-	// EnableChmodHook allows the chmod hook to be injected for device folder permissions.
-	// This hook was originally added as a workaround for a specific crun issue with device
-	// nodes in subdirectories of /dev (e.g., /dev/dri/*, /dev/nvidia-caps/*).
-	// Since this issue has been resolved in newer versions of crun, this hook is disabled
-	// by default. Users who still require this functionality can explicitly enable it.
-	EnableChmodHook *feature `toml:"enable-chmod-hook,omitempty"`
 }
 
 type feature bool

internal/config/features_test.go

@@ -23,4 +23,12 @@ type RuntimeHookConfig struct {
 	Path string `toml:"path"`
 	// SkipModeDetection disables the mode check for the runtime hook.
 	SkipModeDetection bool `toml:"skip-mode-detection"`
+	// DisabledHooks specifies the hooks that are disabled for the NVIDIA
+	// Container Runtime hook.
+	DisabledHooks []string `toml:"disabled,omitempty"`
+	// EnabledHooks specifies the hooks that are enabled for the NVIDIA
+	// Container Runtime hook.
+	// Specify a list of explicitly enabled hooks. If a hook is specified as
+	// both enabled and disabled, it will be enabled.
+	EnabledHooks []string `toml:"enabled,omitempty"`
 }

internal/config/hook.go

@@ -50,6 +50,14 @@ const (
 	defaultNvidiaCDIHookPath = "/usr/bin/nvidia-cdi-hook"
 )
 
+// defaultDisabledHooks defines hooks that are disabled by default.
+// These hooks can be explicitly enabled using the WithEnabledHooks option.
+var defaultDisabledHooks = map[HookName]bool{
+	// ChmodHook is disabled by default as it was a workaround for older
+	// versions of crun that has since been fixed.
+	ChmodHook: true,
+}
+
 var _ Discover = (*Hook)(nil)
 
 // Devices returns an empty list of devices for a Hook discoverer.
@@ -81,8 +89,9 @@ type Option func(*cdiHookCreator)
 
 type cdiHookCreator struct {
 	nvidiaCDIHookPath string
-	disabledHooks     map[HookName]bool
-	enableChmodHook   bool
+	// hookStates tracks the enabled/disabled state of each hook
+	// If not present in the map, the hook uses its default state
+	hookStates map[HookName]bool
 
 	fixedArgs    []string
 	debugLogging bool
@@ -101,12 +110,12 @@ type HookCreator interface {
 	Create(HookName, ...string) *Hook
 }
 
-// WithDisabledHooks sets the set of hooks that are disabled for the CDI hook creator.
+// WithDisabledHooks explicitly disables the specified hooks.
 // This can be specified multiple times.
 func WithDisabledHooks(hooks ...HookName) Option {
 	return func(c *cdiHookCreator) {
 		for _, hook := range hooks {
-			c.disabledHooks[hook] = true
+			c.hookStates[hook] = false
 		}
 	}
 }
@@ -118,25 +127,27 @@ func WithNVIDIACDIHookPath(nvidiaCDIHookPath string) Option {
 	}
 }
 
-// WithEnableChmodHook allows the chmod hook to be enabled.
-// By default, the chmod hook is disabled as it was a workaround for older
-// versions of crun that has since been fixed.
-func WithEnableChmodHook(enabled bool) Option {
+// WithEnabledHooks explicitly enables the specified hooks.
+// This is useful for enabling hooks that are disabled by default.
+func WithEnabledHooks(hooks ...HookName) Option {
 	return func(c *cdiHookCreator) {
-		c.enableChmodHook = enabled
+		for _, hook := range hooks {
+			c.hookStates[hook] = true
+		}
 	}
 }
 
 func NewHookCreator(opts ...Option) HookCreator {
 	cdiHookCreator := &cdiHookCreator{
 		nvidiaCDIHookPath: defaultNvidiaCDIHookPath,
-		disabledHooks:     make(map[HookName]bool),
+		hookStates:        make(map[HookName]bool),
 	}
 	for _, opt := range opts {
 		opt(cdiHookCreator)
 	}
 
-	if cdiHookCreator.disabledHooks[AllHooks] {
+	// Check if all hooks are disabled
+	if state, exists := cdiHookCreator.hookStates[AllHooks]; exists && !state {
 		return &allDisabledHookCreator{}
 	}
 
@@ -162,24 +173,31 @@ func (c cdiHookCreator) Create(name HookName, args ...string) *Hook {
 
 // isDisabled checks if the specified hook name is disabled.
 func (c cdiHookCreator) isDisabled(name HookName, args ...string) bool {
-	if c.disabledHooks[name] {
-		return true
-	}
-
-	switch name {
-	case CreateSymlinksHook:
-		if len(args) == 0 {
+	// Check if we have an explicit state for this hook
+	if state, exists := c.hookStates[name]; exists {
+		// If explicitly disabled, return true
+		if !state {
 			return true
 		}
-	case ChmodHook:
-		// ChmodHook is disabled by default unless explicitly enabled
-		if !c.enableChmodHook {
-			return true
-		}
-		if len(args) == 0 {
-			return true
+		// If explicitly enabled, check only for empty args
+		if name == CreateSymlinksHook || name == ChmodHook {
+			return len(args) == 0
 		}
+		return false
 	}
+
+	// No explicit state, check default behavior
+	// First check if it's disabled by default
+	if defaultDisabledHooks[name] {
+		return true
+	}
+
+	// Apply default logic for hooks that are enabled by default
+	if name == CreateSymlinksHook {
+		// Skip if no symlinks to create
+		return len(args) == 0
+	}
+
 	return false
 }