fix(metadata-collector): skip deleted pods during annotation updates

[sravindra05]

Summary

This PR Fixes #1568. More specifically it:

• Updates UpdatePodDevicesAnnotations in metadata-collector/pkg/mapper/mapper.go to treat NotFound responses while patching Pod device annotations as a benign deletion race.
• Continues reconciling remaining Pods without counting deleted Pods as updated.

Type of Change

• 🐛 Bug fix
• ✨ New feature
• 💥 Breaking change
• 📚 Documentation
• 🔧 Refactoring
• 🔨 Build/CI

Component(s) Affected

• Core Services
• Documentation/CI
• Fault Management
• Health Monitors
• Janitor
• Other: metadata-collector

Testing

• Tests pass locally (make lint-test-metadata-collector)
• Manual testing completed
• No breaking changes (or documented)

Checklist

• Self-review completed
• Documentation updated (if needed)
• Ready for review

Summary by CodeRabbit

• Bug Fixes

  • Device annotation updates now continue when a reported pod no longer exists.
  • Existing pods continue to be updated, while other update errors are still reported.
  • Update counts now reflect only successfully processed pods.

• Tests

  • Added coverage for handling missing pods during annotation updates.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

The mapper now treats Kubernetes NotFound patch errors as non-fatal. It skips deleted pods, continues processing, and counts only successful annotation patches. A test covers deleted and existing pods.

Changes

Pod Annotation Reconciliation

Layer / File(s)	Summary
Skip deleted pods during annotation updates metadata-collector/pkg/mapper/mapper.go, metadata-collector/pkg/mapper/mapper_test.go	NotFound patch errors are logged and skipped. Other patch errors remain fatal. The modification count increases only after successful patches. The test verifies that an existing pod is updated when another pod has been deleted.

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

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The implementation handles Kubernetes NotFound errors as benign, continues reconciliation, and counts only successful updates as required by issue #1568.
Out of Scope Changes check	✅ Passed	The code and test changes directly support issue #1568 and contain no unrelated scope.
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: skipping deleted Pods during annotation updates.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

metadata-collector/pkg/mapper/mapper_test.go (1)

355-355: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename the test to match the required pattern.

Use TestUpdatePodDevicesAnnotations_DeletedPod_SkipsAndUpdatesExistingPod.

As per coding guidelines, “Name tests descriptively following the pattern TestFunctionName_Scenario_ExpectedBehavior in Go.”

🤖 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 `@metadata-collector/pkg/mapper/mapper_test.go` at line 355, Rename the test
function TestUpdatePodDevicesAnnotationsSkipsDeletedPod to
TestUpdatePodDevicesAnnotations_DeletedPod_SkipsAndUpdatesExistingPod,
preserving its existing test logic.

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.

Inline comments:
In `@metadata-collector/pkg/mapper/mapper_test.go`:
- Around line 355-398: Update TestUpdatePodDevicesAnnotationsSkipsDeletedPod and
its newTestDeviceMapper setup to use the repository’s envtest Kubernetes API
server instead of fake.NewSimpleClientset. Exercise the annotation update and
deleted-pod NotFound behavior through the real API server while preserving the
existing assertions for one update and the surviving pod’s annotation.

---

Nitpick comments:
In `@metadata-collector/pkg/mapper/mapper_test.go`:
- Line 355: Rename the test function
TestUpdatePodDevicesAnnotationsSkipsDeletedPod to
TestUpdatePodDevicesAnnotations_DeletedPod_SkipsAndUpdatesExistingPod,
preserving its existing test logic.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ca9d95e4-bd8a-4d8a-a396-2bbd83767db0

📥 Commits

Reviewing files that changed from the base of the PR and between a22fe4b and 5749789.

📒 Files selected for processing (2)

• metadata-collector/pkg/mapper/mapper.go
• metadata-collector/pkg/mapper/mapper_test.go

[lalitadithya]

/ok to test 5749789

[github-actions]

🌿 Fern Docs Preview: https://nvidia-preview-pull-request-1578.docs.buildwithfern.com/nvsentinel

[github-actions]

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/nvidia/nvsentinel/metadata-collector/pkg/mapper	30.93% (+0.06%)	👍

---

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/nvidia/nvsentinel/metadata-collector/pkg/mapper/mapper.go	29.28% (+0.15%)	362 (+5)	106 (+2)	256 (+3)	👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

• github.com/nvidia/nvsentinel/metadata-collector/pkg/mapper/mapper_test.go

[lalitadithya]

/ok to test f7b76a6

[sravindra05]

Thanks @lalitadithya @XRFXLP 🙏🏼