Make NVDRIVER configurable from env file by ArangoGutierrez · Pull Request #280 · NVIDIA/holodeck

ArangoGutierrez · 2025-02-11T10:24:34Z

No description provided.

elezar · 2025-02-11T10:27:47Z

pkg/provisioner/templates/nv-driver.go


 with_retry 3 10s sudo apt-get update
-install_packages_with_retry cuda-drivers
+install_packages_with_retry cuda-drivers{{if .Version}}={{.Version}}{{end}}


As a matter of interest, what are valid values for Version here then?

Looking at the package names these are the "branch numbers": 550, 560, 570, for example?

I am not checking for validity; if the user sets a wrong version, it will fail.
Valid versions are the ones provided by apt list -a cuda-drivers

Valid versions are the ones provided by apt list -a cuda-drivers

Which versions are those?

ubuntu@ip-10-0-0-196:~$ apt list -a cuda-drivers Listing... Done cuda-drivers/unknown 570.86.15-0ubuntu1 amd64 cuda-drivers/unknown 570.86.10-0ubuntu1 amd64 cuda-drivers/unknown 565.57.01-0ubuntu1 amd64 cuda-drivers/unknown 560.35.05-0ubuntu1 amd64 cuda-drivers/unknown 560.35.03-1 amd64 cuda-drivers/unknown 560.28.03-1 amd64 cuda-drivers/unknown 555.42.06-1 amd64 cuda-drivers/unknown 555.42.02-1 amd64 cuda-drivers/unknown 550.144.03-0ubuntu1 amd64 cuda-drivers/unknown 550.127.08-0ubuntu1 amd64 cuda-drivers/unknown 550.127.05-0ubuntu1 amd64 cuda-drivers/unknown 550.90.12-0ubuntu1 amd64 cuda-drivers/unknown 550.90.07-1 amd64 cuda-drivers/unknown 550.54.15-1 amd64 cuda-drivers/unknown 550.54.14-1 amd64 cuda-drivers/unknown 545.23.08-1 amd64 cuda-drivers/unknown 545.23.06-1 amd64 cuda-drivers/unknown 535.230.02-0ubuntu1 amd64 cuda-drivers/unknown 535.216.03-0ubuntu1 amd64 cuda-drivers/unknown 535.216.01-0ubuntu1 amd64 cuda-drivers/unknown 535.183.06-1 amd64 cuda-drivers/unknown 535.183.01-1 amd64 cuda-drivers/unknown 535.161.08-1 amd64 cuda-drivers/unknown 535.161.07-1 amd64 cuda-drivers/unknown 535.154.05-1 amd64 cuda-drivers/unknown 535.129.03-1 amd64 cuda-drivers/unknown 535.104.12-1 amd64 cuda-drivers/unknown 535.104.05-1 amd64 cuda-drivers/unknown 535.86.10-1 amd64 cuda-drivers/unknown 535.54.03-1 amd64 cuda-drivers/unknown 530.30.02-1 amd64 cuda-drivers/unknown 525.147.05-1 amd64 cuda-drivers/unknown 525.125.06-1 amd64 cuda-drivers/unknown 525.105.17-1 amd64 cuda-drivers/unknown 525.85.12-1 amd64 cuda-drivers/unknown 525.60.13-1 amd64 cuda-drivers/unknown 520.61.05-1 amd64 ...

OK. Let's at least add a string to the Version field that indicates that this is (currently) appended to the cuda-drivers package name and installed using apt?

elezar · 2025-02-12T13:45:17Z

pkg/provisioner/templates/nv-driver.go

 `

 type NvDriver struct {
+	Version string


Suggested change

Version string

// Version -- if specified -- indicates the version of the `cuda-drivers` package to install.

Version string

elezar · 2025-02-12T13:46:16Z

pkg/provisioner/templates/nv-driver.go

+	if env.Spec.NVIDIADriver.Version == "" {
+		return &NvDriver{}
+	}
+	return &NvDriver{
+		Version: env.Spec.NVIDIADriver.Version,
+	}


Suggested change

if env.Spec.NVIDIADriver.Version == "" {

return &NvDriver{}

}

return &NvDriver{

Version: env.Spec.NVIDIADriver.Version,

}

return &NvDriver{

Version: env.Spec.NVIDIADriver.Version,

}

The check for the empty string is redundant since golang uses "" as a zero value for strings anyway.

elezar · 2025-02-12T13:47:58Z

pkg/provisioner/templates/nv-driver.go

-install_packages_with_retry cuda-drivers
+install_packages_with_retry cuda-drivers{{if .Version}}={{.Version}}{{end}}

 nvidia-smi -L


A nit here, but we should probably drop the -L so that we can also see the driver version being applied.

elezar · 2025-02-12T14:09:05Z

README.md

      - 192.168.1.0/26
    image:
-      architecture: amd64
+      architecture:


Was this change required?

elezar · 2025-02-12T14:10:26Z

README.md

+
+```
+
+- 570.86.15-0ubuntu1


