Migrate HypervisorMaintenanceController to SSA (status + eviction creation)#339
Migrate HypervisorMaintenanceController to SSA (status + eviction creation)#339fwiesel wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
b0ce964 to
1468c0b
Compare
Replace PatchHypervisorStatusWithRetry with Status().Apply() for ConditionTypeHypervisorDisabled, ConditionTypeEvicting, and the Evicted scalar. ConditionTypeEvicting is removed by omitting it from the apply — SSA prunes map-list entries no longer claimed by the sole owner. The status config is built once in Reconcile and passed to reconcileComputeService and reconcileEviction which mutate it directly. This eliminates intermediate condition return values and makes the single-apply-per-reconcile explicit. The early-return guard for already-enabled/disabled state is inlined into each branch. Seed only the conditions owned by this controller so SSA does not claim ownership over conditions written by others. For the delete-by-omission case, owned conditions are seeded selectively so the target condition is simply absent from the payload. Also clear the forced_down flag when enabling a compute service so an earlier HA event that set forced_down=true does not leave the service reachable but treated as down. SetupWithManager now delegates to registerWithManager instead of inlining the same ctrl.NewControllerManagedBy call, which had left registerWithManager unreachable.
Replace Create + SetControllerReference with Apply for the Eviction CR. The owner reference and labels are set in the apply configuration metadata, and SSA handles the upsert. A Get is still performed after the apply to read the current eviction status conditions. The apply configuration constructor for the cluster-scoped Eviction resource takes only a name (no namespace).
181d69b to
7d13627
Compare
1468c0b to
1adbd01
Compare
|
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
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
|
Two commits, both for HypervisorMaintenanceController:
1. Migrate HypervisorMaintenanceController to SSA status updates
Replace
PatchHypervisorStatusWithRetrywithStatus().Apply()forConditionTypeHypervisorDisabled,ConditionTypeEvicting, and theEvictedscalar.ConditionTypeEvictingis removed by omitting it from the apply — SSA prunes map-list entries no longer claimed by the sole owner.The status config is built once in
Reconcileand passed toreconcileComputeServiceandreconcileEvictionwhich mutate it directly. This eliminates intermediate condition return values and makes the single-apply-per-reconcile explicit. The early-return guard for already-enabled/disabled state is inlined into each branch.Seed only the conditions owned by this controller so SSA does not claim ownership over conditions written by others. For the delete-by-omission case, owned conditions are seeded selectively so the target condition is simply absent from the payload.
Also clear the
forced_downflag when enabling a compute service so an earlier HA event that setforced_down=truedoes not leave the service reachable but treated as down.SetupWithManagernow delegates toregisterWithManagerinstead of inlining the samectrl.NewControllerManagedBycall, which had leftregisterWithManagerunreachable.2. Migrate HypervisorMaintenanceController Eviction creation to SSA
Replace
Create + SetControllerReferencewithApplyfor the Eviction CR. The owner reference and labels are set in the apply configuration metadata, and SSA handles the upsert. AGetis still performed after the apply to read the current eviction status conditions.Depends on #335 (
ssa-conditions-helpers).