diff --git a/api/worker/v1alpha1/generated.pb.go b/api/worker/v1alpha1/generated.pb.go index 44fde8ba9..ffd0be073 100644 --- a/api/worker/v1alpha1/generated.pb.go +++ b/api/worker/v1alpha1/generated.pb.go @@ -3456,14 +3456,16 @@ func (m *KVCacheBackendLeader) MarshalToSizedBuffer(dAtA []byte) (int, error) { dAtA[i] = 0x2a } } - i-- - if m.MultiTenancy { - dAtA[i] = 1 - } else { - dAtA[i] = 0 + if m.MultiTenancy != nil { + i-- + if *m.MultiTenancy { + dAtA[i] = 1 + } else { + dAtA[i] = 0 + } + i-- + dAtA[i] = 0x20 } - i-- - dAtA[i] = 0x20 i -= len(m.AllocationStrategy) copy(dAtA[i:], m.AllocationStrategy) i = encodeVarintGenerated(dAtA, i, uint64(len(m.AllocationStrategy))) @@ -9871,7 +9873,9 @@ func (m *KVCacheBackendLeader) Size() (n int) { } l = len(m.AllocationStrategy) n += 1 + l + sovGenerated(uint64(l)) - n += 2 + if m.MultiTenancy != nil { + n += 2 + } if len(m.ExtraArgs) > 0 { for _, s := range m.ExtraArgs { l = len(s) @@ -12684,7 +12688,7 @@ func (this *KVCacheBackendLeader) String() string { `Replicas:` + valueToStringGenerated(this.Replicas) + `,`, `HighAvailability:` + strings.Replace(this.HighAvailability.String(), "KVCacheBackendLeaderHighAvailability", "KVCacheBackendLeaderHighAvailability", 1) + `,`, `AllocationStrategy:` + fmt.Sprintf("%v", this.AllocationStrategy) + `,`, - `MultiTenancy:` + fmt.Sprintf("%v", this.MultiTenancy) + `,`, + `MultiTenancy:` + valueToStringGenerated(this.MultiTenancy) + `,`, `ExtraArgs:` + fmt.Sprintf("%v", this.ExtraArgs) + `,`, `ExtraEnv:` + repeatedStringForExtraEnv + `,`, `}`, @@ -24400,7 +24404,8 @@ func (m *KVCacheBackendLeader) Unmarshal(dAtA []byte) error { break } } - m.MultiTenancy = bool(v != 0) + b := bool(v != 0) + m.MultiTenancy = &b case 5: if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field ExtraArgs", wireType) diff --git a/api/worker/v1alpha1/generated.proto b/api/worker/v1alpha1/generated.proto index 580e3a006..08e32ae99 100644 --- a/api/worker/v1alpha1/generated.proto +++ b/api/worker/v1alpha1/generated.proto @@ -1548,7 +1548,18 @@ message KVCacheBackendLeader { // domain that belongs to whoever typed it. The store's global -quota_bytes flag stays in // extraArgs for the converse reason: no other API needs to interpret it. // - // Unset and false both mean no ledger, and unset renders NO flag rather than an explicit false. + // IT DEFAULTS TO TRUE, because the ledger is what makes the rest of this API mean what it says: + // without it a KVCachePoolBinding's ceiling is recorded but not enforced, and a master serves one + // reuse domain only, so a second Binding on it is refused. Mooncake has taken the switch since + // 0.3.12, and the default store image is on 0.3.13.post1. + // + // OMITTING THIS KEY AND WRITING `multiTenancy: false` ARE DIFFERENT — the first takes the + // default, the second declines the ledger: only the explicit false renders no switch. A store + // image older than Mooncake 0.3.12 does not recognize the switch and its master exits at + // startup, so a backend on such an image, including the 0.3.10.post2 variants this project + // also publishes, sets false here. Read it through KVCacheBackendLeader.MultiTenancyEnabled. + // + // +k8s:validation:default=true optional bool multiTenancy = 4; // ExtraArgs passes flags this API does not enumerate straight through to the leader, after diff --git a/api/worker/v1alpha1/kv_cache_backend.go b/api/worker/v1alpha1/kv_cache_backend.go index b5058a87d..ebd0b8cfe 100644 --- a/api/worker/v1alpha1/kv_cache_backend.go +++ b/api/worker/v1alpha1/kv_cache_backend.go @@ -339,8 +339,19 @@ type KVCacheBackendLeader struct { // domain that belongs to whoever typed it. The store's global -quota_bytes flag stays in // extraArgs for the converse reason: no other API needs to interpret it. // - // Unset and false both mean no ledger, and unset renders NO flag rather than an explicit false. - MultiTenancy bool `json:"multiTenancy,omitempty" protobuf:"varint,4,opt,name=multiTenancy"` + // IT DEFAULTS TO TRUE, because the ledger is what makes the rest of this API mean what it says: + // without it a KVCachePoolBinding's ceiling is recorded but not enforced, and a master serves one + // reuse domain only, so a second Binding on it is refused. Mooncake has taken the switch since + // 0.3.12, and the default store image is on 0.3.13.post1. + // + // OMITTING THIS KEY AND WRITING `multiTenancy: false` ARE DIFFERENT — the first takes the + // default, the second declines the ledger: only the explicit false renders no switch. A store + // image older than Mooncake 0.3.12 does not recognize the switch and its master exits at + // startup, so a backend on such an image, including the 0.3.10.post2 variants this project + // also publishes, sets false here. Read it through KVCacheBackendLeader.MultiTenancyEnabled. + // + // +k8s:validation:default=true + MultiTenancy *bool `json:"multiTenancy,omitempty" protobuf:"varint,4,opt,name=multiTenancy"` // ExtraArgs passes flags this API does not enumerate straight through to the leader, after // the derived ones. Each entry is one flag token of its own, "-flag" or "-flag=value", and the @@ -375,6 +386,13 @@ type KVCacheBackendLeader struct { ExtraEnv []InstanceEnvVar `json:"extraEnv,omitempty" protobuf:"bytes,6,rep,name=extraEnv"` } +// MultiTenancyEnabled reports whether the leader runs with its tenant ledger. An unset field reads +// as the schema default, true, so an object that never passed the API server, such as one built +// in a test or by a fake client, answers the same as one that did. +func (in KVCacheBackendLeader) MultiTenancyEnabled() bool { + return in.MultiTenancy == nil || *in.MultiTenancy +} + // KVCacheBackendLeaderHighAvailability turns leader election on, and carries how members find the // leader it elects. // diff --git a/api/worker/v1alpha1/zz_generated.crds.go b/api/worker/v1alpha1/zz_generated.crds.go index 7b73469f6..009a7ac3f 100644 --- a/api/worker/v1alpha1/zz_generated.crds.go +++ b/api/worker/v1alpha1/zz_generated.crds.go @@ -2521,8 +2521,12 @@ func crd_gpustack_api_worker_v1alpha1_KVCacheBackend() *v1.CustomResourceDefinit Nullable: true, }, "multiTenancy": { - Description: "MultiTenancy turns on the leader's per-tenant quota ledger and the tenant-scoped shard index\nbehind it. Off, every request falls into one default tenant and the index degrades to a plain\nkey hash, so two callers using different tenant names read each other's cache.\nIt is a FIELD rather than an extraArgs entry because another API validates against it: a\nKVCachePool over a backend with no ledger to write quota into is admitted with a warning that\nno per-tenant quota is in force, withdrawing the flag from a backend a pool already holds is\nrefused, and a webhook reading an unschema'd \"true\", \"1\" or \"True\" would be judging a value\ndomain that belongs to whoever typed it. The store's global -quota_bytes flag stays in\nextraArgs for the converse reason: no other API needs to interpret it.\nUnset and false both mean no ledger, and unset renders NO flag rather than an explicit false.", + Description: "MultiTenancy turns on the leader's per-tenant quota ledger and the tenant-scoped shard index\nbehind it. Off, every request falls into one default tenant and the index degrades to a plain\nkey hash, so two callers using different tenant names read each other's cache.\nIt is a FIELD rather than an extraArgs entry because another API validates against it: a\nKVCachePool over a backend with no ledger to write quota into is admitted with a warning that\nno per-tenant quota is in force, withdrawing the flag from a backend a pool already holds is\nrefused, and a webhook reading an unschema'd \"true\", \"1\" or \"True\" would be judging a value\ndomain that belongs to whoever typed it. The store's global -quota_bytes flag stays in\nextraArgs for the converse reason: no other API needs to interpret it.\nIT DEFAULTS TO TRUE, because the ledger is what makes the rest of this API mean what it says:\nwithout it a KVCachePoolBinding's ceiling is recorded but not enforced, and a master serves one\nreuse domain only, so a second Binding on it is refused. Mooncake has taken the switch since\n0.3.12, and the default store image is on 0.3.13.post1.\nOMITTING THIS KEY AND WRITING `multiTenancy: false` ARE DIFFERENT — the first takes the\ndefault, the second declines the ledger: only the explicit false renders no switch. A store\nimage older than Mooncake 0.3.12 does not recognize the switch and its master exits at\nstartup, so a backend on such an image, including the 0.3.10.post2 variants this project\nalso publishes, sets false here. Read it through KVCacheBackendLeader.MultiTenancyEnabled.", Type: "boolean", + Default: &v1.JSON{ + Raw: []byte(`true`), + }, + Nullable: true, }, "replicas": { Description: "Replicas is how many leader processes run, of which exactly one serves at a time. The rest are\nstandbys: they hold no data, answer no request, and exist to take over.\n- More than one REQUIRES HighAvailability. Electing a leader among several needs a leadership\nrecord, and the webhook refuses the pair without one rather than silently running two\nleaders against the same members.\n- Raising this past one TURNS THE ELECTION ON, and the flip is re-evaluated on every\nreconcile rather than decided at create. It restarts the leader and rolls every member —\nthe member's master entry changes shape with it — so the store's cached contents do not\nsurvive the crossing. The same holds on the way back down to one.\n- Raising this adds no capacity, which members do. The ceiling is here to catch the reading\nthat it does, and it is duplicated in the webhook on purpose: this one still holds when\nthe webhook is not installed, which is when a second leader would be rendered rather than\nrefused. Raise both together; widening a maximum is not a breaking change.", diff --git a/api/worker/v1alpha1/zz_generated.deepcopy.go b/api/worker/v1alpha1/zz_generated.deepcopy.go index 59832d5c1..f8a667a50 100644 --- a/api/worker/v1alpha1/zz_generated.deepcopy.go +++ b/api/worker/v1alpha1/zz_generated.deepcopy.go @@ -1292,6 +1292,11 @@ func (in *KVCacheBackendLeader) DeepCopyInto(out *KVCacheBackendLeader) { *out = new(KVCacheBackendLeaderHighAvailability) **out = **in } + if in.MultiTenancy != nil { + in, out := &in.MultiTenancy, &out.MultiTenancy + *out = new(bool) + **out = **in + } if in.ExtraArgs != nil { in, out := &in.ExtraArgs, &out.ExtraArgs *out = make([]string, len(*in)) diff --git a/api/worker/zz_generated.openapi.go b/api/worker/zz_generated.openapi.go index 1ee69526c..c023e2b70 100644 --- a/api/worker/zz_generated.openapi.go +++ b/api/worker/zz_generated.openapi.go @@ -6482,7 +6482,7 @@ func schema_gpustack_api_worker_v1alpha1_KVCacheBackendLeader(ref common.Referen }, "multiTenancy": { SchemaProps: spec.SchemaProps{ - Description: "MultiTenancy turns on the leader's per-tenant quota ledger and the tenant-scoped shard index behind it. Off, every request falls into one default tenant and the index degrades to a plain key hash, so two callers using different tenant names read each other's cache.\n\nIt is a FIELD rather than an extraArgs entry because another API validates against it: a KVCachePool over a backend with no ledger to write quota into is admitted with a warning that no per-tenant quota is in force, withdrawing the flag from a backend a pool already holds is refused, and a webhook reading an unschema'd \"true\", \"1\" or \"True\" would be judging a value domain that belongs to whoever typed it. The store's global -quota_bytes flag stays in extraArgs for the converse reason: no other API needs to interpret it.\n\nUnset and false both mean no ledger, and unset renders NO flag rather than an explicit false.", + Description: "MultiTenancy turns on the leader's per-tenant quota ledger and the tenant-scoped shard index behind it. Off, every request falls into one default tenant and the index degrades to a plain key hash, so two callers using different tenant names read each other's cache.\n\nIt is a FIELD rather than an extraArgs entry because another API validates against it: a KVCachePool over a backend with no ledger to write quota into is admitted with a warning that no per-tenant quota is in force, withdrawing the flag from a backend a pool already holds is refused, and a webhook reading an unschema'd \"true\", \"1\" or \"True\" would be judging a value domain that belongs to whoever typed it. The store's global -quota_bytes flag stays in extraArgs for the converse reason: no other API needs to interpret it.\n\nIT DEFAULTS TO TRUE, because the ledger is what makes the rest of this API mean what it says: without it a KVCachePoolBinding's ceiling is recorded but not enforced, and a master serves one reuse domain only, so a second Binding on it is refused. Mooncake has taken the switch since 0.3.12, and the default store image is on 0.3.13.post1.\n\nOMITTING THIS KEY AND WRITING `multiTenancy: false` ARE DIFFERENT — the first takes the default, the second declines the ledger: only the explicit false renders no switch. A store image older than Mooncake 0.3.12 does not recognize the switch and its master exits at startup, so a backend on such an image, including the 0.3.10.post2 variants this project also publishes, sets false here. Read it through KVCacheBackendLeader.MultiTenancyEnabled.", Type: []string{"boolean"}, Format: "", }, diff --git a/docs/kv-cache/backend.md b/docs/kv-cache/backend.md index e9bf2a290..11795dcf9 100644 --- a/docs/kv-cache/backend.md +++ b/docs/kv-cache/backend.md @@ -39,7 +39,7 @@ spec: image: docker.io/kvcacheai/mooncake:0.3.13 connection: managed: # or external: — exactly one - leader: {} # replicas and allocationStrategy default + leader: {} # replicas, allocationStrategy and multiTenancy default members: - nodeSelector: {kubernetes.io/os: linux} medium: DRAM # what this group's SEGMENT is made of: DRAM or VRAM @@ -192,6 +192,12 @@ Each variant is built on `0.3.13.post1`, the line vLLM's supported clients are o minimum](../reference/engine-versions.md). SGLang's clients are on the 0.3.12 line, which this project does not build. Which line a backend needs is that table's question. +**A `0.3.10.post2` variant also needs `leader.multiTenancy: false` written out.** The tenant ledger +hangs on the master's `-enable_multi_tenants` switch, which Mooncake took in 0.3.12, and the field +defaults on — so on an older image the default renders a flag the master does not recognize, and it +exits at startup. The explicit false renders no flag, which is the command line such an image has +always run. + **A VRAM group needs a build with VRAM segments compiled in (`USE_VRAM_SEGMENT=ON`), and the stock `-cpu` default is not one.** VRAM segments exist only on the `0.3.13` line — the `0.3.10.post2` variants carry the vendor transfer engine without them — so a VRAM group always names a @@ -235,8 +241,8 @@ write then fails at transfer time with `RPC_FAIL (-900)`. Two posts of one minor line share their RPC signatures and interoperate. The 0.3.12 and 0.3.13 lines do not: the method names are unchanged, so the client reaches the handler and mis-decodes the -arguments. Multi-tenancy moves neither: with it off — the master's own default — every request -resolves to the default tenant. +arguments. Multi-tenancy moves neither: with it off — a declared `multiTenancy: false`, the field +defaulting on — every request resolves to the default tenant. **The client's version is a property of the engine image, not of anything on this CR.** Which client each supported engine's runner image carries, and so which line its store runs, is in the diff --git a/docs/kv-cache/walkthrough.md b/docs/kv-cache/walkthrough.md index d62847e91..3c2949d9f 100644 --- a/docs/kv-cache/walkthrough.md +++ b/docs/kv-cache/walkthrough.md @@ -67,6 +67,11 @@ later. Naming a published upstream image here works until Step 4 and then does n `capacityPerMember` is charged to each member Pod's host memory request, so it is a claim on the node and not a hint. One member Pod runs per node the selector matches. +`leader: {}` takes the field defaults, `multiTenancy` included, so this master keeps a per-tenant +quota ledger and Steps 2 and 3 read against it. A backend pinned to a store image from before +Mooncake 0.3.12 is the one exception and declares `multiTenancy: false` out loud — see +[The project's own build variants](backend.md#the-projects-own-build-variants). + Wait for it, and read what it actually says: ```console @@ -111,18 +116,19 @@ workloads sharing a domain share cached blocks, so a domain that could be edited workload start reading blocks another tokenizer wrote. Pick it to match the model and engine settings the deployments in this namespace will run; a second, different model gets a second Binding. -**`domain.name` is left out, so it is `default`.** The backend from Step 1 runs without -multi-tenancy, so no tenant is forwarded to the engines and the name only records the registration. -On a multi-tenant backend, name each domain; the name is the tenant id the engines are handed. +**`domain.name` is left out, so it is `default`.** The backend from Step 1 runs with multi-tenancy +— `leader.multiTenancy` defaults on — so `default` is the tenant id the engines are handed, the +store's own tenant for a writer that names none. Name each domain once a second Binding shares the +master; the name is what keeps the two apart. -**A quota ceiling is not a reservation.** On a backend with `leader.multiTenancy: true` it is the -most this namespace may hold at once, and going over it does not fail a write — see +**A quota ceiling is not a reservation.** It is the most this namespace may hold at once, and going +over it does not fail a write — see [What a full quota actually does](pool.md#what-a-full-quota-actually-does). -**On the backend from Step 1 the ceiling is recorded but not enforced.** That leader runs without -multi-tenancy, so the master holds no tenant ledger: the pool is admitted with a warning, the Binding -reports `QuotaGranted=True` with reason `Unenforced` and no `EFFECTIVE` figure, and every write lands -in the store's default tenant. Set `leader.multiTenancy: true` in Step 1 to have ceilings enforced. +**The ceiling is enforced because the Step-1 leader carries its tenant ledger, which the default +gave it.** A backend declared `leader.multiTenancy: false` holds no ledger instead: its pool is +admitted with a warning, the Binding reports `QuotaGranted=True` with reason `Unenforced` and no +`EFFECTIVE` figure, and every write lands in the store's default tenant. **A multi-tenant master refuses a tenant name absent from its ledger.** An engine that ignores the injected tenant then needs a second Binding whose domain is `default`, or that leaves `name` out — see diff --git a/pkg/kubeclients/applyconfiguration/worker/v1alpha1/kvcachebackendleader.go b/pkg/kubeclients/applyconfiguration/worker/v1alpha1/kvcachebackendleader.go index 3d689517c..2a4b633ea 100644 --- a/pkg/kubeclients/applyconfiguration/worker/v1alpha1/kvcachebackendleader.go +++ b/pkg/kubeclients/applyconfiguration/worker/v1alpha1/kvcachebackendleader.go @@ -62,7 +62,16 @@ type KVCacheBackendLeaderApplyConfiguration struct { // domain that belongs to whoever typed it. The store's global -quota_bytes flag stays in // extraArgs for the converse reason: no other API needs to interpret it. // - // Unset and false both mean no ledger, and unset renders NO flag rather than an explicit false. + // IT DEFAULTS TO TRUE, because the ledger is what makes the rest of this API mean what it says: + // without it a KVCachePoolBinding's ceiling is recorded but not enforced, and a master serves one + // reuse domain only, so a second Binding on it is refused. Mooncake has taken the switch since + // 0.3.12, and the default store image is on 0.3.13.post1. + // + // OMITTING THIS KEY AND WRITING `multiTenancy: false` ARE DIFFERENT — the first takes the + // default, the second declines the ledger: only the explicit false renders no switch. A store + // image older than Mooncake 0.3.12 does not recognize the switch and its master exits at + // startup, so a backend on such an image, including the 0.3.10.post2 variants this project + // also publishes, sets false here. Read it through KVCacheBackendLeader.MultiTenancyEnabled. MultiTenancy *bool `json:"multiTenancy,omitempty"` // ExtraArgs passes flags this API does not enumerate straight through to the leader, after // the derived ones. Each entry is one flag token of its own, "-flag" or "-flag=value", and the diff --git a/pkg/worker/controllers/worker/kv_cache_backend_test.go b/pkg/worker/controllers/worker/kv_cache_backend_test.go index 3c34fbed4..2e258afad 100644 --- a/pkg/worker/controllers/worker/kv_cache_backend_test.go +++ b/pkg/worker/controllers/worker/kv_cache_backend_test.go @@ -569,8 +569,10 @@ func TestKVCacheBackendReconciler_AHeldTeardownStillConvergesItsWorkloads(t *tes claim := workercore.KVCacheObjectReference{Kind: KVCachePoolKind, Name: "team-a-pool"} // Multi-tenancy is OFF to begin with, which is the state an operator reaches this path in: the - // flag was withdrawn while a pool held the backend, and putting it back is the remedy. + // flag was withdrawn while a pool held the backend, and putting it back is the remedy. Off is + // said out loud: an unset field defaults on, so only the explicit false renders no switch. kvcb := newKVCacheBackendObject(claim) + kvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(false) cli := newKVCacheBackendClient(append([]ctrlcli.Object{kvcb}, kvCachePoolsNamedBy(claim)...)...) leaderArgs := func(t *testing.T) []string { @@ -593,7 +595,7 @@ func TestKVCacheBackendReconciler_AHeldTeardownStillConvergesItsWorkloads(t *tes require.NotNil(t, live.DeletionTimestamp, "the finalizer must have held it") // The remedy, applied to an object that is already Deleting. - live.Spec.Connection.Managed.Leader.MultiTenancy = true + live.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(true) require.NoError(t, cli.Update(ctx, live)) held := reconcileKVCacheBackend(t, cli, kvcb.Name) @@ -1215,7 +1217,10 @@ func TestKVCacheBackendReconciler_ConvergesAnEFASwitch(t *testing.T) { // already moves, and a pass that moved one without the other would be refused by the API server on // every reconcile — while this object went on reporting Ready. func TestKVCacheBackendReconciler_ConvergesAMultiTenancySwitch(t *testing.T) { + // Off to begin with, and said out loud for the same reason: an unset field defaults on, so the + // starting render this test asserts has to be asked for. kvcb := newKVCacheBackendObject() + kvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(false) cli := newKVCacheBackendClient(kvcb) ctx := context.Background() @@ -1224,7 +1229,7 @@ func TestKVCacheBackendReconciler_ConvergesAMultiTenancySwitch(t *testing.T) { setMultiTenancy := func(on bool) { got := new(workercore.KVCacheBackend) require.NoError(t, cli.Get(ctx, ctrlcli.ObjectKey{Name: kvcb.Name}, got)) - got.Spec.Connection.Managed.Leader.MultiTenancy = on + got.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(on) require.NoError(t, cli.Update(ctx, got)) require.NotNil(t, reconcileKVCacheBackend(t, cli, kvcb.Name)) } diff --git a/pkg/worker/controllers/worker/kv_cache_pool.go b/pkg/worker/controllers/worker/kv_cache_pool.go index 285275e7e..b3738fcc9 100644 --- a/pkg/worker/controllers/worker/kv_cache_pool.go +++ b/pkg/worker/controllers/worker/kv_cache_pool.go @@ -160,7 +160,7 @@ func KVCacheMasterSeparatesTenants(kvcb *workercore.KVCacheBackend, kvcp *worker return false } if managed := kvcb.Spec.Connection.Managed; managed != nil { - return managed.Leader.MultiTenancy + return managed.Leader.MultiTenancyEnabled() } return true } @@ -1086,7 +1086,7 @@ func (r *KVCachePoolReconciler) convergeTenantLedger( // The declaration is exact for a managed backend — this operator renders the flag onto the // leader's command line — so a managed master declared without multi-tenancy is never asked a // question it can only refuse. - if managed := kvcb.Spec.Connection.Managed; managed != nil && !managed.Leader.MultiTenancy { + if managed := kvcb.Spec.Connection.Managed; managed != nil && !managed.Leader.MultiTenancyEnabled() { r.reportTenantLedgerAbsent(holder) metrics, scraped := r.observeAllocatableCapacity(ctx, admin, holder, true) return kvCachePoolLedgerPass{converged: true, noLedger: true, metrics: metrics, scraped: scraped} @@ -2035,7 +2035,7 @@ func (r *KVCachePoolReconciler) resolveKVCachePoolAdmin( // rendered from the cluster alone, so asking would only make the pool's deletion depend on a // leader that is running — and a leader that never came up would then hold the pool, which holds // the backend, with nothing left able to move. - if managed := kvcb.Spec.Connection.Managed; managed != nil && !managed.Leader.MultiTenancy { + if managed := kvcb.Spec.Connection.Managed; managed != nil && !managed.Leader.MultiTenancyEnabled() { logger.V(2).Info("tearing down a pool on a backend declared without a tenant ledger") return nil, kvcb, false, nil } diff --git a/pkg/worker/controllers/worker/kv_cache_pool_reconcile_test.go b/pkg/worker/controllers/worker/kv_cache_pool_reconcile_test.go index b61e67f76..840f9ae9a 100644 --- a/pkg/worker/controllers/worker/kv_cache_pool_reconcile_test.go +++ b/pkg/worker/controllers/worker/kv_cache_pool_reconcile_test.go @@ -17,6 +17,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" meta "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" ctrlcli "sigs.k8s.io/controller-runtime/pkg/client" ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -355,7 +356,7 @@ func newReconcileBackend(name, admin string) *workercore.KVCacheBackend { // backend whose leader this operator starts without the multi-tenancy flag. func newManagedSingleTenantReconcileBackend(name, admin string) *workercore.KVCacheBackend { kvcb := newManagedReconcileBackend(name, admin) - kvcb.Spec.Connection.Managed.Leader.MultiTenancy = false + kvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(false) return kvcb } diff --git a/pkg/worker/controllers/worker/kv_cache_pool_teardown_test.go b/pkg/worker/controllers/worker/kv_cache_pool_teardown_test.go index 54ddf2e87..aec2dd536 100644 --- a/pkg/worker/controllers/worker/kv_cache_pool_teardown_test.go +++ b/pkg/worker/controllers/worker/kv_cache_pool_teardown_test.go @@ -1155,7 +1155,7 @@ func newManagedReconcileBackend(name, admin string) *workercore.KVCacheBackend { Managed: &workercore.KVCacheBackendManaged{ Leader: workercore.KVCacheBackendLeader{ Replicas: ptr.To[int32](1), - MultiTenancy: true, + MultiTenancy: ptr.To(true), }, Members: []workercore.KVCacheBackendMember{{ NodeSelector: map[string]string{"kvcache-dram": "true"}, @@ -1211,7 +1211,7 @@ func TestKVCachePoolTeardown_MultiTenancyWithdrawnMidFlightStillDeletesBothObjec // written here is the object an operator ALREADY has — the state the refusal cannot reach back // into — and the master answers its ledger accordingly. live := readBackend(t, cli, "mooncake-dram") - live.Spec.Connection.Managed.Leader.MultiTenancy = false + live.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(false) require.NoError(t, cli.Update(ctx, live)) master.refuse(409, `{"success":false,"error_code":-1011,"error_message":"UNAVAILABLE_IN_CURRENT_MODE"}`) diff --git a/pkg/worker/kvcache/mooncake/leader_flags.go b/pkg/worker/kvcache/mooncake/leader_flags.go index 5b00956ab..fae7c7094 100644 --- a/pkg/worker/kvcache/mooncake/leader_flags.go +++ b/pkg/worker/kvcache/mooncake/leader_flags.go @@ -154,7 +154,7 @@ func RenderLeaderFlags(kvcb *workercore.KVCacheBackend) []string { // the switch alone is a process that does not start. -tenant_quota_connector_type stays absent // under the rule above: "file" is already its default, and the workload provides no other kind // of source. - if leader.MultiTenancy { + if leader.MultiTenancyEnabled() { flags = append(flags, "-enable_multi_tenants=true", "-tenant_quota_connector_uri="+QuotaPolicyFilePath) diff --git a/pkg/worker/kvcache/mooncake/leader_flags_test.go b/pkg/worker/kvcache/mooncake/leader_flags_test.go index 2db65e52a..7bf412069 100644 --- a/pkg/worker/kvcache/mooncake/leader_flags_test.go +++ b/pkg/worker/kvcache/mooncake/leader_flags_test.go @@ -25,7 +25,9 @@ func TestRenderLeaderFlags(t *testing.T) { want []string }{ { - name: "the canonical leader, as admission leaves it", + // The field is unset here on purpose: the schema defaults it on, and an unset field + // renders as that default, so this is the case that pins nil to mean the ledger. + name: "an unset multi-tenancy renders the ledger the schema defaults on", leader: workercore.KVCacheBackendLeader{ Replicas: ptr.To[int32](1), AllocationStrategy: "FreeRatioFirst", @@ -35,12 +37,15 @@ func TestRenderLeaderFlags(t *testing.T) { "-metrics_port=9003", "-default_kv_lease_ttl=5m", "-allocation_strategy=free_ratio_first", + "-enable_multi_tenants=true", + "-tenant_quota_connector_uri=/var/lib/mooncake/tenant-quota-policy.yaml", }, }, { name: "the other strategy maps to the artifact's own spelling", leader: workercore.KVCacheBackendLeader{ AllocationStrategy: "Random", + MultiTenancy: ptr.To(false), }, want: []string{ "-rpc_port=50051", @@ -51,7 +56,7 @@ func TestRenderLeaderFlags(t *testing.T) { }, { name: "an unset strategy renders no flag rather than a guess", - leader: workercore.KVCacheBackendLeader{}, + leader: workercore.KVCacheBackendLeader{MultiTenancy: ptr.To(false)}, want: []string{ "-rpc_port=50051", "-metrics_port=9003", @@ -67,7 +72,7 @@ func TestRenderLeaderFlags(t *testing.T) { leader: workercore.KVCacheBackendLeader{ Replicas: ptr.To[int32](1), AllocationStrategy: "FreeRatioFirst", - MultiTenancy: true, + MultiTenancy: ptr.To(true), }, want: []string{ "-rpc_port=50051", @@ -79,13 +84,14 @@ func TestRenderLeaderFlags(t *testing.T) { }, }, { - // Both flags are absent rather than rendered false and empty, so a backend nobody asked - // to be multi-tenant runs the command line it ran before this field existed. + // Both flags are absent rather than rendered false and empty, so a backend that asks for + // no ledger — the explicit false an older store image needs — runs the command line it + // ran before this field existed. name: "multi-tenancy off renders nothing at all", leader: workercore.KVCacheBackendLeader{ Replicas: ptr.To[int32](1), AllocationStrategy: "FreeRatioFirst", - MultiTenancy: false, + MultiTenancy: ptr.To(false), }, want: []string{ "-rpc_port=50051", @@ -104,6 +110,7 @@ func TestRenderLeaderFlags(t *testing.T) { name: "extraArgs come last, in the order written", leader: workercore.KVCacheBackendLeader{ AllocationStrategy: "FreeRatioFirst", + MultiTenancy: ptr.To(false), ExtraArgs: []string{ "-offload_cap_ratio=0.5", "-client_ttl=30", @@ -127,6 +134,7 @@ func TestRenderLeaderFlags(t *testing.T) { name: "a one-token boolean entry renders as itself", leader: workercore.KVCacheBackendLeader{ AllocationStrategy: "FreeRatioFirst", + MultiTenancy: ptr.To(false), ExtraArgs: []string{"-client_verbose_logging"}, }, want: []string{ @@ -196,6 +204,7 @@ func TestRenderLeaderFlags_HighAvailability(t *testing.T) { kvcb := leaderBackend(workercore.KVCacheBackendLeader{ Replicas: ptr.To[int32](3), AllocationStrategy: "FreeRatioFirst", + MultiTenancy: ptr.To(false), HighAvailability: &workercore.KVCacheBackendLeaderHighAvailability{}, }) diff --git a/pkg/worker/kvcache/mooncake/leader_snapshot_test.go b/pkg/worker/kvcache/mooncake/leader_snapshot_test.go index a89b2df19..abc212a79 100644 --- a/pkg/worker/kvcache/mooncake/leader_snapshot_test.go +++ b/pkg/worker/kvcache/mooncake/leader_snapshot_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/utils/ptr" workercore "gpustack.ai/gpustack/api/worker/v1alpha1" ) @@ -27,7 +28,7 @@ func TestRenderLeader_RendersNoSnapshot(t *testing.T) { kvcb.Spec.Connection.Managed.Leader.HighAvailability = &workercore.KVCacheBackendLeaderHighAvailability{} })}, {"an elected leader under multi-tenancy", haBackend(func(kvcb *workercore.KVCacheBackend) { - kvcb.Spec.Connection.Managed.Leader.MultiTenancy = true + kvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(true) })}, } diff --git a/pkg/worker/kvcache/mooncake/leader_workload.go b/pkg/worker/kvcache/mooncake/leader_workload.go index 29c01e8e7..22f40f8ce 100644 --- a/pkg/worker/kvcache/mooncake/leader_workload.go +++ b/pkg/worker/kvcache/mooncake/leader_workload.go @@ -383,7 +383,7 @@ func RenderLeaderDeployment(kvcb *workercore.KVCacheBackend, image string) *apps } podSpec := &deploy.Spec.Template.Spec - if leader.MultiTenancy { + if leader.MultiTenancyEnabled() { podSpec.Volumes = append(podSpec.Volumes, quotaPolicyVolumes(kvcb)...) podSpec.InitContainers = []core.Container{ quotaPolicySeedContainer(image, kvcache.EffectivePullPolicy(kvcb, image)), @@ -412,7 +412,7 @@ func leaderContainerSpec( leader := kvcb.Spec.Connection.Managed.Leader var volumeMounts []core.VolumeMount - if leader.MultiTenancy { + if leader.MultiTenancyEnabled() { // Not read-only, and that is the point: the master writes a temp file into this directory // and renames it over the policy on every admin-API change. // diff --git a/pkg/worker/kvcache/mooncake/leader_workload_test.go b/pkg/worker/kvcache/mooncake/leader_workload_test.go index af4265c01..8563e346f 100644 --- a/pkg/worker/kvcache/mooncake/leader_workload_test.go +++ b/pkg/worker/kvcache/mooncake/leader_workload_test.go @@ -419,7 +419,9 @@ func TestLeaderWorkload_PodIdentityEnv(t *testing.T) { // is the side that needs those, and a leader that acquired them would be a privilege nobody asked // for. func TestLeaderWorkload_ClaimsNoHost(t *testing.T) { - deploy := RenderLeaderDeployment(testBackend(), "mooncake:v0.3.13") + // Single-tenant, because the ledger is the one thing that mounts a volume pair here, and it has + // its own tests below. + deploy := RenderLeaderDeployment(singleTenantBackend(), "mooncake:v0.3.13") podSpec := deploy.Spec.Template.Spec assert.False(t, podSpec.HostNetwork, "the leader is not hostNetwork") @@ -433,11 +435,21 @@ func TestLeaderWorkload_ClaimsNoHost(t *testing.T) { } } -// multiTenantBackend is the canonical backend with the quota ledger turned on — the one shape that -// makes the leader mount anything at all. +// multiTenantBackend is the canonical backend with the quota ledger turned on — said out loud, +// though the field defaults on, so the cases below read as what they are about rather than as what +// an unset field happens to mean. func multiTenantBackend() *workercore.KVCacheBackend { return testBackend(func(k *workercore.KVCacheBackend) { - k.Spec.Connection.Managed.Leader.MultiTenancy = true + k.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(true) + }) +} + +// singleTenantBackend is the canonical backend with the quota ledger declined. The false is +// explicit for the same reason: an unset field defaults on, so only saying false renders without +// the policy volume pair. +func singleTenantBackend() *workercore.KVCacheBackend { + return testBackend(func(k *workercore.KVCacheBackend) { + k.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(false) }) } @@ -524,7 +536,7 @@ func TestLeaderWorkload_QuotaPolicySeedFallsBackToTheEmptyPolicy(t *testing.T) { // TestLeaderWorkload_QuotaPolicyVolumeIsGatedOnMultiTenancy is the negative half, asserted field by // field rather than as one shape comparison, so the switch cannot half-apply. func TestLeaderWorkload_QuotaPolicyVolumeIsGatedOnMultiTenancy(t *testing.T) { - deploy := RenderLeaderDeployment(testBackend(), "mooncake:v0.3.13") + deploy := RenderLeaderDeployment(singleTenantBackend(), "mooncake:v0.3.13") podSpec := deploy.Spec.Template.Spec assert.Empty(t, podSpec.Volumes) diff --git a/pkg/worker/webhooks/worker/kv_cache_backend.go b/pkg/worker/webhooks/worker/kv_cache_backend.go index 3d1c4df65..62b6cc438 100644 --- a/pkg/worker/webhooks/worker/kv_cache_backend.go +++ b/pkg/worker/webhooks/worker/kv_cache_backend.go @@ -1319,7 +1319,7 @@ func validateKVCacheBackendMultiTenancyWithdrawal( if oldManaged == nil || newManaged == nil { return nil } - if !oldManaged.Leader.MultiTenancy || newManaged.Leader.MultiTenancy { + if !oldManaged.Leader.MultiTenancyEnabled() || newManaged.Leader.MultiTenancyEnabled() { return nil } if len(oldKvcb.Status.UsedBy) == 0 { diff --git a/pkg/worker/webhooks/worker/kv_cache_backend_test.go b/pkg/worker/webhooks/worker/kv_cache_backend_test.go index 0d67b4175..dd0957c12 100644 --- a/pkg/worker/webhooks/worker/kv_cache_backend_test.go +++ b/pkg/worker/webhooks/worker/kv_cache_backend_test.go @@ -1194,8 +1194,8 @@ func TestKVCacheBackendWebhook_MultiTenancyCannotBeWithdrawnFromAClaimedBackend( if tc.external { oldKvcb.Spec, newKvcb.Spec = newExternalKVCacheBackendSpec(), newExternalKVCacheBackendSpec() } else { - oldKvcb.Spec.Connection.Managed.Leader.MultiTenancy = tc.was - newKvcb.Spec.Connection.Managed.Leader.MultiTenancy = tc.now + oldKvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(tc.was) + newKvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(tc.now) } // On BOTH, because status is a subresource: an update to the spec carries whatever status // the API server already holds, so a fixture that put the claim on only one of them would diff --git a/pkg/worker/webhooks/worker/kv_cache_pool.go b/pkg/worker/webhooks/worker/kv_cache_pool.go index 776dee30b..0a4c10414 100644 --- a/pkg/worker/webhooks/worker/kv_cache_pool.go +++ b/pkg/worker/webhooks/worker/kv_cache_pool.go @@ -180,7 +180,7 @@ func (r *KVCachePoolWebhook) validateKVCachePoolBackend( // process was started is answered where it can be — by the reconciler reading the master's own // 409, and by the Pod webhook refusing injection until the pool reports it. managed := kvcb.Spec.Connection.Managed - if managed == nil || managed.Leader.MultiTenancy { + if managed == nil || managed.Leader.MultiTenancyEnabled() { return nil, nil } diff --git a/pkg/worker/webhooks/worker/kv_cache_pool_binding.go b/pkg/worker/webhooks/worker/kv_cache_pool_binding.go index d7933ff45..dfd4640a8 100644 --- a/pkg/worker/webhooks/worker/kv_cache_pool_binding.go +++ b/pkg/worker/webhooks/worker/kv_cache_pool_binding.go @@ -468,7 +468,7 @@ func (r *KVCachePoolBindingWebhook) masterSeparatesDomains( } } if managed := kvcb.Spec.Connection.Managed; managed != nil { - return managed.Leader.MultiTenancy, nil + return managed.Leader.MultiTenancyEnabled(), nil } return true, nil diff --git a/pkg/worker/webhooks/worker/kv_cache_pool_binding_test.go b/pkg/worker/webhooks/worker/kv_cache_pool_binding_test.go index 9b6e071c2..f9127fac6 100644 --- a/pkg/worker/webhooks/worker/kv_cache_pool_binding_test.go +++ b/pkg/worker/webhooks/worker/kv_cache_pool_binding_test.go @@ -547,7 +547,7 @@ func TestKVCachePoolBindingWebhook_ASecondDistinctDomainNeedsAMasterThatSeparate { name: "a managed master with no ledger refuses the second domain", objs: []ctrlcli.Object{ - newKVCachePool(), newKVCacheBackend(), otherKVCachePoolBinding("team-b-batch"), + newKVCachePool(), newSingleTenantKVCacheBackend(), otherKVCachePoolBinding("team-b-batch"), }, wantMsg: "that backend holds no tenant ledger", }, @@ -555,7 +555,7 @@ func TestKVCachePoolBindingWebhook_ASecondDistinctDomainNeedsAMasterThatSeparate // The word SECOND is load-bearing: one domain on a ledger-less master is what such a master // serves correctly, so nothing is refused and nothing is warned about. name: "a ledger-less master takes the first domain", - objs: []ctrlcli.Object{newKVCachePool(), newKVCacheBackend()}, + objs: []ctrlcli.Object{newKVCachePool(), newSingleTenantKVCacheBackend()}, wantMsg: "", }, { @@ -595,7 +595,7 @@ func TestKVCachePoolBindingWebhook_ASecondDistinctDomainNeedsAMasterThatSeparate otherPool.Spec.Backends = []string{"mooncake-other"} holder := otherKVCachePoolBinding("team-b-batch") holder.Spec.PoolRef.Name = "other-pool" - return []ctrlcli.Object{newKVCachePool(), newKVCacheBackend(), otherPool, holder} + return []ctrlcli.Object{newKVCachePool(), newSingleTenantKVCacheBackend(), otherPool, holder} }(), wantMsg: "", }, @@ -707,7 +707,7 @@ func TestKVCachePoolBindingWebhook_EverySharedMasterIsAsked(t *testing.T) { badHolder.Namespace, badHolder.Name = "team-c", "zzz-rag" badHolder.Spec.PoolRef.Name = badPool.Name - ledgerless := newKVCacheBackend() + ledgerless := newSingleTenantKVCacheBackend() ledgerless.Name = "mooncake-second" wh := newKVCachePoolBindingWebhook( @@ -777,7 +777,7 @@ func TestKVCachePoolBindingWebhook_TheWarningClaimsNoSeparationItDidNotEstablish // what can be changed here; a message naming only the collision leaves the reader with no next step. func TestKVCachePoolBindingWebhook_TheSeparationRefusalSaysWhatToDo(t *testing.T) { wh := newKVCachePoolBindingWebhook( - newKVCachePool(), newKVCacheBackend(), otherKVCachePoolBinding("team-b-batch")) + newKVCachePool(), newSingleTenantKVCacheBackend(), otherKVCachePoolBinding("team-b-batch")) _, err := wh.ValidateCreate(context.Background(), newKVCachePoolBinding()) require.Error(t, err) @@ -806,7 +806,7 @@ func TestKVCachePoolBindingWebhook_TheSeparationRefusalSaysWhatToDo(t *testing.T // removes a finalizer, which would leave the Binding undeletable. func TestKVCachePoolBindingWebhook_SeparationIsNotRejudgedOnUpdate(t *testing.T) { wh := newKVCachePoolBindingWebhook( - newKVCachePool(), newKVCacheBackend(), otherKVCachePoolBinding("team-b-batch")) + newKVCachePool(), newSingleTenantKVCacheBackend(), otherKVCachePoolBinding("team-b-batch")) // The collision is real: the same cluster state refuses this object at CREATE. _, err := wh.ValidateCreate(context.Background(), newKVCachePoolBinding()) diff --git a/pkg/worker/webhooks/worker/kv_cache_pool_test.go b/pkg/worker/webhooks/worker/kv_cache_pool_test.go index b25ebf085..351da2211 100644 --- a/pkg/worker/webhooks/worker/kv_cache_pool_test.go +++ b/pkg/worker/webhooks/worker/kv_cache_pool_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/resource" meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" ctrlcli "sigs.k8s.io/controller-runtime/pkg/client" ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -39,7 +40,15 @@ func newKVCachePool() *workercore.KVCachePool { // the ledger F5 requires. func newMultiTenantKVCacheBackend() *workercore.KVCacheBackend { kvcb := newKVCacheBackend() - kvcb.Spec.Connection.Managed.Leader.MultiTenancy = true + kvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(true) + return kvcb +} + +// newSingleTenantKVCacheBackend is the fixture backend declared WITHOUT the ledger. The false is +// explicit because the field defaults on: only an object that says false runs no ledger. +func newSingleTenantKVCacheBackend() *workercore.KVCacheBackend { + kvcb := newKVCacheBackend() + kvcb.Spec.Connection.Managed.Leader.MultiTenancy = ptr.To(false) return kvcb } @@ -163,7 +172,7 @@ func TestKVCachePoolWebhook_ValidateCreate(t *testing.T) { // store is a topology rather than a broken one — and the admission says what the shape costs, because // "no per-tenant quota is in force" is not something the object itself can say. func TestKVCachePoolWebhook_ValidateCreate_SingleTenantBackendWarns(t *testing.T) { - wh := newKVCachePoolWebhook(newKVCacheBackend()) + wh := newKVCachePoolWebhook(newSingleTenantKVCacheBackend()) warnings, err := wh.ValidateCreate(context.Background(), newKVCachePool())