Add GKE COS NVIDIA library mounts#2764
Conversation
This comment has been minimized.
This comment has been minimized.
- update readme for datadog
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45b1507acb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mountPath: /var/run/nvidia-container-devices/all | ||
| {{- if .Values.providers.gke.cos }} | ||
| - name: gke-nvidia-driver-lib64 | ||
| mountPath: /host/run/nvidia/driver/usr/lib/x86_64-linux-gnu |
There was a problem hiding this comment.
question: the container path /host/run/nvidia/driver/usr/lib/x86_64-linux-gnu has to match what the agent's NVML detection actually searches, and nothing in the chart wires the two together. Grepping the whole chart, this path appears only in these three template lines (here, _container-system-probe.yaml:153, and the hostPath source at _daemonset-volumes-linux.yaml:255); there's no LD_LIBRARY_PATH or path-pointing env var, so the agent binary must have this directory hardcoded in its search logic. The /host/run/nvidia/driver prefix mirrors the GPU-Operator driver-container root (/run/nvidia/driver) with the chart's /host host-mount convention, so it reads as deliberate rather than arbitrary.
Could you confirm against the agent's NVML library-search code that this exact directory is on the search path? The live validation shows the .so files are present and readable at the mount, but not that NVML loaded them. Worth confirming GPU metrics flow end-to-end, not just that ls finds the libraries.
There was a problem hiding this comment.
Yes, this is part of the default search paths of the agent. I deliberately chose this mount path as it's less likely to conflict with other libraries. Adding a comment mentioning it.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
mergequeue build completed successfully, but the github api returned an error while merging the pr.
DetailsError: PUT https://api.github.com/repos/DataDog/helm-charts/pulls/2764/merge: 405 Required status check "pr-validated" is expected. [] (Request ID: C2C4:37ADC:4AF0F6E:10ECCB5C:6A479587) FullStacktrace: |
…8.0 (#2787) The gpu_monitoring_cos.yaml baseline was left at operator 1.27.0 while the rest of the datadog chart moved to 1.28.0, so any PR touching charts/datadog or test/datadog fails the "Go Test Datadog" build job on Test_baseline_inputs/gpu_monitoring_cos.yaml. Root cause: #2764 added this baseline (operator 1.27.0) after #2771 had already branched, so when #2771 bumped operator.image.tag to 1.28.0 and ran `make update-test-baselines`, it regenerated gpu_monitoring.yaml but not the newly-added gpu_monitoring_cos.yaml. The two PRs merged cleanly and left a stale baseline on main. Regenerated via `make update-test-baselines-datadog-agent`; the diff is the operator appVersion/image bump to 1.28.0 plus the operator ClusterRole RBAC changes carried by the datadog-operator 2.23->2.24 subchart bump. Co-authored-by: Claude Opus 4.8 <[email protected]>
What this PR does / why we need it:
Mounts GKE COS NVIDIA driver libraries into the Agent and system-probe containers when GPU monitoring runs with
providers.gke.cos=true.This lets GPU monitoring find NVIDIA libraries on GKE COS nodes where they live under
/home/kubernetes/bin/nvidia/lib64.Which issue this PR fixes
Special notes for your reviewer:
Validated with
helm templateand a temporary deployment on a GKE COS GPU cluster. The Agent DaemonSet reached3/3 Running, and the Agent container could read NVIDIA libraries from the mounted path.The test cluster did not have a
nvidiaRuntimeClass, so deployment validation used--set-string datadog.gpuMonitoring.runtimeClassName=.Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]
<chartName>/minor-version,<chartName>/patch-version, or<chartName>/no-version-bump)datadogordatadog-operatorchart or value changes, update the test baselines (run:make update-test-baselines)datadogchart changes, received ✅ from a member of your teamGitHub CI takes care of the below, but are still required:
.github/helm-docs.sh)CHANGELOG.mdhas been updatedREADME.md