From 371aa36521775d67e10cac9459cc0bd04b2a4ef8 Mon Sep 17 00:00:00 2001 From: Balaji Ganesan Date: Fri, 4 Sep 2026 08:36:19 -0700 Subject: [PATCH 1/4] fix(nvca): provision the shared filesystem cache writer on the model cache class The shared filesystem path created its writer claim on nvcf-miniservice-sc, the class whose presence selects that path. Every other backend provisions on the model cache class, nvcf-sc or the configured override, and the persisted selection that now drives Helm backend selection records that class as well. On a cluster that has a shared filesystem behind nvcf-sc but no nvcf-miniservice-sc, the selection picked this path and the writer claim then waited for a class that did not exist. The writer now takes the model cache class through the same helper the NVMesh and Samba paths use. nvcf-miniservice-sc keeps one job: its presence tells legacy selection that an operator provided a shared filesystem. Co-Authored-By: Balaji Ganesan --- docs/user/cluster-management/model-cache.md | 12 ++++++------ .../nvca/pkg/storage/cachebackend.go | 7 ++++--- .../nvca/pkg/storage/modelcache.go | 7 +++++-- .../nvca/pkg/storage/modelcache_test.go | 9 ++++++--- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/docs/user/cluster-management/model-cache.md b/docs/user/cluster-management/model-cache.md index b08f33554..ea48913e5 100644 --- a/docs/user/cluster-management/model-cache.md +++ b/docs/user/cluster-management/model-cache.md @@ -64,14 +64,14 @@ NVCA uses the first matching backend in this order: | Priority | Cluster condition | Backend | Reuse behavior | | --- | --- | --- | --- | | 1 | The model cache StorageClass (`nvcf-sc` unless overridden) is provisioned by `nvmesh-csi.excelero.com` | NVMesh | Durable reuse across namespaces | -| 2 | `nvcf-miniservice-sc` exists and supports `ReadOnlyMany` or `ReadWriteMany` | Operator-provided shared filesystem | Durable reuse across namespaces | +| 2 | `nvcf-miniservice-sc` exists | Shared filesystem on the model cache StorageClass | Durable reuse across namespaces | | 3 | `HelmSharedStorage` is enabled | NVCA-managed Samba | Durable reuse across namespaces | | 4 | No shared backend is available | `emptyDir` | Pod-local caching only | -NVCA does not create `nvcf-miniservice-sc`. If you provide this StorageClass, -it must support `ReadOnlyMany` or `ReadWriteMany`, and separate claims must -expose the same underlying cached data. NVCA prefers `ReadOnlyMany` for reader -claims and uses `ReadWriteMany` as a fallback. A provisioner that creates an +NVCA does not create `nvcf-miniservice-sc`. Its presence only tells NVCA that +the cluster has a shared filesystem; the cache itself is written to the model +cache StorageClass (`nvcf-sc` unless overridden), and readers are derived from +the writer's volume rather than provisioned separately. A provisioner that creates an isolated directory, access point, or subvolume for every claim does not provide cross-namespace reuse through this backend. @@ -194,7 +194,7 @@ The PV identifies the attachment type: primary volume. - A Samba reader uses the `smb.csi.k8s.io` driver and points to the cache handle's SMB share. -- A shared-filesystem reader is provisioned through `nvcf-miniservice-sc`. +- A shared-filesystem reader is a static PV on the writer's volume, on the model cache StorageClass. Repeat these checks for every workload namespace. Later workloads with the same cache handle should receive their own read-only attachment without creating diff --git a/src/compute-plane-services/nvca/pkg/storage/cachebackend.go b/src/compute-plane-services/nvca/pkg/storage/cachebackend.go index 044b6c7c8..cda632254 100644 --- a/src/compute-plane-services/nvca/pkg/storage/cachebackend.go +++ b/src/compute-plane-services/nvca/pkg/storage/cachebackend.go @@ -57,9 +57,10 @@ const ( ) const ( - // HelmCacheSharedStorageClassName is the shared storage class used for - // non-NVMesh cross-namespace model caching. It is either pre-provisioned - // by the operator or created by NVCA pointing at a Samba server. + // HelmCacheSharedStorageClassName is a legacy detection signal only: its + // presence tells SelectHelmCacheBackend that an operator provided a shared + // filesystem. Model cache volumes are provisioned on the model cache class, + // not on this one. HelmCacheSharedStorageClassName = "nvcf-miniservice-sc" ) diff --git a/src/compute-plane-services/nvca/pkg/storage/modelcache.go b/src/compute-plane-services/nvca/pkg/storage/modelcache.go index 02dfd990a..bddc1c934 100644 --- a/src/compute-plane-services/nvca/pkg/storage/modelcache.go +++ b/src/compute-plane-services/nvca/pkg/storage/modelcache.go @@ -260,8 +260,11 @@ func (r *Reconciler) doModelCacheSharedFS(ctx context.Context, if err != nil { return reconcile.Result{}, r.terminalErrorWithMetricErr(modelcachetypes.ReasonCacheSpecInvalid, fmt.Errorf("find and decode artifacts: %w", err)) } - sharedSC := HelmCacheSharedStorageClassName - rwPVC.Spec.StorageClassName = &sharedSC + // The writer lands on the model cache class like every other backend. The + // class that selected this path, nvcf-miniservice-sc, is only how legacy + // selection detects an operator-provided shared filesystem; provisioning on + // it would strand the cache when a cluster has only the model cache class. + r.applyModelCacheStorageClass(ctx, rwPVC) // Gate on the durable populated marker, mirroring the NVMesh/Samba // getPrimaryPV gate. Shared storage shares data across namespaces natively diff --git a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go index 8e2fb28e6..3a95c1a46 100644 --- a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go +++ b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go @@ -1369,11 +1369,14 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { assert.NoError(ct, err) }, 5*time.Second, 50*time.Millisecond) - // The writer RW PVC is on the shared class, not an NVMesh class. + // The writer RW PVC lands on the model cache class like every other + // backend. nvcf-miniservice-sc only selected the path; provisioning on it + // would strand the cache on a cluster that has only the model cache class. rwPVC := &corev1.PersistentVolumeClaim{} require.NoError(t, c.Get(ctx, client.ObjectKey{Name: "rw-pvc-" + cacheHandle, Namespace: ModelCacheInitNamespace}, rwPVC)) if assert.NotNil(t, rwPVC.Spec.StorageClassName) { - assert.Equal(t, HelmCacheSharedStorageClassName, *rwPVC.Spec.StorageClassName) + assert.Equal(t, DefaultModelCacheStorageClassName, *rwPVC.Spec.StorageClassName, + "the shared-FS writer must use the model cache class, not the detection class") } // Drive the writer job to "started" so the request moves to InitRunning. @@ -1406,7 +1409,7 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { Capacity: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse("1Gi")}, AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimRetain, - StorageClassName: HelmCacheSharedStorageClassName, + StorageClassName: DefaultModelCacheStorageClassName, PersistentVolumeSource: corev1.PersistentVolumeSource{ CSI: &corev1.CSIPersistentVolumeSource{ Driver: SMBCSIDriverName, From 8b0fb2f14ced74fbe126111c3c3b690e9e06c885 Mon Sep 17 00:00:00 2001 From: Balaji Ganesan Date: Fri, 4 Sep 2026 09:15:33 -0700 Subject: [PATCH 2/4] test(nvca): prove the shared filesystem writer honors the model cache class override The existing test only asserted the default class, which the old hard-coded value could never produce but which a default could satisfy by accident. A second envtest configures a class override and asserts the writer claim carries it; with the old code no configuration reaches the writer, so the assertion cannot pass. The manager bootstrap and request setup move into helpers shared by both tests. Co-Authored-By: Balaji Ganesan --- .../nvca/pkg/storage/modelcache_test.go | 70 +++++++++++++++---- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go index 3a95c1a46..106d1f238 100644 --- a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go +++ b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go @@ -1297,7 +1297,12 @@ func newModelCacheICMSSpec(cacheHandle string) nvcav2beta1.ICMSRequestSpec { // primary/secondary PV), then a per-namespace read-only PVC on the shared class // is created and the request becomes Ready. The CSI probe is pre-seeded as ROX // so the path does not attempt a live probe under envtest. -func TestReconcile_ModelCacheSharedFS(t *testing.T) { +// startModelCacheController boots envtest and the model cache controller with +// the given agent config, returning a context, the manager client, and the +// manager error channel. Several model-cache envtests run in one process, so +// controller name validation is skipped. +func startModelCacheController(t *testing.T, agentCfg nvcaconfig.Config) (context.Context, client.Client, <-chan error) { + t.Helper() ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -1311,14 +1316,12 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { BaseContext: func() context.Context { return ctx }, WebhookServer: nvcaenvtest.NewFakeWebhookServer(), Metrics: nvcaenvtest.NewFakeMetricsOptions(), - // Two model-cache envtests run in one process; the controller name - // "modelcache" is otherwise globally unique per controller-runtime. - Controller: ctrlconfig.Controller{SkipNameValidation: newBool(true)}, + Controller: ctrlconfig.Controller{SkipNameValidation: newBool(true)}, }) require.NoError(t, err) defaultTimeConfig := (&k8sutil.TimeConfig{}).Complete() - err = BuildController(nvcaconfig.Config{}, nvcav1new.ModelCacheRequest, mgr, "my-cluster", "us-west-1", defaultTimeConfig, ControllerOptions{}) + err = BuildController(agentCfg, nvcav1new.ModelCacheRequest, mgr, "my-cluster", "us-west-1", defaultTimeConfig, ControllerOptions{}) require.NoError(t, err) mgrErrCh, err := nvcaenvtest.StartManager(ctx, mgr) @@ -1328,22 +1331,21 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { mgr.GetCache().WaitForCacheSync(cctx) ccancel() - c := mgr.GetClient() + return ctx, mgr.GetClient(), mgrErrCh +} +// createSharedFSModelCacheRequest creates the namespaces, ICMSRequest, and a +// StorageRequest already routed to the shared filesystem backend for +// cacheHandle, and returns the workload namespace. +func createSharedFSModelCacheRequest(t *testing.T, ctx context.Context, c client.Client, cacheHandle, workloadNSName string) *corev1.Namespace { + t.Helper() srNamespace := &corev1.Namespace{} srNamespace.Name = types.DefaultICMSRequestNamespace require.NoError(t, c.Create(ctx, srNamespace)) require.NoError(t, c.Create(ctx, NewModelCacheInitNamespace())) - // The shared class exists (operator- or Samba-provided). - require.NoError(t, c.Create(ctx, &storagev1.StorageClass{ - ObjectMeta: metav1.ObjectMeta{Name: HelmCacheSharedStorageClassName}, - Provisioner: SMBCSIDriverName, - })) - - cacheHandle := "sharedfshandle" workloadNS := &corev1.Namespace{} - workloadNS.Name = "sr-sharedfs" + workloadNS.Name = workloadNSName require.NoError(t, c.Create(ctx, workloadNS)) sr := &nvcav2beta1.ICMSRequest{} @@ -1361,6 +1363,46 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { Backend: string(HelmCacheBackendSharedFS), } require.NoError(t, c.Create(ctx, st)) + return workloadNS +} + +// TestReconcile_ModelCacheSharedFSWriterHonorsClassOverride pins that the shared +// filesystem writer claim takes the configured model cache class. The old code +// hard-coded nvcf-miniservice-sc, so no configuration could reach the writer +// and this assertion cannot pass against it. +func TestReconcile_ModelCacheSharedFSWriterHonorsClassOverride(t *testing.T) { + const override = "custom-block-sc" + ctx, c, _ := startModelCacheController(t, nvcaconfig.Config{ + Agent: nvcaconfig.AgentConfig{ModelCache: nvcaconfig.ModelCacheConfig{StorageClassName: override}}, + }) + cacheHandle := "sharedfsoverride" + createSharedFSModelCacheRequest(t, ctx, c, cacheHandle, "sr-sharedfs-override") + + rwPVC := &corev1.PersistentVolumeClaim{} + assert.EventuallyWithT(t, func(ct *assert.CollectT) { + err := c.Get(ctx, client.ObjectKey{Name: "rw-pvc-" + cacheHandle, Namespace: ModelCacheInitNamespace}, rwPVC) + assert.NoError(ct, err) + }, 5*time.Second, 50*time.Millisecond) + require.NotNil(t, rwPVC.Spec.StorageClassName) + assert.Equal(t, override, *rwPVC.Spec.StorageClassName, + "the shared-FS writer must follow the configured model cache class") + assert.NotEqual(t, HelmCacheSharedStorageClassName, *rwPVC.Spec.StorageClassName, + "the detection class must never hold cache data") +} + +func TestReconcile_ModelCacheSharedFS(t *testing.T) { + ctx, c, mgrErrCh := startModelCacheController(t, nvcaconfig.Config{}) + + // The shared class exists (operator- or Samba-provided). + require.NoError(t, c.Create(ctx, &storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{Name: HelmCacheSharedStorageClassName}, + Provisioner: SMBCSIDriverName, + })) + + cacheHandle := "sharedfshandle" + workloadNS := createSharedFSModelCacheRequest(t, ctx, c, cacheHandle, "sr-sharedfs") + srNamespace := &corev1.Namespace{} + srNamespace.Name = types.DefaultICMSRequestNamespace // The writer job is created on the shared backend. initJob := &batchv1.Job{} From dd41637769efb9ec27432d4d636fe9786ebc6619 Mon Sep 17 00:00:00 2001 From: Balaji Ganesan Date: Fri, 4 Sep 2026 09:18:10 -0700 Subject: [PATCH 3/4] test(nvca): return the request objects and cancel func from the shared filesystem test helpers The previous commit moved setup into helpers but the existing test still used the ICMSRequest, StorageRequest, and cancel func they created, so the package did not compile. The helpers now return them. Co-Authored-By: Balaji Ganesan --- .../nvca/pkg/storage/modelcache_test.go | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go index 106d1f238..0cfddfd78 100644 --- a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go +++ b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go @@ -1301,7 +1301,9 @@ func newModelCacheICMSSpec(cacheHandle string) nvcav2beta1.ICMSRequestSpec { // the given agent config, returning a context, the manager client, and the // manager error channel. Several model-cache envtests run in one process, so // controller name validation is skipped. -func startModelCacheController(t *testing.T, agentCfg nvcaconfig.Config) (context.Context, client.Client, <-chan error) { +func startModelCacheController( + t *testing.T, agentCfg nvcaconfig.Config, +) (context.Context, client.Client, <-chan error, context.CancelFunc) { t.Helper() ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -1331,13 +1333,15 @@ func startModelCacheController(t *testing.T, agentCfg nvcaconfig.Config) (contex mgr.GetCache().WaitForCacheSync(cctx) ccancel() - return ctx, mgr.GetClient(), mgrErrCh + return ctx, mgr.GetClient(), mgrErrCh, cancel } // createSharedFSModelCacheRequest creates the namespaces, ICMSRequest, and a // StorageRequest already routed to the shared filesystem backend for // cacheHandle, and returns the workload namespace. -func createSharedFSModelCacheRequest(t *testing.T, ctx context.Context, c client.Client, cacheHandle, workloadNSName string) *corev1.Namespace { +func createSharedFSModelCacheRequest( + t *testing.T, ctx context.Context, c client.Client, cacheHandle, workloadNSName string, +) (*corev1.Namespace, *nvcav2beta1.ICMSRequest, *nvcav1new.StorageRequest) { t.Helper() srNamespace := &corev1.Namespace{} srNamespace.Name = types.DefaultICMSRequestNamespace @@ -1363,7 +1367,7 @@ func createSharedFSModelCacheRequest(t *testing.T, ctx context.Context, c client Backend: string(HelmCacheBackendSharedFS), } require.NoError(t, c.Create(ctx, st)) - return workloadNS + return workloadNS, sr, st } // TestReconcile_ModelCacheSharedFSWriterHonorsClassOverride pins that the shared @@ -1372,11 +1376,11 @@ func createSharedFSModelCacheRequest(t *testing.T, ctx context.Context, c client // and this assertion cannot pass against it. func TestReconcile_ModelCacheSharedFSWriterHonorsClassOverride(t *testing.T) { const override = "custom-block-sc" - ctx, c, _ := startModelCacheController(t, nvcaconfig.Config{ + ctx, c, _, _ := startModelCacheController(t, nvcaconfig.Config{ Agent: nvcaconfig.AgentConfig{ModelCache: nvcaconfig.ModelCacheConfig{StorageClassName: override}}, }) cacheHandle := "sharedfsoverride" - createSharedFSModelCacheRequest(t, ctx, c, cacheHandle, "sr-sharedfs-override") + _, _, _ = createSharedFSModelCacheRequest(t, ctx, c, cacheHandle, "sr-sharedfs-override") rwPVC := &corev1.PersistentVolumeClaim{} assert.EventuallyWithT(t, func(ct *assert.CollectT) { @@ -1391,7 +1395,7 @@ func TestReconcile_ModelCacheSharedFSWriterHonorsClassOverride(t *testing.T) { } func TestReconcile_ModelCacheSharedFS(t *testing.T) { - ctx, c, mgrErrCh := startModelCacheController(t, nvcaconfig.Config{}) + ctx, c, mgrErrCh, cancel := startModelCacheController(t, nvcaconfig.Config{}) // The shared class exists (operator- or Samba-provided). require.NoError(t, c.Create(ctx, &storagev1.StorageClass{ @@ -1400,7 +1404,7 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { })) cacheHandle := "sharedfshandle" - workloadNS := createSharedFSModelCacheRequest(t, ctx, c, cacheHandle, "sr-sharedfs") + workloadNS, sr, st := createSharedFSModelCacheRequest(t, ctx, c, cacheHandle, "sr-sharedfs") srNamespace := &corev1.Namespace{} srNamespace.Name = types.DefaultICMSRequestNamespace From e4d515fd61bcbad5184aead8efdfe7963d989b03 Mon Sep 17 00:00:00 2001 From: Balaji Ganesan Date: Fri, 4 Sep 2026 10:17:26 -0700 Subject: [PATCH 4/4] docs(nvca): describe the reader PV and Samba volume classes accurately The reader PV carries no StorageClass, so a pre-bound pair matches; only the writer claim uses the model cache class. The Samba backing volume uses the configured model cache class, not always nvcf-sc. Co-Authored-By: Balaji Ganesan --- docs/user/cluster-management/model-cache.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user/cluster-management/model-cache.md b/docs/user/cluster-management/model-cache.md index ea48913e5..4fae32925 100644 --- a/docs/user/cluster-management/model-cache.md +++ b/docs/user/cluster-management/model-cache.md @@ -75,8 +75,8 @@ the writer's volume rather than provisioned separately. A provisioner that creat isolated directory, access point, or subvolume for every claim does not provide cross-namespace reuse through this backend. -The Samba backend creates a separate Samba server and `nvcf-sc` backing volume -for each cache handle. Readers mount the same SMB share with read-only +The Samba backend creates a separate Samba server and a backing volume on the +model cache StorageClass (`nvcf-sc` unless overridden) for each cache handle. Readers mount the same SMB share with read-only credentials. ## Workload Mounts @@ -194,7 +194,7 @@ The PV identifies the attachment type: primary volume. - A Samba reader uses the `smb.csi.k8s.io` driver and points to the cache handle's SMB share. -- A shared-filesystem reader is a static PV on the writer's volume, on the model cache StorageClass. +- A shared-filesystem reader is a static PV bound to the writer's volume by name, with no StorageClass; only the writer claim uses the model cache StorageClass. Repeat these checks for every workload namespace. Later workloads with the same cache handle should receive their own read-only attachment without creating