[alien1403]

What type of PR is this?

/kind feature
/area was
/area tas

What this PR does / why we need it?

Introduces virtual pod synthesis and snapshot integration for the WAS simulator. Admitted workloads without running pods are represented as virtual pods based on their PodSets and TAS TopologyAssignments, deduplicated against real pods from the podTracker to accurately reflect node resource usage during feasibility and preemption simulations.

flowchart TD
    %% Step 1: Inputs
    subgraph S1 ["1. Inputs to Simulator"]
        WLs["Assumed Workloads<br/>(with TopologyAssignment)"]
        RealPods["Real Pods in Cluster"]
        Nodes["Cluster Nodes"]
    end

    %% Step 2: Virtual Pod Generation
    subgraph S2 ["2. Generate Virtual Pods (virtual_pods.go)"]
        direction TB
        VPG["PodsForWorkload(workload)"]
        VPG --> Details["• Copy PodSet Template & Namespace<br/>• Set Spec.NodeName from TopologyAssignment<br/>• Generate deterministic truncated name"]
    end

    %% Step 3: Deduplication
    subgraph S3 ["3. Deduplicate Pods (scheduling_simulator.go)"]
        Merge["Combine: Take virtual pods if the workload has any,<br/>otherwise keep real pods"]
    end

    %% Step 4: Snapshot
    subgraph S4 ["4. Simulator Snapshot"]
        Snap["cache.NewSnapshot(CombinedPods, Nodes)<br/>(Accurate node reservations & preemption support)"]
    end

    %% Flow connections
    WLs --> VPG
    Details --> Merge
    RealPods --> Merge
    Merge --> Snap
    Nodes --> Snap

Loading

Part of #13700

Useful notes for your reviewer

Release note

WorkloadAwareScheduler: Scheduling simulations now include resources reserved by Workloads with TAS assignments before their Pods are scheduled. Real Pods and simulated Pods for the same Workload are not counted twice. Requires the SchedulerLibraryIntegration feature gate.

AI summary

The WAS simulator now synthesizes deterministic virtual pods for admitted or quota-reserved workloads from TAS topology assignments. It uses virtual pods in resource, feasibility, and preemption simulations, while retaining real pods when generation fails.

Suggested release note

WorkloadAwareScheduler: Added virtual pod synthesis for admitted workloads in scheduling simulations.

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	6db329c
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6ab27bb43d0efe0008d5deeb

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: kubernetes-sigs/kueue/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: dd7b4548-5818-46b1-94df-ad946c809415

📥 Commits

Reviewing files that changed from the base of the PR and between 8346b4c and c99d379.

📒 Files selected for processing (2)

• pkg/cache/scheduler/was/pod_tracker.go
• pkg/cache/scheduler/was/scheduling_simulator.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/cache/scheduler/was/scheduling_simulator.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The snapshot API now accepts workloads. Topology-aware snapshots collect admitted workloads, generate virtual Pods from topology assignments, and include those Pods in WAS scheduling state. Tests cover virtual Pod placement, deduplication, host-port preemption, and snapshot behavior.

Changes

Virtual workload scheduling

Layer / File(s)	Summary
Snapshot contract and workload wiring pkg/cache/scheduler/..., pkg/scheduler/scheduler_tas_test.go	SchedulingSimulator.Snapshot now accepts workloads. Topology-aware cache snapshots pass admitted workloads, while other callers pass nil where needed.
Virtual pod generation pkg/cache/scheduler/was/virtual_pods.go, pkg/cache/scheduler/was/virtual_pods_test.go	Added virtual Pod name generation and workload conversion from TAS assignments. Generated Pods include topology nodes, workload metadata, labels, annotations, running status, and preserved container ports.
Simulator virtual-pod integration and validation pkg/cache/scheduler/was/pod_tracker.go, pkg/cache/scheduler/was/scheduling_simulator.go, pkg/cache/scheduler/was/scheduling_simulator_test.go	The WAS simulator copies tracker state, replaces tracked workload Pods with generated virtual Pods, converts keyed Pod state to slices, and validates host-port reservations, deduplication, preemption, and revert behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant CacheSnapshot
  participant WASSnapshot
  participant PodsForWorkload
  participant SchedulingSnapshot
  CacheSnapshot->>WASSnapshot: pass nodes and assumed workloads
  WASSnapshot->>PodsForWorkload: generate virtual Pods for each admitted workload
  PodsForWorkload-->>WASSnapshot: return topology-assigned virtual Pods
  WASSnapshot->>SchedulingSnapshot: replace matching tracked Pods and add virtual Pods

Loading

Merge Risk: 🟡 Moderate · up to c99d3

Workloads with partially representable topology may lose real Pods from scheduling simulations, causing resource and preemption decisions to be inaccurate. Preserve those Pods or require complete virtual coverage before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 11 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main changes: adding virtual pod generation and integrating it with the WAS simulator snapshot flow.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[alien1403]

/cc olekzabl

[olekzabl]

I think there's no point in virtualizing already-admitted Pods without a known node.
Because, for those, WAS library won't handle them reasonably without knowing where they are.

That's why, when !hasNode, I'd return nil right away.
The benefit of this would be that, if [almost] all Pods are already existing & scheduled, we'll let the real Pods into the snapshot, leading to a more precise simulation.

