fix(autoscaling): Skip per-pod metric division in replica calculator when replicas is 0 to avoid overflow/underflow

Project: kubernetes/kubernetes | Language: Go | Concepts: error-handling, refactoring, testing-strategy | Difficulty: mid
Link: https://github.com/kubernetes/kubernetes/pull/140958

The problem

When the Kubernetes Horizontal Pod Autoscaler (HPA) calculates per-pod metrics, it divides the total usage by the number of replicas. If the number of replicas is zero, this results in a division by zero, causing integer overflow or underflow. This could lead to nonsensical metric values and potentially unstable autoscaling behavior.

Why it's non-trivial

A naive fix might just skip the calculation or return a default value, but the function is used in multiple places, and the result must be consistent and safe for further calculations. The fix must also handle overflows and underflows robustly, and ensure that changes are covered by tests to prevent regressions.

The approach

1. Refactor per-pod usage calculation into a helper function

The calculation for per-pod usage is extracted into a new helper, getPerPodUsage, to centralize the logic and ensure consistent handling of edge cases:

// getPerPodUsage calculates the per-pod usage based on total usage and replica count,
// returning MaxInt64 on overflow to prevent wrapping and 0 when replicas is 0 to avoid division by zero.
func getPerPodUsage(usage int64, statusReplicas int32) int64 {
	if float64(usage) >= float64(math.MaxInt64) {
		return math.MaxInt64
	}
	if statusReplicas == 0 {
		return 0
	}
	return int64(math.Ceil(float64(usage) / float64(statusReplicas)))
}

• What it does:
  • If usage is at or above math.MaxInt64, it returns math.MaxInt64 to avoid overflow.
  • If statusReplicas is zero, it returns 0 to avoid division by zero and underflow.
  • Otherwise, it performs the division as intended.

2. Replace direct division in replica calculator functions

The direct division is replaced with a call to the new helper, ensuring all usages benefit from the safe logic:

-	usage = int64(math.Ceil(float64(usage) / float64(statusReplicas)))
+	usage = getPerPodUsage(usage, statusReplicas)

• What it does:
  • Ensures that all per-pod usage calculations are safe and consistent.

3. Add and update tests for edge cases

New test cases are added to verify correct behavior when statusReplicas is zero and when usage is at overflow boundaries:

{
	name: "statusReplicas zero does not overflow usage",
	fixture: calcScenario{
		currentReplicas: 0,
		metric:          externalPerPodMetric(20000),
	},
	perPodTargetUsage: 5000,
	expectedReplicas:  4,
	expectedUsage:     0,
},
{
	name: "statusReplicas zero and metric zero does not underflow",
	fixture: calcScenario{
		currentReplicas: 0,
		metric:          externalPerPodMetric(0),
	},
	perPodTargetUsage: 5000,
	expectedReplicas:  0,
	expectedUsage:     0,
},

• What it does:
  • Ensures that when replicas are zero, usage is always zero (no overflow/underflow).
  • Ensures correct behavior for both non-zero and zero usage inputs.

A dedicated unit test for the helper is also added:

func TestGetPerPodUsage(t *testing.T) {
	cases := []struct {
		name           string
		usage          int64
		statusReplicas int32
		expected       int64
	}{
		{
			name:           "zero replicas returns zero",
			usage:          20000,
			statusReplicas: 0,
			expected:       0,
		},
		// ...
	}
	// ...
}

• What it does:
  • Directly tests the helper for all critical edge cases.

From the review thread

Reviewer:

Can the usage be non-zero if statusReplicas=0?

Author:

In transient states yes (when usage gets picked up but the replica is still 0) in those cases it overflows.

If the usage is zero the zero division (usage = 0, replicas = 0) leads to negative underflow (-9223372036854775808m) as shown in https://github.com/kubernetes/kubernetes/issues/140956.

i.e.

	var usage int64 = 0
	var statusReplicas int64 = 0
	usage = int64(math.Ceil(float64(usage) / float64(statusReplicas)))
	fmt.Println(usage) // -9223372036854775808

Reviewer:

TBH I don't know what would be the result of returning inconsistent data from the function (replicas=0, usage >0) so maybe let's try with returning usage = 0.

Author:

Sounds good, I moved the overflow check logic to a separate function and returning 0 ... as this is used in two places.

Takeaways

• Always guard against division by zero, especially in metric calculations that may see transient or edge-case inputs.
• Centralize error-prone logic (like overflow/underflow handling) in a helper function to ensure consistent, safe behavior across the codebase.
• Add explicit tests for edge cases (zero, overflow, underflow) to prevent regressions and clarify intended behavior.