Repository navigation
feat(components): deletion protection finalizer lifecycle and teardown engine - #604
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces deletion protection and finalizer management for XMC Component custom resources, adding a deletion reconciliation flow and updating status and finalizer synchronization. The review feedback highlights opportunities to improve robustness and consistency: specifically, returning the reconciliation result instead of nil on deletion errors to prevent nil-pointer dereferences, adding a nil check for the result in syncFinalizers, and restoring status assertions in tests to maintain comprehensive test coverage.
b816548 to
2311dc3
Compare
4085971 to
6e3a7b7
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces deletion protection and finalizer management to the XMC bridge controller, updating the reconciliation loop to handle resource deletion and execute teardown operations in the CMS control plane. The review feedback highlights three key areas for improvement: always attaching the finalizer to guarantee controlled teardown and prevent resource leaks when deletion protection is disabled; setting failure conditions on deletion errors to avoid wiping out existing status conditions; and using the resource's namespace instead of the controller's namespace during updates to improve multi-namespace robustness.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces deletion protection and finalizer management for XMC Component custom resources. It updates the controller to attach or remove deletion protection finalizers, handles component deletion flows (including workload and CMS deletion), and updates the reconciler and status conditions accordingly. Feedback suggests explicitly setting the Degraded condition to False when deletion is blocked by deletion protection to prevent status loss, and adding a nil check for the reconciliation result to avoid a potential nil pointer dereference panic.
8b0c0c0 to
3a323f0
Compare
3a323f0 to
a95f7a6
Compare
a95f7a6 to
6136cd0
Compare
6136cd0 to
9d9e786
Compare
9d9e786 to
f706b7f
Compare
25d2e42 to
6264972
Compare
3b3ccc5 to
75708ae
Compare
75708ae to
808334e
Compare
808334e to
170b212
Compare
170b212 to
ee61f81
Compare
…n engine - Implement FinalizerDeletionProtection and deletion blocking state machine in Component Reconciler - Enforce DeletionBlocked condition when deletionProtection is enabled - Wire ExecuteDeletion and workload de-provisioning during unblocked Component deletion - Implement syncFinalizers in cmd/xmc-bridge daemon runtime for dynamic finalizer management - Add unit tests for deletion protection blocking, unblocked deletion, and finalizer synchronization TAG=agy
…on, nil safety, and status assertion - Return ReconcileResult on 1P workload and CMS deletion errors in reconcileDeletion to preserve partial state and avoid nil pointer dereferences - Add defensive nil check for ReconcileResult in Controller.syncFinalizers - Assert status state COMPONENT_STATE_ENABLED in TestController_RunReconcilePass - Add TestController_SyncFinalizers_NilResult and verify non-nil ReconcileResult in ReconcileComponent tests TAG=agy
…ate nestif lint complexity
…ions, and use item namespace - Always attach deletion protection finalizer on active components so deletion lifecycle is guaranteed - Set Ready=False and Degraded=True failure conditions on 1P workload and CMS deletion failures - Use item.GetNamespace() in syncFinalizers and syncStatus for robustness Comment links: - #604 (comment) - #604 (comment) - #604 (comment) TAG=agy
…letion blocked and defensive nil check
ee61f81 to
c730eaf
Compare
There was a problem hiding this comment.
one logic question about handling sync errors
…solation semantics
Summary
Implements deletion protection and dynamic finalizer management for XMC Component custom resources.
Architectural Deliverables
finalizers.xmc.dev/deletion-protectionlifecycle on protected Component CRs.DeletionBlocked=Truecondition whenspec.deletionProtectionis enabled.CMSClient.ExecuteDeletion.syncFinalizersincmd/xmc-bridgecontroller loop.reconciler_test.goandmain_test.go.TAG=agy