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
1468c0b to
1adbd01
Compare
1adbd01 to
103edf5
Compare
103edf5 to
93b87be
Compare
joker-at-work
left a comment
There was a problem hiding this comment.
As written in DMs, please check if we need to transport the ObservedGeneration into the calls to SetApplyConfigurationStatusCondition to not wipe the previous values or the whole field or its ownership.
joker-at-work
left a comment
There was a problem hiding this comment.
ObservedGeneration is unused so my previous comment doesn't matter.
93d738b to
4cfbe3a
Compare
4cfbe3a to
39dfbca
Compare
39dfbca to
75d66e6
Compare
75d66e6 to
f2acde6
Compare
b4268ca to
4c2679e
Compare
Server-side apply lets controllers own individual status fields without overwriting conditions managed elsewhere. Status must also remain useful when Nova operations fail. Persist the resulting failure condition before returning the operational error so users can distinguish a failed transition from the last successful state. Signed-off-by: Fabian Wiesel <fabian.wiesel@sap.com>
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). Signed-off-by: Fabian Wiesel <fabian.wiesel@sap.com>
4c2679e to
be49afa
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 status updates to Server-Side Apply
Server-Side Apply lets the controller own only
HypervisorDisabled,Evicting, andEvicted, without overwriting status managed by other controllers.Nova compute-service failures are reflected as
HypervisorDisabled=Unknownbefore the error is returned for retry. This keeps status useful when a requested transition cannot be completed and prevents eviction from starting when disabling the service fails.Enabling a compute service also clears
forced_down, so a previous HA event cannot leave an enabled service treated as down.2. Migrate Eviction creation to Server-Side Apply
SSA makes Eviction creation idempotent while preserving labels and owner references. The controller still reads the Eviction after applying it to observe its current status.
Depends on #335.