Merge pull request #487 from sapcc/limes-fix-base-quota-distribution

fix ACPQ test cases to always take zero-valued inputs in the "any" AZ
sapcc · Jun 20, 2024 · 63e28f0 · 63e28f0
2 parents 636a13a + 51b17ee
commit 63e28f0
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 4 deletions.
diff --git a/internal/datamodel/apply_computed_project_quota.go b/internal/datamodel/apply_computed_project_quota.go
@@ -338,7 +338,7 @@ func acpqComputeQuotas(stats map[limes.AvailabilityZone]clusterAZAllocationStats
 			}
 		}
 	}
-	target.EnforceConstraints(constraints, allAZsInOrder, isProjectResourceID)
+	target.EnforceConstraints(constraints, allAZsInOrder, isProjectResourceID, isAZAware)
 	target.TryFulfillDesired(stats, cfg, allowsQuotaOvercommit)
 
 	// phase 3: try granting desired_quota
@@ -356,7 +356,7 @@ func acpqComputeQuotas(stats map[limes.AvailabilityZone]clusterAZAllocationStats
 			target[az][resourceID].Desired = desiredQuota
 		}
 	}
-	target.EnforceConstraints(constraints, allAZsInOrder, isProjectResourceID)
+	target.EnforceConstraints(constraints, allAZsInOrder, isProjectResourceID, isAZAware)
 	target.TryFulfillDesired(stats, cfg, allowsQuotaOvercommit)
 
 	// phase 4: try granting additional "any" quota until sum of all quotas is ProjectBaseQuota
@@ -375,7 +375,7 @@ func acpqComputeQuotas(stats map[limes.AvailabilityZone]clusterAZAllocationStats
 		if !slices.Contains(allAZsInOrder, limes.AvailabilityZoneAny) {
 			allAZsInOrder = append(allAZsInOrder, limes.AvailabilityZoneAny)
 		}
-		target.EnforceConstraints(constraints, allAZsInOrder, isProjectResourceID)
+		target.EnforceConstraints(constraints, allAZsInOrder, isProjectResourceID, isAZAware)
 		target.TryFulfillDesired(stats, cfg, allowsQuotaOvercommit)
 	}
 
@@ -384,19 +384,26 @@ func acpqComputeQuotas(stats map[limes.AvailabilityZone]clusterAZAllocationStats
 
 // After increasing Desired, but before increasing Allocated, this decreases
 // Desired in order to fit into project-local quota constraints.
-func (target acpqGlobalTarget) EnforceConstraints(constraints map[db.ProjectResourceID]projectLocalQuotaConstraints, allAZs []limes.AvailabilityZone, isProjectResourceID map[db.ProjectResourceID]struct{}) {
+func (target acpqGlobalTarget) EnforceConstraints(constraints map[db.ProjectResourceID]projectLocalQuotaConstraints, allAZs []limes.AvailabilityZone, isProjectResourceID map[db.ProjectResourceID]struct{}, isAZAware bool) {
 	for resourceID, c := range constraints {
 		// raise Allocated as necessary to fulfil minimum quota
 		if c.MinQuota != nil && *c.MinQuota > 0 {
 			totalAllocated := uint64(0)
 			desireScalePerAZ := make(map[limes.AvailabilityZone]uint64)
 			for _, az := range allAZs {
+				// Quota should not be assgined to ANY AZ on AZ aware resources. This causes unusable quota distribution on manual quota overrides.
+				if az == limes.AvailabilityZoneAny && isAZAware {
+					continue
+				}
 				t := target[az][resourceID]
 				totalAllocated += t.Allocated
 				desireScalePerAZ[az] = *c.MinQuota * max(1, subtractOrZero(t.Desired, t.Allocated))
 			}
 			extraAllocatedPerAZ := util.DistributeFairly(subtractOrZero(*c.MinQuota, totalAllocated), desireScalePerAZ)
 			for _, az := range allAZs {
+				if az == limes.AvailabilityZoneAny && isAZAware {
+					continue
+				}
 				target[az][resourceID].Allocated += extraAllocatedPerAZ[az]
 			}
 		}

diff --git a/internal/datamodel/apply_computed_project_quota_test.go b/internal/datamodel/apply_computed_project_quota_test.go
@@ -101,13 +101,27 @@ func TestACPQBasicWithAZAwareness(t *testing.T) {
 			Capacity: 200,
 			ProjectStats: map[db.ProjectResourceID]projectAZAllocationStats{
 				401: constantUsage(20),
+				402: constantUsage(0),
 				403: {Usage: 20, MinHistoricalUsage: 19, MaxHistoricalUsage: 20},
 				404: {Usage: 0, MinHistoricalUsage: 0, MaxHistoricalUsage: 15},
 				405: constantUsage(40),
 				406: constantUsage(0),
 				407: constantUsage(1),
 			},
 		},
+		// The scraper creates empty fallback entries in project_az_resources for AZ "any", so we will always see those in the input, too.
+		"any": {
+			Capacity: 0,
+			ProjectStats: map[db.ProjectResourceID]projectAZAllocationStats{
+				401: {},
+				402: {},
+				403: {},
+				404: {},
+				405: {},
+				406: {},
+				407: {},
+			},
+		},
 	}
 	cfg := core.AutogrowQuotaDistributionConfiguration{
 		GrowthMultiplier: 1.2,
@@ -258,6 +272,17 @@ func TestACPQQuotaOvercommitTurnsOffAboveAllocationThreshold(t *testing.T) {
 				405: constantUsage(0),
 			},
 		},
+		// The scraper creates empty fallback entries in project_az_resources for AZ "any", so we will always see those in the input, too.
+		"any": {
+			Capacity: 0,
+			ProjectStats: map[db.ProjectResourceID]projectAZAllocationStats{
+				401: {},
+				402: {},
+				403: {},
+				404: {},
+				405: {},
+			},
+		},
 	}
 	cfg := core.AutogrowQuotaDistributionConfiguration{
 		GrowthMultiplier: 1.2,
@@ -321,6 +346,14 @@ func TestACPQWithProjectLocalQuotaConstraints(t *testing.T) {
 				402: {Usage: 40, MinHistoricalUsage: 40, MaxHistoricalUsage: 60},
 			},
 		},
+		// The scraper creates empty fallback entries in project_az_resources for AZ "any", so we will always see those in the input, too.
+		"any": {
+			Capacity: 0,
+			ProjectStats: map[db.ProjectResourceID]projectAZAllocationStats{
+				401: {},
+				402: {},
+			},
+		},
 	}
 	cfg := core.AutogrowQuotaDistributionConfiguration{
 		GrowthMultiplier: 1.2,
@@ -446,6 +479,16 @@ func TestEmptyRegionDoesNotPrecludeQuotaOvercommit(t *testing.T) {
 				405: constantUsage(0),
 			},
 		},
+		"any": {
+			Capacity: 0,
+			ProjectStats: map[db.ProjectResourceID]projectAZAllocationStats{
+				401: {},
+				402: {},
+				403: {},
+				404: {},
+				405: {},
+			},
+		},
 	}
 	// Quota overcommit should always be allowed.
 	cfg := core.AutogrowQuotaDistributionConfiguration{