feat(nvca): add storage-aware model cache runtime - #1357
Open
balajinvda wants to merge 18 commits into
Open
Conversation
Relates to #1326 Signed-off-by: balaji <balaji7@gmail.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Contributor
Add the provider-neutral regular model-cache transition that populates one RWX claim and publishes that same claim to workloads with read-only mount intent. Persist and validate storage identity, fence publication with the exact completed writer job, and fail closed on ownership, lifecycle, race, or cleanup drift. Keep all external provider entries disabled pending binding-safe writer inputs and functional qualification.\n\nRefs NVCF-11476 Signed-off-by: balaji <balaji7@gmail.com>
balajinvda
changed the title
balajinvda
marked this pull request as draft
August 30, 2026 22:03
4 tasks
Describe the exact PVC, Job, and Pod-template metadata canonicalization performed by the RWX read-only path.\n\nRefs NVCF-11476 Signed-off-by: balaji <balaji7@gmail.com>
balajinvda
marked this pull request as ready for review
August 30, 2026 23:18
Signed-off-by: balaji <balaji7@gmail.com>
…orage-runtime-selection # Conflicts: # deploy/helm/nvca-operator/nvca-operator/files/nvcf-storage-capabilities-v1alpha1.schema.json # deploy/helm/nvca-operator/nvca-operator/files/nvcf-storage-capabilities-v1alpha1.yaml # docs/dev/sdd-storage-agnostic-cache-architecture.md # src/compute-plane-services/nvca/deployments/nvca-operator/files/nvcf-storage-capabilities-v1alpha1.schema.json # src/compute-plane-services/nvca/deployments/nvca-operator/files/nvcf-storage-capabilities-v1alpha1.yaml # src/compute-plane-services/nvca/pkg/storage/storage_capabilities.go # src/compute-plane-services/nvca/pkg/storage/storage_capabilities_test.go # src/compute-plane-services/nvca/scripts/lint_helm.sh
The source chart grants modelcachebindings and modelcachebindings/status in both the operator Role and the pre-delete cleanup RBAC, but the vendored release chart still granted only storagerequests. The operator builds the agent ClusterRole from these rules, so Kubernetes privilege-escalation prevention can reject the delegation when the operator does not hold the permission itself. Regenerated with make vendor-chart. The RBAC lines are the only drift, so the rest of the vendored chart was already in sync. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
The binding decision hashed the exact catalog ConfigMap payload, including comments, key ordering, and whitespace, and stored it in the immutable binding spec. ValidateModelCacheBindingIntent then compares the whole spec with DeepEqual, so any catalog edit invalidated every existing Active binding, not only requests that had not yet committed one. Editing the catalog is exactly how a new provider gets qualified, so the catalog could not be extended without breaking warm caches on every cluster. Replace Decision.catalogDigest with Decision.profileDigest, computed over a canonical form of only the driver entry and workflow a decision selected: provisioner, provider, workflow, transition, qualified access modes, and reader mount options. Access modes are sorted because they are a set; reader mount options keep their order because order is behavior. Record the payload hash as Decision.catalogRevision, audit metadata that equalBindingIntent excludes from comparison. The live drift check compares the profile instead of the payload, which also tightens it: it now catches a change to the selected driver's own access modes or reader mount options, which the previous provider and transition field checks missed. ModelCacheBinding is introduced in this same PR stack, so the CRD schema change needs no migration. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
roxReadOnly was pinned to the NVMesh provisioner and provider in both the catalog validator and the persisted-selection validator, and both also required the reader mount options ro, norecovery and nouuid. norecovery and nouuid are XFS flags NVMesh needs; requiring them of every driver meant a second qualified backend was rejected for not carrying another vendor's filesystem options. Enabling a provider therefore needed a code change in two files, which defeats the purpose of shipping a catalog. The execution path was already vendor-agnostic: it takes the CSI driver from Decision.Provisioner and the reader options from Decision.RequiredMountOptions. Only validation was holding the abstraction back. Gate roxReadOnly on what the transition mechanically needs instead: proven ReadWriteOnce and ReadOnlyMany access modes, and a "ro" reader mount, which is a property of a read-only reader on any backend. Vendor-specific options stay where they belong, as that driver's own catalog data. Adding a qualified backend is now a catalog edit. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
…ility Correcting the previous commit, which removed the NVMesh restriction from roxReadOnly for both workflows. For regular caching that was right: the reader stays in the request's namespace, so proven ReadWriteOnce and ReadOnlyMany access modes are the whole requirement and any qualified driver can run it. For Helm caching it was wrong. Helm caching is cross-namespace, and NVCA reaches the second namespace by deriving a reader PV from the writer's CSI volume handle, rewriting the segment after the last colon to the reader namespace (updateSecondaryPVVolumeHandle, modelcache.go). That only means "same volume, other namespace" on a driver whose handles encode the namespace that way. Access modes do not express it, so no combination of them can gate it. Add drivers[].capabilities.crossNamespaceVolumeSharing to the catalog and require it for roxReadOnly on the Helm workflow only. NVMesh declares it. The gate names the mechanism instead of the vendor, so the catalog stays the control surface: a driver that gains the capability is a catalog edit, and a driver without it is told in the error that Helm caching needs a ReadWriteMany claim instead. Note that the ReadWriteMany route for Helm caching is not implemented yet: rwxReadOnly remains regular-only in the schema enum and both validators. Until that lands, a non-NVMesh backend can cache for regular functions but must leave helmModelCache disabled. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
The catalog declared a transition per workflow per driver, on top of the access modes that already implied it, plus a capability flag and a strategy enum. That is three ways to say one thing, and it let the file disagree with itself. The catalog now records only what a qualification run establishes: the exact CSI provisioner, its provider id, the PVC access modes proven end to end in a cache workflow, and the reader mount options NVCA must apply. NVCA derives the rest: ReadWriteMany -> one shared claim, readers mount it read-only ReadWriteOnce+ReadOnlyMany -> writer takes the claim, readers get ROX on it Regular caching keeps its readers in the request namespace, so either shape serves it. Helm caching must reach other namespaces, which a ReadWriteMany claim does natively and the ROX shape does not, except on NVMesh, whose CSI volume handles encode the namespace. That exception is one clause in transitionForWorkflow rather than a schema concept. Enabling a backend is now an edit to its accessModes. An empty list means nothing is qualified yet and both workflows stay off, which is how Weka, FSS and Lustre are recorded until their qualification runs land: their previously listed modes came from claim-level tests, not from a cache workflow, so carrying them over would have silently enabled both workflows. Removes the transitions block, the transition strategy enums, the crossNamespaceVolumeSharing capability, and the validation that policed them: 637 lines deleted against 269 added. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
Documents what the storage capability catalog means, what a qualification run has to prove before a driver is enabled, and what has been measured. Weka is qualified on csi.weka.io: a static PV in a second namespace reusing the writer's volume handle serves the cache read-only with EROFS enforced, for both ReadWriteMany and ReadOnlyMany claims. Weka handles carry no namespace, so the handle is reused verbatim; NVMesh needs the same static PV plus a rewrite because its handles are namespace scoped. Also records the negative result that matters. A reader claim naming only a storage class gets a new empty volume, which is what doModelCacheSharedFS creates. That path assumes every dynamically provisioned claim on the class resolves to the same data, which is not what EFS, CephFS, Weka or FSS do. It holds only for a class pinned to one export with no per-volume subdirectory, as the NFS and SMB CSI drivers behave when subDir or source is fixed. It fails quietly: the claim binds, the pod starts, and the model is missing. FSS and Lustre remain unqualified. The jbt-ct4 cluster used for the August FSS performance work is gone, and the OCI dev clusters register the FSS driver but have no FSS storage class. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
The FSS target is nvcf-dgxc-k8s-oci-jbp-ct4 behind the production Teleport proxy, not a dev cluster. The previous text named the cluster incorrectly and suggested the OCI dev clusters as candidates. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
FSS qualifies for ReadWriteMany and ReadOnlyMany on nvcf-dgxc-k8s-oci-jbt-ct4, by the same mechanism as Weka: a static PV in the reader namespace reusing the writer's volume handle unchanged. Handles are <filesystem-ocid>:<mount-target-ip>:<export-path> and carry no namespace. Read-only is enforced with EROFS. A fresh dynamic claim on the same class gets a new empty export, which is the second measurement of the shared filesystem assumption failing. Also records three cluster facts that constrain a deployment there. The FSS CSI driver declares fsGroupPolicy ReadWriteOnceWithFSType, so Kubernetes does not chown a ReadWriteMany volume and the writer must run as root; Weka declares File and does not have this constraint. All three FSS classes use reclaim policy Delete while the model cache class must be Retain. CRI-O enforces short-name resolution, so image references must be fully qualified. Corrects the previous commit, which named the cluster jbp-ct4 and claimed it was unreachable. The name is jbt-ct4 and a working kubeconfig already existed. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
The FSS section said "FSS classes" where it meant the three FSS StorageClasses on the cluster, and named them only by reclaim policy. Name them, note they share one mount target and differ only in mount options, and use StorageClass consistently where the Kubernetes object is meant. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
… volume Enables Weka and OCI FSS for both cache workflows, and fixes the defect that would have made Helm caching silently useless on them. doModelCacheSharedFS created its reader as a claim naming only the shared StorageClass, trusting the class to make every claim resolve to the same data. A dynamic provisioner does not: it answers each claim with a new volume. The reader therefore mounted an empty directory while binding cleanly, so nothing alerted. Measured on Weka and on OCI FSS, where a fresh claim on the writer's own StorageClass came back empty and writable. The reader is now a PV derived from the volume the writer populated, claimed by name with an empty StorageClass so no provisioner is involved, ReadOnlyMany and Retain so one namespace's reader can never destroy a cache others are reading. That is the shape NVMesh and Samba already used. deriveReaderVolumeHandle now holds the only vendor specific step: NVMesh encodes the consuming namespace in its CSI volume handle and needs the reader namespace substituted in, while Weka and FSS address one volume by one handle and reuse the writer's unchanged. Both were measured. HelmCacheBackendFromSelection routes rwxReadOnly to that path, and the persisted selection no longer rejects rwxReadOnly for the Helm workflow, so a qualified shared claim backend reaches an executor instead of erroring. Removes pkg/storage/cacheprobe. It existed to discover at run time, by creating a PVC and a Pod, whether the shared class supported ROX or RWX. The catalog states that now, from a qualification run, which is better evidence than a probe with a TTL and a fallback state. Weka and FSS are set to ReadWriteMany and ReadOnlyMany, citing the runs in docs/dev/storage-provider-qualification.md. Lustre stays unqualified. Corrects a test fixture that gave an NVMesh PV the CSI driver "nvmesh"; the real name is nvmesh-csi.excelero.com, which is what the rest of the model cache code compares against the selection provisioner. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
Bindings were created Active with a protection finalizer, and releasing the last request only emptied the reference list. Nothing ever set the Retiring phase the API declares, and nothing ever removed the finalizer: the only finalizer removal in the tree is the operator's, for NVCFBackend, ConfigMap and ClusterRole. Binding-owned PVs are also outside the existing GC scope. Every binding and the resources its finalizer protects therefore accumulated for the life of the cluster, and could not be deleted without editing finalizers by hand. Add retireIdleModelCacheBindings to the idle model cache cleanup pass, where the Samba and shared filesystem reclaims already run, so a binding is released only after its backing store has been reclaimed. Retirement is two phased and idle gated so a warm cache survives a function scaling to zero: an unreferenced binding idle past ModelCacheIdlePeriod moves to Retiring, and a Retiring binding has its declared resources deleted before the finalizer is dropped. Release deletes exactly what the binding recorded in its resource intent. Nothing is inferred from labels or names, so a retirement cannot reach another cache's resources, and a binding that regains a reference before the second phase is left alone. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
The SDD flagged that a separately provisioned reader PVC is not guaranteed to resolve to the writer's data. That is now measured on Weka and OCI FSS, where it does not, and the code derives the reader PV from the writer volume instead. Describe what the three shared backends now do, which differs only in the volume handle. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
…che class The shared filesystem writer claim was pinned to nvcf-miniservice-sc, a second StorageClass separate from the one the catalog resolves the provisioner from. That predates the catalog: back then the class name was how a backend was chosen, so the cache had to live on the class that identified it. The writer now lands on the cluster's model cache class through applyModelCacheStorageClass, the same class every other model cache volume uses and the one nvcf-sc resolves to. nvcf-miniservice-sc keeps one job, as a detection signal for the legacy selector, and disappears with that selector once every cluster resolves through the catalog. This is safe for caches populated before the change. Readers are derived from whatever volume the writer claim is bound to, not from a class, so an existing claim on the older class keeps serving its readers. Relates to #1326 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
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
2 participants
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.
Customer Summary
Adds deterministic model-cache storage selection and read-only reader publication. Existing NVMesh behavior is named
roxReadOnly. A new regular-cacherwxReadOnlypath reuses one populated RWX claim without a clone or data copy. No external storage provider is enabled by this PR.TL;DR
nvcf-scprovisioner against the public catalog from feat(nvca): publish storage capability catalog #1334.ModelCacheBindingper cache identity and make retries use that binding.roxReadOnlyfor the existing NVMesh regular and Helm paths.rwxReadOnlyusing the populated RWX claim directly.disabled.Additional Details
This PR is stacked on #1334, which publishes the closed transition contract and storage capability catalog.
For a new model-cache request, NVCA evaluates the workflow gates, reads
StorageClass/nvcf-sc, requiresRetain, loads the exact provisioner entry, and persists the selection. It then creates or joins an immutable binding before it creates storage resources. Retries, restarts, catalog changes, and feature-gate changes continue from the recorded decision.Transition behavior
roxReadOnlyis restricted to the exact NVMesh provisioner and provider:The shipped required options are
ro,norecovery, andnouuid. They are persisted in the request selection andModelCacheBinding, copied defensively, repaired on retry, and validated before publication and cleanup. The legacynvca-cache-mount-optionsConfigMap is used only for annotation-free compatibility requests. The operatorcacheMountOptionssetting remains additive; any value that negates a provider requirement is ignored.rwxReadOnlyis restricted to regular model cache and requiresReadWriteMany:This path does not create a reader PVC, rewrite the PV, wait for detach, clone data, or make another copy. Its catalog entry must use an empty
readerMountOptionsarray. It does not claim backend-enforced write denial.Helm model cache supports only
roxReadOnly. Its writer and readers use different namespaces, sorwxReadOnlyis rejected by the schema, loader, persisted-selection validator, and runtime dispatcher.Enablement boundary
The shared-writer path accepts only credential-free Jobs. Current translated writer artifacts contain inputs that it rejects. Provider enablement still requires:
Container cache, CSI installation, performance qualification, and Helm support for non-NVMesh providers are outside this PR.
No third-party dependency changed.
NOTICEdoes not change.For the Reviewer
Please focus on:
rwxReadOnlysame-claim publication with no PV mutation or data copyrwxReadOnlyThe complete contract, limitations, rollout, and qualification plan are in
docs/dev/sdd-storage-agnostic-cache-architecture.md.For QA
Local validation completed:
pkg/storage,pkg/nvca,pkg/apis/nvca/v2beta1, andinternal/miniservicetests with Kubernetes 1.34.1 envtest assets: passgo vetfor those four packages: passgit diff --check, ASCII documentation style, and public-safety scans: passThe tests cover selection, persistence, generated Kubernetes objects, read-only fields, mount-option repair, identity preservation, retry, drift, ownership, cleanup, and RWX no-mutation behavior. They use fake or envtest Kubernetes APIs. They do not mount a real CSI volume or qualify Weka, OCI FSS, OCI Lustre, backend write denial, restart behavior on a live cluster, or performance. Those provider entries remain disabled.
Tickets
Relates to #1326
Checklist