fix(device): return deep copied pods in GetScheduledPods to prevent data race

[aryansri05]

Fixes #2471

Re-opening after #2501 was closed due to an unrelated commit accidentally pushed to the branch (a benchmark graph file, now removed).

---

Currently, GetScheduledPods() returns a shallow copy of the pods map via maps.Copy. While this prevents concurrent map iteration/write panics on the map itself, the map values are *PodInfo pointers — the underlying structs are still shared across goroutines.

This causes a data race between the metrics scraping goroutine (which reads PodInfo fields) and the scheduler goroutine (which writes to the same PodInfo structs).

Fix: call v.DeepCopy() on each value so callers receive fully independent copies.

Signed-off-by: Aryan Srivastava aryansriva05@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Improved scheduled pod data handling to prevent unintended changes from propagating between copied results.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aryansri05
Once this PR has been reviewed and has the lgtm label, please assign fouof for approval. For more information see the Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

📝 Walkthrough

Walkthrough

GetScheduledPods now returns a copied map containing deep-copied PodInfo values instead of sharing stored pointers.

Changes

Scheduled pod snapshot

Layer / File(s)	Summary
Deep-copy scheduled pod entries pkg/device/pods.go	GetScheduledPods iterates through stored pods and applies PodInfo.DeepCopy() to each entry before returning the map.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related issues

• #2163 — Both changes update GetScheduledPods to deep-copy PodInfo values.

Possibly related PRs

• Project-HAMi/HAMi#2164 — Updates GetScheduledPods to return deep-copied PodInfo data.
• Project-HAMi/HAMi#2472 — Implements the same GetScheduledPods deep-copy fix.
• Project-HAMi/HAMi#2333 — Adds deep-copy behavior to getter methods for internal objects and maps.

Suggested reviewers: lakshya77089, fouof

Poem

A rabbit checks the pod map twice,
Then copies each entry nice.
No shared pointers hop away,
Safe snapshots greet the scrape today.
Squeak, the race now loses pace!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change deep-copies each PodInfo in GetScheduledPods, addressing the pointer-sharing race described in [#2471].
Out of Scope Changes check	✅ Passed	The change is limited to the GetScheduledPods copy behavior and contains no unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: returning deep-copied pods from GetScheduledPods to prevent data races.

✨ Finishing Touches 💡 2

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch fix/data-race-scheduled-pods

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 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.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/device/pods.go (1)

241-243: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression test for snapshot independence.

TestGetScheduledPods currently checks value equality only. Mutate a returned PodInfo field, nested Pod field, and Devices map, then verify the manager’s stored value is unchanged. This directly protects the deep-copy contract from regressing to a shallow copy.

As per coding guidelines, keep the test runnable with the repository’s go test ... -short --race -count=1 convention.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/device/pods.go` around lines 241 - 243, Extend TestGetScheduledPods to
verify snapshot independence by mutating a returned PodInfo’s field, its nested
Pod field, and its Devices map, then assert the manager’s stored pod remains
unchanged. Keep the existing equality assertions and ensure the regression test
runs under the repository’s go test -short --race -count=1 convention.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/device/pods.go`:
- Around line 241-243: Extend TestGetScheduledPods to verify snapshot
independence by mutating a returned PodInfo’s field, its nested Pod field, and
its Devices map, then assert the manager’s stored pod remains unchanged. Keep
the existing equality assertions and ensure the regression test runs under the
repository’s go test -short --race -count=1 convention.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a921595b-f231-4227-9d0a-f650d45a8805

📥 Commits

Reviewing files that changed from the base of the PR and between 634bf2b and 985c224.

📒 Files selected for processing (1)

• pkg/device/pods.go

[mesutoezdil]

This is being closed because it does not comply with the contribution guidelines.

[aryansri05]

Apologies for not following the contribution guidelines — I should have disclosed AI assistance upfront and checked for existing PRs before opening this. Won't happen again.