From 185d521f901305a45d78817373862104f9708a2f Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Tue, 7 May 2024 15:56:35 -0700 Subject: [PATCH 1/6] Create driver-ready file with content and simplify operand startup scripts This commit updates the driver validation to always create a 'driver-ready' file, regardless if the driver is installed on the host or not. It also populates this file with a list of environment variables, one per line, which are required by some operands. The startup scripts for several operands are simplified to simply source the content of this file before executing the main program for the container. Signed-off-by: Christopher Desiniotis --- .../0400_configmap.yaml | 18 ++++----- .../state-device-plugin/0400_configmap.yaml | 29 ++++----------- assets/state-mig-manager/0420_configmap.yaml | 37 ++++++------------- validator/main.go | 32 ++++++++-------- validator/metrics.go | 5 --- 5 files changed, 44 insertions(+), 77 deletions(-) diff --git a/assets/state-container-toolkit/0400_configmap.yaml b/assets/state-container-toolkit/0400_configmap.yaml index ada0872fe..cca4ee4e9 100644 --- a/assets/state-container-toolkit/0400_configmap.yaml +++ b/assets/state-container-toolkit/0400_configmap.yaml @@ -9,17 +9,15 @@ data: entrypoint.sh: |- #!/bin/bash - set -e + until [[ -f /run/nvidia/validations/driver-ready ]] + do + echo "waiting for the driver validations to be ready..." + sleep 5 + done - driver_root=/run/nvidia/driver - driver_root_ctr_path=$driver_root - if [[ -f /run/nvidia/validations/host-driver-ready ]]; then - driver_root=/ - driver_root_ctr_path=/host - fi - - export NVIDIA_DRIVER_ROOT=$driver_root - export DRIVER_ROOT_CTR_PATH=$driver_root_ctr_path + set -o allexport + cat /run/nvidia/validations/driver-ready + . /run/nvidia/validations/driver-ready # # The below delay is a workaround for an issue affecting some versions diff --git a/assets/state-device-plugin/0400_configmap.yaml b/assets/state-device-plugin/0400_configmap.yaml index 6bfba993b..651fd5421 100644 --- a/assets/state-device-plugin/0400_configmap.yaml +++ b/assets/state-device-plugin/0400_configmap.yaml @@ -9,28 +9,15 @@ data: entrypoint.sh: |- #!/bin/bash - driver_root="" - container_driver_root="" - while true; do - if [[ -f /run/nvidia/validations/host-driver-ready ]]; then - driver_root=/ - container_driver_root=/host - break - elif [[ -f /run/nvidia/validations/driver-ready ]]; then - driver_root=/run/nvidia/driver - container_driver_root=$driver_root - break - else - echo "waiting for the driver validations to be ready..." - sleep 5 - fi + until [[ -f /run/nvidia/validations/driver-ready ]] + do + echo "waiting for the driver validations to be ready..." + sleep 5 done - - export NVIDIA_DRIVER_ROOT=$driver_root - echo "NVIDIA_DRIVER_ROOT=$NVIDIA_DRIVER_ROOT" - - export CONTAINER_DRIVER_ROOT=$container_driver_root - echo "CONTAINER_DRIVER_ROOT=$CONTAINER_DRIVER_ROOT" + + set -o allexport + cat /run/nvidia/validations/driver-ready + . /run/nvidia/validations/driver-ready echo "Starting nvidia-device-plugin" exec nvidia-device-plugin diff --git a/assets/state-mig-manager/0420_configmap.yaml b/assets/state-mig-manager/0420_configmap.yaml index 5c9e9f1ab..7fbfc0d78 100644 --- a/assets/state-mig-manager/0420_configmap.yaml +++ b/assets/state-mig-manager/0420_configmap.yaml @@ -9,34 +9,19 @@ data: entrypoint.sh: |- #!/bin/bash - host_driver="" - driver_root="" - driver_root_ctr_path="" - while true; do - if [[ -f /run/nvidia/validations/host-driver-ready ]]; then - host_driver=true - driver_root="/" - driver_root_ctr_path="/host" - break - elif [[ -f /run/nvidia/validations/driver-ready ]]; then - host_driver=false - driver_root="/run/nvidia/driver" - driver_root_ctr_path="/run/nvidia/driver" - break - else - echo "waiting for the driver validations to be ready..." - sleep 5 - fi + until [[ -f /run/nvidia/validations/driver-ready ]] + do + echo "waiting for the driver validations to be ready..." + sleep 5 done - - export WITH_SHUTDOWN_HOST_GPU_CLIENTS=$host_driver + + set -o allexport + cat /run/nvidia/validations/driver-ready + . /run/nvidia/validations/driver-ready + + # manually export additional envs required by mig-manager + export WITH_SHUTDOWN_HOST_GPU_CLIENTS=$IS_HOST_DRIVER echo "WITH_SHUTDOWN_HOST_GPU_CLIENTS=$WITH_SHUTDOWN_HOST_GPU_CLIENTS" - export DRIVER_ROOT=$driver_root - echo "DRIVER_ROOT=$DRIVER_ROOT" - - export DRIVER_ROOT_CTR_PATH=$driver_root_ctr_path - echo "DRIVER_ROOT_CTR_PATH=$DRIVER_ROOT_CTR_PATH" - echo "Starting nvidia-mig-manager" exec nvidia-mig-manager diff --git a/validator/main.go b/validator/main.go index 057bba35d..4a3b99707 100644 --- a/validator/main.go +++ b/validator/main.go @@ -140,8 +140,6 @@ const ( driverContainerRoot = "/run/nvidia/driver" // driverStatusFile indicates status file for containerizeddriver readiness driverStatusFile = "driver-ready" - // hostDriverStatusFile indicates status file for host driver readiness - hostDriverStatusFile = "host-driver-ready" // nvidiaFsStatusFile indicates status file for nvidia-fs driver readiness nvidiaFsStatusFile = "nvidia-fs-ready" // toolkitStatusFile indicates status file for toolkit readiness @@ -641,12 +639,6 @@ func (d *Driver) validate() error { return err } - // delete host driver status file is already present - err = deleteStatusFile(outputDirFlag + "/" + hostDriverStatusFile) - if err != nil { - return err - } - driverRoot, isHostDriver, err := d.runValidation(false) if err != nil { log.Error("driver is not ready") @@ -675,17 +667,27 @@ func (d *Driver) validate() error { } } - statusFile := driverStatusFile + return d.createStatusFile(isHostDriver) +} + +func (d *Driver) createStatusFile(isHostDriver bool) error { + var nvidiaDriverRoot, driverRootCtrPath string if isHostDriver { - statusFile = hostDriverStatusFile + nvidiaDriverRoot = "/" + driverRootCtrPath = "/host" + } else { + nvidiaDriverRoot = "/run/nvidia/driver" + driverRootCtrPath = "/run/nvidia/driver" } + statusFileContent := strings.Join([]string{ + fmt.Sprintf("NVIDIA_DRIVER_ROOT=%s", nvidiaDriverRoot), + fmt.Sprintf("DRIVER_ROOT_CTR_PATH=%s", driverRootCtrPath), + fmt.Sprintf("IS_HOST_DRIVER=%v", isHostDriver), + }, "\n") + "\n" + // create driver status file - err = createStatusFile(outputDirFlag + "/" + statusFile) - if err != nil { - return err - } - return nil + return createStatusFileWithContent(outputDirFlag+"/"+driverStatusFile, statusFileContent) } // createDevCharSymlinks creates symlinks in /host-dev-char that point to all possible NVIDIA devices nodes. diff --git a/validator/metrics.go b/validator/metrics.go index d9ac75e5e..f500ff152 100644 --- a/validator/metrics.go +++ b/validator/metrics.go @@ -163,11 +163,6 @@ func (nm *NodeMetrics) watchStatusFile(statusFile *promcli.Gauge, statusFileFile for { _, err := os.Stat(outputDirFlag + "/" + statusFileFilename) ready = !os.IsNotExist(err) - if !ready && statusFileFilename == driverStatusFile { - // check if the driver status file for pre-installed driver exists - _, err = os.Stat(outputDirFlag + "/" + hostDriverStatusFile) - ready = !os.IsNotExist(err) - } if ready != prevReady { prevReady = ready From 8ebc3a6d7119ef5fc72b7e0830a874f2e2affce0 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Tue, 7 May 2024 16:51:15 -0700 Subject: [PATCH 2/6] Add logic to the driver validator to support driver containers not managed by GPU Operator The commit updates our driver validator to only check the presence of .driver-ctr.ready, a file created by our driver daemonset readiness probe, if the driver container is managed by GPU Operator. This allows us to support non-standard environments, like GKE, where a driver container is deployed but not managed by GPU Operator. Signed-off-by: Christopher Desiniotis --- api/nvidia/v1/clusterpolicy_types.go | 4 ++ assets/state-container-toolkit/0200_role.yaml | 6 ++ .../0500_daemonset.yaml | 4 ++ .../0500_daemonset.yaml | 4 ++ validator/main.go | 58 +++++++++++++++++-- 5 files changed, 71 insertions(+), 5 deletions(-) diff --git a/api/nvidia/v1/clusterpolicy_types.go b/api/nvidia/v1/clusterpolicy_types.go index 1cd15a185..6fffbca21 100644 --- a/api/nvidia/v1/clusterpolicy_types.go +++ b/api/nvidia/v1/clusterpolicy_types.go @@ -34,6 +34,10 @@ import ( // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. +const ( + ClusterPolicyCRDName = "ClusterPolicy" +) + // ClusterPolicySpec defines the desired state of ClusterPolicy type ClusterPolicySpec struct { // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster diff --git a/assets/state-container-toolkit/0200_role.yaml b/assets/state-container-toolkit/0200_role.yaml index ad8f93a7a..22b86bea9 100644 --- a/assets/state-container-toolkit/0200_role.yaml +++ b/assets/state-container-toolkit/0200_role.yaml @@ -12,3 +12,9 @@ rules: - use resourceNames: - privileged +- apiGroups: + - apps + resources: + - daemonsets + verbs: + - list diff --git a/assets/state-container-toolkit/0500_daemonset.yaml b/assets/state-container-toolkit/0500_daemonset.yaml index 85f68869d..493a48bd6 100644 --- a/assets/state-container-toolkit/0500_daemonset.yaml +++ b/assets/state-container-toolkit/0500_daemonset.yaml @@ -36,6 +36,10 @@ spec: value: "true" - name: COMPONENT value: driver + - name: OPERATOR_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace securityContext: privileged: true seLinuxOptions: diff --git a/assets/state-operator-validation/0500_daemonset.yaml b/assets/state-operator-validation/0500_daemonset.yaml index e25f060bf..d764326cb 100644 --- a/assets/state-operator-validation/0500_daemonset.yaml +++ b/assets/state-operator-validation/0500_daemonset.yaml @@ -35,6 +35,10 @@ spec: value: "true" - name: COMPONENT value: driver + - name: OPERATOR_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace securityContext: privileged: true seLinuxOptions: diff --git a/validator/main.go b/validator/main.go index 4a3b99707..b75718534 100644 --- a/validator/main.go +++ b/validator/main.go @@ -44,6 +44,8 @@ import ( "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + nvidiav1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1" + nvidiav1alpha1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1alpha1" "github.com/NVIDIA/gpu-operator/internal/info" ) @@ -55,7 +57,9 @@ type Component interface { } // Driver component -type Driver struct{} +type Driver struct { + ctx context.Context +} // NvidiaFs GDS Driver component type NvidiaFs struct{} @@ -205,6 +209,8 @@ const ( gpuWorkloadConfigVMVgpu = "vm-vgpu" // CCCapableLabelKey represents NFD label name to indicate if the node is capable to run CC workloads CCCapableLabelKey = "nvidia.com/cc.capable" + // appComponentLabelKey indicates the label key of the component + appComponentLabelKey = "app.kubernetes.io/component" ) func main() { @@ -465,7 +471,9 @@ func start(c *cli.Context) error { switch componentFlag { case "driver": - driver := &Driver{} + driver := &Driver{ + ctx: c.Context, + } err := driver.validate() if err != nil { return fmt.Errorf("error validating driver installation: %s", err) @@ -612,12 +620,52 @@ func assertDriverContainerReady(silent bool) error { return runCommand(command, args, silent) } +// isDriverManagedByOperator determines if the NVIDIA driver is managed by the GPU Operator. +// We check if at least one driver DaemonSet exists in the operator namespace that is +// owned by the ClusterPolicy or NVIDIADriver controllers. +func isDriverManagedByOperator(ctx context.Context) (bool, error) { + kubeConfig, err := rest.InClusterConfig() + if err != nil { + return false, fmt.Errorf("error getting cluster config: %w", err) + } + + kubeClient, err := kubernetes.NewForConfig(kubeConfig) + if err != nil { + return false, fmt.Errorf("error getting k8s client: %w", err) + } + + opts := meta_v1.ListOptions{LabelSelector: labels.Set{appComponentLabelKey: "nvidia-driver"}.AsSelector().String()} + dsList, err := kubeClient.AppsV1().DaemonSets(namespaceFlag).List(ctx, opts) + if err != nil { + return false, fmt.Errorf("error listing daemonsets: %w", err) + } + + for i := range dsList.Items { + ds := dsList.Items[i] + owner := meta_v1.GetControllerOf(&ds) + if owner == nil { + continue + } + if strings.HasPrefix(owner.APIVersion, "nvidia.com/") && (owner.Kind == nvidiav1.ClusterPolicyCRDName || owner.Kind == nvidiav1alpha1.NVIDIADriverCRDName) { + return true, nil + } + } + + return false, nil +} + func (d *Driver) runValidation(silent bool) (string, bool, error) { driverRoot, isHostDriver := getDriverRoot() - if !isHostDriver { - log.Infof("Driver is not pre-installed on the host. Checking driver container status.") + + driverManagedByOperator, err := isDriverManagedByOperator(d.ctx) + if err != nil { + return "", false, fmt.Errorf("error checking if driver is managed by GPU Operator: %w", err) + } + + if !isHostDriver && driverManagedByOperator { + log.Infof("Driver is not pre-installed on the host and is managed by GPU Operator. Checking driver container status.") if err := assertDriverContainerReady(silent); err != nil { - return "", false, fmt.Errorf("error checking driver container status: %v", err) + return "", false, fmt.Errorf("error checking driver container status: %w", err) } } From c46ade78beebc4ae3dd7de934974f5871baa0bd3 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Tue, 7 May 2024 23:25:18 -0700 Subject: [PATCH 3/6] Only chroot when validating a host installed driver This commit updates our driver validator code to only chroot when validating a host installed driver. When validating a driver container install, we discover the paths to libnvidia-ml.so.1 and nvidia-smi at the driver container root and then run 'LD_PRELOAD=/driverRoot/path/to/libnvidia-ml.so.1 nvidia-smi'. This sets the stage for validating driver container installs where driverRoot does not represent a full filesystem hiearchy that one can 'chroot' into. Signed-off-by: Christopher Desiniotis --- validator/find.go | 89 +++++++++++++++++++++++++++++++++++++++++++++ validator/main.go | 91 +++++++++++++++++++++++++++++++++++++---------- 2 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 validator/find.go diff --git a/validator/find.go b/validator/find.go new file mode 100644 index 000000000..d48140846 --- /dev/null +++ b/validator/find.go @@ -0,0 +1,89 @@ +/* +# Copyright 2024 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 main + +import ( + "fmt" + "path/filepath" +) + +type root string + +// getDriverLibraryPath returns path to `libnvidia-ml.so.1` in the driver root. +// The folder for this file is also expected to be the location of other driver files. +func (r root) getDriverLibraryPath() (string, error) { + librarySearchPaths := []string{ + "/usr/lib64", + "/usr/lib/x86_64-linux-gnu", + "/usr/lib/aarch64-linux-gnu", + "/lib64", + "/lib/x86_64-linux-gnu", + "/lib/aarch64-linux-gnu", + } + + libraryPath, err := r.findFile("libnvidia-ml.so.1", librarySearchPaths...) + if err != nil { + return "", err + } + + return libraryPath, nil +} + +// getNvidiaSMIPath returns path to the `nvidia-smi` executable in the driver root. +func (r root) getNvidiaSMIPath() (string, error) { + binarySearchPaths := []string{ + "/usr/bin", + "/usr/sbin", + "/bin", + "/sbin", + } + + binaryPath, err := r.findFile("nvidia-smi", binarySearchPaths...) + if err != nil { + return "", err + } + + return binaryPath, nil +} + +// findFile searches the root for a specified file. +// A number of folders can be specified to search in addition to the root itself. +// If the file represents a symlink, this is resolved and the final path is returned. +func (r root) findFile(name string, searchIn ...string) (string, error) { + + for _, d := range append([]string{"/"}, searchIn...) { + l := filepath.Join(string(r), d, name) + candidate, err := resolveLink(l) + if err != nil { + continue + } + return candidate, nil + } + + return "", fmt.Errorf("error locating %q", name) +} + +// resolveLink finds the target of a symlink or the file itself in the +// case of a regular file. +// This is equivalent to running `readlink -f ${l}`. +func resolveLink(l string) (string, error) { + resolved, err := filepath.EvalSymlinks(l) + if err != nil { + return "", fmt.Errorf("error resolving link '%s': %w", l, err) + } + return resolved, nil +} diff --git a/validator/main.go b/validator/main.go index b75718534..d90b4ff17 100644 --- a/validator/main.go +++ b/validator/main.go @@ -33,7 +33,6 @@ import ( devchar "github.com/NVIDIA/nvidia-container-toolkit/cmd/nvidia-ctk/system/create-dev-char-symlinks" log "github.com/sirupsen/logrus" cli "github.com/urfave/cli/v2" - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -597,14 +596,26 @@ func runCommandWithWait(command string, args []string, sleepSeconds int, silent } } -func getDriverRoot() (string, bool) { - // check if driver is pre-installed on the host and use host path for validation - if fileInfo, err := os.Lstat("/host/usr/bin/nvidia-smi"); err == nil && fileInfo.Size() != 0 { - log.Infof("Detected pre-installed driver on the host") - return "/host", true +// prependPathListEnvvar prepends a specified list of strings to a specified envvar and returns its value. +func prependPathListEnvvar(envvar string, prepend ...string) string { + if len(prepend) == 0 { + return os.Getenv(envvar) } + current := filepath.SplitList(os.Getenv(envvar)) + return strings.Join(append(prepend, current...), string(filepath.ListSeparator)) +} - return driverContainerRoot, false +// setEnvVar adds or updates an envar to the list of specified envvars and returns it. +func setEnvVar(envvars []string, key, value string) []string { + var updated []string + for _, envvar := range envvars { + pair := strings.SplitN(envvar, "=", 2) + if pair[0] == key { + continue + } + updated = append(updated, envvar) + } + return append(updated, fmt.Sprintf("%s=%s", key, value)) } // For driver container installs, check existence of .driver-ctr-ready to confirm running driver @@ -654,30 +665,72 @@ func isDriverManagedByOperator(ctx context.Context) (bool, error) { return false, nil } -func (d *Driver) runValidation(silent bool) (string, bool, error) { - driverRoot, isHostDriver := getDriverRoot() +func validateHostDriver(silent bool) error { + log.Info("Attempting to validate a pre-installed driver on the host") + command := "chroot" + args := []string{"/host", "nvidia-smi"} + + return runCommand(command, args, silent) +} - driverManagedByOperator, err := isDriverManagedByOperator(d.ctx) +func validateDriverContainer(silent bool, ctx context.Context) error { + driverManagedByOperator, err := isDriverManagedByOperator(ctx) if err != nil { - return "", false, fmt.Errorf("error checking if driver is managed by GPU Operator: %w", err) + return fmt.Errorf("error checking if driver is managed by GPU Operator: %w", err) } - if !isHostDriver && driverManagedByOperator { + if driverManagedByOperator { log.Infof("Driver is not pre-installed on the host and is managed by GPU Operator. Checking driver container status.") if err := assertDriverContainerReady(silent); err != nil { - return "", false, fmt.Errorf("error checking driver container status: %w", err) + return fmt.Errorf("error checking driver container status: %w", err) } } - // invoke validation command - command := "chroot" - args := []string{driverRoot, "nvidia-smi"} + driverRoot := root(driverContainerRoot) - if withWaitFlag { - return driverRoot, isHostDriver, runCommandWithWait(command, args, sleepIntervalSecondsFlag, silent) + validateDriver := func(silent bool) error { + driverLibraryPath, err := driverRoot.getDriverLibraryPath() + if err != nil { + return fmt.Errorf("failed to locate driver libraries: %w", err) + } + + nvidiaSMIPath, err := driverRoot.getNvidiaSMIPath() + if err != nil { + return fmt.Errorf("failed to locate nvidia-smi: %w", err) + } + cmd := exec.Command(nvidiaSMIPath) + // In order for nvidia-smi to run, we need to update LD_PRELOAD to include the path to libnvidia-ml.so.1. + cmd.Env = setEnvVar(os.Environ(), "LD_PRELOAD", prependPathListEnvvar("LD_PRELOAD", driverLibraryPath)) + if !silent { + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } + return cmd.Run() + } + + for { + log.Info("Attempting to validate a driver container installation") + err := validateDriver(silent) + if err != nil { + if !withWaitFlag { + return fmt.Errorf("error validating driver: %w", err) + } + log.Warningf("failed to validate the driver, retrying after %d seconds\n", sleepIntervalSecondsFlag) + time.Sleep(time.Duration(sleepIntervalSecondsFlag) * time.Second) + continue + } + return nil + } +} + +func (d *Driver) runValidation(silent bool) (string, bool, error) { + err := validateHostDriver(silent) + if err == nil { + log.Info("Detected a pre-installed driver on the host") + return "/host", true, nil } - return driverRoot, isHostDriver, runCommand(command, args, silent) + return driverContainerRoot, false, validateDriverContainer(silent, d.ctx) } func (d *Driver) validate() error { From 7685d9fb19dbd9b62f29dc665c2b26dd3b845728 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Wed, 8 May 2024 17:36:32 -0700 Subject: [PATCH 4/6] Add a 'hostPaths.rootFS' field to ClusterPolicy RootFS represents the path to the root filesystem of the host. This is used by components that need to interact with the host filesystem and as such this must be a chroot-able filesystem. Examples include the MIG Manager and Toolkit Container which may need to stop, start, or restart systemd services. Signed-off-by: Christopher Desiniotis Co-authored-by: Angelos Kolaitis --- api/nvidia/v1/clusterpolicy_types.go | 12 ++ api/nvidia/v1/zz_generated.deepcopy.go | 16 ++ .../manifests/nvidia.com_clusterpolicies.yaml | 13 ++ .../crd/bases/nvidia.com_clusterpolicies.yaml | 13 ++ controllers/object_controls.go | 47 ++++++ controllers/transforms_test.go | 141 ++++++++++++++++++ .../crds/nvidia.com_clusterpolicies_crd.yaml | 13 ++ .../gpu-operator/templates/clusterpolicy.yaml | 2 + deployments/gpu-operator/values.yaml | 8 + internal/state/driver.go | 30 ++-- internal/state/driver_test.go | 1 + manifests/state-driver/0500_daemonset.yaml | 2 +- validator/main.go | 10 +- 13 files changed, 293 insertions(+), 15 deletions(-) create mode 100644 controllers/transforms_test.go diff --git a/api/nvidia/v1/clusterpolicy_types.go b/api/nvidia/v1/clusterpolicy_types.go index 6fffbca21..38fe7a277 100644 --- a/api/nvidia/v1/clusterpolicy_types.go +++ b/api/nvidia/v1/clusterpolicy_types.go @@ -92,6 +92,8 @@ type ClusterPolicySpec struct { KataManager KataManagerSpec `json:"kataManager,omitempty"` // CCManager component spec CCManager CCManagerSpec `json:"ccManager,omitempty"` + // HostPaths defines various paths on the host needed by GPU Operator components + HostPaths HostPathsSpec `json:"hostPaths,omitempty"` } // Runtime defines container runtime type @@ -148,6 +150,16 @@ type OperatorSpec struct { UseOpenShiftDriverToolkit *bool `json:"use_ocp_driver_toolkit,omitempty"` } +// HostPathsSpec defines various paths on the host needed by GPU Operator components +type HostPathsSpec struct { + // RootFS represents the path to the root filesystem of the host. + // This is used by components that need to interact with the host filesystem + // and as such this must be a chroot-able filesystem. + // Examples include the MIG Manager and Toolkit Container which may need to + // stop, start, or restart systemd services. + RootFS string `json:"rootFS,omitempty"` +} + // EnvVar represents an environment variable present in a Container. type EnvVar struct { // Name of the environment variable. diff --git a/api/nvidia/v1/zz_generated.deepcopy.go b/api/nvidia/v1/zz_generated.deepcopy.go index 4f9ec6e68..6d876f675 100644 --- a/api/nvidia/v1/zz_generated.deepcopy.go +++ b/api/nvidia/v1/zz_generated.deepcopy.go @@ -208,6 +208,7 @@ func (in *ClusterPolicySpec) DeepCopyInto(out *ClusterPolicySpec) { in.CDI.DeepCopyInto(&out.CDI) in.KataManager.DeepCopyInto(&out.KataManager) in.CCManager.DeepCopyInto(&out.CCManager) + out.HostPaths = in.HostPaths } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterPolicySpec. @@ -862,6 +863,21 @@ func (in *GPUFeatureDiscoverySpec) DeepCopy() *GPUFeatureDiscoverySpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HostPathsSpec) DeepCopyInto(out *HostPathsSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostPathsSpec. +func (in *HostPathsSpec) DeepCopy() *HostPathsSpec { + if in == nil { + return nil + } + out := new(HostPathsSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InitContainerSpec) DeepCopyInto(out *InitContainerSpec) { *out = *in diff --git a/bundle/manifests/nvidia.com_clusterpolicies.yaml b/bundle/manifests/nvidia.com_clusterpolicies.yaml index 93543fa95..051211565 100644 --- a/bundle/manifests/nvidia.com_clusterpolicies.yaml +++ b/bundle/manifests/nvidia.com_clusterpolicies.yaml @@ -1175,6 +1175,19 @@ spec: description: GFD image tag type: string type: object + hostPaths: + description: HostPaths defines various paths on the host needed by + GPU Operator components + properties: + rootFS: + description: |- + RootFS represents the path to the root filesystem of the host. + This is used by components that need to interact with the host filesystem + and as such this must be a chroot-able filesystem. + Examples include the MIG Manager and Toolkit Container which may need to + stop, start, or restart systemd services. + type: string + type: object kataManager: description: KataManager component spec properties: diff --git a/config/crd/bases/nvidia.com_clusterpolicies.yaml b/config/crd/bases/nvidia.com_clusterpolicies.yaml index 93543fa95..051211565 100644 --- a/config/crd/bases/nvidia.com_clusterpolicies.yaml +++ b/config/crd/bases/nvidia.com_clusterpolicies.yaml @@ -1175,6 +1175,19 @@ spec: description: GFD image tag type: string type: object + hostPaths: + description: HostPaths defines various paths on the host needed by + GPU Operator components + properties: + rootFS: + description: |- + RootFS represents the path to the root filesystem of the host. + This is used by components that need to interact with the host filesystem + and as such this must be a chroot-able filesystem. + Examples include the MIG Manager and Toolkit Container which may need to + stop, start, or restart systemd services. + type: string + type: object kataManager: description: KataManager component spec properties: diff --git a/controllers/object_controls.go b/controllers/object_controls.go index 07114c39f..e0ed4c37f 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -168,6 +168,8 @@ const ( MPSRootEnvName = "MPS_ROOT" // DefaultMPSRoot is the default MPS root path on the host DefaultMPSRoot = "/run/nvidia/mps" + // HostRootEnvName is the name of the envvar representing the root path of the underlying host + HostRootEnvName = "HOST_ROOT" ) // ContainerProbe defines container probe types @@ -712,6 +714,9 @@ func preProcessDaemonSet(obj *appsv1.DaemonSet, n ClusterPolicyController) error return err } + // transform the host-root and host-dev-char volumes if a custom host root is configured with the operator + transformForHostRoot(obj, n.singleton.Spec.HostPaths.RootFS) + // apply per operand Daemonset config err = t(obj, &n.singleton.Spec, n) if err != nil { @@ -773,6 +778,48 @@ func applyCommonDaemonsetConfig(obj *appsv1.DaemonSet, config *gpuv1.ClusterPoli return nil } +// apply necessary transforms if a custom host root path is configured +func transformForHostRoot(obj *appsv1.DaemonSet, hostRoot string) { + if hostRoot == "" || hostRoot == "/" { + return + } + + transformHostRootVolume(obj, hostRoot) + transformHostDevCharVolume(obj, hostRoot) +} + +func transformHostRootVolume(obj *appsv1.DaemonSet, hostRoot string) { + containsHostRootVolume := false + for _, volume := range obj.Spec.Template.Spec.Volumes { + if volume.Name == "host-root" { + volume.HostPath.Path = hostRoot + containsHostRootVolume = true + break + } + } + + if !containsHostRootVolume { + return + } + + for index := range obj.Spec.Template.Spec.InitContainers { + setContainerEnv(&(obj.Spec.Template.Spec.InitContainers[index]), HostRootEnvName, hostRoot) + } + + for index := range obj.Spec.Template.Spec.Containers { + setContainerEnv(&(obj.Spec.Template.Spec.Containers[index]), HostRootEnvName, hostRoot) + } +} + +func transformHostDevCharVolume(obj *appsv1.DaemonSet, hostRoot string) { + for _, volume := range obj.Spec.Template.Spec.Volumes { + if volume.Name == "host-dev-char" { + volume.HostPath.Path = filepath.Join(hostRoot, "/dev/char") + break + } + } +} + // TransformGPUDiscoveryPlugin transforms GPU discovery daemonset with required config as per ClusterPolicy func TransformGPUDiscoveryPlugin(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, n ClusterPolicyController) error { // update validation container diff --git a/controllers/transforms_test.go b/controllers/transforms_test.go new file mode 100644 index 000000000..d7373f804 --- /dev/null +++ b/controllers/transforms_test.go @@ -0,0 +1,141 @@ +/* + * Copyright (c) 2024, NVIDIA CORPORATION. All rights reserved. + * + * 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 controllers + +import ( + "testing" + + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Daemonset is a DaemonSet wrapper used for testing +type Daemonset struct { + *appsv1.DaemonSet +} + +func NewDaemonset() Daemonset { + ds := &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ds", + Namespace: "test-ns", + }, + Spec: appsv1.DaemonSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + {Name: "foo", Image: "foo"}, + }, + Containers: []corev1.Container{ + {Name: "foo", Image: "foo"}, + }, + }, + }, + }, + } + return Daemonset{ds} +} + +func (d Daemonset) WithHostPathVolume(name string, path string) Daemonset { + volume := corev1.Volume{ + Name: name, + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: path, + }, + }, + } + d.Spec.Template.Spec.Volumes = append(d.Spec.Template.Spec.Volumes, volume) + return d +} + +func (d Daemonset) WithEnvVar(name string, value string) Daemonset { + for index := range d.Spec.Template.Spec.InitContainers { + ctr := &d.Spec.Template.Spec.InitContainers[index] + ctr.Env = append(ctr.Env, corev1.EnvVar{Name: name, Value: value}) + } + for index := range d.Spec.Template.Spec.Containers { + ctr := &d.Spec.Template.Spec.Containers[index] + ctr.Env = append(ctr.Env, corev1.EnvVar{Name: name, Value: value}) + } + return d +} + +func TestTransformForHostRoot(t *testing.T) { + hostRootVolumeName := "host-root" + hostDevCharVolumeName := "host-dev-char" + testCases := []struct { + description string + hostRoot string + input Daemonset + expectedOutput Daemonset + }{ + { + description: "no host root or host-dev-char volume in daemonset", + hostRoot: "/custom-root", + input: NewDaemonset(), + expectedOutput: NewDaemonset(), + }, + { + description: "empty host root is a no-op", + hostRoot: "", + input: NewDaemonset(). + WithHostPathVolume(hostRootVolumeName, "/"). + WithHostPathVolume(hostDevCharVolumeName, "/"), + expectedOutput: NewDaemonset(). + WithHostPathVolume(hostRootVolumeName, "/"). + WithHostPathVolume(hostDevCharVolumeName, "/"), + }, + { + description: "custom host root with host-root and host-dev-char volumes", + hostRoot: "/custom-root", + input: NewDaemonset(). + WithHostPathVolume(hostRootVolumeName, "/"). + WithHostPathVolume(hostDevCharVolumeName, "/"), + expectedOutput: NewDaemonset(). + WithHostPathVolume(hostRootVolumeName, "/custom-root"). + WithHostPathVolume(hostDevCharVolumeName, "/custom-root/dev/char"). + WithEnvVar(HostRootEnvName, "/custom-root"), + }, + { + description: "custom host root with host-root volume", + hostRoot: "/custom-root", + input: NewDaemonset(). + WithHostPathVolume(hostRootVolumeName, "/"), + expectedOutput: NewDaemonset(). + WithHostPathVolume(hostRootVolumeName, "/custom-root"). + WithEnvVar(HostRootEnvName, "/custom-root"), + }, + { + description: "custom host root with host-dev-char volume", + hostRoot: "/custom-root", + input: NewDaemonset(). + WithHostPathVolume(hostDevCharVolumeName, "/"), + expectedOutput: NewDaemonset(). + WithHostPathVolume(hostDevCharVolumeName, "/custom-root/dev/char"), + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + transformForHostRoot(tc.input.DaemonSet, tc.hostRoot) + require.EqualValues(t, tc.expectedOutput, tc.input) + }) + } +} diff --git a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml index 93543fa95..051211565 100644 --- a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml +++ b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml @@ -1175,6 +1175,19 @@ spec: description: GFD image tag type: string type: object + hostPaths: + description: HostPaths defines various paths on the host needed by + GPU Operator components + properties: + rootFS: + description: |- + RootFS represents the path to the root filesystem of the host. + This is used by components that need to interact with the host filesystem + and as such this must be a chroot-able filesystem. + Examples include the MIG Manager and Toolkit Container which may need to + stop, start, or restart systemd services. + type: string + type: object kataManager: description: KataManager component spec properties: diff --git a/deployments/gpu-operator/templates/clusterpolicy.yaml b/deployments/gpu-operator/templates/clusterpolicy.yaml index 12defaced..d0f294962 100644 --- a/deployments/gpu-operator/templates/clusterpolicy.yaml +++ b/deployments/gpu-operator/templates/clusterpolicy.yaml @@ -12,6 +12,8 @@ metadata: "helm.sh/resource-policy": keep {{- end }} spec: + hostPaths: + rootFS: {{ .Values.hostPaths.rootFS }} operator: {{- if .Values.operator.defaultRuntime }} defaultRuntime: {{ .Values.operator.defaultRuntime }} diff --git a/deployments/gpu-operator/values.yaml b/deployments/gpu-operator/values.yaml index 4d528d1ff..1bf6c95b9 100644 --- a/deployments/gpu-operator/values.yaml +++ b/deployments/gpu-operator/values.yaml @@ -20,6 +20,14 @@ sandboxWorkloads: enabled: false defaultWorkload: "container" +hostPaths: + # rootFS represents the path to the root filesystem of the host. + # This is used by components that need to interact with the host filesystem + # and as such this must be a chroot-able filesystem. + # Examples include the MIG Manager and Toolkit Container which may need to + # stop, start, or restart systemd services + rootFS: "/" + daemonsets: labels: {} annotations: {} diff --git a/internal/state/driver.go b/internal/state/driver.go index b7254bfc6..47de0c61d 100644 --- a/internal/state/driver.go +++ b/internal/state/driver.go @@ -37,6 +37,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/source" + gpuv1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1" nvidiav1alpha1 "github.com/NVIDIA/gpu-operator/api/nvidia/v1alpha1" "github.com/NVIDIA/gpu-operator/controllers/clusterinfo" "github.com/NVIDIA/gpu-operator/internal/consts" @@ -90,6 +91,7 @@ type driverRenderData struct { Openshift *openshiftSpec Precompiled *precompiledSpec AdditionalConfigs *additionalConfigs + HostRoot string } func NewStateDriver( @@ -121,23 +123,12 @@ func (s *stateDriver) Sync(ctx context.Context, customResource interface{}, info return SyncStateError, fmt.Errorf("NVIDIADriver CR not provided as input to Sync()") } - info := infoCatalog.Get(InfoTypeClusterPolicyCR) - if info == nil { - return SyncStateError, fmt.Errorf("failed to get ClusterPolicy CR from info catalog") - } - - info = infoCatalog.Get(InfoTypeClusterInfo) - if info == nil { - return SyncStateNotReady, fmt.Errorf("failed to get cluster info from info catalog") - } - clusterInfo := info.(clusterinfo.Interface) - err := s.cleanupStaleDriverDaemonsets(ctx, cr) if err != nil { return SyncStateNotReady, fmt.Errorf("failed to cleanup stale driver DaemonSets: %w", err) } - objs, err := s.getManifestObjects(ctx, cr, clusterInfo) + objs, err := s.getManifestObjects(ctx, cr, infoCatalog) if err != nil { return SyncStateNotReady, fmt.Errorf("failed to create k8s objects from manifests: %v", err) } @@ -200,9 +191,21 @@ func (s *stateDriver) cleanupStaleDriverDaemonsets(ctx context.Context, cr *nvid return nil } -func (s *stateDriver) getManifestObjects(ctx context.Context, cr *nvidiav1alpha1.NVIDIADriver, clusterInfo clusterinfo.Interface) ([]*unstructured.Unstructured, error) { +func (s *stateDriver) getManifestObjects(ctx context.Context, cr *nvidiav1alpha1.NVIDIADriver, infoCatalog InfoCatalog) ([]*unstructured.Unstructured, error) { logger := log.FromContext(ctx) + info := infoCatalog.Get(InfoTypeClusterPolicyCR) + if info == nil { + return nil, fmt.Errorf("failed to get ClusterPolicy CR from info catalog") + } + clusterPolicy := info.(gpuv1.ClusterPolicy) + + info = infoCatalog.Get(InfoTypeClusterInfo) + if info == nil { + return nil, fmt.Errorf("failed to get cluster info from info catalog") + } + clusterInfo := info.(clusterinfo.Interface) + runtimeSpec, err := getRuntimeSpec(ctx, s.client, clusterInfo, &cr.Spec) if err != nil { return nil, fmt.Errorf("failed to construct cluster runtime spec: %w", err) @@ -213,6 +216,7 @@ func (s *stateDriver) getManifestObjects(ctx context.Context, cr *nvidiav1alpha1 renderData := &driverRenderData{ GPUDirectRDMA: gpuDirectRDMASpec, Runtime: runtimeSpec, + HostRoot: clusterPolicy.Spec.HostPaths.RootFS, } if len(runtimeSpec.NodePools) == 0 { diff --git a/internal/state/driver_test.go b/internal/state/driver_test.go index 7de78621c..a2a352b8a 100644 --- a/internal/state/driver_test.go +++ b/internal/state/driver_test.go @@ -595,6 +595,7 @@ func getMinimalDriverRenderData() *driverRenderData { Namespace: "test-operator", KubernetesVersion: "1.28.0", }, + HostRoot: "", } } diff --git a/manifests/state-driver/0500_daemonset.yaml b/manifests/state-driver/0500_daemonset.yaml index a87ffaed9..8ceb7820c 100644 --- a/manifests/state-driver/0500_daemonset.yaml +++ b/manifests/state-driver/0500_daemonset.yaml @@ -617,7 +617,7 @@ spec: type: DirectoryOrCreate - name: host-root hostPath: - path: "/" + path: {{ .HostRoot | default "/" }} - name: host-sys hostPath: path: /sys diff --git a/validator/main.go b/validator/main.go index d90b4ff17..3b4349fa7 100644 --- a/validator/main.go +++ b/validator/main.go @@ -124,6 +124,7 @@ var ( metricsPort int defaultGPUWorkloadConfigFlag string disableDevCharSymlinkCreation bool + hostRootFlag string ) // defaultGPUWorkloadConfig is "vm-passthrough" unless @@ -321,6 +322,13 @@ func main() { Destination: &disableDevCharSymlinkCreation, EnvVars: []string{"DISABLE_DEV_CHAR_SYMLINK_CREATION"}, }, + &cli.StringFlag{ + Name: "host-root", + Value: "/", + Usage: "root path of the underlying host", + Destination: &hostRootFlag, + EnvVars: []string{"HOST_ROOT"}, + }, } // Log version info @@ -774,7 +782,7 @@ func (d *Driver) validate() error { func (d *Driver) createStatusFile(isHostDriver bool) error { var nvidiaDriverRoot, driverRootCtrPath string if isHostDriver { - nvidiaDriverRoot = "/" + nvidiaDriverRoot = hostRootFlag driverRootCtrPath = "/host" } else { nvidiaDriverRoot = "/run/nvidia/driver" From 8e4021c8bb9d1c81976fa1066632635bf90b13b5 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Thu, 9 May 2024 14:31:43 -0700 Subject: [PATCH 5/6] Always mount a driver container install to '/driver-root' in our operand containers except the driver-validator Having a static path inside our containers will make it easier when driverRoot is a configurable field. If driverRoot is set to a custom path, we can transform the host path for the volume while keeping the container path unchanged. The driver-validation initContainer is the exception to this rule. From the driver validation initContainer, the container path must match the host path otherwise the /dev/char symlinks will not resolve correctly on the host. The target of the symlinks must correspond to the path of the device nodes on the host. For example, when the NVIDIA device nodes are present under `/run/nvidia/driver/dev` on the host, running the following command from inside the container would create an invalid symlink: ln -s /driver-root/dev/nvidiactl /host-dev-char/195:255 while running the below command from inside the container would create a valid symlink: ln -s /run/nvidia/driver/dev/nvidiactl /host-dev-char/195:255 Signed-off-by: Christopher Desiniotis --- .../0500_daemonset.yaml | 21 ++++++++++++------- .../state-device-plugin/0500_daemonset.yaml | 20 +++++++++++------- assets/state-mig-manager/0600_daemonset.yaml | 20 +++++++++++------- .../0500_daemonset.yaml | 4 ++-- validator/main.go | 2 +- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/assets/state-container-toolkit/0500_daemonset.yaml b/assets/state-container-toolkit/0500_daemonset.yaml index 493a48bd6..b10949460 100644 --- a/assets/state-container-toolkit/0500_daemonset.yaml +++ b/assets/state-container-toolkit/0500_daemonset.yaml @@ -45,7 +45,7 @@ spec: seLinuxOptions: level: "s0" volumeMounts: - - name: driver-install-path + - name: driver-install-dir mountPath: /run/nvidia/driver mountPropagation: HostToContainer - name: run-nvidia-validations @@ -71,6 +71,8 @@ spec: value: "management.nvidia.com/gpu" - name: NVIDIA_VISIBLE_DEVICES value: "void" + - name: TOOLKIT_PID_FILE + value: "/run/nvidia/toolkit/toolkit.pid" imagePullPolicy: IfNotPresent name: nvidia-container-toolkit-ctr securityContext: @@ -82,13 +84,17 @@ spec: readOnly: true mountPath: /bin/entrypoint.sh subPath: entrypoint.sh - - name: nvidia-run-path - mountPath: /run/nvidia - mountPropagation: Bidirectional + - name: toolkit-root + mountPath: /run/nvidia/toolkit + - name: run-nvidia-validations + mountPath: /run/nvidia/validations - name: toolkit-install-dir mountPath: /usr/local/nvidia - name: crio-hooks mountPath: /usr/share/containers/oci/hooks.d + - name: driver-install-dir + mountPath: /driver-root + mountPropagation: HostToContainer - name: host-root mountPath: /host readOnly: true @@ -100,17 +106,18 @@ spec: configMap: name: nvidia-container-toolkit-entrypoint defaultMode: 448 - - name: nvidia-run-path + - name: toolkit-root hostPath: - path: /run/nvidia + path: /run/nvidia/toolkit type: DirectoryOrCreate - name: run-nvidia-validations hostPath: path: /run/nvidia/validations type: DirectoryOrCreate - - name: driver-install-path + - name: driver-install-dir hostPath: path: /run/nvidia/driver + type: DirectoryOrCreate - name: host-root hostPath: path: / diff --git a/assets/state-device-plugin/0500_daemonset.yaml b/assets/state-device-plugin/0500_daemonset.yaml index d82e590d2..76de43420 100644 --- a/assets/state-device-plugin/0500_daemonset.yaml +++ b/assets/state-device-plugin/0500_daemonset.yaml @@ -32,8 +32,8 @@ spec: securityContext: privileged: true volumeMounts: - - name: run-nvidia - mountPath: /run/nvidia + - name: run-nvidia-validations + mountPath: /run/nvidia/validations mountPropagation: HostToContainer - image: "FILLED BY THE OPERATOR" name: config-manager-init @@ -91,8 +91,10 @@ spec: subPath: entrypoint.sh - name: device-plugin mountPath: /var/lib/kubelet/device-plugins - - name: run-nvidia - mountPath: /run/nvidia + - name: run-nvidia-validations + mountPath: /run/nvidia/validations + - name: driver-install-dir + mountPath: /driver-root mountPropagation: HostToContainer - name: host-root mountPath: /host @@ -141,10 +143,14 @@ spec: - name: device-plugin hostPath: path: /var/lib/kubelet/device-plugins - - name: run-nvidia + - name: run-nvidia-validations hostPath: - path: "/run/nvidia" - type: Directory + path: "/run/nvidia/validations" + type: DirectoryOrCreate + - name: driver-install-dir + hostPath: + path: "/run/nvidia/driver" + type: DirectoryOrCreate - name: host-root hostPath: path: / diff --git a/assets/state-mig-manager/0600_daemonset.yaml b/assets/state-mig-manager/0600_daemonset.yaml index 2aadec4d5..e8676b27b 100644 --- a/assets/state-mig-manager/0600_daemonset.yaml +++ b/assets/state-mig-manager/0600_daemonset.yaml @@ -32,8 +32,8 @@ spec: securityContext: privileged: true volumeMounts: - - name: run-nvidia - mountPath: /run/nvidia + - name: run-nvidia-validations + mountPath: /run/nvidia/validations mountPropagation: HostToContainer containers: - name: nvidia-mig-manager @@ -62,6 +62,8 @@ spec: readOnly: true mountPath: /bin/entrypoint.sh subPath: entrypoint.sh + - name: run-nvidia-validations + mountPath: /run/nvidia/validations - mountPath: /sys name: host-sys - mountPath: /mig-parted-config @@ -71,8 +73,8 @@ spec: mountPropagation: HostToContainer - mountPath: /gpu-clients name: gpu-clients - - name: run-nvidia - mountPath: /run/nvidia + - name: driver-install-dir + mountPath: /driver-root mountPropagation: HostToContainer - name: cdi-root mountPath: /var/run/cdi @@ -88,10 +90,14 @@ spec: - name: mig-parted-config configMap: name: "FILLED_BY_OPERATOR" - - name: run-nvidia + - name: run-nvidia-validations hostPath: - path: "/run/nvidia" - type: Directory + path: "/run/nvidia/validations" + type: DirectoryOrCreate + - name: driver-install-dir + hostPath: + path: "/run/nvidia/driver" + type: DirectoryOrCreate - name: host-root hostPath: path: "/" diff --git a/assets/state-operator-validation/0500_daemonset.yaml b/assets/state-operator-validation/0500_daemonset.yaml index d764326cb..72a7d72a6 100644 --- a/assets/state-operator-validation/0500_daemonset.yaml +++ b/assets/state-operator-validation/0500_daemonset.yaml @@ -48,7 +48,7 @@ spec: mountPath: /host readOnly: true mountPropagation: HostToContainer - - name: driver-install-path + - name: driver-install-dir mountPath: /run/nvidia/driver mountPropagation: HostToContainer - name: run-nvidia-validations @@ -164,7 +164,7 @@ spec: hostPath: path: /run/nvidia/validations type: DirectoryOrCreate - - name: driver-install-path + - name: driver-install-dir hostPath: path: /run/nvidia/driver - name: host-root diff --git a/validator/main.go b/validator/main.go index 3b4349fa7..2d43d4c80 100644 --- a/validator/main.go +++ b/validator/main.go @@ -786,7 +786,7 @@ func (d *Driver) createStatusFile(isHostDriver bool) error { driverRootCtrPath = "/host" } else { nvidiaDriverRoot = "/run/nvidia/driver" - driverRootCtrPath = "/run/nvidia/driver" + driverRootCtrPath = "/driver-root" } statusFileContent := strings.Join([]string{ From d53e3e3cc9cf031f0512905da283a6e9a8ac8cc8 Mon Sep 17 00:00:00 2001 From: Christopher Desiniotis Date: Fri, 10 May 2024 11:51:17 -0700 Subject: [PATCH 6/6] Add 'hostPaths.driverInstallDir' field to ClusterPolicy This allows for non-standard driver container installations, where the driver installation path and device nodes are rooted at paths other than '/run/nvidia/driver'. Note, setting driverInstallDir to a custom value is currently only supported for driver container installations not managed by by GPU Operator. For example, in the GKE use case where a driver daemonset is deployed prior to installing GPU Operator and the GPU Operator managed driver is disabled. The GPU Operator's driver container daemonset still assumes that the full driver installation is made available at '/run/nvidia/driver' on the host, and consequently, we always mount '/run/nvidia/driver' into the GPU Operator managed daemonset. We may consider removing this assumption in the future and support driver container implementations which allow for a custom driverInstallDir to be specified. Signed-off-by: Christopher Desiniotis --- api/nvidia/v1/clusterpolicy_types.go | 4 + .../manifests/nvidia.com_clusterpolicies.yaml | 5 + .../crd/bases/nvidia.com_clusterpolicies.yaml | 5 + controllers/object_controls.go | 43 ++++++ controllers/transforms_test.go | 85 ++++++++++++ .../crds/nvidia.com_clusterpolicies_crd.yaml | 5 + .../gpu-operator/templates/clusterpolicy.yaml | 1 + deployments/gpu-operator/values.yaml | 4 + validator/driver.go | 73 ++++++++++ validator/find.go | 20 +++ validator/main.go | 129 +++++++++++------- validator/metrics.go | 2 +- 12 files changed, 324 insertions(+), 52 deletions(-) create mode 100644 validator/driver.go diff --git a/api/nvidia/v1/clusterpolicy_types.go b/api/nvidia/v1/clusterpolicy_types.go index 38fe7a277..07e424761 100644 --- a/api/nvidia/v1/clusterpolicy_types.go +++ b/api/nvidia/v1/clusterpolicy_types.go @@ -158,6 +158,10 @@ type HostPathsSpec struct { // Examples include the MIG Manager and Toolkit Container which may need to // stop, start, or restart systemd services. RootFS string `json:"rootFS,omitempty"` + + // DriverInstallDir represents the root at which driver files including libraries, + // config files, and executables can be found. + DriverInstallDir string `json:"driverInstallDir,omitempty"` } // EnvVar represents an environment variable present in a Container. diff --git a/bundle/manifests/nvidia.com_clusterpolicies.yaml b/bundle/manifests/nvidia.com_clusterpolicies.yaml index 051211565..6fc6f5435 100644 --- a/bundle/manifests/nvidia.com_clusterpolicies.yaml +++ b/bundle/manifests/nvidia.com_clusterpolicies.yaml @@ -1179,6 +1179,11 @@ spec: description: HostPaths defines various paths on the host needed by GPU Operator components properties: + driverInstallDir: + description: |- + DriverInstallDir represents the root at which driver files including libraries, + config files, and executables can be found. + type: string rootFS: description: |- RootFS represents the path to the root filesystem of the host. diff --git a/config/crd/bases/nvidia.com_clusterpolicies.yaml b/config/crd/bases/nvidia.com_clusterpolicies.yaml index 051211565..6fc6f5435 100644 --- a/config/crd/bases/nvidia.com_clusterpolicies.yaml +++ b/config/crd/bases/nvidia.com_clusterpolicies.yaml @@ -1179,6 +1179,11 @@ spec: description: HostPaths defines various paths on the host needed by GPU Operator components properties: + driverInstallDir: + description: |- + DriverInstallDir represents the root at which driver files including libraries, + config files, and executables can be found. + type: string rootFS: description: |- RootFS represents the path to the root filesystem of the host. diff --git a/controllers/object_controls.go b/controllers/object_controls.go index e0ed4c37f..2a0fc51d0 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -170,6 +170,13 @@ const ( DefaultMPSRoot = "/run/nvidia/mps" // HostRootEnvName is the name of the envvar representing the root path of the underlying host HostRootEnvName = "HOST_ROOT" + // DefaultDriverInstallDir represents the default path of a driver container installation + DefaultDriverInstallDir = "/run/nvidia/driver" + // DriverInstallDirEnvName is the name of the envvar used by the driver-validator to represent the driver install dir + DriverInstallDirEnvName = "DRIVER_INSTALL_DIR" + // DriverInstallDirCtrPathEnvName is the name of the envvar used by the driver-validator to represent the path + // of the driver install dir mounted in the container + DriverInstallDirCtrPathEnvName = "DRIVER_INSTALL_DIR_CTR_PATH" ) // ContainerProbe defines container probe types @@ -717,6 +724,9 @@ func preProcessDaemonSet(obj *appsv1.DaemonSet, n ClusterPolicyController) error // transform the host-root and host-dev-char volumes if a custom host root is configured with the operator transformForHostRoot(obj, n.singleton.Spec.HostPaths.RootFS) + // transform the driver-root volume if a custom driver install dir is configured with the operator + transformForDriverInstallDir(obj, n.singleton.Spec.HostPaths.DriverInstallDir) + // apply per operand Daemonset config err = t(obj, &n.singleton.Spec, n) if err != nil { @@ -820,6 +830,39 @@ func transformHostDevCharVolume(obj *appsv1.DaemonSet, hostRoot string) { } } +// apply necessary transforms if a custom driver install directory is configured +func transformForDriverInstallDir(obj *appsv1.DaemonSet, driverInstallDir string) { + if driverInstallDir == "" || driverInstallDir == DefaultDriverInstallDir { + return + } + + containsDriverInstallDirVolume := false + podSpec := obj.Spec.Template.Spec + for _, volume := range podSpec.Volumes { + if volume.Name == "driver-install-dir" { + volume.HostPath.Path = driverInstallDir + containsDriverInstallDirVolume = true + break + } + } + + if !containsDriverInstallDirVolume { + return + } + + for i, ctr := range podSpec.InitContainers { + if ctr.Name == "driver-validation" { + setContainerEnv(&(podSpec.InitContainers[i]), DriverInstallDirEnvName, driverInstallDir) + setContainerEnv(&(podSpec.InitContainers[i]), DriverInstallDirCtrPathEnvName, driverInstallDir) + for j, volumeMount := range ctr.VolumeMounts { + if volumeMount.Name == "driver-install-dir" { + podSpec.InitContainers[i].VolumeMounts[j].MountPath = driverInstallDir + } + } + } + } +} + // TransformGPUDiscoveryPlugin transforms GPU discovery daemonset with required config as per ClusterPolicy func TransformGPUDiscoveryPlugin(obj *appsv1.DaemonSet, config *gpuv1.ClusterPolicySpec, n ClusterPolicyController) error { // update validation container diff --git a/controllers/transforms_test.go b/controllers/transforms_test.go index d7373f804..cd8d397c1 100644 --- a/controllers/transforms_test.go +++ b/controllers/transforms_test.go @@ -77,6 +77,11 @@ func (d Daemonset) WithEnvVar(name string, value string) Daemonset { return d } +func (d Daemonset) WithInitContainer(container corev1.Container) Daemonset { + d.Spec.Template.Spec.InitContainers = append(d.Spec.Template.Spec.InitContainers, container) + return d +} + func TestTransformForHostRoot(t *testing.T) { hostRootVolumeName := "host-root" hostDevCharVolumeName := "host-dev-char" @@ -139,3 +144,83 @@ func TestTransformForHostRoot(t *testing.T) { }) } } + +func TestTransformForDriverInstallDir(t *testing.T) { + driverInstallDirVolumeName := "driver-install-dir" + testCases := []struct { + description string + driverInstallDir string + input Daemonset + expectedOutput Daemonset + }{ + { + description: "no driver-install-dir volume in daemonset", + driverInstallDir: "/custom-root", + input: NewDaemonset(), + expectedOutput: NewDaemonset(), + }, + { + description: "empty driverInstallDir is a no-op", + driverInstallDir: "", + input: NewDaemonset(). + WithHostPathVolume(driverInstallDirVolumeName, "/run/nvidia/driver"). + WithInitContainer( + corev1.Container{ + Name: "driver-validation", + VolumeMounts: []corev1.VolumeMount{ + {Name: driverInstallDirVolumeName, MountPath: "/run/nvidia/driver"}, + }, + }), + expectedOutput: NewDaemonset(). + WithHostPathVolume(driverInstallDirVolumeName, "/run/nvidia/driver"). + WithInitContainer( + corev1.Container{ + Name: "driver-validation", + VolumeMounts: []corev1.VolumeMount{ + {Name: driverInstallDirVolumeName, MountPath: "/run/nvidia/driver"}, + }, + }), + }, + { + description: "custom driverInstallDir with driver-install-dir volume", + driverInstallDir: "/custom-root", + input: NewDaemonset(). + WithHostPathVolume(driverInstallDirVolumeName, "/run/nvidia/driver"), + expectedOutput: NewDaemonset(). + WithHostPathVolume(driverInstallDirVolumeName, "/custom-root"), + }, + { + description: "custom driverInstallDir with driver-install-dir volume and driver-validation initContainer", + driverInstallDir: "/custom-root", + input: NewDaemonset(). + WithHostPathVolume(driverInstallDirVolumeName, "/run/nvidia/driver"). + WithInitContainer( + corev1.Container{ + Name: "driver-validation", + VolumeMounts: []corev1.VolumeMount{ + {Name: driverInstallDirVolumeName, MountPath: "/run/nvidia/driver"}, + }, + }), + expectedOutput: NewDaemonset(). + WithHostPathVolume(driverInstallDirVolumeName, "/custom-root"). + WithInitContainer( + corev1.Container{ + Name: "driver-validation", + VolumeMounts: []corev1.VolumeMount{ + {Name: driverInstallDirVolumeName, MountPath: "/custom-root"}, + }, + Env: []corev1.EnvVar{ + {Name: DriverInstallDirEnvName, Value: "/custom-root"}, + {Name: DriverInstallDirCtrPathEnvName, Value: "/custom-root"}, + }, + }), + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + transformForDriverInstallDir(tc.input.DaemonSet, tc.driverInstallDir) + require.EqualValues(t, tc.expectedOutput, tc.input) + }) + } +} diff --git a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml index 051211565..6fc6f5435 100644 --- a/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml +++ b/deployments/gpu-operator/crds/nvidia.com_clusterpolicies_crd.yaml @@ -1179,6 +1179,11 @@ spec: description: HostPaths defines various paths on the host needed by GPU Operator components properties: + driverInstallDir: + description: |- + DriverInstallDir represents the root at which driver files including libraries, + config files, and executables can be found. + type: string rootFS: description: |- RootFS represents the path to the root filesystem of the host. diff --git a/deployments/gpu-operator/templates/clusterpolicy.yaml b/deployments/gpu-operator/templates/clusterpolicy.yaml index d0f294962..835d5ab64 100644 --- a/deployments/gpu-operator/templates/clusterpolicy.yaml +++ b/deployments/gpu-operator/templates/clusterpolicy.yaml @@ -14,6 +14,7 @@ metadata: spec: hostPaths: rootFS: {{ .Values.hostPaths.rootFS }} + driverInstallDir: {{ .Values.hostPaths.driverInstallDir }} operator: {{- if .Values.operator.defaultRuntime }} defaultRuntime: {{ .Values.operator.defaultRuntime }} diff --git a/deployments/gpu-operator/values.yaml b/deployments/gpu-operator/values.yaml index 1bf6c95b9..3ad30403b 100644 --- a/deployments/gpu-operator/values.yaml +++ b/deployments/gpu-operator/values.yaml @@ -28,6 +28,10 @@ hostPaths: # stop, start, or restart systemd services rootFS: "/" + # driverInstallDir represents the root at which driver files including libraries, + # config files, and executables can be found. + driverInstallDir: "/run/nvidia/driver" + daemonsets: labels: {} annotations: {} diff --git a/validator/driver.go b/validator/driver.go new file mode 100644 index 000000000..6c25fada3 --- /dev/null +++ b/validator/driver.go @@ -0,0 +1,73 @@ +/* +# Copyright 2024 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 main + +// driverInfo contains information about an NVIDIA driver installation. +// +// isHostDriver indicates whether the driver is installed directly on +// the host at the host's root filesystem. +// +// hostRoot represents the host's root filesystem (typically '/'). +// +// driverRoot and devRoot represent the absolute paths of the driver install +// and NVIDIA device nodes on the host. +// +// driverRootCtrPath and devRootCtrPath represent the paths of the driver install +// and NVIDIA device nodes in the management containers that require them, like +// the Toolkit Container, the Device Plugin, and MIG Manager. +type driverInfo struct { + isHostDriver bool + hostRoot string + driverRoot string + driverRootCtrPath string + devRoot string + devRootCtrPath string +} + +func getDriverInfo(isHostDriver bool, hostRoot string, driverInstallDir string, driverInstallDirCtrPath string) driverInfo { + if isHostDriver { + return driverInfo{ + isHostDriver: true, + hostRoot: hostRoot, + driverRoot: hostRoot, + driverRootCtrPath: "/host", + devRoot: hostRoot, + devRootCtrPath: "/host", + } + } + + // For drivers not installed directly on the host, devRoot can either be + // hostRoot or driverInstallDir + var devRoot, devRootCtrPath string + devRoot = root(driverInstallDirCtrPath).getDevRoot() + if devRoot == "/" { + devRoot = hostRoot + devRootCtrPath = "/host" + } else { + devRoot = driverInstallDir + devRootCtrPath = "/driver-root" + } + + return driverInfo{ + isHostDriver: false, + hostRoot: hostRoot, + driverRoot: driverInstallDir, + driverRootCtrPath: "/driver-root", + devRoot: devRoot, + devRootCtrPath: devRootCtrPath, + } +} diff --git a/validator/find.go b/validator/find.go index d48140846..0d0d39697 100644 --- a/validator/find.go +++ b/validator/find.go @@ -18,6 +18,7 @@ package main import ( "fmt" + "os" "path/filepath" ) @@ -60,6 +61,25 @@ func (r root) getNvidiaSMIPath() (string, error) { return binaryPath, nil } +// isDevRoot checks whether the specified root is a dev root. +// A dev root is defined as a root containing a /dev folder. +func (r root) isDevRoot() bool { + stat, err := os.Stat(filepath.Join(string(r), "dev")) + if err != nil { + return false + } + return stat.IsDir() +} + +// getDevRoot returns the dev root associated with the root. +// If the root is not a dev root, this defaults to "/". +func (r root) getDevRoot() string { + if r.isDevRoot() { + return string(r) + } + return "/" +} + // findFile searches the root for a specified file. // A number of folders can be specified to search in addition to the root itself. // If the file represents a symlink, this is resolved and the final path is returned. diff --git a/validator/main.go b/validator/main.go index 2d43d4c80..f4f8e74fa 100644 --- a/validator/main.go +++ b/validator/main.go @@ -125,6 +125,8 @@ var ( defaultGPUWorkloadConfigFlag string disableDevCharSymlinkCreation bool hostRootFlag string + driverInstallDirFlag string + driverInstallDirCtrPathFlag string ) // defaultGPUWorkloadConfig is "vm-passthrough" unless @@ -140,8 +142,10 @@ const ( defaultMetricsPort = 0 // hostDevCharPath indicates the path in the container where the host '/dev/char' directory is mounted to hostDevCharPath = "/host-dev-char" - // driverContainerRoot indicates the path on the host where driver container mounts it's root filesystem - driverContainerRoot = "/run/nvidia/driver" + // defaultDriverInstallDir indicates the default path on the host where the driver container installation is made available + defaultDriverInstallDir = "/run/nvidia/driver" + // defaultDriverInstallDirCtrPath indicates the default path where the NVIDIA driver install dir is mounted in the container + defaultDriverInstallDirCtrPath = "/run/nvidia/driver" // driverStatusFile indicates status file for containerizeddriver readiness driverStatusFile = "driver-ready" // nvidiaFsStatusFile indicates status file for nvidia-fs driver readiness @@ -329,6 +333,20 @@ func main() { Destination: &hostRootFlag, EnvVars: []string{"HOST_ROOT"}, }, + &cli.StringFlag{ + Name: "driver-install-dir", + Value: defaultDriverInstallDir, + Usage: "the path on the host where a containerized NVIDIA driver installation is made available", + Destination: &driverInstallDirFlag, + EnvVars: []string{"DRIVER_INSTALL_DIR"}, + }, + &cli.StringFlag{ + Name: "driver-install-dir-ctr-path", + Value: defaultDriverInstallDirCtrPath, + Usage: "the path where the NVIDIA driver install dir is mounted in the container", + Destination: &driverInstallDirCtrPathFlag, + EnvVars: []string{"DRIVER_INSTALL_DIR_CTR_PATH"}, + }, } // Log version info @@ -694,7 +712,7 @@ func validateDriverContainer(silent bool, ctx context.Context) error { } } - driverRoot := root(driverContainerRoot) + driverRoot := root(driverInstallDirCtrPathFlag) validateDriver := func(silent bool) error { driverLibraryPath, err := driverRoot.getDriverLibraryPath() @@ -731,14 +749,18 @@ func validateDriverContainer(silent bool, ctx context.Context) error { } } -func (d *Driver) runValidation(silent bool) (string, bool, error) { +func (d *Driver) runValidation(silent bool) (driverInfo, error) { err := validateHostDriver(silent) if err == nil { log.Info("Detected a pre-installed driver on the host") - return "/host", true, nil + return getDriverInfo(true, hostRootFlag, hostRootFlag, "/host"), nil } - return driverContainerRoot, false, validateDriverContainer(silent, d.ctx) + err = validateDriverContainer(silent, d.ctx) + if err != nil { + return driverInfo{}, err + } + return getDriverInfo(false, hostRootFlag, driverInstallDirFlag, driverInstallDirCtrPathFlag), nil } func (d *Driver) validate() error { @@ -748,51 +770,41 @@ func (d *Driver) validate() error { return err } - driverRoot, isHostDriver, err := d.runValidation(false) + driverInfo, err := d.runValidation(false) if err != nil { - log.Error("driver is not ready") + log.Errorf("driver is not ready: %v", err) return err } - if !disableDevCharSymlinkCreation { - log.Info("creating symlinks under /dev/char that correspond to NVIDIA character devices") - err = createDevCharSymlinks(driverRoot, isHostDriver) - if err != nil { - msg := strings.Join([]string{ - "Failed to create symlinks under /dev/char that point to all possible NVIDIA character devices.", - "The existence of these symlinks is required to address the following bug:", - "", - " https://github.com/NVIDIA/gpu-operator/issues/430", - "", - "This bug impacts container runtimes configured with systemd cgroup management enabled.", - "To disable the symlink creation, set the following envvar in ClusterPolicy:", - "", - " validator:", - " driver:", - " env:", - " - name: DISABLE_DEV_CHAR_SYMLINK_CREATION", - " value: \"true\""}, "\n") - return fmt.Errorf("%v\n\n%s", err, msg) - } + err = createDevCharSymlinks(driverInfo, disableDevCharSymlinkCreation) + if err != nil { + msg := strings.Join([]string{ + "Failed to create symlinks under /dev/char that point to all possible NVIDIA character devices.", + "The existence of these symlinks is required to address the following bug:", + "", + " https://github.com/NVIDIA/gpu-operator/issues/430", + "", + "This bug impacts container runtimes configured with systemd cgroup management enabled.", + "To disable the symlink creation, set the following envvar in ClusterPolicy:", + "", + " validator:", + " driver:", + " env:", + " - name: DISABLE_DEV_CHAR_SYMLINK_CREATION", + " value: \"true\""}, "\n") + return fmt.Errorf("%w\n\n%s", err, msg) } - return d.createStatusFile(isHostDriver) + return d.createStatusFile(driverInfo) } -func (d *Driver) createStatusFile(isHostDriver bool) error { - var nvidiaDriverRoot, driverRootCtrPath string - if isHostDriver { - nvidiaDriverRoot = hostRootFlag - driverRootCtrPath = "/host" - } else { - nvidiaDriverRoot = "/run/nvidia/driver" - driverRootCtrPath = "/driver-root" - } - +func (d *Driver) createStatusFile(driverInfo driverInfo) error { statusFileContent := strings.Join([]string{ - fmt.Sprintf("NVIDIA_DRIVER_ROOT=%s", nvidiaDriverRoot), - fmt.Sprintf("DRIVER_ROOT_CTR_PATH=%s", driverRootCtrPath), - fmt.Sprintf("IS_HOST_DRIVER=%v", isHostDriver), + fmt.Sprintf("IS_HOST_DRIVER=%t", driverInfo.isHostDriver), + fmt.Sprintf("NVIDIA_DRIVER_ROOT=%s", driverInfo.driverRoot), + fmt.Sprintf("DRIVER_ROOT_CTR_PATH=%s", driverInfo.driverRootCtrPath), + fmt.Sprintf("NVIDIA_DEV_ROOT=%s", driverInfo.devRoot), + fmt.Sprintf("DEV_ROOT_CTR_PATH=%s", driverInfo.devRootCtrPath), }, "\n") + "\n" // create driver status file @@ -800,21 +812,36 @@ func (d *Driver) createStatusFile(isHostDriver bool) error { } // createDevCharSymlinks creates symlinks in /host-dev-char that point to all possible NVIDIA devices nodes. -func createDevCharSymlinks(driverRoot string, isHostDriver bool) error { - // If the host driver is being used, we rely on the fact that we are running a privileged container and as such - // have access to /dev - devRoot := driverRoot - if isHostDriver { - devRoot = "/" +func createDevCharSymlinks(driverInfo driverInfo, disableDevCharSymlinkCreation bool) error { + if disableDevCharSymlinkCreation { + log.WithField("disableDevCharSymlinkCreation", true). + Info("skipping the creation of symlinks under /dev/char that correspond to NVIDIA character devices") + return nil + } + + log.Info("creating symlinks under /dev/char that correspond to NVIDIA character devices") + + // Only attempt to load NVIDIA kernel modules when we can chroot into driverRoot + loadKernelModules := driverInfo.isHostDriver || (driverInfo.devRoot == driverInfo.driverRoot) + + // driverRootCtrPath is the path of the driver install dir in the container. This will either be + // driverInstallDirCtrPathFlag or '/host'. + // Note, if we always mounted the driver install dir to '/driver-root' in the validation container + // instead, then we could simplify to always use driverInfo.driverRootCtrPath -- which would be + // either '/host' or '/driver-root', both paths would exist in the validation container. + driverRootCtrPath := driverInstallDirCtrPathFlag + if driverInfo.isHostDriver { + driverRootCtrPath = "/host" } + // We now create the symlinks in /dev/char. creator, err := devchar.NewSymlinkCreator( - devchar.WithDriverRoot(driverRoot), - devchar.WithDevRoot(devRoot), + devchar.WithDriverRoot(driverRootCtrPath), + devchar.WithDevRoot(driverInfo.devRoot), devchar.WithDevCharPath(hostDevCharPath), devchar.WithCreateAll(true), devchar.WithCreateDeviceNodes(true), - devchar.WithLoadKernelModules(true), + devchar.WithLoadKernelModules(loadKernelModules), ) if err != nil { return fmt.Errorf("error creating symlink creator: %v", err) diff --git a/validator/metrics.go b/validator/metrics.go index f500ff152..c7062222d 100644 --- a/validator/metrics.go +++ b/validator/metrics.go @@ -231,7 +231,7 @@ func (nm *NodeMetrics) watchDriverValidation() { driver := &Driver{} for { - _, _, err := driver.runValidation(true) + _, err := driver.runValidation(true) if err == nil { nm.driverValidation.Set(1) nm.driverValidationLastSuccess.Set(float64(time.Now().Unix()))