diff --git a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml index dab714b8c..fd631a303 100644 --- a/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml +++ b/helm/bundles/cortex-nova/templates/pipelines_kvm.yaml @@ -120,6 +120,8 @@ spec: requests without headroom, add `params: [{key: enforce, boolValue: true}]` to this step. The shadow default is intentional so that newly-rolled-out releases never silently start rejecting requests. + params: + - {key: crStateFilter, stringListValue: {{ .Values.kvm.crStateFilter | toJson }}} weighers: - name: kvm_prefer_smaller_hosts params: @@ -293,6 +295,8 @@ spec: requests without headroom, add `params: [{key: enforce, boolValue: true}]` to this step. The shadow default is intentional so that newly-rolled-out releases never silently start rejecting requests. + params: + - {key: crStateFilter, stringListValue: {{ .Values.kvm.crStateFilter | toJson }}} weighers: - name: kvm_prefer_smaller_hosts params: diff --git a/helm/bundles/cortex-nova/values.yaml b/helm/bundles/cortex-nova/values.yaml index 4d2287ab4..0ef1ace0b 100644 --- a/helm/bundles/cortex-nova/values.yaml +++ b/helm/bundles/cortex-nova/values.yaml @@ -109,6 +109,12 @@ kvm: # Domain names that are explicitly treated as non-external-customer domains and # skipped by the filter, even if they match one of the prefixes above. ignoredDomainNames: [] + # CommittedResource states that count toward the CR portion of project quota. + # Shared by the quota controller (PaygUsage derivation) and the + # filter_quota_enforcement pipeline step (CR deduction) — the two MUST agree, + # so this single value feeds both. Empty means no CR states count. + crStateFilter: &crStateFilter + - confirmed cortex: &cortex crd: {enable: false} @@ -118,6 +124,9 @@ cortex: &cortex prometheus: {enable: false} conf: &cortexConf schedulingDomain: nova + # Consumed by the quota controller; mirrors the filter_quota_enforcement + # step's crStateFilter. See kvm.crStateFilter for the shared source. + crStateFilter: *crStateFilter apiservers: home: gvks: diff --git a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go index 697701b6a..c51a0a9e9 100644 --- a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go +++ b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go @@ -38,24 +38,15 @@ type FilterQuotaEnforcementOpts struct { // (headroom = Quota - sum(CR amounts) - PaygUsage). It MUST mirror the quota // controller's crStateFilter (see internal/scheduling/reservations/quota/config.go) // so the filter's CR deduction and the controller's PaygUsage derivation agree - // on what quota already accounts for. Empty means the shared default (confirmed). + // on what quota already accounts for. Empty means no CR states count; the + // deployment supplies the default (confirmed) via Helm. CRStateFilter []v1alpha1.CommitmentStatus `json:"crStateFilter,omitempty"` } -// defaultCRStateFilter mirrors the quota controller's default -// (internal/scheduling/reservations/quota/config.go): confirmed only. -var defaultCRStateFilter = []v1alpha1.CommitmentStatus{ - v1alpha1.CommitmentStatusConfirmed, -} - // crStateIncluded reports whether a CR in the given state counts toward the CR -// portion of quota. An unset CRStateFilter falls back to defaultCRStateFilter. +// portion of quota. func (o FilterQuotaEnforcementOpts) crStateIncluded(state v1alpha1.CommitmentStatus) bool { - states := o.CRStateFilter - if len(states) == 0 { - states = defaultCRStateFilter - } - return slices.Contains(states, state) + return slices.Contains(o.CRStateFilter, state) } func (FilterQuotaEnforcementOpts) Validate() error { return nil } diff --git a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go index bb9b3d856..96b655b99 100644 --- a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go +++ b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go @@ -1007,7 +1007,13 @@ func TestFilterQuotaEnforcement_Run(t *testing.T) { filter := &FilterQuotaEnforcement{} filter.Client = fakeClient filter.Options.Enforce = tt.enforce - filter.Options.CRStateFilter = tt.crStateFilter + // Production supplies the CR state filter via Helm; mirror that + // default here so cases that don't set one exercise confirmed-only. + crStateFilter := tt.crStateFilter + if crStateFilter == nil { + crStateFilter = []v1alpha1.CommitmentStatus{v1alpha1.CommitmentStatusConfirmed} + } + filter.Options.CRStateFilter = crStateFilter result, err := filter.Run(logr.Discard(), tt.request) if err != nil { diff --git a/internal/scheduling/reservations/quota/config.go b/internal/scheduling/reservations/quota/config.go index ae99d3805..02d6bfd08 100644 --- a/internal/scheduling/reservations/quota/config.go +++ b/internal/scheduling/reservations/quota/config.go @@ -18,7 +18,9 @@ type QuotaControllerConfig struct { FullReconcileInterval metav1.Duration `json:"fullReconcileInterval"` // CRStateFilter defines which CommittedResource states to include - // when summing cr_actual_usage. Default: ["confirmed"] + // when summing cr_actual_usage. Empty means no states count; the + // deployment supplies the default (confirmed) via Helm. It MUST mirror + // the filter_quota_enforcement step's crStateFilter. CRStateFilter []v1alpha1.CommitmentStatus `json:"crStateFilter"` // EnableHVDiff enables incremental TotalUsage updates via HV instance diffs. @@ -31,15 +33,14 @@ type QuotaControllerConfig struct { FlavorGroupResourceConfig map[string]commitments.FlavorGroupResourcesConfig `json:"flavorGroupResourceConfig,omitempty"` } -// ApplyDefaults fills in any unset values with defaults. +// ApplyDefaults fills in any unset values with defaults. CRStateFilter is +// intentionally not defaulted here: an empty filter means no states count, and +// the deployment supplies the intended set (confirmed) via Helm. func (c *QuotaControllerConfig) ApplyDefaults() { defaults := DefaultQuotaControllerConfig() if c.FullReconcileInterval.Duration == 0 { c.FullReconcileInterval = defaults.FullReconcileInterval } - if len(c.CRStateFilter) == 0 { - c.CRStateFilter = defaults.CRStateFilter - } if c.EnableHVDiff == nil { c.EnableHVDiff = defaults.EnableHVDiff } @@ -53,14 +54,12 @@ func (c *QuotaControllerConfig) IsHVDiffEnabled() bool { return *c.EnableHVDiff } -// DefaultQuotaControllerConfig returns a default configuration. +// DefaultQuotaControllerConfig returns a default configuration. CRStateFilter +// has no code-level default; the deployment supplies it via Helm. func DefaultQuotaControllerConfig() QuotaControllerConfig { enableHVDiff := true return QuotaControllerConfig{ FullReconcileInterval: metav1.Duration{Duration: 5 * time.Minute}, - CRStateFilter: []v1alpha1.CommitmentStatus{ - v1alpha1.CommitmentStatusConfirmed, - }, - EnableHVDiff: &enableHVDiff, + EnableHVDiff: &enableHVDiff, } } diff --git a/internal/scheduling/reservations/quota/controller_test.go b/internal/scheduling/reservations/quota/controller_test.go index 17104231d..ac7eb4f6d 100644 --- a/internal/scheduling/reservations/quota/controller_test.go +++ b/internal/scheduling/reservations/quota/controller_test.go @@ -18,8 +18,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +// testQuotaConfig returns a controller config mirroring the production +// deployment, where Helm supplies the CR state filter (confirmed). +func testQuotaConfig() QuotaControllerConfig { + c := DefaultQuotaControllerConfig() + c.CRStateFilter = []v1alpha1.CommitmentStatus{v1alpha1.CommitmentStatusConfirmed} + return c +} + func TestComputeTotalUsage(t *testing.T) { - ctrl := &QuotaController{Config: DefaultQuotaControllerConfig()} + ctrl := &QuotaController{Config: testQuotaConfig()} flavorGroups := map[string]compute.FlavorGroupFeature{ "hana_v2": { @@ -146,7 +154,7 @@ func TestComputeTotalUsage(t *testing.T) { } func TestComputeCRUsage(t *testing.T) { - ctrl := &QuotaController{Config: DefaultQuotaControllerConfig()} + ctrl := &QuotaController{Config: testQuotaConfig()} // Flavor groups — MemoryMB value is no longer used for unit conversion (now fixed at 1 GiB). testFlavorGroups := map[string]compute.FlavorGroupFeature{ @@ -390,7 +398,7 @@ func TestIncrementDecrementUsage(t *testing.T) { } func TestIsCRStateIncluded(t *testing.T) { - ctrl := &QuotaController{Config: DefaultQuotaControllerConfig()} + ctrl := &QuotaController{Config: testQuotaConfig()} if !ctrl.isCRStateIncluded(v1alpha1.CommitmentStatusConfirmed) { t.Error("expected confirmed to be included") @@ -449,7 +457,7 @@ func TestUsageDelta(t *testing.T) { func TestReconcile_NilTotalUsage(t *testing.T) { // When TotalUsage is nil, Reconcile should skip and return no error. // This validates the early-return branch logic used in Reconcile(). - ctrl := &QuotaController{Config: DefaultQuotaControllerConfig()} + ctrl := &QuotaController{Config: testQuotaConfig()} // computeCRUsage on nil slice should return empty map (no panic) result := ctrl.computeCRUsage(nil, nil) @@ -466,7 +474,7 @@ func TestReconcile_NilTotalUsage(t *testing.T) { func TestAccumulateAddedVM_UnknownFlavor(t *testing.T) { // Verifies that accumulateAddedVM gracefully handles a VM with an unknown flavor - ctrl := &QuotaController{Config: DefaultQuotaControllerConfig()} + ctrl := &QuotaController{Config: testQuotaConfig()} flavorGroups := map[string]compute.FlavorGroupFeature{ "hana_v2": { @@ -529,7 +537,7 @@ func TestAccumulateAddedVM_KnownFlavor(t *testing.T) { qc := &QuotaController{ Client: k8sClient, - Config: DefaultQuotaControllerConfig(), + Config: testQuotaConfig(), } flavorGroups := map[string]compute.FlavorGroupFeature{ diff --git a/internal/scheduling/reservations/quota/integration_test.go b/internal/scheduling/reservations/quota/integration_test.go index 6314034d9..e1c0e259a 100644 --- a/internal/scheduling/reservations/quota/integration_test.go +++ b/internal/scheduling/reservations/quota/integration_test.go @@ -1135,7 +1135,7 @@ func newIntegrationTestEnv(t *testing.T, tc IntegrationTestCase) *integrationTes controller := &QuotaController{ Client: k8sClient, VMSource: vmSrc, - Config: DefaultQuotaControllerConfig(), + Config: testQuotaConfig(), Metrics: NewQuotaMetrics(nil), // no-op metrics }