Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

fix driver bug #1829

Merged
merged 1 commit into from Dec 6, 2018
Merged

fix driver bug #1829

merged 1 commit into from Dec 6, 2018

Conversation

xudifsd
Copy link
Member

@xudifsd xudifsd commented Dec 5, 2018

it seems rest-server will not pass environment variable into yarn-script. just hardcode in yarn-script since it is hardcoded anyway.

@xudifsd xudifsd mentioned this pull request Dec 5, 2018
Copy link
Member

@Gerhut Gerhut left a comment

Choose a reason for hiding this comment

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

@@ -52,8 +52,6 @@ spec:
value: {{ cluster_cfg['rest-server']['default-pai-admin-password'] }}
- name: ETCD_URI
value: {{ cluster_cfg['rest-server']['etcd-uris'] }}
- name: NV_DRIVER
value: /var/drivers/nvidia/current
Copy link
Contributor

Choose a reason for hiding this comment

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

for example if on aks env dir is diff with this, need tell user to change this config?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this can be hardcoded. My plan is all gpu-consumers(job-exporter, node-manager) use this symbolic link to find current active driver. And it's driver's responsibility to make this symbolic link pointing to the right place, whether it is pre-installed place or our installation place.

@xudifsd
Copy link
Member Author

xudifsd commented Dec 6, 2018

@Gerhut yes, I know that, but I didn't know when I submit previous PR, since it can be hardcoded, I'll just hardcoding in script instead of passing the env variable.

@xudifsd xudifsd merged commit af921fa into master Dec 6, 2018
@xudifsd xudifsd deleted the dixu/fix-driver-bug branch December 6, 2018 02:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants