Skip to content

Reduce BMC API calls in applyBootOrder for empty spec.bootOrder #911

Merged
afritzler merged 1 commit into
mainfrom
907-server-controller-applybootorder-makes-unnecessary-bmc-api-call-every-reconcile-when-specbootorder-is-empty
May 26, 2026
Merged

Reduce BMC API calls in applyBootOrder for empty spec.bootOrder #911
afritzler merged 1 commit into
mainfrom
907-server-controller-applybootorder-makes-unnecessary-bmc-api-call-every-reconcile-when-specbootorder-is-empty

Conversation

@atd9876
Copy link
Copy Markdown
Contributor

@atd9876 atd9876 commented May 26, 2026

Proposed Changes

  • skip the BMC GetBootOrder call entirely when spec.bootOrder is empty
  • log message should only appear when the boot order is actually modified

Fixes #907

Summary by CodeRabbit

  • Bug Fixes
    • Improved BIOS boot order change detection accuracy when comparing boot order specifications and device values.
    • Optimized boot order application to skip unnecessary operations when specifications are empty.
    • Enhanced error messages for clearer feedback during boot order operation failures.
    • Added detailed logging to assist with debugging boot order update operations.

Review Change Stack

Signed-off-by: Andrew Dodds <andrew.dodds@sap.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

📝 Walkthrough

Walkthrough

The PR improves boot order application logic in the server reconciler by adding an early-exit optimization for empty boot orders, refining the change detection to compare both list lengths and individual device values, updating error messages, and adding verbose logging when boot order updates are applied.

Changes

Boot Order Application Improvements

Layer / File(s) Summary
Boot order application and error handling
internal/controller/server_controller.go
applyBootOrder returns early when Spec.BootOrder is empty; change detection is improved to account for differing list lengths and per-index device values (including cases where reported order has fewer entries); GetBootOrder error message is updated; verbose logging of the computed newOrder is added when an update is applied; reconcile-level error message for boot order failures is updated.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

  • #907: The changes directly address this issue by short-circuiting when spec.bootOrder is empty, avoiding unnecessary GetBootOrder calls and improving update and logging behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description covers the main objectives but lacks detailed context about why these changes are beneficial or their impact.
Title check ✅ Passed The title clearly summarizes the main change: reducing BMC API calls by short-circuiting when spec.bootOrder is empty, which directly matches the primary objective and code changes.

✏️ 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 907-server-controller-applybootorder-makes-unnecessary-bmc-api-call-every-reconcile-when-specbootorder-is-empty

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@afritzler afritzler changed the title Server controller: applyBootOrder remove unnecessary BMC API call every reconcile when spec.bootOrder is empty Reduce BMC API calls in applyBootOrder for empty spec.bootOrder May 26, 2026
@afritzler afritzler merged commit 9e4f9fd into main May 26, 2026
26 checks passed
@afritzler afritzler deleted the 907-server-controller-applybootorder-makes-unnecessary-bmc-api-call-every-reconcile-when-specbootorder-is-empty branch May 26, 2026 11:24
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Metal Automation May 26, 2026
@github-project-automation github-project-automation Bot moved this to Done in Roadmap May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size/S

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

Server controller: applyBootOrder makes unnecessary BMC API call every reconcile when spec.bootOrder is empty

2 participants