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

[aryansri05]

Fixes #2471

Currently, GetScheduledPods() returns a shallow copy of the pods map via maps.Copy. While this prevents concurrent map iteration and map write panics on the map itself, the map values are pointers to *PodInfo. As a result, the underlying struct pointers are still shared across threads.

When the metrics collector reads the scheduled pods concurrently, it races with scheduling routines (like AddPod or UpdatePod) mutating these same structs.

This PR fixes the data race by iterating and calling v.DeepCopy() on the PodInfo pointers, returning fully independent structs.

Before:

podsCopy := make(map[k8stypes.UID]*PodInfo, podCount)
maps.Copy(podsCopy, m.pods)

After:

podsCopy := make(map[k8stypes.UID]*PodInfo, podCount)
for k, v := range m.pods {
	podsCopy[k] = v.DeepCopy()
}

Summary by CodeRabbit

• Bug Fixes
  • Improved scheduled pod data isolation by returning independent copies, preventing unintended changes to the original pod information.

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

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

• docs/benchmarks/hami_contention.png is excluded by !**/*.png

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d83d0bdd-10ae-4a82-8d0d-f40f6f0723a4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

GetScheduledPods now deep-copies each PodInfo before returning the scheduled pod map.

Changes

Scheduled pod data isolation

Layer / File(s)	Summary
Deep-copy scheduled pod entries pkg/device/pods.go	GetScheduledPods now calls PodInfo.DeepCopy() for each map entry instead of using a shallow maps.Copy.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

• Project-HAMi/HAMi issue 2163 — Deep-copying PodInfo prevents shared mutable state in GetScheduledPods.

Possibly related PRs

• Project-HAMi/HAMi#2164 — Changes the same API to return deep-copied PodInfo data.
• Project-HAMi/HAMi PR 2472 — Describes the same GetScheduledPods deep-copy implementation.
• Project-HAMi/HAMi#2055 — Adds deep-copy behavior to a related pod data API.

Suggested reviewers: dsfans2014

Poem

A rabbit copied pods with care,
So shared pointers no longer share.
Each scrape gets its own safe view,
While informer updates pass through.
Hop, hop—clean data everywhere!

🚥 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 identifies the GetScheduledPods deep-copy fix and its purpose of preventing a data race.
Linked Issues check	✅ Passed	The change reuses PodInfo.DeepCopy in GetScheduledPods, isolating fields and nested data from concurrent manager updates described in [#2471].
Out of Scope Changes check	✅ Passed	The only change modifies GetScheduledPods to deep-copy PodInfo values, which directly supports the linked issue objectives.
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.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/device/pods.go (1)

241-243: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Add a regression test for copy isolation.

The current TestGetScheduledPods test checks only value equality. It would also pass if GetScheduledPods returned shared *PodInfo pointers. Mutate the returned Pod, Devices map, and nested device slice, then assert that the manager’s stored PodInfo remains unchanged.

🤖 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 deep-copy isolation, not just value equality. After retrieving the result
from GetScheduledPods, mutate the returned Pod, Devices map, and nested device
slice, then assert the manager’s stored PodInfo and all nested data remain
unchanged.

🤖 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 deep-copy
isolation, not just value equality. After retrieving the result from
GetScheduledPods, mutate the returned Pod, Devices map, and nested device slice,
then assert the manager’s stored PodInfo and all nested data remain unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 887c5af8-4544-4317-83e2-3468c9fb1c75

📥 Commits

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

📒 Files selected for processing (1)

• pkg/device/pods.go

[hami-robot]

Thanks for your pull request. Before we can look at it, you'll need to add a 'DCO signoff' to your commits.

📝 Please follow instructions in the contributing guide to update your commits with the DCO

Full details of the Developer Certificate of Origin can be found at developercertificate.org.

The list of commits missing DCO signoff:

• 56190be docs: add init lock contention benchmark graph (Tesla T4)

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[mesutoezdil]

You can view the relevant rule here.
https://github.com/Project-HAMi/HAMi/blob/master/CONTRIBUTING.md#contribution-gates

[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.