Fix findExistingSubscription bailing on partial CollectionError

[asergeant01]

fixes #862 (follow-up fix)

Problem

After the initial fix (merged), the BMC reconciler could still fail to find an
existing subscription if any individual subscription in the collection failed
to fetch. Gofish's ev.Subscriptions() fetches members concurrently and
returns a CollectionError when any member fails — alongside the successfully
fetched results. The original code bailed immediately on any error, discarding
the partial results:

subscriptions, err := ev.Subscriptions()
if err != nil {
    return "", fmt.Errorf("failed to list subscriptions: %w", err)
}

If the target subscription was fetched successfully but another subscription in
the collection returned an error, the match would never be found.

Fix

Tolerate CollectionError and continue searching through partial results. Only
fail on other error types:

var collErr *schemas.CollectionError
if !errors.As(err, &collErr) {
    return "", fmt.Errorf("failed to list subscriptions: %w", err)
}

Note: gofish wraps even a failure on the collection endpoint itself as a
CollectionError, so both "some members failed" and "collection endpoint
unreachable" fall through to a "not found" result rather than a hard error.

Changes

• bmc/redfish.go: Tolerate CollectionError in findExistingSubscription()
• bmc/redfish_test.go: Added 2 tests — one verifying a match is found when a
  sibling subscription fails to fetch, one verifying behaviour when the
  collection endpoint itself is unavailable

Summary by CodeRabbit

• Bug Fixes
  • Improved event subscription retrieval reliability. The system now uses available subscription data when some individual subscriptions encounter errors, instead of failing completely.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 589d1a3e-97d4-4d34-9f7d-53082d08bb36

📥 Commits

Reviewing files that changed from the base of the PR and between 70aa1c0 and 6547d13.

📒 Files selected for processing (2)

• bmc/redfish.go
• bmc/redfish_test.go

---

📝 Walkthrough

Walkthrough

The pull request refines findExistingSubscription in the Redfish BMC module to handle partial subscription collection results gracefully. When the subscriptions listing returns a CollectionError, the function now proceeds with available partial results instead of failing immediately. Non-CollectionError failures continue to cause immediate returns. Two new test cases validate both the partial-failure and collection-endpoint-failure scenarios.

Changes

CollectionError Tolerance in Subscription Lookup

Layer / File(s)	Summary
Error Handling Logic bmc/redfish.go	findExistingSubscription branches on ev.Subscriptions() error type: CollectionError allows continuation with partial results; other errors return immediately. Destination and context matching behavior unchanged.
Test Coverage bmc/redfish_test.go	New test case verifies matching subscription is found despite one member returning HTTP 500. Second test case verifies function returns "not found" when collection endpoint itself returns HTTP 403.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• ironcore-dev/metal-operator#756: Added the initial findExistingSubscription helper for duplicate-subscription detection; this PR refines its error handling for robustness.
• ironcore-dev/metal-operator#865: Modifies the same findExistingSubscription function and its tests, focusing on matching logic and Context parameter changes.

Suggested labels

bug, size/S, area/metal-automation

Suggested reviewers

• afritzler

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately describes the main change: fixing findExistingSubscription to handle partial CollectionError rather than bailing immediately.
Description check	✅ Passed	The description provides comprehensive context: problem statement with code example, solution explanation, and clear summary of changes. It follows the basic structure of the template.
Linked Issues check	✅ Passed	The PR implements the core requirement from #862: checking for existing subscriptions before creation by making findExistingSubscription more robust to partial fetch failures, allowing subscriptions to be found and reused.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue #862: modifying findExistingSubscription error handling in redfish.go and adding related tests in redfish_test.go with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/ignore-partial-fetch-failures-on-subscriptions

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

---

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 and usage tips.}