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

replace CONTAINER_DRIVER_ROOT to DRIVER_ROOT_CTR_PATH #211

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lengrongfu
Copy link

Fixes: #209

@klueska
Copy link
Collaborator

klueska commented Dec 11, 2024

I believe you will also need to change this in the helm chart

@cdesiniotis
Copy link

I believe you will also need to change this in the helm chart

This env is not currently being set in the helm chart.

env:
- name: MASK_NVIDIA_DRIVER_PARAMS
value: "{{ .Values.maskNvidiaDriverParams }}"
- name: NVIDIA_CTK_PATH
value: "{{ .Values.nvidiaCtkPath }}"
- name: NVIDIA_DRIVER_ROOT
value: "{{ .Values.nvidiaDriverRoot }}"
- name: NVIDIA_VISIBLE_DEVICES
value: void
- name: CDI_ROOT
value: /var/run/cdi
- name: NVIDIA_MIG_CONFIG_DEVICES
value: all
- name: DEVICE_CLASSES
value: {{ .Values.deviceClasses | join "," }}
- name: NODE_NAME
valueFrom:
fieldRef:
fieldPath: spec.nodeName
- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace

@lengrongfu
Copy link
Author

we can add this env to helm chart if need.

@lengrongfu lengrongfu force-pushed the feat/replace-CONTAINER_DRIVER_ROOT branch from 3ebd09f to d5d906a Compare December 17, 2024 09:01
@elezar
Copy link
Member

elezar commented Jan 7, 2025

I don't think that we should be setting this in the helm chart as part of this change. The /driver-root is an internal path and was not configurable before. If we decide that it should be user definable, let's do that as a follow-up.

Copy link
Member

@elezar elezar left a comment

Choose a reason for hiding this comment

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

I don't think that we should be setting this in the helm chart as part of this change. The /driver-root is an internal path and was not configurable before. If we decide that it should be user definable, let's do that as a follow-up.

Sorry for the back and forth on this @lengrongfu

@lengrongfu
Copy link
Author

It's all good, happy to help! please take a look. @elezar

@lengrongfu lengrongfu force-pushed the feat/replace-CONTAINER_DRIVER_ROOT branch from d5d906a to 178a715 Compare January 10, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace CONTAINER_DRIVER_ROOT with DRIVER_ROOT_CTR_PATH
4 participants