diff --git a/docs/user/cluster-management/model-cache.md b/docs/user/cluster-management/model-cache.md index b08f335548..4fae32925c 100644 --- a/docs/user/cluster-management/model-cache.md +++ b/docs/user/cluster-management/model-cache.md @@ -64,19 +64,19 @@ 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. -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 provisioned through `nvcf-miniservice-sc`. +- 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 diff --git a/src/compute-plane-services/nvca/pkg/storage/cachebackend.go b/src/compute-plane-services/nvca/pkg/storage/cachebackend.go index 044b6c7c8d..cda6322549 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 02dfd990a0..bddc1c934f 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 8e2fb28e61..0cfddfd788 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,14 @@ 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, context.CancelFunc) { + t.Helper() ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -1311,14 +1318,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 +1333,23 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { mgr.GetCache().WaitForCacheSync(cctx) ccancel() - c := mgr.GetClient() + 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, *nvcav2beta1.ICMSRequest, *nvcav1new.StorageRequest) { + 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 +1367,46 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { Backend: string(HelmCacheBackendSharedFS), } require.NoError(t, c.Create(ctx, st)) + return workloadNS, sr, st +} + +// 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, cancel := 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, sr, st := 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{} @@ -1369,11 +1415,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 +1455,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,