refactor(scheduler): delegate effective-request accounting to k8s.io/component-helpers

[gshaibi]

Description

Prototype / discussion PR — targets the #2021 branch, not main.

Replaces the hand-rolled KEP-1287 effective-request accounting introduced in #2021 with the upstream implementation in k8s.io/component-helpers/resource, already a direct dependency (go.mod, v0.35.4).

Upstream's determineEffectiveRequests is semantically identical to what #2021 implements:

func determineEffectiveRequests(pod *v1.Pod, rs *ResourceState) v1.ResourceList {
	if IsPodResizeInfeasible(pod) {
		return max(rs.Actuated, rs.Allocated)
	}
	return max(rs.Spec, rs.Actuated, rs.Allocated)
}

and AggregateContainerRequests already implements the full sidecar formula (KEP-753) — the restartable-init running sum and the init-phase peak — with effective requests applied.

1. Scheduler side

Removed	Replaced by
effective_requests.go (whole file, 3 funcs)	upstream
getPodResourceWithoutInitContainers	AggregateContainerRequests
initContainerEffects	same call
logIfErr (became dead)	—

sidecarSum and initPhasePeak were never consumed separately — getPodResourceRequest recombined them immediately with Add + SetMaxResource, which is exactly how upstream folds them internally. So the split served no purpose.

2. Admission side

podResizeDelta + accumulateDeltaSums (~130 lines) collapse to three aggregate calls:

newSpec      := resourcehelpers.AggregateContainerRequests(newPod, specOnly)
oldSpec      := resourcehelpers.AggregateContainerRequests(oldPod, specOnly)
effectiveOld := resourcehelpers.AggregateContainerRequests(oldPod, withStatus)

The UseStatusResources flag is the spec/effective distinction. Both the scheduler and the webhook now call the same function, so the invariant "the delta baseline equals what the queue charges" holds by construction rather than by parallel hand-written logic. All three review findings on #2021's podResizeDelta came from exactly that drift.

The webhook's duplicated isPodResizeInfeasible is also gone, in favour of resource.IsPodResizeInfeasible.

This also fixes a latent bug in #2021

New test TestPodResizeValidator_InitPeakDominates_NoDelta covers a pod whose queue charge is dominated by the init-phase peak rather than steady state:

• ordinary init container 10 CPU; regular container 1 → 2 CPU; queue limit 10, allocated 10
• pod charge = max(steady, initPeak) = max(1,10) = 10 before, max(2,10) = 10 after — unchanged

On the #2021 branch this test fails:

Error: Should be empty, but was map[cpu:{{1 0}...}]
Error: Should be true — queue charge is unchanged, so the resize must be admitted at the limit

The per-container sum never sees the max(), so it invents a 1-CPU delta and rejects a resize that costs the queue nothing. With upstream aggregates the delta is empty and the resize is admitted.

If this refactor is not taken, that test is worth cherry-picking to #2021 as a known-failing case — the bug exists there today.

Semantic change: the Status guard

Upstream keys off Reason alone:

if condition.Type == v1.PodResizePending {
	return condition.Reason == v1.PodReasonInfeasible   // no Status check
}

#2021 added a Status == ConditionTrue guard, which does not survive the move. Decision taken: accept upstream semantics. The kubelet deletes the condition rather than setting it False, so the case is not expected to be reachable.

Rather than dropping the coverage, TestIsPodResizeInfeasible now pins upstream's behaviour as a characterization test — if upstream ever tightens the check, it fails and the change surfaces instead of passing silently.

Related Issues

Follow-up to #2021. Not for independent merge.

Checklist

• Self-reviewed
• Added/updated tests (if needed)
• Updated documentation (if needed)
• Added a changelog fragment via make changelog (or applied the skip-changelog label)

Refactor; the one semantic change is documented above. Labelled skip-changelog.

Breaking Changes

None intended. The Status guard difference is the only behavioural change, described above.

Additional Notes

Test results: full suite green.

• 19/19 existing webhook tests pass unmodified
• TestGetPodResourceRequest passes unchanged — covers init containers, sidecars and overhead end-to-end, and confirms the RequirementsFromResourceList round-trip
• new coverage: sidecar mid-downsize (scheduler), init-peak-dominated resize (admission)

effective_requests_test.go was retargeted from the deleted unexported helpers onto the public aggregation path, so it guards against drift if the dependency is bumped. TestGetPodResourceWithoutInitContainers was deleted — it tested a decomposition that no longer exists and is subsumed by TestGetPodResourceRequest.

Not verified: the GPU round-trip is exercised only via CPU/memory cases. If fraction-GPU pods route through getPodResourceRequest in a way those cases don't cover, that needs a targeted check before this merges anywhere.