diff --git a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go index 09bc67fa5..32992942b 100644 --- a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go +++ b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/go-logr/logr" "math" + "slices" "time" api "github.com/cobaltcore-dev/cortex/api/external/nova" @@ -29,6 +30,30 @@ type FilterQuotaEnforcementOpts struct { // enforcement, and so that newly-rolled-out pipelines never silently start // rejecting requests. Enforce bool `json:"enforce,omitempty"` + + // CRStateFilter selects which CommittedResource states count toward the CR + // portion of quota, i.e. which CRs are deducted when computing PAYG headroom + // (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). + 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. +func (o FilterQuotaEnforcementOpts) crStateIncluded(state v1alpha1.CommitmentStatus) bool { + states := o.CRStateFilter + if len(states) == 0 { + states = defaultCRStateFilter + } + return slices.Contains(states, state) } func (FilterQuotaEnforcementOpts) Validate() error { return nil } @@ -199,7 +224,8 @@ func (s *FilterQuotaEnforcement) Run(traceLog logr.Logger, request api.ExternalS spec = cr.Status.AcceptedSpec } - // Must match project, AZ, flavor group, resource type, and be active. + // Must match project, AZ, flavor group, resource type, and be in a + // quota-counting state (see Options.CRStateFilter). if spec.ProjectID != projectID { continue } @@ -212,7 +238,7 @@ func (s *FilterQuotaEnforcement) Run(traceLog logr.Logger, request api.ExternalS if spec.ResourceType != v1alpha1.CommittedResourceTypeMemory { continue } - if !spec.IsActive() { + if !s.Options.crStateIncluded(spec.State) { continue } now := time.Now() 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 2543c5e20..f646ccdcd 100644 --- a/internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go +++ b/internal/scheduling/nova/plugins/filters/filter_quota_enforcement_test.go @@ -57,6 +57,8 @@ func TestFilterQuotaEnforcement_Run(t *testing.T) { objects []client.Object request api.ExternalSchedulerRequest enforce bool + // crStateFilter sets Options.CRStateFilter; nil exercises the default (confirmed). + crStateFilter []v1alpha1.CommitmentStatus expectAccept bool @@ -98,7 +100,7 @@ func TestFilterQuotaEnforcement_Run(t *testing.T) { expectFG: "hana_v2", }, { - name: "ACCEPT: CR has exact headroom (guaranteed state)", + name: "ACCEPT: CR has exact headroom (confirmed state)", objects: []client.Object{ &v1alpha1.CommittedResource{ ObjectMeta: metav1.ObjectMeta{Name: "cr-1"}, @@ -107,7 +109,7 @@ func TestFilterQuotaEnforcement_Run(t *testing.T) { AvailabilityZone: "az-1", FlavorGroupName: "hana_v2", ResourceType: v1alpha1.CommittedResourceTypeMemory, - State: v1alpha1.CommitmentStatusGuaranteed, + State: v1alpha1.CommitmentStatusConfirmed, Amount: resource.MustParse("100Gi"), }, Status: v1alpha1.CommittedResourceStatus{ @@ -443,6 +445,87 @@ func TestFilterQuotaEnforcement_Run(t *testing.T) { expectAZ: "az-1", expectFG: "hana_v2", }, + { + name: "confirmed CR is deducted from PAYG headroom (reject)", + objects: []client.Object{ + &v1alpha1.CommittedResource{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-confirmed-full"}, + Spec: v1alpha1.CommittedResourceSpec{ + ProjectID: "project-1", + AvailabilityZone: "az-1", + FlavorGroupName: "hana_v2", + ResourceType: v1alpha1.CommittedResourceTypeMemory, + State: v1alpha1.CommitmentStatusConfirmed, + Amount: resource.MustParse("100Gi"), + }, + // Fully used, so it provides no CR headroom itself — the request + // falls through to the PAYG check where its amount is deducted. + Status: v1alpha1.CommittedResourceStatus{ + UsedResources: map[string]resource.Quantity{"memory": resource.MustParse("100Gi")}, + }, + }, + &v1alpha1.ProjectQuota{ + ObjectMeta: metav1.ObjectMeta{Name: "quota-project-1-az-1"}, + Spec: v1alpha1.ProjectQuotaSpec{ + ProjectID: "project-1", + AvailabilityZone: "az-1", + Quota: map[string]int64{"hw_version_hana_v2_ram": 150}, + }, + Status: v1alpha1.ProjectQuotaStatus{ + PaygUsage: map[string]int64{"hw_version_hana_v2_ram": 45}, + }, + }, + }, + // headroom = 150 - 100 (confirmed CR) - 45 = 5 < 10 requested → reject. + request: makeQuotaEnforcementRequest("project-1", "az-1", "hana_v2", 10240, 1, nil), + expectAccept: false, + enforce: true, + expectMode: "enforce", + expectDecision: "reject", + expectResource: "ram", + expectAZ: "az-1", + expectFG: "hana_v2", + }, + { + name: "guaranteed CR is excluded from PAYG headroom under default filter (accept)", + objects: []client.Object{ + &v1alpha1.CommittedResource{ + ObjectMeta: metav1.ObjectMeta{Name: "cr-guaranteed-full"}, + Spec: v1alpha1.CommittedResourceSpec{ + ProjectID: "project-1", + AvailabilityZone: "az-1", + FlavorGroupName: "hana_v2", + ResourceType: v1alpha1.CommittedResourceTypeMemory, + State: v1alpha1.CommitmentStatusGuaranteed, + Amount: resource.MustParse("100Gi"), + }, + Status: v1alpha1.CommittedResourceStatus{ + UsedResources: map[string]resource.Quantity{"memory": resource.MustParse("100Gi")}, + }, + }, + &v1alpha1.ProjectQuota{ + ObjectMeta: metav1.ObjectMeta{Name: "quota-project-1-az-1"}, + Spec: v1alpha1.ProjectQuotaSpec{ + ProjectID: "project-1", + AvailabilityZone: "az-1", + Quota: map[string]int64{"hw_version_hana_v2_ram": 150}, + }, + Status: v1alpha1.ProjectQuotaStatus{ + PaygUsage: map[string]int64{"hw_version_hana_v2_ram": 45}, + }, + }, + }, + // Default filter (confirmed only) excludes the guaranteed CR: + // headroom = 150 - 0 - 45 = 105 >= 10 requested → accept via PAYG. + request: makeQuotaEnforcementRequest("project-1", "az-1", "hana_v2", 10240, 1, nil), + expectAccept: true, + enforce: true, + expectMode: "enforce", + expectDecision: "accept_payg", + expectResource: "", + expectAZ: "az-1", + expectFG: "hana_v2", + }, { name: "ACCEPT: multiple instances, enough PAYG headroom", objects: []client.Object{ @@ -906,6 +989,7 @@ func TestFilterQuotaEnforcement_Run(t *testing.T) { filter := &FilterQuotaEnforcement{} filter.Client = fakeClient filter.Options.Enforce = tt.enforce + filter.Options.CRStateFilter = tt.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 03e9378cd..ae99d3805 100644 --- a/internal/scheduling/reservations/quota/config.go +++ b/internal/scheduling/reservations/quota/config.go @@ -18,7 +18,7 @@ type QuotaControllerConfig struct { FullReconcileInterval metav1.Duration `json:"fullReconcileInterval"` // CRStateFilter defines which CommittedResource states to include - // when summing cr_actual_usage. Default: ["confirmed", "guaranteed"] + // when summing cr_actual_usage. Default: ["confirmed"] CRStateFilter []v1alpha1.CommitmentStatus `json:"crStateFilter"` // EnableHVDiff enables incremental TotalUsage updates via HV instance diffs. @@ -60,7 +60,6 @@ func DefaultQuotaControllerConfig() QuotaControllerConfig { FullReconcileInterval: metav1.Duration{Duration: 5 * time.Minute}, CRStateFilter: []v1alpha1.CommitmentStatus{ v1alpha1.CommitmentStatusConfirmed, - v1alpha1.CommitmentStatusGuaranteed, }, EnableHVDiff: &enableHVDiff, } diff --git a/internal/scheduling/reservations/quota/controller_test.go b/internal/scheduling/reservations/quota/controller_test.go index a5e5c937b..91a77eb92 100644 --- a/internal/scheduling/reservations/quota/controller_test.go +++ b/internal/scheduling/reservations/quota/controller_test.go @@ -175,7 +175,7 @@ func TestComputeCRUsage(t *testing.T) { FlavorGroupName: "hana_v2", AvailabilityZone: "az-1", ResourceType: v1alpha1.CommittedResourceTypeMemory, - State: v1alpha1.CommitmentStatusGuaranteed, + State: v1alpha1.CommitmentStatusConfirmed, }, Status: v1alpha1.CommittedResourceStatus{ UsedResources: map[string]resource.Quantity{"memory": resource.MustParse("3Gi")}, @@ -225,7 +225,7 @@ func TestComputeCRUsage(t *testing.T) { crsByProject := groupCRsByProject(allCRs) result := ctrl.computeCRUsage(crsByProject["project-a"], testFlavorGroups) - // Should include confirmed + guaranteed for project-a only + // Should include confirmed CRs for project-a only ramUsage := result["hw_version_hana_v2_ram"] if ramUsage["az-1"] != 8 { // 5 + 3 t.Errorf("expected cr ram usage az-1 = 8, got %d", ramUsage["az-1"]) @@ -395,8 +395,8 @@ func TestIsCRStateIncluded(t *testing.T) { if !ctrl.isCRStateIncluded(v1alpha1.CommitmentStatusConfirmed) { t.Error("expected confirmed to be included") } - if !ctrl.isCRStateIncluded(v1alpha1.CommitmentStatusGuaranteed) { - t.Error("expected guaranteed to be included") + if ctrl.isCRStateIncluded(v1alpha1.CommitmentStatusGuaranteed) { + t.Error("expected guaranteed to NOT be included") } if ctrl.isCRStateIncluded(v1alpha1.CommitmentStatusPending) { t.Error("expected pending to NOT be included")