From e51621aa7fa99d5bc6d5fe43282ec162a1456427 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 19 Jul 2023 13:12:20 +0200 Subject: [PATCH 1/2] Handle empty root in config If the config.toml has an empty root specified, this could be passed to the NVIDIA Container CLI through the --root flag which causes argument parsing to fail. This change only adds the --root flag if the config option is specified and is non-empty. Signed-off-by: Evan Lezar --- cmd/nvidia-container-runtime-hook/main.go | 2 +- scripts/release-packages.sh | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/cmd/nvidia-container-runtime-hook/main.go b/cmd/nvidia-container-runtime-hook/main.go index 2db60247..b0d88ee1 100644 --- a/cmd/nvidia-container-runtime-hook/main.go +++ b/cmd/nvidia-container-runtime-hook/main.go @@ -92,7 +92,7 @@ func doPrestart() { rootfs := getRootfsPath(container) args := []string{getCLIPath(cli)} - if cli.Root != nil { + if cli.Root != nil && *cli.Root != "" { args = append(args, fmt.Sprintf("--root=%s", *cli.Root)) } if cli.LoadKmods { diff --git a/scripts/release-packages.sh b/scripts/release-packages.sh index 1d777d70..10f7fbf8 100755 --- a/scripts/release-packages.sh +++ b/scripts/release-packages.sh @@ -102,9 +102,20 @@ function sync() { ubuntu*) pkg_type=deb ;; *) echo "ERROR: unexpected distribution ${src_dist}" + exit 1 ;; esac + if [[ $# -ge 4 && $4 == "package_type" ]] ; then + if [[ "${src_dist}" != "ubuntu18.04" && "${src_dist}" != "centos7" ]]; then + echo "Package type repos require ubuntu18.04 or centos7 as the source" + echo "skipping" + return + fi + dst_dist=$pkg_type + fi + + local arch=${target##*-} local dst_arch=${arch} case ${src_dist} in @@ -174,11 +185,13 @@ git -C ${PACKAGE_REPO_ROOT} clean -fdx ${REPO} for target in ${targets[@]}; do sync ${target} ${PACKAGE_CACHE}/packages ${PACKAGE_REPO_ROOT}/${REPO} + # We also create a `package_type` repo; internally we skip this for non-ubuntu18.04 or centos7 distributions + sync ${target} ${PACKAGE_CACHE}/packages ${PACKAGE_REPO_ROOT}/${REPO} "package_type" done git -C ${PACKAGE_REPO_ROOT} add ${REPO} -if [[ ${REPO} == "stable" ]]; then +if [[ "${REPO}" == "stable" ]]; then # Stable release git -C ${PACKAGE_REPO_ROOT} commit -s -F- < Date: Wed, 19 Jul 2023 14:37:49 +0200 Subject: [PATCH 2/2] Ensure default config comments are consistent Signed-off-by: Evan Lezar --- internal/config/cli.go | 7 +++--- internal/config/config.go | 12 +++++---- internal/config/config_test.go | 45 ++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/internal/config/cli.go b/internal/config/cli.go index bd801a39..cb8d615c 100644 --- a/internal/config/cli.go +++ b/internal/config/cli.go @@ -18,7 +18,8 @@ package config // ContainerCLIConfig stores the options for the nvidia-container-cli type ContainerCLIConfig struct { - Root string `toml:"root"` - LoadKmods bool `toml:"load-kmods"` - Ldconfig string `toml:"ldconfig"` + Root string `toml:"root"` + LoadKmods bool `toml:"load-kmods"` + Ldconfig string `toml:"ldconfig"` + Environment []string `toml:"environment"` } diff --git a/internal/config/config.go b/internal/config/config.go index 576e16d7..c281acd9 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -57,6 +57,7 @@ var ( // Config represents the contents of the config.toml file for the NVIDIA Container Toolkit // Note: This is currently duplicated by the HookConfig in cmd/nvidia-container-toolkit/hook_config.go type Config struct { + DisableRequire bool `toml:"disable-require"` AcceptEnvvarUnprivileged bool `toml:"accept-nvidia-visible-devices-envvar-when-unprivileged"` NVIDIAContainerCLIConfig ContainerCLIConfig `toml:"nvidia-container-cli"` @@ -279,25 +280,26 @@ func (c Config) asCommentedToml() (*toml.Tree, error) { } for k, v := range commentedDefaults { set := asToml.Get(k) - fmt.Printf("k=%v v=%+v set=%+v\n", k, v, set) if !shouldComment(k, v, set) { continue } - fmt.Printf("set=%+v v=%+v\n", set, v) asToml.SetWithComment(k, "", true, v) } return asToml, nil } -func shouldComment(key string, value interface{}, set interface{}) bool { +func shouldComment(key string, defaultValue interface{}, setTo interface{}) bool { + if key == "nvidia-container-cli.root" && setTo == "" { + return true + } if key == "nvidia-container-cli.user" && !getCommentedUserGroup() { return false } - if key == "nvidia-container-runtime.debug" && set == "/dev/null" { + if key == "nvidia-container-runtime.debug" && setTo == "/dev/null" { return true } - if set == nil || value == set { + if setTo == nil || defaultValue == setTo { return true } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 029a0109..3f94bc26 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -17,6 +17,7 @@ package config import ( + "bytes" "io/ioutil" "os" "path/filepath" @@ -234,3 +235,47 @@ func TestGetConfig(t *testing.T) { }) } } + +func TestConfigDefault(t *testing.T) { + config, err := getDefault() + require.NoError(t, err) + + buffer := new(bytes.Buffer) + _, err = config.Save(buffer) + require.NoError(t, err) + + var lines []string + for _, l := range strings.Split(buffer.String(), "\n") { + l = strings.TrimSpace(l) + if strings.HasPrefix(l, "# ") { + l = "#" + strings.TrimPrefix(l, "# ") + } + lines = append(lines, l) + } + + // We take the lines from the config that was included in previous packages. + expectedLines := []string{ + "disable-require = false", + "#swarm-resource = \"DOCKER_RESOURCE_GPU\"", + "#accept-nvidia-visible-devices-envvar-when-unprivileged = true", + "#accept-nvidia-visible-devices-as-volume-mounts = false", + + "#root = \"/run/nvidia/driver\"", + "#path = \"/usr/bin/nvidia-container-cli\"", + "environment = []", + "#debug = \"/var/log/nvidia-container-toolkit.log\"", + "#ldcache = \"/etc/ld.so.cache\"", + "load-kmods = true", + "#no-cgroups = false", + "#user = \"root:video\"", + + "[nvidia-container-runtime]", + "#debug = \"/var/log/nvidia-container-runtime.log\"", + "log-level = \"info\"", + "mode = \"auto\"", + + "mount-spec-path = \"/etc/nvidia-container-runtime/host-files-for-container.d\"", + } + + require.Subset(t, lines, expectedLines) +}