Suggested change

- 570.86.15-0ubuntu1

Where `<version>` can be a prefix of any package version. The following are example package versions:

- 570.86.15-0ubuntu1

elezar · 2025-02-12T14:10:38Z

README.md

+  nvidiaDriver:
+    install: true
+    version: <version>
+


Suggested change

nvidiaDriver:

install: true

version: <version>

nvidiaDriver:

install: true

version: <version>

elezar · 2025-02-12T14:11:14Z

pkg/provisioner/templates/test.bash

@@ -0,0 +1,207 @@
+#!/bin/bash


Should this file have been committed?

elezar · 2025-02-12T14:11:42Z

pkg/provisioner/templates/nv-driver.go


 with_retry 3 10s sudo apt-get update
-install_packages_with_retry cuda-drivers
+install_packages_with_retry cuda-drivers{{if .Version}}={{.Version}}{{end}}


Question: do we add --no-install-recommends here somewhere?

No, here's the install_packages_with_retry function

install_packages_with_retry() { local packages=("$@") local max_retries=5 local retry_delay=5 for ((i=1; i<=$max_retries; i++)); do echo "Attempt $i to install packages: ${packages[@]}" # Attempt to install packages sudo apt-get install -y "${packages[@]}" # Check if the last command failed and the error is related to unsigned repository if [ $? -ne 0 ] && grep -q 'NO_PUBKEY' <<< "$(tail -n 1 /var/lib/dpkg/status 2>/dev/null)"; then echo "Error: Unsigned repository. Retrying in $retry_delay seconds..." sleep $retry_delay else # Exit loop if installation is successful or the error is not related to unsigned repository break fi done }

added to the function

Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>

ArangoGutierrez self-assigned this Feb 11, 2025

ArangoGutierrez requested review from cdesiniotis, elezar and tariq1890 as code owners February 11, 2025 10:24

elezar reviewed Feb 11, 2025

View reviewed changes

elezar reviewed Feb 12, 2025

View reviewed changes

ArangoGutierrez requested a review from elezar February 12, 2025 14:03

ArangoGutierrez force-pushed the nv_driver_version branch from 2a7950a to 571b07a Compare February 12, 2025 14:03

elezar reviewed Feb 12, 2025

View reviewed changes

README.md Outdated

- 192.168.1.0/26

image:

architecture: amd64

architecture:

Copy link

Member

elezar Feb 12, 2025

Choose a reason for hiding this comment

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

Was this change required?

elezar reviewed Feb 12, 2025

View reviewed changes

ArangoGutierrez force-pushed the nv_driver_version branch from 571b07a to a4bc41d Compare February 12, 2025 14:14

Make NVDRIVER configurable from env file

8c347ce

Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>

ArangoGutierrez force-pushed the nv_driver_version branch from a4bc41d to 8c347ce Compare February 12, 2025 14:16

ArangoGutierrez requested a review from elezar February 12, 2025 14:16

elezar approved these changes Feb 12, 2025

View reviewed changes

ArangoGutierrez merged commit 9a28600 into NVIDIA:main Feb 12, 2025
5 checks passed

	Version string
	// Version -- if specified -- indicates the version of the `cuda-drivers` package to install.
	Version string

	- 570.86.15-0ubuntu1
	Where `<version>` can be a prefix of any package version. The following are example package versions:
	- 570.86.15-0ubuntu1


		```

		- 570.86.15-0ubuntu1

Conversation

ArangoGutierrez commented Feb 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArangoGutierrez Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ArangoGutierrez Feb 11, 2025 •

edited

Loading