fix(nvca): default NVMesh model cache volumes to nvcf-sc-30, not nvcf-sc - #1311
Draft
apartha-nv wants to merge 1 commit into
Draft
fix(nvca): default NVMesh model cache volumes to nvcf-sc-30, not nvcf-sc#1311apartha-nv wants to merge 1 commit into
apartha-nv wants to merge 1 commit into
Conversation
doModelCacheNVMesh only runs once SelectHelmCacheBackend has confirmed
NVMeshStorageClassName ("nvcf-sc-30") exists in the cluster, but the
unconfigured storage class default it applied to the PVC was the
Samba-oriented DefaultModelCacheStorageClassName ("nvcf-sc"). Both
classes share the same NVMesh CSI provisioner, so the volume silently
landed on the wrong tier instead of failing loudly.
Add nvmeshModelCacheStorageClassName, used only by NewReconciler's
one-time resolution of modelCacheStorageClass (consumed exclusively by
doModelCacheNVMesh), defaulting to NVMeshStorageClassName. Split off a
new sambaModelCacheStorageClass field, resolved from the same override
via the original ModelCacheStorageClassName default, so
doModelCacheSamba's backing PVC keeps landing on nvcf-sc regardless of
the NVMesh-side default change.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
doModelCacheNVMeshonly runs onceSelectHelmCacheBackendhas confirmednvcf-sc-30exists in the cluster, but the unconfigured storage-class default it applied to the PVC wasDefaultModelCacheStorageClassName("nvcf-sc") — the Samba-oriented default. Since both classes share the same NVMesh CSI provisioner (nvmesh-csi.excelero.com), volumes silently landed on the wrong tier instead of failing loudly.nvmeshModelCacheStorageClassName, used only forNewReconciler's one-time resolution ofmodelCacheStorageClass(consumed exclusively bydoModelCacheNVMesh), defaulting toNVMeshStorageClassName.sambaModelCacheStorageClassfield, resolved from the same override via the originalModelCacheStorageClassNamedefault, sodoModelCacheSamba's backing PVC keeps landing onnvcf-scregardless of the NVMesh-side default change.Test plan
TestNvmeshModelCacheStorageClassNameResolution— new unit test pinning the NVMesh defaultTestModelCacheStorageClassResolvedOnce— updated to expectnvcf-sc-30as the NVMesh defaultTestSambaModelCacheStorageClassResolvedOnce— new unit test confirming Samba's field independently still defaults tonvcf-scTestApplyModelCacheStorageClass— updated expectationTestReconcile_ModelCacheSamba(envtest) — extended to assert the Samba backing PVC's storage class end-to-endgo test ./pkg/storage/...passes in full (envtest viaKUBEBUILDER_ASSETS)go vet ./pkg/storage/...cleanMarked as draft — opening for early feedback before final review.