feat(helm): support externally-managed PVC via pvcExistingClaimName (#938)

[Anai-Guo]

Summary

Fixes #938.

Adds an optional per-model pvcExistingClaimName to servingEngineSpec.modelSpec[], so operators can mount a PersistentVolumeClaim that is managed outside the chart instead of one the chart creates. This is useful for GitOps environments where PVC policy requires claims to be provisioned separately and brought in via existingClaim.

Behavior

• pvcExistingClaimName set → the chart skips rendering the model PVC (pvc.yaml) and the serving-engine deployment mounts the named claim directly.
• pvcExistingClaimName unset (default) → unchanged: the chart creates and mounts <release>-<model>-storage-claim.

Changes

• helm/templates/pvc.yaml — guard PVC creation with (empty $modelSpec.pvcExistingClaimName).
• helm/templates/deployment-vllm-multi.yaml — use pvcExistingClaimName for the volume claimName when set, falling back to the generated name.
• helm/values.yaml — document the option as a commented example. Kept as a plain comment (not a # -- helm-docs key) so the generated values.schema.json is unaffected; since modelSpec items do not set additionalProperties: false, the new key still validates.
• helm/tests/pvc_test.yaml — add a case asserting no PVC document is rendered when pvcExistingClaimName is set.

Scope is limited to the per-model storage claim referenced by the serving-engine deployment; the separate sharedPvcStorage path is unchanged.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces the pvcExistingClaimName parameter to allow using an externally-managed PersistentVolumeClaim instead of having the Helm chart create one. The feedback highlights a few issues with this implementation: first, skipping PVC rendering can lead to empty YAML documents due to the placement of the --- separator; second, the deployment template still implicitly requires pvcStorage to be set to a valid string to mount the volume, which should be documented; and third, the outer template guards in the deployment should be updated to check for pvcExistingClaimName directly to prevent silent failures if pvcStorage is omitted.

[gemini-code-assist]

Skipping PVC rendering when pvcExistingClaimName is set will result in empty YAML documents (consecutive --- separators) because the --- separator at line 39 is rendered for every model in the loop, regardless of whether the PVC itself is rendered.

To prevent generating empty documents or trailing/consecutive --- separators, the --- separator should be moved inside the if block (e.g., at the very beginning of the if block, before apiVersion: v1), and the hardcoded --- at line 39 should be removed.

{{- if and (hasKey $modelSpec "pvcStorage") (not (empty $modelSpec.pvcStorage)) (kindIs "string" $modelSpec.pvcStorage) (empty $modelSpec.pvcExistingClaimName) -}}
---

[gemini-code-assist]

When pvcExistingClaimName is used, the chart's deployment template still requires pvcStorage to be set to a valid string (e.g., "50Gi") to trigger the creation of the volume and volume mounts. If a user unsets pvcStorage or sets it to null (which is a common expectation when using an existing PVC), the volume will not be mounted at all.

Please document this requirement in the comments so users are aware they must keep pvcStorage defined.

    # Use an externally-managed PersistentVolumeClaim instead of one created by
    # this chart. When set, the chart skips rendering the PVC and mounts the
    # named claim directly (useful for GitOps setups where PVCs are managed
    # outside the chart). Leave unset to have the chart create the PVC.
    # Note: pvcStorage must still be set to a valid string (e.g., "50Gi") to trigger
    # the volume and volumeMounts creation in the deployment.
    # pvcExistingClaimName: "my-existing-claim"

[gemini-code-assist]

The persistentVolumeClaim block is nested inside {{- else if kindIs "string" $modelSpec.pvcStorage }} (line 472). This means that if a user sets pvcExistingClaimName but unsets pvcStorage or sets it to a non-string value, the persistentVolumeClaim volume will not be rendered at all.

To make this more robust and avoid silent failures when pvcStorage is omitted, consider updating the outer template guards (such as lines 418, 464, and 472) to also check for pvcExistingClaimName (e.g., or (hasKey $modelSpec "pvcStorage") (hasKey $modelSpec "pvcExistingClaimName")).