Merge branch 'fix-user-group' into 'main'

Fix bug in determining CLI user on SUSE systems

See merge request nvidia/container-toolkit/container-toolkit!532

CHANGELOG.md

@@ -7,6 +7,7 @@
 * Remove package dependency on libseccomp.
 * Added detection of libnvdxgdmal.so.1 on WSL2
 * Use devRoot to resolve MIG device nodes.
+* Fix bug in determining default nvidia-container-runtime.user config value on SUSE-based systems.
 
 ## v1.15.0-rc.1
 * Skip update of ldcache in containers without ldconfig. The .so.SONAME symlinks are still created.

internal/config/config.go

@@ -95,6 +95,7 @@ func GetDefault() (*Config, error) {
 		NVIDIAContainerCLIConfig: ContainerCLIConfig{
 			LoadKmods: true,
 			Ldconfig:  getLdConfigPath(),
+			User:      getUserGroup(),
 		},
 		NVIDIACTKConfig: CTKConfig{
 			Path: nvidiaCTKExecutable,
@@ -126,24 +127,32 @@ func getLdConfigPath() string {
 	return NormalizeLDConfigPath("@/sbin/ldconfig")
 }
 
-// getCommentedUserGroup returns whether the nvidia-container-cli user and group config option should be commented.
-func getCommentedUserGroup() bool {
-	uncommentIf := map[string]bool{
+func getUserGroup() string {
+	if isSuse() {
+		return "root:video"
+	}
+	return ""
+}
+
+// isSuse returns whether a SUSE-based distribution was detected.
+func isSuse() bool {
+	suseDists := map[string]bool{
 		"suse":     true,
 		"opensuse": true,
 	}
 
 	idsLike := getDistIDLike()
 	for _, id := range idsLike {
-		if uncommentIf[id] {
-			return false
-		}
-	}
+		if suseDists[id] {
 			return true
+		}
+	}
+	return false
 }
 
 // getDistIDLike returns the ID_LIKE field from /etc/os-release.
-func getDistIDLike() []string {
+// We can override this for testing.
+var getDistIDLike = func() []string {
 	releaseFile, err := os.Open("/etc/os-release")
 	if err != nil {
 		return nil

internal/config/config_test.go

@@ -48,6 +48,7 @@ func TestGetConfig(t *testing.T) {
 		contents        []string
 		expectedError   error
 		inspectLdconfig bool
+		distIdsLike     []string
 		expectedConfig  *Config
 	}{
 		{
@@ -93,6 +94,7 @@ func TestGetConfig(t *testing.T) {
 				"nvidia-container-cli.root = \"/bar/baz\"",
 				"nvidia-container-cli.load-kmods = false",
 				"nvidia-container-cli.ldconfig = \"/foo/bar/ldconfig\"",
+				"nvidia-container-cli.user = \"foo:bar\"",
 				"nvidia-container-runtime.debug = \"/foo/bar\"",
 				"nvidia-container-runtime.discover-mode = \"not-legacy\"",
 				"nvidia-container-runtime.log-level = \"debug\"",
@@ -112,6 +114,7 @@ func TestGetConfig(t *testing.T) {
 					Root:      "/bar/baz",
 					LoadKmods: false,
 					Ldconfig:  "/foo/bar/ldconfig",
+					User:      "foo:bar",
 				},
 				NVIDIAContainerRuntimeConfig: RuntimeConfig{
 					DebugFilePath: "/foo/bar",
@@ -152,6 +155,7 @@ func TestGetConfig(t *testing.T) {
 				"root = \"/bar/baz\"",
 				"load-kmods = false",
 				"ldconfig = \"/foo/bar/ldconfig\"",
+				"user = \"foo:bar\"",
 				"[nvidia-container-runtime]",
 				"debug = \"/foo/bar\"",
 				"discover-mode = \"not-legacy\"",
@@ -176,6 +180,7 @@ func TestGetConfig(t *testing.T) {
 					Root:      "/bar/baz",
 					LoadKmods: false,
 					Ldconfig:  "/foo/bar/ldconfig",
+					User:      "foo:bar",
 				},
 				NVIDIAContainerRuntimeConfig: RuntimeConfig{
 					DebugFilePath: "/foo/bar",
@@ -207,10 +212,88 @@ func TestGetConfig(t *testing.T) {
 				},
 			},
 		},
+		{
+			description:     "suse config",
+			distIdsLike:     []string{"suse", "opensuse"},
+			inspectLdconfig: true,
+			expectedConfig: &Config{
+				AcceptEnvvarUnprivileged:    true,
+				SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video",
+				NVIDIAContainerCLIConfig: ContainerCLIConfig{
+					Root:      "",
+					LoadKmods: true,
+					Ldconfig:  "WAS_CHECKED",
+					User:      "root:video",
+				},
+				NVIDIAContainerRuntimeConfig: RuntimeConfig{
+					DebugFilePath: "/dev/null",
+					LogLevel:      "info",
+					Runtimes:      []string{"docker-runc", "runc"},
+					Mode:          "auto",
+					Modes: modesConfig{
+						CSV: csvModeConfig{
+							MountSpecPath: "/etc/nvidia-container-runtime/host-files-for-container.d",
+						},
+						CDI: cdiModeConfig{
+							DefaultKind:        "nvidia.com/gpu",
+							AnnotationPrefixes: []string{"cdi.k8s.io/"},
+							SpecDirs:           []string{"/etc/cdi", "/var/run/cdi"},
+						},
+					},
+				},
+				NVIDIAContainerRuntimeHookConfig: RuntimeHookConfig{
+					Path: "nvidia-container-runtime-hook",
+				},
+				NVIDIACTKConfig: CTKConfig{
+					Path: "nvidia-ctk",
+				},
+			},
+		},
+		{
+			description:     "suse config overrides user",
+			distIdsLike:     []string{"suse", "opensuse"},
+			inspectLdconfig: true,
+			contents: []string{
+				"nvidia-container-cli.user = \"foo:bar\"",
+			},
+			expectedConfig: &Config{
+				AcceptEnvvarUnprivileged:    true,
+				SupportedDriverCapabilities: "compat32,compute,display,graphics,ngx,utility,video",
+				NVIDIAContainerCLIConfig: ContainerCLIConfig{
+					Root:      "",
+					LoadKmods: true,
+					Ldconfig:  "WAS_CHECKED",
+					User:      "foo:bar",
+				},
+				NVIDIAContainerRuntimeConfig: RuntimeConfig{
+					DebugFilePath: "/dev/null",
+					LogLevel:      "info",
+					Runtimes:      []string{"docker-runc", "runc"},
+					Mode:          "auto",
+					Modes: modesConfig{
+						CSV: csvModeConfig{
+							MountSpecPath: "/etc/nvidia-container-runtime/host-files-for-container.d",
+						},
+						CDI: cdiModeConfig{
+							DefaultKind:        "nvidia.com/gpu",
+							AnnotationPrefixes: []string{"cdi.k8s.io/"},
+							SpecDirs:           []string{"/etc/cdi", "/var/run/cdi"},
+						},
+					},
+				},
+				NVIDIAContainerRuntimeHookConfig: RuntimeHookConfig{
+					Path: "nvidia-container-runtime-hook",
+				},
+				NVIDIACTKConfig: CTKConfig{
+					Path: "nvidia-ctk",
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
+			defer setGetDistIDLikeForTest(tc.distIdsLike)()
 			reader := strings.NewReader(strings.Join(tc.contents, "\n"))
 
 			tomlCfg, err := loadConfigTomlFrom(reader)
@@ -236,3 +319,19 @@ func TestGetConfig(t *testing.T) {
 		})
 	}
 }
+
+// setGetDistIDsLikeForTest overrides the distribution IDs that would normally be read from the /etc/os-release file.
+func setGetDistIDLikeForTest(ids []string) func() {
+	if ids == nil {
+		return func() {}
+	}
+	original := getDistIDLike
+
+	getDistIDLike = func() []string {
+		return ids
+	}
+
+	return func() {
+		getDistIDLike = original
+	}
+}

internal/config/toml.go

@@ -201,7 +201,7 @@ func (t *Toml) commentDefaults() *Toml {
 }
 
 func shouldComment(key string, defaultValue interface{}, setTo interface{}) bool {
-	if key == "nvidia-container-cli.user" && !getCommentedUserGroup() {
+	if key == "nvidia-container-cli.user" && defaultValue == setTo && isSuse() {
 		return false
 	}
 	if key == "nvidia-container-runtime.debug" && setTo == "/dev/null" {