From 232df647c1b9d1bd61b866783a22778a7acde322 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 14 Nov 2023 16:56:50 +0100 Subject: [PATCH] Resolve LDConfig path passed to nvidia-container-cli Instead of relying solely on a static config, we resolve the path to ldconfig. The path is checked for existence and a .real suffix is preferred. Signed-off-by: Evan Lezar --- CHANGELOG.md | 2 + cmd/nvidia-container-runtime-hook/main.go | 4 +- internal/config/cli.go | 29 ++++++++ internal/config/cli_test.go | 83 +++++++++++++++++++++++ internal/config/config.go | 5 +- tools/container/toolkit/toolkit.go | 15 ++-- 6 files changed, 125 insertions(+), 13 deletions(-) create mode 100644 internal/config/cli_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 173319c6..2bd59bfd 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cmd/nvidia-container-runtime-hook/main.go b/cmd/nvidia-container-runtime-hook/main.go index 0c61e558..ac807074 100644 --- a/cmd/nvidia-container-runtime-hook/main.go +++ b/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") diff --git a/internal/config/cli.go b/internal/config/cli.go index f2a55a7a..4a1e199d 100644 --- a/internal/config/cli.go +++ b/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 +} diff --git a/internal/config/cli_test.go b/internal/config/cli_test.go new file mode 100644 index 00000000..100bea95 --- /dev/null +++ b/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()) + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index f29afc95..bda139df 100644 --- a/internal/config/config.go +++ b/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. diff --git a/tools/container/toolkit/toolkit.go b/tools/container/toolkit/toolkit.go index 9e090e68..d57ba9c1 100644 --- a/tools/container/toolkit/toolkit.go +++ b/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) }