Skip to content

Conversation

@defo89
Copy link
Contributor

@defo89 defo89 commented Jan 27, 2026

Proposed Changes

  • To align with CCM flow, we will also label the ServerClaim with ServerMaintenanceNeeded label
  • Later, same annotation will be removed

Summary by CodeRabbit

  • New Features

    • Server maintenance status is now recorded on ServerClaim objects using both labels and annotations, improving visibility and compatibility with Kubernetes tooling; status is set and removed consistently across lifecycle stages.
  • Tests

    • Test suite updated to verify both label and annotation presence, values and removal across maintenance transitions, including pending, approval, and cleanup flows.

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

@defo89 defo89 requested a review from a team as a code owner January 27, 2026 11:19
@defo89 defo89 added the bug Something isn't working label Jan 27, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 27, 2026

Walkthrough

Reconciler now sets and removes the ServerMaintenanceNeeded key on both ServerClaim.Labels and .Annotations during pending and cleanup flows; the previous annotations-only helper was removed and tests were updated to assert both label and annotation presence and removal.

Changes

Cohort / File(s) Summary
ServerMaintenance Controller
internal/controller/servermaintenance_controller.go
Inlined logic to ensure ObjectMeta.Labels/Annotations exist, set ServerMaintenanceNeededLabelKey in both, and patch the ServerClaim via client.Patch (MergeFrom). Removed patchServerClaimAnnotations helper and added label removal via metautils.DeleteLabels in cleanup.
Controller Tests
internal/controller/servermaintenance_controller_test.go
Tests updated to assert both ObjectMeta.Annotations and ObjectMeta.Labels include ServerMaintenanceNeededLabelKey (and related maintenance keys) during pending state and verify their removal during cleanup; introduced maintenanceLabels expected map and adjusted assertions/messages.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested labels

enhancement, size/L, area/metal-automation

Suggested reviewers

  • afritzler
  • Nuckal777
  • nagadeesh-nagaraja
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Set maintenance needed label on ServerClaim' accurately reflects the main change: adding a label to ServerClaim for maintenance state tracking.
Description check ✅ Passed The description adequately explains the proposed changes by referencing the CCM flow alignment and noting that the label will be removed later, though it could be more detailed about implementation specifics.
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

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.

@defo89 defo89 merged commit b77248f into main Jan 27, 2026
16 checks passed
@defo89 defo89 deleted the maintenance-needed-label branch January 27, 2026 13:09
@github-project-automation github-project-automation bot moved this from Backlog to Done in Metal Automation Jan 27, 2026
@github-project-automation github-project-automation bot moved this to Done in Roadmap Jan 27, 2026
defo89 added a commit to sapcc/helm-charts that referenced this pull request Jan 27, 2026
defo89 added a commit to sapcc/helm-charts that referenced this pull request Jan 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants