Skip to content

[Bugfix] to shared storage not working with dynamic PV provisioning#933

Merged
ruizhang0101 merged 5 commits into
vllm-project:mainfrom
NiccoloTosato:fix-sharedpvc-7a9ed88-1
May 7, 2026
Merged

[Bugfix] to shared storage not working with dynamic PV provisioning#933
ruizhang0101 merged 5 commits into
vllm-project:mainfrom
NiccoloTosato:fix-sharedpvc-7a9ed88-1

Conversation

@NiccoloTosato

@NiccoloTosato NiccoloTosato commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

This will fix #932

Summary

If shared storage is enabled, change HF_HOME from /data to /data/shared-pvc-storage


Checklist

  • Make sure the code changes pass the pre-commit checks.
  • Sign-off your commit by using -s when doing git commit
  • Try to classify PRs for easy understanding of the type of changes, such as [Bugfix], [Feat], and [CI].

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for shared PVC storage by updating the HF_HOME environment variable across vLLM deployments and Ray clusters. However, several critical issues were identified in the Helm templates. The logic in shared-storage.yaml incorrectly restricts PersistentVolume creation and storageClassName assignment when a storage class is defined, which breaks static provisioning. Additionally, while HF_HOME was updated in ray-cluster.yaml, the corresponding volume mounts for the shared storage are missing in both the head and worker groups, which will lead to runtime failures.

Comment thread helm/templates/shared-storage.yaml Outdated
Comment thread helm/templates/shared-storage.yaml
Comment thread helm/templates/shared-storage.yaml Outdated
Comment thread helm/templates/ray-cluster.yaml Outdated
Comment thread helm/templates/ray-cluster.yaml Outdated
@ruizhang0101

Copy link
Copy Markdown
Collaborator

@NiccoloTosato Hi, could you sign the commit?

@NiccoloTosato NiccoloTosato force-pushed the fix-sharedpvc-7a9ed88-1 branch from c03e535 to 2e8d772 Compare May 6, 2026 18:40
@NiccoloTosato

Copy link
Copy Markdown
Contributor Author

@NiccoloTosato Hi, could you sign the commit?

Hi, signature added, checklist updated.

@ruizhang0101

Copy link
Copy Markdown
Collaborator

Hi I think there are still some past commits haven't signed, they need to be all signed for the CI to pass. The CI task is called DCO

…o storageClass; set PVC.volumeName then

Signed-off-by: Niccolo Tosato <niccolo.tosato@yahoo.it>
Signed-off-by: Niccolo Tosato <niccolo.tosato@yahoo.it>
Signed-off-by: Niccolo Tosato <niccolo.tosato@yahoo.it>
Signed-off-by: Niccolo Tosato <niccolo.tosato@yahoo.it>
@NiccoloTosato NiccoloTosato force-pushed the fix-sharedpvc-7a9ed88-1 branch from 2e8d772 to 14cd90a Compare May 6, 2026 18:55

@ruizhang0101 ruizhang0101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@ruizhang0101 ruizhang0101 merged commit dfe0551 into vllm-project:main May 7, 2026
11 checks passed
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.

bug: sharedPvcStorage does not support dynamic CSI provisioning (requires hostPath)

2 participants