fix(device): hand out copies from PodManager instead of stored pointers

[Lakshya77089]

What type of PR is this?

/kind bug

What this PR does / why we need it:

PodManager.GetScheduledPods copies the map but returns the same *PodInfo
pointers it stores:

// Return a shallow copy of the pods map to avoid race conditions.
// This prevents a "concurrent map iteration and map write" fatal error.
podsCopy := make(map[k8stypes.UID]*PodInfo, podCount)
maps.Copy(podsCopy, m.pods)

That does keep callers off the manager's own map, but AddPod writes to the
stored structs in place:

m.pods[pod.UID].Devices = devices   // pods.go:72

and UpdatePod does the same with pi.Pod.

The only caller is the Prometheus collector in cmd/scheduler/metrics.go, which
runs on the scrape goroutine and ranges over val.Devices and reads
val.Namespace after GetScheduledPods has returned and the read lock is
gone. The pod informer calls AddPod on every pod add and update. The pointer
handoff is synchronised; the field reads are not.

go test ./pkg/device/ --race reports it on master:

WARNING: DATA RACE
Write at 0x00c000243a18 by goroutine 10:
  (*PodManager).AddPod()   pkg/device/pods.go:72
Previous read at 0x00c000243a18 by goroutine 11:
  ... ranging val.Devices, as the collector does

with a second report on the devices map itself, mapIterStart against
mapassign_faststr.

The fix

Copy the entries as well as the map, reusing the PodInfo.DeepCopy that
ListPodsInfo in the same file already relies on. GetPod gets the same
treatment: it has no non-test callers today, but it returns the stored pointer
in exactly the same way and is the same trap for the next caller.

This mirrors #2333, which made nodeManager.GetNode return a deep copy for the
node cache.

Tests

TestGetScheduledPodsCopiesEntries drives AddPod on one goroutine while a
second does what the collector does. TestGetScheduledPodsReturnsDetachedEntries
and TestGetPodReturnsDetachedCopy mutate what was handed back and assert the
manager's own state is untouched. All three fail on master, the first with five
DATA RACE reports, and pass here.

Which issue(s) this PR fixes:
Fixes #2471

Special notes for your reviewer:

The cost is a deep copy per scrape rather than a map copy. ListPodsInfo
already pays exactly that, so this is not a new class of cost, but it is worth
naming: on a large cluster the collector now copies every tracked pod's device
list once per scrape interval.

The cheaper alternative is to hold the read lock while the collector builds its
metric values, which avoids the copy but holds the lock across metric
construction. I went with the copy because it matches ListPodsInfo and #2333,
and keeps the locking discipline inside the manager rather than spreading it to
callers. Happy to switch if you would rather have the other trade.

Not validated on real hardware. The change is confined to the scheduler's
in-memory pod cache and is covered by unit tests, which CONTRIBUTING allows for
scheduler-scoped changes.

Does this PR introduce a user-facing change?:

NONE

---

AI assistance disclosure: this change was developed with Claude Code — spotting
the race, the fix, and the tests. Flagging the extent up front per CONTRIBUTING.

Summary by CodeRabbit

• Bug Fixes
  • Improved pod data retrieval to prevent callers from unintentionally modifying cached state.
  • Added safer handling for concurrent pod updates and scheduled-pod reads.
  • Missing pod lookups continue to be handled reliably.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ca8f720d-d5bf-4809-b752-74c78cfb8367

📥 Commits

Reviewing files that changed from the base of the PR and between 3616313 and 4b4b849.

📒 Files selected for processing (2)

• pkg/device/pod_test.go
• pkg/device/pods.go

---

📝 Walkthrough

Walkthrough

PodManager.GetPod and GetScheduledPods now return deep-copied pod data. Tests verify detached results, missing-pod behavior, caller mutation isolation, and concurrent AddPod updates.

Changes

Pod snapshot isolation

Layer / File(s)	Summary
Detached getter snapshots pkg/device/pods.go	GetPod returns a deep copy for existing pods and (nil, false) for missing pods. GetScheduledPods deep-copies each PodInfo entry.
Snapshot isolation validation pkg/device/pod_test.go	Tests verify concurrent updates, detached scheduled-pod entries, detached single-pod results, and missing-pod behavior.

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

Possibly related issues

• Issue 2163: It also covers deep-copying GetScheduledPods results to isolate cached PodInfo state.

Possibly related PRs

• Project-HAMi/HAMi#2055: It also changes PodManager read APIs to return deep-copied PodInfo objects.
• Project-HAMi/HAMi#2164: It also updates GetScheduledPods and deep-copy tests.
• Project-HAMi/HAMi#2333: It also makes getter results deep-copied snapshots with concurrency tests.

Suggested reviewers: peachest, archlitchi

Poem

A rabbit checks each pod with care,
And finds a copy waiting there.
Updates hop while reads stay still,
No shared leaves to cause a spill.
Deep copies guard the manager’s hill.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: PodManager now returns copies instead of stored pointers.
Linked Issues check	✅ Passed	The changes satisfy issue #2471 by deep-copying results from GetScheduledPods and GetPod and adding concurrency and detachment tests.
Out of Scope Changes check	✅ Passed	All changes directly support issue #2471 by fixing shared PodInfo state and testing the required copy behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
unittests	64.27% <100.00%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/device/pods.go	97.22% <100.00%> (+0.05%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mesutoezdil]

/lgtm

[FouoF]

/lgtm

[hami-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FouoF, Lakshya77089

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 [FouoF]

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