Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rootFS and driverInstallDir fields to ClusterPolicy #747

Merged
merged 6 commits into from
Jun 17, 2024
Merged

Add rootFS and driverInstallDir fields to ClusterPolicy #747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
185d521
Create driver-ready file with content and simplify operand startup sc…
cdesiniotis May 7, 2024
8ebc3a6
Add logic to the driver validator to support driver containers not ma…
cdesiniotis May 7, 2024
c46ade7
Only chroot when validating a host installed driver
cdesiniotis May 8, 2024
7685d9f
Add a 'hostPaths.rootFS' field to ClusterPolicy
cdesiniotis May 9, 2024
8e4021c
Always mount a driver container install to '/driver-root' in our oper…
cdesiniotis May 9, 2024
d53e3e3
Add 'hostPaths.driverInstallDir' field to ClusterPolicy
cdesiniotis May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions api/nvidia/v1/clusterpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -88,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
Expand Down Expand Up @@ -144,6 +150,20 @@ 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"`

// 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.
type EnvVar struct {
// Name of the environment variable.
Expand Down
16 changes: 16 additions & 0 deletions api/nvidia/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions assets/state-container-toolkit/0200_role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,9 @@ rules:
- use
resourceNames:
- privileged
- apiGroups:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware of the toolkit inspecting daemonsets? Was thsi change intentional?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because the validator runs here too? I thought we changed the logic to just look for the ready file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is because the validator runs here as an init container. We still need to run the driver validation here since the operator-validator pod runs with the nvidia runtime class, and thus, needs the toolkit pod to run first before it can run and start performing any validations.

- apps
resources:
- daemonsets
verbs:
- list
18 changes: 8 additions & 10 deletions assets/state-container-toolkit/0400_configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 18 additions & 7 deletions assets/state-container-toolkit/0500_daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,16 @@ spec:
value: "true"
- name: COMPONENT
value: driver
- name: OPERATOR_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
securityContext:
privileged: true
seLinuxOptions:
level: "s0"
volumeMounts:
- name: driver-install-path
- name: driver-install-dir
mountPath: /run/nvidia/driver
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question (and maybe out of scope for this PR): Does it make sense to set this to /driver-root in the container instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using the exact hostPath so that dev-char symlink creation works correctly. I want to make sure the target of the symlink resolves correctly on the host. So for example, we create a symlink /host-dev-char/195:0 --> /run/nvidia/driver/dev/nvidia0 in the driver validator container. I guess if we transformed the target path before creating the symlink, then we could always mount the driver install directory at /driver-root or /driver-install-dir. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we'd be able to create the symlink regardless of whether the target exists or not, so as lon as NVIDIA_DEV_ROOT (or NVIDIA_DEV_PATH) refers to the correct path on the host the symlinks should work as expected.

In the medium term we could consider moving this out of the toolkit container anyway ... since it really should be a postcondition of the driver container.

Let's leave it as is for now though.

mountPropagation: HostToContainer
- name: run-nvidia-validations
Expand All @@ -67,6 +71,8 @@ spec:
value: "management.nvidia.com/gpu"
- name: NVIDIA_VISIBLE_DEVICES
value: "void"
- name: TOOLKIT_PID_FILE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference. This requires NVIDIA/nvidia-container-toolkit#544

value: "/run/nvidia/toolkit/toolkit.pid"
imagePullPolicy: IfNotPresent
name: nvidia-container-toolkit-ctr
securityContext:
Expand All @@ -78,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
Expand All @@ -96,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: /
Expand Down
29 changes: 8 additions & 21 deletions assets/state-device-plugin/0400_configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]]
cdesiniotis marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it also export the custom toolkit installation path from values in toolkit to the device plugin for nvidia-ctk tool ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. But we set the NVIDIA_CTK_PATH variable here for the device-plugin in the clusterpolicy controller: https://github.com/NVIDIA/gpu-operator/blob/main/controllers/object_controls.go#L1284.

. /run/nvidia/validations/driver-ready

echo "Starting nvidia-device-plugin"
exec nvidia-device-plugin
20 changes: 13 additions & 7 deletions assets/state-device-plugin/0500_daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
cdesiniotis marked this conversation as resolved.
Show resolved Hide resolved
type: DirectoryOrCreate
- name: host-root
hostPath:
path: /
Expand Down
37 changes: 11 additions & 26 deletions assets/state-mig-manager/0420_configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 13 additions & 7 deletions assets/state-mig-manager/0600_daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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: "/"
Expand Down
8 changes: 6 additions & 2 deletions assets/state-operator-validation/0500_daemonset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ spec:
value: "true"
- name: COMPONENT
value: driver
- name: OPERATOR_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
securityContext:
privileged: true
seLinuxOptions:
Expand All @@ -44,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
Expand Down Expand Up @@ -160,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
Expand Down
Loading