diff --git a/src/compute-plane-services/nvca/internal/miniservice/reconcile_storagerequests_test.go b/src/compute-plane-services/nvca/internal/miniservice/reconcile_storagerequests_test.go index 646b4063f..29de4835c 100644 --- a/src/compute-plane-services/nvca/internal/miniservice/reconcile_storagerequests_test.go +++ b/src/compute-plane-services/nvca/internal/miniservice/reconcile_storagerequests_test.go @@ -97,6 +97,24 @@ func TestMakeStorageRequests_BackendHandling(t *testing.T) { assert.Empty(t, sts, "cacheLaunchRequested gates durable backends like the ephemeral path") }) + t.Run("none backend emits no ModelCacheRequest", func(t *testing.T) { + // SelectHelmCacheBackend returns None when CachingSupport or the + // HelmModelCaching sub-gate is off; a valid cache spec must still + // produce no ModelCacheRequest. + icmsReq := &nvcav2beta1.ICMSRequest{} + icmsReq.Spec.Action = common.FunctionCreationAction + icmsReq.Spec.CreationMsgInfo.FunctionLaunchSpecification = &function.LaunchSpecification{ + CacheLaunchSpecification: &common.CacheLaunchSpecification{ + CacheArtifacts: true, + CacheHandle: "h1", + CacheSize: 262144000, + }, + } + sts, err := r.makeStorageRequests(icmsReq, nil, job, pvc, nvcastorage.HelmCacheBackendNone) + require.NoError(t, err) + assert.Empty(t, sts) + }) + t.Run("ephemeral backend emits no ModelCacheRequest", func(t *testing.T) { icmsReq := &nvcav2beta1.ICMSRequest{} icmsReq.Spec.Action = common.FunctionCreationAction diff --git a/src/compute-plane-services/nvca/pkg/featureflag/featureflag.go b/src/compute-plane-services/nvca/pkg/featureflag/featureflag.go index 9496ec555..7bac81f6c 100644 --- a/src/compute-plane-services/nvca/pkg/featureflag/featureflag.go +++ b/src/compute-plane-services/nvca/pkg/featureflag/featureflag.go @@ -34,8 +34,13 @@ var ( ) var ( - LogPosting = newFeatureFlag("LogPosting", newBool(false)) - CachingSupport = newFeatureFlag("CachingSupport", newBool(false)) + LogPosting = newFeatureFlag("LogPosting", newBool(false)) + CachingSupport = newFeatureFlag("CachingSupport", newBool(false)) + // HelmModelCaching gates model caching for Helm-based workloads. It is a + // sub-gate of CachingSupport: both must be on before a backend is selected + // in storage.SelectHelmCacheBackend. When off, no ModelCacheRequest is + // created and no ephemeral model-cache-init container is injected. + HelmModelCaching = newFeatureFlag("HelmModelCaching", newBool(false)) NVMeshEncryption = newFeatureFlag("NVMeshEncryption", newBool(false)) PeriodicInstanceStatusUpdate = newFeatureFlag("PeriodicInstanceStatusUpdate", newBool(true)) HelmRBACEnforcement = newFeatureFlag("HelmRBACEnforcement", newBool(true)) diff --git a/src/compute-plane-services/nvca/pkg/featureflag/featureflag_test.go b/src/compute-plane-services/nvca/pkg/featureflag/featureflag_test.go index e5a80d517..2d6f37225 100644 --- a/src/compute-plane-services/nvca/pkg/featureflag/featureflag_test.go +++ b/src/compute-plane-services/nvca/pkg/featureflag/featureflag_test.go @@ -103,6 +103,40 @@ func TestMaintenanceModeFeatureFlags(t *testing.T) { assert.Equal(t, CordonAndDrainMaintenance, flag) } +func TestHelmModelCachingFeatureFlag(t *testing.T) { + // parseFlags mutates package globals. Restore them via Cleanup so state + // cannot leak into other tests even if an assertion aborts this one. + origCaching, origHelm := CachingSupport.enabled, HelmModelCaching.enabled + t.Cleanup(func() { + CachingSupport.enabled, HelmModelCaching.enabled = origCaching, origHelm + }) + + // Helm model caching must be opted into. Enabled() prefers the override, so + // clear it to assert the declared default is what an unset flag reports. + HelmModelCaching.enabled = nil + require.NotNil(t, HelmModelCaching.defaultValue) + assert.False(t, *HelmModelCaching.defaultValue, "HelmModelCaching must default to off") + assert.False(t, HelmModelCaching.Enabled(), "the declared default must reach Enabled()") + + _ = parseFlags(fmt.Sprintf("+%s", HelmModelCaching.Key)) + assert.True(t, HelmModelCaching.Enabled()) + + _ = parseFlags(fmt.Sprintf("-%s", HelmModelCaching.Key)) + assert.False(t, HelmModelCaching.Enabled()) + + // CachingSupport is the parent gate, but it must not imply the Helm + // sub-gate. Clear the child override first, so this covers an unset child + // rather than one explicitly disabled just above. + HelmModelCaching.enabled = nil + _ = parseFlags(fmt.Sprintf("+%s", CachingSupport.Key)) + assert.True(t, CachingSupport.Enabled()) + assert.False(t, HelmModelCaching.Enabled()) + + flag, err := Get(HelmModelCaching.Key) + require.NoError(t, err) + assert.Equal(t, HelmModelCaching, flag) +} + func TestHelmAllowCPUNodesFeatureFlag(t *testing.T) { // Reset flags to defaults _ = parseFlags("-HelmAllowCPUNodes,-HelmResourceConstraints") diff --git a/src/compute-plane-services/nvca/pkg/storage/cachebackend.go b/src/compute-plane-services/nvca/pkg/storage/cachebackend.go index e4f6c4b9d..1dcdde29e 100644 --- a/src/compute-plane-services/nvca/pkg/storage/cachebackend.go +++ b/src/compute-plane-services/nvca/pkg/storage/cachebackend.go @@ -31,14 +31,15 @@ import ( ) // HelmCacheBackend identifies the storage backend selected for the Helm model -// cache. The backend is resolved from the CachingSupport gate plus the storage -// classes available in the cluster, replacing the old HelmCachingSupport flag. +// cache. The backend is resolved from the CachingSupport and HelmModelCaching +// gates plus the storage classes available in the cluster. // The type and values live in pkg/types so metrics can share them; they are // aliased here for the storage-facing API. type HelmCacheBackend = nvcatypes.HelmCacheBackend const ( - // HelmCacheBackendNone means caching is disabled (CachingSupport off). + // HelmCacheBackendNone means caching is disabled (CachingSupport or + // HelmModelCaching off). HelmCacheBackendNone = nvcatypes.HelmCacheBackendNone // HelmCacheBackendNVMesh uses NVMesh 3.x cross-namespace PV sharing // (the existing doModelCacheNVMesh path). @@ -79,10 +80,10 @@ func ModelCacheStorageClassName(override string) string { } // SelectHelmCacheBackend resolves the Helm model-cache storage backend. All -// caching is gated on CachingSupport; the mechanism is then chosen by which -// storage class the cluster provides, falling back to Samba (when -// HelmSharedStorage is enabled and the block class Samba needs exists) and -// finally to a per-pod ephemeral cache: +// caching is gated on CachingSupport plus the HelmModelCaching sub-gate; the +// mechanism is then chosen by which storage class the cluster provides, +// falling back to Samba (when HelmSharedStorage is enabled and the block class +// Samba needs exists) and finally to a per-pod ephemeral cache: // // 1. nvcf-sc-30 present -> NVMesh 3.x installed -> NVMesh // 2. nvcf-miniservice-sc present -> operator shared storage -> SharedFS @@ -98,7 +99,8 @@ func SelectHelmCacheBackend( ff featureflag.Fetcher, modelCacheStorageClass string, ) (HelmCacheBackend, error) { - if !ff.IsFeatureFlagEnabled(featureflag.CachingSupport) { + if !ff.IsFeatureFlagEnabled(featureflag.CachingSupport) || + !ff.IsFeatureFlagEnabled(featureflag.HelmModelCaching) { return HelmCacheBackendNone, nil } diff --git a/src/compute-plane-services/nvca/pkg/storage/cachebackend_test.go b/src/compute-plane-services/nvca/pkg/storage/cachebackend_test.go index f2ccff933..9c9dc0a1f 100644 --- a/src/compute-plane-services/nvca/pkg/storage/cachebackend_test.go +++ b/src/compute-plane-services/nvca/pkg/storage/cachebackend_test.go @@ -51,9 +51,13 @@ func cacheBackendClient(t *testing.T, scs ...*storagev1.StorageClass) *fake.Clie } func TestSelectHelmCacheBackend(t *testing.T) { - cachingOnly := []*featureflag.FeatureFlag{featureflag.CachingSupport} + cachingOnly := []*featureflag.FeatureFlag{ + featureflag.CachingSupport, + featureflag.HelmModelCaching, + } cachingAndSamba := []*featureflag.FeatureFlag{ featureflag.CachingSupport, + featureflag.HelmModelCaching, &featureflag.HelmSharedStorage.FeatureFlag, } @@ -72,6 +76,20 @@ func TestSelectHelmCacheBackend(t *testing.T) { storageClasses: []*storagev1.StorageClass{storageClass(NVMeshStorageClassName)}, want: HelmCacheBackendNone, }, + { + name: "HelmModelCaching off -> none", + flags: []*featureflag.FeatureFlag{featureflag.CachingSupport}, + // CachingSupport on and nvcf-sc-30 present, but the Helm sub-gate + // is off: no backend is selected. + storageClasses: []*storagev1.StorageClass{storageClass(NVMeshStorageClassName)}, + want: HelmCacheBackendNone, + }, + { + name: "CachingSupport off, HelmModelCaching on -> none", + flags: []*featureflag.FeatureFlag{featureflag.HelmModelCaching}, + storageClasses: []*storagev1.StorageClass{storageClass(NVMeshStorageClassName)}, + want: HelmCacheBackendNone, + }, { name: "nvcf-sc-30 present -> nvmesh", flags: cachingOnly, @@ -192,6 +210,7 @@ func TestSelectHelmCacheBackend_SambaClassLookupError(t *testing.T) { }).Build() ff := &featureflagmock.Fetcher{EnabledFFs: []*featureflag.FeatureFlag{ featureflag.CachingSupport, + featureflag.HelmModelCaching, &featureflag.HelmSharedStorage.FeatureFlag, }}