TAS: the final Fits check admits Workloads the assignment path would reject #14437

What happened:

TASFlavorSnapshot.Fits is the last check between a nominated Workload and admission, and it answers a weaker question than the assignment that produced the nomination. Two ways it says yes where the assignment path would have said no.

It never asks about Pod slots. findTopologyAssignment adds resources.OnePodRequest to what each Pod wants, and updateTASUsage records pods against the domain, so the capacity side of the ledger tracks them. The request side doesn't. SinglePodRequests is built by resources.NewRequestsFromPodSpec, which carries only what the containers asked for, and CountIn can constrain on a resource only if the request names it. A node with no Pod slots left still reports room.

It also re-reads untouched capacity for every entry in the list. A Workload with two PodSets in one domain produces two TopologyDomainRequests with the same values, and Fits measures each against the full remaining capacity as if the other weren't there:

for _, domainUsage := range flavorUsage {
	...
	for _, leaf := range leaves {
		remainingCapacity := s.remainingCapacityForLeaf(leaf, false, cachingEnabled)
		fitCount += domainUsage.SinglePodRequests.CountIn(remainingCapacity.Get())

What you expected to happen:

A Workload that fits only because Pod slots went uncounted, or because a sibling PodSet's share of the domain went uncounted, should fail this check.

How to reproduce it (as minimally and precisely as possible):

Both cases in pkg/cache/scheduler, against 01c45cacf, on a topology whose lowest level is the hostname:

package scheduler

import (
	"context"
	"testing"

	corev1 "k8s.io/api/core/v1"
	"k8s.io/apimachinery/pkg/api/resource"

	"sigs.k8s.io/kueue/pkg/resources"
	utiltas "sigs.k8s.io/kueue/pkg/util/tas"
	utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
	testingnode "sigs.k8s.io/kueue/pkg/util/testingjobs/node"
	"sigs.k8s.io/kueue/pkg/workload"
)

func probeSnap(t *testing.T, ctx context.Context, nodes ...*corev1.Node) *TASFlavorSnapshot {
	t.Helper()
	_, log := utiltesting.ContextWithLog(t)
	tasCache := NewTASCache(nil, newDefaultSimulator(), resources.NewResourceFormatter())
	for _, n := range nodes {
		tasCache.SyncNode(n)
	}
	fc := tasCache.NewTASFlavorCache(
		topologyInformation{Levels: []string{treeTestBlockLabel, treeTestRackLabel, corev1.LabelHostname}},
		flavorInformation{TopologyName: "default"},
	)
	s, err := fc.snapshot(ctx, log, nil)
	if err != nil {
		t.Fatalf("snapshot: %v", err)
	}
	return s
}

func probeNode(name string, alloc corev1.ResourceList) *corev1.Node {
	return testingnode.MakeNode(name).
		Label(treeTestBlockLabel, "b1").
		Label(treeTestRackLabel, "r1").
		Label(corev1.LabelHostname, name).
		StatusAllocatable(alloc).Ready().Obj()
}

func TestProbeFitsIgnoresPodSlots(t *testing.T) {
	ctx, _ := utiltesting.ContextWithLog(t)
	s := probeSnap(t, ctx, probeNode("n1", corev1.ResourceList{
		corev1.ResourceCPU:  resource.MustParse("100"),
		corev1.ResourcePods: resource.MustParse("1"),
	}))

	// One Pod already admitted here, so the node's only Pod slot is gone.
	s.updateTASUsage(utiltas.TopologyDomainID("n1"),
		resources.NewRequestsFromMap(map[corev1.ResourceName]int64{corev1.ResourceCPU: 1000}), add, 1)

	usage := workload.TASFlavorUsage{{
		Values:            []string{"n1"},
		SinglePodRequests: resources.NewRequestsFromMap(map[corev1.ResourceName]int64{corev1.ResourceCPU: 1000}),
		Count:             1,
	}}
	t.Logf("Fits() with zero Pod slots left = %v", s.Fits(usage))
}

func TestProbeFitsUsagesDoNotConsume(t *testing.T) {
	ctx, _ := utiltesting.ContextWithLog(t)
	s := probeSnap(t, ctx, probeNode("n1", corev1.ResourceList{
		corev1.ResourceCPU:  resource.MustParse("4"),
		corev1.ResourcePods: resource.MustParse("110"),
	}))

	four := resources.NewRequestsFromMap(map[corev1.ResourceName]int64{corev1.ResourceCPU: 4000})
	one := workload.TASFlavorUsage{{Values: []string{"n1"}, SinglePodRequests: four, Count: 1}}
	two := workload.TASFlavorUsage{
		{Values: []string{"n1"}, SinglePodRequests: four, Count: 1},
		{Values: []string{"n1"}, SinglePodRequests: four, Count: 1},
	}
	t.Logf("one PodSet wanting 4 CPU on a 4 CPU node: Fits=%v", s.Fits(one))
	t.Logf("two PodSets wanting 4 CPU each, same node: Fits=%v", s.Fits(two))
}

Fits() with zero Pod slots left = true

one PodSet wanting 4 CPU on a 4 CPU node: Fits=true
two PodSets wanting 4 CPU each, same node: Fits=true

Anything else we need to know?:

No feature gate is involved. Both come back the same with TASCachingRemainingResources off, and both reproduce whether or not the topology declares the hostname level.

The place it matters is updateAssignmentIfNeeded, where a FitsCheckOk short-circuits before getAssignments runs again:

needsTASRecompute := fitsCheck == schdcache.FitsCheckNoTAS && features.Enabled(features.TASRecomputeAssignmentWithinSchedulingCycle)
...
default:
	// Short-circuit, nothing to recompute.
	return usage, schdcache.FitsCheckOk == fitsCheck

So an assignment nominated against the snapshot at the start of a cycle is admitted on this check alone, and the two cases above are how it can pass while the topology no longer has room. TASRecomputeAssignmentWithinSchedulingCycle is Beta and on since 0.19, which is what makes this check load-bearing rather than advisory.

Adding resources.OnePodRequest to the incoming SinglePodRequests closes the first. The second needs the loop to carry a running deduction rather than reading remainingCapacityForLeaf fresh for every entry, and the deduction has to be per leaf, since which leaf absorbs an entry decides what's left for the next one.

I'd like to send a fix for both if nobody's on it already.

Environment:

• Kubernetes version: not applicable, both cases reproduce in a unit test
• Kueue version (use git describe --tags --dirty --always): v0.20.0-devel-305-g01c45cacf
• Cloud provider or hardware configuration: not applicable
• OS: not applicable
• Kernel: not applicable
• Install tools: not applicable
• Others: TopologyAwareScheduling on, everything else at its default

This issue was written in part with the assistance of generative AI. The reproducer is mine and the output above is what it printed.