(For the code to be less surprising, I'd even make this check outside this loop.
That is: check if levels are not empty, and if IsLowestLevelHostname(levels).
Otherwise return nil even without this loop.
Then, in the loop, we can expect hasNode to be true. If it isn't, you could log some error, and then return nil)

[olekzabl]

Ouch, but we also have an outer loop. for _, psa := ...
Which indicates a flaw in my thinking so far: "virtualizability" actually applies per PodSet, not per Workload.

For example, in 1 Workload we may have PodSet A with node-level TAS, PodSet B with rack-level TAS, and PodSet C with no TAS at all (because they all got assigned different ResourceFlavors).
And then, ideally, we'd try to virtualize A while taking real Pods for B & C.

I think we don't need to address this in this PR.
Right now, we're focused on TPU critical path, where we agreed to make assumption that everything is TAS, with node-level Topology (whenever it simplifies the code).
So this assumption seems to apply nicely here.

Still, I'm leaving this comment to document a post-MVP follow-up action item.

[olekzabl]

This is going to be useful for currently-considered workload - but not for already-admitted ones.
(Cf. this comment).

I'd suggest keeping this out of this PR, to prevent confusion.

[olekzabl]

A Job of size 5 may have 2 of its Pods already terminated (yet, still retained for visibility).

This is a follow-up to this comment but a different story because here the whole Workload is still not Finished.

And this case is more problematic. The TopologyAssignment will still have all 5 Pods present - possibly assigned to different nodes - and Workload.Status.ReclaimablePods will:

• tell us that 2 Pods already completed
• tell us which PodSet they belong to
• not tell us which nodes specifically have been freed.

AFAIU even the current TAS logic does not deal with this particularly well.
Kueue TAS caches will "block" all the 5 nodes even if 2 of them have been freed.

So I'd propose to ignore this problem in this PR - but keep this in mind when planning follow-up work.

[olekzabl]

While your diagram in the PR description is generally nice - thank you for it! - I find 2 places confusing:

• From box # 2, we have two successors in box # 3:

  Track Virtual pods in map for preemption

  Combine: Other cluster pods + Virtual pods

  I don't get why they both exist. For me, it's just a single mechanism.
  The virtual Pods are only combined with the real ones, I don't see any additional "tracking" done for them.
  (Or did you mean this? But then tell me why do this :)

• When you say

  Filter: Drop real pods of admitted workloads

  This is almost right - but with the exception that we don't drop real Pods for the admitted Workloads that we've failed to virtualize (code). And I'd say rightly so, see this comment.

  So, I think I'd reduce box # 3 to a single blue item, saying e.g.

  Combine:
  Take virtual pods if the workload has any,
  otherwise take real pods

WDYT?

[olekzabl]

/lgtm

[kubernetes-prow]

LGTM label has been added.

Details

Git tree hash: beb68ab7557e42db4d2d2eb61736407d1d1d76bc

[alien1403]

/cc mimowo

[mimowo]

Could you maybe move the assumedWorkloads param to an option and follow the variadic option pattern? To avoid passing nil in many places which don't need this param.

So I imagine the SnapshotOption new struct.

[alien1403]

I will do the cleanup in a separate PR, thanks!

[mimowo]

extract getting the assumed workloads to a helper function

[mimowo]

/approve
My comments aren't blocking, but it would be good to follow up

@coderabbitai please open a cleanup issue to address my comments.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alien1403, mimowo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mimowo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mimowo]

@coderabbitai please open a cleanup issue to address my comments.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -HI '^AGENTS\.md$' . -x sh -c 'echo "--- $1"; cat "$1"' sh {}

Length of output: 3824

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
cat cmd/experimental/skills/kueue-pr-issue-workflow/SKILL.md

Length of output: 1690

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
fd -HI . .github/ISSUE_TEMPLATE -t f -x sh -c 'echo "--- $1"; cat "$1"' sh {}

Length of output: 8672

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh api repos/kubernetes-sigs/kueue/issues/15720/comments --paginate \
  --jq '.[] | select(.user.login == "mimowo" and (.body | contains("`@coderabbitai` please open a cleanup issue"))) | {html_url, body}'

Length of output: 539

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

body=$(cat <<'EOF'
**What would you like to be cleaned**:

Update the virtual-pod flow diagram in PR `#15720`.

- Combine virtual-pod tracking and real-pod replacement into one step: use virtual pods when generation succeeds for a workload; otherwise use its real pods.
- Remove the statement that real cluster pods have an empty `NodeName`.
- State that real pods can have a populated `NodeName`.

**Why is this needed**:

The diagram must match the simulator behavior. Virtual-pod generation can fail, so the simulator retains real pods in that case. Real pods can already be assigned to nodes.

Requested by `@mimowo`: https://github.com/kubernetes-sigs/kueue/pull/15720#issuecomment-5778615489

Related PR: https://github.com/kubernetes-sigs/kueue/pull/15720

AI usage disclosure: CodeRabbit AI created this issue on behalf of `@mimowo`.
EOF
)

gh issue create \
  --repo kubernetes-sigs/kueue \
  --title "🧹 Clarify virtual-pod simulator diagram" \
  --body "$body" \
  --label "kind/cleanup" \
  --assignee "mimowo"

Length of output: 1241

---

Kueue Development
@​mimowo: Opened and assigned #16007: 🧹 Clarify virtual-pod simulator diagram.

The issue includes the requested diagram corrections, links to PR #15720 and your comment, and the required AI usage disclosure.

You are interacting with an AI system.

[mimowo]

Opened by hand #16008, because CodeRabbit didn't get which comments I was asking it seems.

[mimowo]

/release-note-edit

WorkloadAwareScheduler: Scheduling simulations now include resources reserved by Workloads with TAS assignments before their Pods are scheduled. Real Pods and simulated Pods for the same Workload are not counted twice. Requires the SchedulerLibraryIntegration feature gate.