CNTRLPLANE-3532: Add shared status patching helpers with optimistic locking - #8782
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
the
jira/valid-reference
|
@vsolanki12: This pull request references CNTRLPLANE-3532 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThe Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ 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 |
openshift-ci
Bot
added
area/control-plane-operator
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@support/statuspatching/statuspatching.go`:
- Line 108: The comment at line 108 that says "8. Apply the patch" has duplicate
step numbering since step 8 already appears at line 102. Update the step number
in the comment from 8 to 9 to maintain sequential numbering in the patch
application logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4447923e-c270-401e-b7ee-5cf73e96f58a
📒 Files selected for processing (2)
support/statuspatching/statuspatching.gosupport/statuspatching/statuspatching_test.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8782 +/- ##
==========================================
+ Coverage 41.43% 42.30% +0.87%
==========================================
Files 756 773 +17
Lines 93658 97413 +3755
==========================================
+ Hits 38807 41215 +2408
- Misses 52128 53315 +1187
- Partials 2723 2883 +160
... and 100 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
support/statuspatching/statuspatching_test.go (1)
69-76: ⚡ Quick winNarrow the conflict interceptor to the
statussubresource.Line 70 currently returns a conflict for every subresource patch. That can let this test pass even if callers patch the wrong subresource. Restricting this to
"status"makes the test precise.Proposed diff
return interceptor.NewClient(underlying, interceptor.Funcs{ SubResourcePatch: func(ctx context.Context, c client.Client, subResourceName string, obj client.Object, patch client.Patch, opts ...client.SubResourcePatchOption) error { + if subResourceName != "status" { + return fmt.Errorf("unexpected subresource patch: %s", subResourceName) + } return apierrors.NewConflict( schema.GroupResource{Group: "", Resource: "nodes"}, obj.GetName(), fmt.Errorf("the object has been modified"), ) }, })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@support/statuspatching/statuspatching_test.go` around lines 69 - 76, The SubResourcePatch interceptor function currently returns a conflict error for any subresource patch operation, but it should only return this conflict when patching the status subresource. Add a condition at the beginning of the SubResourcePatch function to check if the subResourceName parameter equals "status", and only return the conflict error in that case. For other subresources, delegate to the underlying client by calling its SubResourcePatch method or return nil to allow the patch to proceed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@support/statuspatching/statuspatching_test.go`:
- Around line 69-76: The SubResourcePatch interceptor function currently returns
a conflict error for any subresource patch operation, but it should only return
this conflict when patching the status subresource. Add a condition at the
beginning of the SubResourcePatch function to check if the subResourceName
parameter equals "status", and only return the conflict error in that case. For
other subresources, delegate to the underlying client by calling its
SubResourcePatch method or return nil to allow the patch to proceed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3c69c41d-fb19-40ac-9d8f-35c5ab55677b
📒 Files selected for processing (2)
support/statuspatching/statuspatching.gosupport/statuspatching/statuspatching_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- support/statuspatching/statuspatching.go
b929e39 to
1c7030d
Compare
| original := obj.DeepCopyObject().(client.Object) | ||
| mutate() | ||
| if equality.Semantic.DeepEqual(original, obj) { | ||
| return nil | ||
| } | ||
| return c.Status().Patch(ctx, obj, client.MergeFromWithOptions(original, client.MergeFromWithOptimisticLock{})) |
There was a problem hiding this comment.
to minimize requeues, I think we should get the original first and wrap in retryOnConflict
see:
hypershift/support/k8sutil/object.go
Lines 27 to 40 in dca6f75
There was a problem hiding this comment.
Done. All three functions now use retry.RetryOnConflict(retry.DefaultBackoff, ...).
| // last TransitionTime in certain states). | ||
| // mutate() must only modify status fields on obj. | ||
| // The patch is filtered to only /status/* operations before applying. | ||
| func PatchStatusJSON(ctx context.Context, c client.Client, obj client.Object, mutate func()) error { |
There was a problem hiding this comment.
why do we need this? this was meant as a workaround, we should not expose it as reusable function.
There was a problem hiding this comment.
Removed. Dropped from this PR since it was a workaround specific to hcpstatus.go.
| // It captures a deep copy before calling mutate, then patches with | ||
| // MergeFromWithOptimisticLock so concurrent writers get a conflict error instead of silently overwriting. | ||
| // Mutate must only modify status fields. | ||
| func PatchStatus(ctx context.Context, c client.Client, obj client.Object, mutate func()) error { |
There was a problem hiding this comment.
mutate() should return an error
There was a problem hiding this comment.
Done. Changed to mutate func() error.
| return nil | ||
| } | ||
| return c.Status().Patch(ctx, obj, client.MergeFromWithOptions(original, client.MergeFromWithOptimisticLock{})) | ||
| } |
There was a problem hiding this comment.
the main CPO reconciler finction does multiple changes in sequence to the HCP status, then a bulk patch at the end, which doesn't fit the mutate() pattern. We should have a separate function to simply patch an object. e.g.
func PatchObjectStatus(ctx context.Context, c client.Client, obj client.Object) error {
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
existing := obj.DeepCopyObject().(client.Object)
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), existing); err != nil {
return err
}
return c.Status().Patch(ctx, obj, client.MergeFromWithOptions(existing, client.MergeFromWithOptimisticLock{}))
})
}There was a problem hiding this comment.
Done. Added PatchObjectStatusthat patches an already mutated object against server state, for the CPO bulk-mutation-then-patch pattern.
There was a problem hiding this comment.
how is this better/different from just passing a no op mutate func here and we keep a single authoritative func?
There was a problem hiding this comment.
If really need to keep the separation I couldn't tell the intent different by the current func naming
There was a problem hiding this comment.
Is there a reason why we can't have one single exported function and let the others be part of the implementation?
There was a problem hiding this comment.
vibe coded example fwiw
// Condition binds a condition to the slice it should be set on.
type Condition struct {
Target *[]metav1.Condition
Condition metav1.Condition
}
// PatchStatus re-fetches obj, applies mutate (if non-nil), sets any
// conditions, and patches the status subresource with optimistic locking.
// Conflicts are retried automatically. mutate must only modify status fields.
//
// Usage patterns:
//
// // Mutate callback only:
// statuspatching.PatchStatus(ctx, c, hcp, func() error {
// hcp.Status.Version = newVersion
// return nil
// }, nil)
//
// // Single condition only:
// statuspatching.PatchStatus(ctx, c, hcp, nil, &statuspatching.Condition{
// Target: &hcp.Status.Conditions,
// Condition: metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"},
// })
//
// // Both:
// statuspatching.PatchStatus(ctx, c, hcp, func() error {
// hcp.Status.Version = newVersion
// return nil
// }, &statuspatching.Condition{
// Target: &hcp.Status.Conditions,
// Condition: metav1.Condition{Type: "Ready", Status: metav1.ConditionTrue, Reason: "AllGood"},
// })
func PatchStatus(ctx context.Context, c client.Client, obj client.Object, mutate func() error, cond *Condition) error {
return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil {
return err
}
original := obj.DeepCopyObject().(client.Object)
if mutate != nil {
if err := mutate(); err != nil {
return err
}
}
if cond != nil {
meta.SetStatusCondition(cond.Target, cond.Condition)
}
if equality.Semantic.DeepEqual(original, obj) {
return nil
}
return c.Status().Patch(ctx, obj, client.MergeFromWithOptions(original, client.MergeFromWithOptimisticLock{}))
})
}
There was a problem hiding this comment.
how is this better/different from just passing a no op mutate func here and we keep a single authoritative func?
the original function fetches the obj and save it in the passed obj and then apply mutate. This fetches the existing obj in a separate variable to be used a original for patching the passed in obj
There was a problem hiding this comment.
Dropped PatchObjectStatus after cblecker identified the stale-state retry bug, same class of issue as OCPBUGS-93462. On consolidating into a single function, PatchStatusCondition relies on SetStatusCondition changed return value for no-op detection, which DeepEqual can't do reliably due to LastTransitionTime being stamped on every call. Keeping them as two separate functions for now, open to revisiting if you feel otherwise.
1c7030d to
2525891
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
support/statuspatching/statuspatching_test.go (1)
350-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd optimistic-lock payload assertions to the remaining helper test suites.
TestPatchStatusConditionandTestPatchObjectStatusverify invocation (and type for one path), but they don’t assert that the generated patch payload carriesresourceVersion. Since optimistic locking is a key contract, asserting this in both suites would better protect against regressions.Suggested hardening
@@ err := PatchStatusCondition(context.Background(), c, svc, &svc.Status.Conditions, tt.newCondition) g.Expect(err).ToNot(HaveOccurred()) g.Expect(recorder.called).To(Equal(tt.expectPatchCalled)) + if tt.expectPatchCalled { + g.Expect(recorder.patchType).To(Equal(types.MergePatchType)) + g.Expect(string(recorder.patchData)).To(ContainSubstring("resourceVersion")) + } @@ if tt.expectPatchCalled { g.Expect(recorder.patchType).To(Equal(types.MergePatchType)) + g.Expect(string(recorder.patchData)).To(ContainSubstring("resourceVersion")) }Also applies to: 420-422
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@support/statuspatching/statuspatching_test.go` around lines 350 - 354, The test suites TestPatchStatusCondition and TestPatchObjectStatus currently verify that the patch functions are invoked but do not assert that the generated patch payload includes the resourceVersion field for optimistic locking. Add assertions in both test suites to inspect the patch payload captured by the recorder (likely in the recorder.patch field or similar) and verify that it contains the resourceVersion field. This should be done after verifying the patch was called to ensure the optimistic locking contract is maintained across both test cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@support/statuspatching/statuspatching_test.go`:
- Around line 50-53: The SubResourcePatch function at line 52 ignores the error
returned by patch.Data(obj) using a blank identifier, which violates Go security
guidelines about handling error returns. Capture the error return value from
patch.Data(obj) instead of discarding it with _, check if the error is not nil,
and handle it appropriately by either returning the error or failing the test
with an error message. This same fix must also be applied to the identical
pattern at line 85 in the file where patch.Data is called.
---
Nitpick comments:
In `@support/statuspatching/statuspatching_test.go`:
- Around line 350-354: The test suites TestPatchStatusCondition and
TestPatchObjectStatus currently verify that the patch functions are invoked but
do not assert that the generated patch payload includes the resourceVersion
field for optimistic locking. Add assertions in both test suites to inspect the
patch payload captured by the recorder (likely in the recorder.patch field or
similar) and verify that it contains the resourceVersion field. This should be
done after verifying the patch was called to ensure the optimistic locking
contract is maintained across both test cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0efa4a0d-5110-4408-b62a-c45a714d112c
📒 Files selected for processing (2)
support/statuspatching/statuspatching.gosupport/statuspatching/statuspatching_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- support/statuspatching/statuspatching.go
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
2525891 to
dfafe87
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: muraee, vsolanki12 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
| // TEST: PatchStatusCondition | ||
| //================================================================== | ||
|
|
||
| func TestPatchStatusCondition(t *testing.T) { |
There was a problem hiding this comment.
PatchStatus and PatchObjectStatus both have dedicated RetryOnConflict tests, but PatchStatusCondition doesn't. The retry path is especially worth testing here because of the conditions pointer — after c.Get refreshes obj on retry, the *[]metav1.Condition pointer must still reference the correct slice. A TestPatchStatusCondition_RetryOnConflict using newFakeClientWithConflictThenSuccess would lock that in.
There was a problem hiding this comment.
Added TestPatchStatusCondition_RetryOnConflict to verify that the conditions pointer remains valid after re-fetching the object during a retry.
| if err := c.Get(ctx, client.ObjectKeyFromObject(obj), existing); err != nil { | ||
| return err | ||
| } | ||
| return c.Status().Patch(ctx, obj, client.MergeFromWithOptions(existing, client.MergeFromWithOptimisticLock{})) |
There was a problem hiding this comment.
PatchStatus skips no-ops via DeepEqual (line 36) and PatchStatusCondition via SetStatusCondition's return value (line 55), but PatchObjectStatus always calls Status().Patch() even when the local object matches server state. The package doc says these helpers "skip no-op updates" — this function doesn't.
Unnecessary patches bump resourceVersion, triggering watch events and reconcile loops across every controller watching the object. Adding if equality.Semantic.DeepEqual(existing, obj) { return nil } before the patch would make this consistent. (Or if this function gets folded into PatchStatus per the other thread, the skip comes for free.)
There was a problem hiding this comment.
Removed PatchObjectStatus entirely — no longer applies.
| // current server state. Use this when the caller has already made multiple | ||
| // status mutations in sequence (e.g. the main CPO reconciler) and wants a | ||
| // single bulk patch at the end rather than the mutate-callback pattern. | ||
| func PatchObjectStatus(ctx context.Context, c client.Client, obj client.Object) error { |
There was a problem hiding this comment.
Building on the API consolidation discussion — there's a concrete correctness issue with the retry semantics here. When PatchObjectStatus retries after a 409, line 68 deep-copies the caller's stale, pre-mutated object on every iteration. Line 69 fetches fresh server state into that copy. The merge patch is then diff(fresh server, stale caller) — which includes ALL fields that differ, not just what the caller intended to change. This silently reverts concurrent status changes from other controllers.
Contrast with PatchStatus which re-fetches into obj and re-runs the mutate callback on every retry, always operating on fresh state. That's another argument for consolidating into PatchStatus — the retry-replay-mutation pattern is inherently correct for concurrent writers, while retry-with-stale-diff is not.
There was a problem hiding this comment.
Agreed, this has the same stale-state problem as OCPBUGS-93462. Dropped PatchObjectStatus and its tests. If we need a bulk-patch pattern later, it can be designed with correct retry semantics during the CPO migration.
| // avoiding false positives from LastTransitionTime being stamped with time.Now(). | ||
| // Pass a pointer to the object's conditions slice (e.g. &hcp.Status.Conditions) | ||
| // since HCP types expose conditions as a bare field, not via getter/setter methods. | ||
| func PatchStatusCondition(ctx context.Context, c client.Client, obj client.Object, conditions *[]metav1.Condition, condition metav1.Condition) error { |
There was a problem hiding this comment.
Minor: the conditions pointer must alias a field within obj for correctness (so Get refreshes the same memory the pointer references). The doc comment explains this well, and Go's type system can't enforce it, so this is inherently documentation-enforced. Just flagging it — enxebre's Condition struct proposal in the other thread would make the relationship more visible at call sites.
There was a problem hiding this comment.
This follows the same pattern as &hcp.Status.Conditions used throughout the codebase. Wrapping it in a struct would only relocate the aliasing, not eliminate it.
| } | ||
| } | ||
|
|
||
| func TestPatchObjectStatus_RetryOnConflict(t *testing.T) { |
There was a problem hiding this comment.
TestPatchObjectStatus_RetryOnConflict verifies the retry happened (recorder.called) but doesn't assert the correct status values were written afterward. Fetching the node from the client after the call and checking Phase == NodeTerminated would strengthen the contract.
There was a problem hiding this comment.
Removed along with PatchObjectStatus.
| // mutate must only modify status fields on obj. | ||
| func PatchStatus(ctx context.Context, c client.Client, obj client.Object, mutate func() error) error { | ||
| return retry.RetryOnConflict(retry.DefaultBackoff, func() error { | ||
| if err := c.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { |
There was a problem hiding this comment.
No test covers the path where the initial Get fails (e.g., object was deleted). A single test calling PatchStatus against a non-seeded fake client and asserting NotFound is returned would document the error-propagation contract.
There was a problem hiding this comment.
Added TestPatchStatus_GetFailure to cover the unseeded object scenario, verifying a NotFound error is returned and no patch is applied.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
c958425 to
d5ab2b5
Compare
cblecker
left a comment
cblecker
left a comment
There was a problem hiding this comment.
Nit: PatchStatus now has TestPatchStatus_GetFailure (thanks for adding that), but there's no equivalent for PatchStatusCondition. The code path is identical so the risk is low, but a symmetric TestPatchStatusCondition_GetFailure would round out the error contract coverage.
| node.Status.Phase = corev1.NodeTerminated | ||
| return nil | ||
| }) | ||
| g.Expect(err).ToNot(HaveOccurred()) |
There was a problem hiding this comment.
Same note I left on the removed PatchObjectStatus retry test — this verifies the retry mechanism fired but doesn't assert the final persisted state. TestPatchStatusCondition_RetryOnConflict (below) does this correctly by checking the conditions after the call. Would be good to add a c.Get + assert on result.Status.Phase == corev1.NodeTerminated here too, to catch subtle retry-loop bugs like the stale-state issue we saw before.
There was a problem hiding this comment.
Done — added c.Get + result.Status.Phase == NodeTerminated assertion to TestPatchStatus_RetryOnConflict in the latest push, so it now verifies the mutation actually persisted.
…king Signed-off-by: Vimal Solanki <vsolanki@redhat.com>
d5ab2b5 to
3114a9c
Compare
vsolanki12
left a comment
vsolanki12
left a comment
There was a problem hiding this comment.
Added TestPatchStatusCondition_GetFailure — creates a Service not seeded into the fake client, calls PatchStatusCondition, and asserts IsNotFound + no patch call. Symmetric with TestPatchStatus_GetFailure. Both changes are in the latest push (3114a9c).
|
/lgtm nit: your PR description may be slightly out of date with the changes you made, but not blocking |
|
Scheduling tests matching the |
|
thank you @cblecker updated as per suggestion. |
|
/test e2e-aws-upgrade-hypershift-operator earlier it got failed due to resource issues on CI side. |
|
Confirmed: 37 tests, 0 failures, 0 errors. The test itself passed completely. Now I have all the evidence I need. Let me compile the final report. Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryAll 37 e2e tests (TestUpgradeHyperShiftOperator) passed successfully with 0 failures. The pre phase (cluster creation, HyperShift install) and the test phase (upgrade e2e tests) both completed without errors. The job failed solely because the Root CauseThe root cause is a CI infrastructure pod scheduling failure on the shared OpenShift CI build cluster, not a product or test code bug. When the post phase began at
The combination of all constraints left zero eligible nodes for the entire 1-hour timeout period. After the timeout, ci-operator deleted the pending pod and marked the post phase as failed, which caused the overall job to be reported as failed despite all actual tests passing. Critically:
This is a flaky CI infrastructure issue — not caused by PR #8782's code changes. Recommendations
Evidence
|
|
/override ci/prow/e2e-aws-upgrade-hypershift-operator This job succeeded but failed in the dump. Considering this PR doesn't wire in these changes yet, this is okay. Setting verified to later for the same reason -- will be wired in, in a follow up. |
openshift-ci-robot
added
verified-later
verified
|
@cblecker: This PR has been marked to be verified later by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@cblecker: Overrode contexts on behalf of cblecker: ci/prow/e2e-aws-upgrade-hypershift-operator DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@cblecker: Overrode contexts on behalf of cblecker: ci/prow/e2e-aws-upgrade-hypershift-operator DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@vsolanki12: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
5 participants
What does this PR do?
Introduces a
support/statuspatchingpackage with two helpers that give controllers a single, consistent way to patch status subresources safely.PatchStatus takes a mutate callback, re-fetches the object, deep-copies it, runs the callback, and uses
MergeFromWithOptimisticLockso a stale write returns a conflict instead of silently winning. It skips the API call entirely when nothing changed. On conflict, the entire cycle (re-fetch → mutate → patch) is retried automatically.PatchStatusCondition does the same thing for a single
metav1.Condition. It accepts a*[]metav1.Conditionpointer (e.g.&hcp.Status.Conditions) because HCP exposes conditions as a bare field rather than through getter/setter methods. It usesSetStatusCondition's own change detection to skip no-ops reliably, avoiding false positives fromLastTransitionTimebeing stamped withtime.Now().Why is this needed?
Several controllers (CPO, HCCO, HO, karpenter-operator) write to the same HostedControlPlane status. Without optimistic locking the last writer silently wins. This package gives everyone a single correct implementation to call, with built-in retry-on-conflict handling so callers never need to handle 409s themselves.
How to test
go test ./support/statuspatching/...Jira
CNTRLPLANE-3532