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.

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

Changes required for WSL2 #1180

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 4 additions & 4 deletions assets/gpu-feature-discovery/0500_daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ spec:
volumeMounts:
- name: output-dir
mountPath: "/etc/kubernetes/node-feature-discovery/features.d"
- name: dmi-info-dir
mountPath: "/sys/class/dmi/id"
- name: host-sys
mountPath: /sys
readOnly: true
- name: config
mountPath: /config
Expand Down Expand Up @@ -169,9 +169,9 @@ spec:
- name: output-dir
hostPath:
path: "/etc/kubernetes/node-feature-discovery/features.d"
- name: dmi-info-dir
- name: host-sys
hostPath:
path: "/sys/class/dmi/id"
path: /sys
- name: run-nvidia-validations
hostPath:
path: "/run/nvidia/validations"
Expand Down
5 changes: 5 additions & 0 deletions validator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,11 @@ func isDriverManagedByOperator(ctx context.Context) (bool, error) {

func validateHostDriver(silent bool) error {
log.Info("Attempting to validate a pre-installed driver on the host")
if fileInfo, err := os.Lstat("/host/usr/lib/wsl/lib/nvidia-smi"); err == nil && fileInfo.Size() != 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense to use the info package from go-nvlib here instead (for example https://github.com/NVIDIA/k8s-device-plugin/blob/44b263c37e13370f857ef0691b8ab8ff02135c0d/internal/plugin/factory.go#L129).

This will allow for consistent detection across components.

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 agree with the overall notion of aligning the detection across components, so I have added a commit to use the info package as you have suggested.

However, the usage of the platform resolver in the driver-validator does not work as expected and returns unknown as the platform. This is because the libdxcore.so library is not in the ldcache, and likewise, neither is libnvidia-ml.so. The platform resolver works in the device-plugin because the NVIDIA Container Toolkit takes care of injecting the libdxcore.so library into the container and updating the ldcache.

Let me know if you have any suggestions to get the platform detection working in the driver-validator, otherwise I will revert the latest commit I added in favor of the original logic.

Copy link
Member

Choose a reason for hiding this comment

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

Looking at the implementiong in the info package, I don't think there's a way to get this working with the current implementation. I think for the time being, let's revert the proposed change and stick with the existing implementation.

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 have reverted the commit.

log.Infof("WSL2 system detected, assuming driver is pre-installed")
disableDevCharSymlinkCreation = true
return nil
}
fileInfo, err := os.Lstat("/host/usr/bin/nvidia-smi")
if err != nil {
return fmt.Errorf("no 'nvidia-smi' file present on the host: %w", err)
Expand Down
Loading