Merge branch 'fix-ldconfig-resolution' into 'main'

Resolve LDConfig path

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

CHANGELOG.md

@@ -1,6 +1,8 @@
 # NVIDIA Container Toolkit Changelog
 
 * Skip update of ldcache in containers without ldconfig. The .so.SONAME symlinks are still created.
+* Normalize ldconfig path on use. This automatically adjust the ldconfig setting applied to ldconfig.real on systems where this exists.
+
 * [libnvidia-container] Fix device permission check when using cgroupv2 (fixes #227)
 
 ## v1.14.3

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

@@ -111,8 +111,8 @@ func doPrestart() {
 	}
 	args = append(args, "configure")
 
-	if cli.Ldconfig != "" {
-		args = append(args, fmt.Sprintf("--ldconfig=%s", cli.Ldconfig))
+	if ldconfigPath := cli.NormalizeLDConfigPath(); ldconfigPath != "" {
+		args = append(args, fmt.Sprintf("--ldconfig=%s", ldconfigPath))
 	}
 	if cli.NoCgroups {
 		args = append(args, "--no-cgroups")

cmd/nvidia-ctk/hook/update-ldcache/update-ldcache.go

@@ -22,6 +22,7 @@ import (
 	"path/filepath"
 	"syscall"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
 	"github.com/urfave/cli/v2"
@@ -31,7 +32,7 @@ type command struct {
 	logger logger.Interface
 }
 
-type config struct {
+type options struct {
 	folders       cli.StringSlice
 	containerSpec string
 }
@@ -46,7 +47,7 @@ func NewCommand(logger logger.Interface) *cli.Command {
 
 // build the update-ldcache command
 func (m command) build() *cli.Command {
-	cfg := config{}
+	cfg := options{}
 
 	// Create the 'update-ldcache' command
 	c := cli.Command{
@@ -73,7 +74,7 @@ func (m command) build() *cli.Command {
 	return &c
 }
 
-func (m command) run(c *cli.Context, cfg *config) error {
+func (m command) run(c *cli.Context, cfg *options) error {
 	s, err := oci.LoadContainerState(cfg.containerSpec)
 	if err != nil {
 		return fmt.Errorf("failed to load container state: %v", err)
@@ -84,7 +85,8 @@ func (m command) run(c *cli.Context, cfg *config) error {
 		return fmt.Errorf("failed to determined container root: %v", err)
 	}
 
-	args := []string{"/sbin/ldconfig"}
+	ldconfigPath := m.resolveLDConfigPath("/sbin/ldconfig")
+	args := []string{filepath.Base(ldconfigPath)}
 	if containerRoot != "" {
 		args = append(args, "-r", containerRoot)
 	}
@@ -118,6 +120,13 @@ func (r root) hasPath(path string) bool {
 	return true
 }
 
+// resolveLDConfigPath determines the LDConfig path to use for the system.
+// On systems such as Ubuntu where `/sbin/ldconfig` is a wrapper around
+// /sbin/ldconfig.real, the latter is returned.
+func (m command) resolveLDConfigPath(path string) string {
+	return config.NormalizeLDConfigPath("@" + path)
+}
+
 // createConfig creates (or updates) /etc/ld.so.conf.d/nvcr-<RANDOM_STRING>.conf in the container
 // to include the required paths.
 func (m command) createConfig(root string, folders []string) error {

internal/config/cli.go

@@ -16,6 +16,11 @@
 
 package config
 
+import (
+	"os"
+	"strings"
+)
+
 // ContainerCLIConfig stores the options for the nvidia-container-cli
 type ContainerCLIConfig struct {
 	Root        string   `toml:"root"`
@@ -31,3 +36,27 @@ type ContainerCLIConfig struct {
 	User      string `toml:"user"`
 	Ldconfig  string `toml:"ldconfig"`
 }
+
+// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
+// This is only done for host LDConfigs and is required to handle systems where
+// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
+func (c *ContainerCLIConfig) NormalizeLDConfigPath() string {
+	return NormalizeLDConfigPath(c.Ldconfig)
+}
+
+// NormalizeLDConfigPath returns the resolved path of the configured LDConfig binary.
+// This is only done for host LDConfigs and is required to handle systems where
+// /sbin/ldconfig is a wrapper around /sbin/ldconfig.real.
+func NormalizeLDConfigPath(path string) string {
+	if !strings.HasPrefix(path, "@") {
+		return path
+	}
+
+	trimmedPath := strings.TrimSuffix(strings.TrimPrefix(path, "@"), ".real")
+	// If the .real path exists, we return that.
+	if _, err := os.Stat(trimmedPath + ".real"); err == nil {
+		return "@" + trimmedPath + ".real"
+	}
+	// If the .real path does not exists (or cannot be read) we return the non-.real path.
+	return "@" + trimmedPath
+}

internal/config/cli_test.go

@@ -0,0 +1,83 @@
+/**
+# Copyright 2023 NVIDIA CORPORATION
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+**/
+
+package config
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNormalizeLDConfigPath(t *testing.T) {
+	testDir := t.TempDir()
+
+	f, err := os.Create(filepath.Join(testDir, "exists.real"))
+	require.NoError(t, err)
+	_ = f.Close()
+
+	testCases := []struct {
+		description string
+		ldconfig    string
+		expected    string
+	}{
+		{
+			description: "empty input",
+		},
+		{
+			description: "non-host with .real suffix returns as is",
+			ldconfig:    "/some/path/ldconfig.real",
+			expected:    "/some/path/ldconfig.real",
+		},
+		{
+			description: "non-host without .real suffix returns as is",
+			ldconfig:    "/some/path/ldconfig",
+			expected:    "/some/path/ldconfig",
+		},
+		{
+			description: "host .real file exists is returned",
+			ldconfig:    "@" + filepath.Join(testDir, "exists.real"),
+			expected:    "@" + filepath.Join(testDir, "exists.real"),
+		},
+		{
+			description: "host resolves .real file",
+			ldconfig:    "@" + filepath.Join(testDir, "exists"),
+			expected:    "@" + filepath.Join(testDir, "exists.real"),
+		},
+		{
+			description: "host .real file not exists strips suffix",
+			ldconfig:    "@/does/not/exist.real",
+			expected:    "@/does/not/exist",
+		},
+		{
+			description: "host file returned as is if no .real file exsits",
+			ldconfig:    "@/does/not/exist",
+			expected:    "@/does/not/exist",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			c := ContainerCLIConfig{
+				Ldconfig: tc.ldconfig,
+			}
+
+			require.Equal(t, tc.expected, c.NormalizeLDConfigPath())
+		})
+	}
+}

internal/config/config.go

@@ -122,10 +122,7 @@ func GetDefault() (*Config, error) {
 }
 
 func getLdConfigPath() string {
-	if _, err := os.Stat("/sbin/ldconfig.real"); err == nil {
-		return "@/sbin/ldconfig.real"
-	}
-	return "@/sbin/ldconfig"
+	return NormalizeLDConfigPath("@/sbin/ldconfig")
 }
 
 // getCommentedUserGroup returns whether the nvidia-container-cli user and group config option should be commented.

tools/container/toolkit/toolkit.go

@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/config"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/system/nvdevices"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi"
 	"github.com/NVIDIA/nvidia-container-toolkit/pkg/nvcdi/transform"
@@ -383,7 +384,7 @@ func installLibrary(libName string, toolkitRoot string) error {
 func installToolkitConfig(c *cli.Context, toolkitConfigPath string, nvidiaContainerCliExecutablePath string, nvidiaCTKPath string, nvidaContainerRuntimeHookPath string, opts *options) error {
 	log.Infof("Installing NVIDIA container toolkit config '%v'", toolkitConfigPath)
 
-	config, err := loadConfig(nvidiaContainerToolkitConfigSource)
+	cfg, err := loadConfig(nvidiaContainerToolkitConfigSource)
 	if err != nil {
 		return fmt.Errorf("could not open source config file: %v", err)
 	}
@@ -396,9 +397,9 @@ func installToolkitConfig(c *cli.Context, toolkitConfigPath string, nvidiaContai
 
 	// Read the ldconfig path from the config as this may differ per platform
 	// On ubuntu-based systems this ends in `.real`
-	ldconfigPath := fmt.Sprintf("%s", config.GetDefault("nvidia-container-cli.ldconfig", "/sbin/ldconfig"))
+	ldconfigPath := fmt.Sprintf("%s", cfg.GetDefault("nvidia-container-cli.ldconfig", "/sbin/ldconfig"))
 	// Use the driver run root as the root:
-	driverLdconfigPath := "@" + filepath.Join(opts.DriverRoot, strings.TrimPrefix(ldconfigPath, "@/"))
+	driverLdconfigPath := config.NormalizeLDConfigPath("@" + filepath.Join(opts.DriverRoot, strings.TrimPrefix(ldconfigPath, "@/")))
 
 	configValues := map[string]interface{}{
 		// Set the options in the root toml table
@@ -415,7 +416,7 @@ func installToolkitConfig(c *cli.Context, toolkitConfigPath string, nvidiaContai
 		"nvidia-container-runtime-hook.skip-mode-detection": opts.ContainerRuntimeHookSkipModeDetection,
 	}
 	for key, value := range configValues {
-		config.Set(key, value)
+		cfg.Set(key, value)
 	}
 
 	// Set the optional config options
@@ -452,15 +453,15 @@ func installToolkitConfig(c *cli.Context, toolkitConfigPath string, nvidiaContai
 			log.Warningf("Unexpected type for option %v=%v: %T", key, value, v)
 		}
 
-		config.Set(key, value)
+		cfg.Set(key, value)
 	}
 
-	if _, err := config.WriteTo(targetConfig); err != nil {
+	if _, err := cfg.WriteTo(targetConfig); err != nil {
 		return fmt.Errorf("error writing config: %v", err)
 	}
 
 	os.Stdout.WriteString("Using config:\n")
-	if _, err = config.WriteTo(os.Stdout); err != nil {
+	if _, err = cfg.WriteTo(os.Stdout); err != nil {
 		log.Warningf("Failed to output config to STDOUT: %v", err)
 	}