Skip to content

Conversation

@asergeant01
Copy link
Contributor

@asergeant01 asergeant01 commented Jan 6, 2026

metal operator changed uuid to system uuid, function was missed during that change https://github.com/ironcore-dev/metal-operator/pull/561/files

Summary by CodeRabbit

  • Bug Fixes
    • Corrected server boot configuration to use the proper system UUID identifier.

✏️ Tip: You can customize this high-level summary in your review settings.

@asergeant01 asergeant01 self-assigned this Jan 6, 2026
@github-actions github-actions bot added size/XS bug Something isn't working labels Jan 6, 2026
metal operator changed uuid to system uuid, function was missed during that change
https://github.com/ironcore-dev/metal-operator/pull/561/files
@asergeant01 asergeant01 force-pushed the fixes-issue-retrieving-systemuuid-from-boot-config branch from 5177c95 to 8d59037 Compare January 6, 2026 16:20
@github-actions github-actions bot added size/S and removed size/XS labels Jan 6, 2026
Copy link
Contributor

@xkonni xkonni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

@hardikdr hardikdr changed the title fixes issue retrieving system uuid from boot config Fix issue retrieving system uuid from boot config Jan 7, 2026
@hardikdr
Copy link
Member

hardikdr commented Jan 7, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Jan 7, 2026

Walkthrough

The change updates the system UUID retrieval logic to use SystemUUID instead of UUID from the Server specification. Both the controller and corresponding tests are updated to reference the new field name.

Changes

Cohort / File(s) Summary
Field reference update
internal/controller/serverbootconfiguration_pxe_controller.go, internal/controller/serverbootconfiguration_pxe_controller_test.go
Changed system UUID sourcing from server.Spec.UUID to server.Spec.SystemUUID in getSystemUUIDFromBootConfig function and updated all test setup and assertions to match

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description explains the context and reason for the change but does not follow the repository's template structure with "Proposed Changes" and "Fixes" sections. Restructure the description to follow the template with a "Proposed Changes" section listing the changes and a "Fixes" section referencing the related issue or PR.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing the retrieval of system UUID from boot config, which directly corresponds to the code changes replacing UUID with SystemUUID.
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

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 65dd86c and 8d59037.

📒 Files selected for processing (2)
  • internal/controller/serverbootconfiguration_pxe_controller.go
  • internal/controller/serverbootconfiguration_pxe_controller_test.go
🔇 Additional comments (3)
internal/controller/serverbootconfiguration_pxe_controller.go (1)

194-201: LGTM!

The field reference update from UUID to SystemUUID correctly aligns with the upstream metal-operator API change. The function name and implementation are now consistent.

internal/controller/serverbootconfiguration_pxe_controller_test.go (2)

36-44: LGTM!

Test setup and assertions correctly updated to use SystemUUID field, maintaining consistency with the controller change.

Also applies to: 80-92


97-105: LGTM!

Second test case correctly updated with the same SystemUUID field changes, ensuring both OCI image variants are tested consistently.

Also applies to: 141-153


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.

@hardikdr hardikdr merged commit 3b3366f into main Jan 7, 2026
15 checks passed
@hardikdr hardikdr deleted the fixes-issue-retrieving-systemuuid-from-boot-config branch January 7, 2026 09:27
@github-project-automation github-project-automation bot moved this to Done in Roadmap Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size/S

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants