What happened:
QuotaManager keys limits by namespace and device resource name only (pkg/device/quota.go:38-40), with nothing recording which ResourceQuota object set them. addQuotaLocked writes straight into the slot and delQuotaLocked clears it unconditionally, so with two objects in one namespace that both set limits.hami.io/gpumem:
after adding strict(1000) and loose(5000): {Used:0 Limit:5000 LimitSet:true}
-> a 3000 MiB pod is admitted, which "strict" forbids
after deleting loose: {Used:0 Limit:0 LimitSet:false}
-> a 1000000 MiB pod is admitted, although "strict" is still in the cluster
That is master (b5ec6b1), driven through s.onAddQuota / s.onDelQuota and then fitResourceQuota. Which object wins is informer delivery order, so it can flip on a scheduler restart with no cluster change. NewQuotaManager hands back the same localCache as GetLocalCache, so the webhook check and the per-backend re-check added in #2397 all read this one map. #2386 made the del/add pair atomic, which does not help here: two objects share one slot, this is not a gap in time.
What you expected to happen:
Kubernetes applies every ResourceQuota in a namespace, so a platform baseline quota plus a tenant quota should leave the more restrictive limit in force.
Before I send a patch I would like your call on the shape. Keying the limit side by object name and recomputing the minimum on add and delete is contained, but Spec.Scopes and Spec.ScopeSelector are not read anywhere in the tree, so a minimum taken over all objects would let a scoped quota restrict pods outside its scope. Happy to write it either way.
How to reproduce it (as minimally and precisely as possible):
Two ResourceQuota objects in one namespace, both with a limits.<hami memory resource> entry and different values.
Environment:
- HAMi version: master (
b5ec6b1)
What happened:
QuotaManagerkeys limits by namespace and device resource name only (pkg/device/quota.go:38-40), with nothing recording which ResourceQuota object set them.addQuotaLockedwrites straight into the slot anddelQuotaLockedclears it unconditionally, so with two objects in one namespace that both setlimits.hami.io/gpumem:That is master (
b5ec6b1), driven throughs.onAddQuota/s.onDelQuotaand thenfitResourceQuota. Which object wins is informer delivery order, so it can flip on a scheduler restart with no cluster change.NewQuotaManagerhands back the samelocalCacheasGetLocalCache, so the webhook check and the per-backend re-check added in #2397 all read this one map. #2386 made the del/add pair atomic, which does not help here: two objects share one slot, this is not a gap in time.What you expected to happen:
Kubernetes applies every ResourceQuota in a namespace, so a platform baseline quota plus a tenant quota should leave the more restrictive limit in force.
Before I send a patch I would like your call on the shape. Keying the limit side by object name and recomputing the minimum on add and delete is contained, but
Spec.ScopesandSpec.ScopeSelectorare not read anywhere in the tree, so a minimum taken over all objects would let a scoped quota restrict pods outside its scope. Happy to write it either way.How to reproduce it (as minimally and precisely as possible):
Two ResourceQuota objects in one namespace, both with a
limits.<hami memory resource>entry and different values.Environment:
b5ec6b1)