8000 feat(components): deletion protection finalizer lifecycle and teardown engine by VLukiantsev · Pull Request #604 · google/xmc · GitHub
[go: up one dir, main page]

Skip to content

feat(components): deletion protection finalizer lifecycle and teardown engine - #604

Merged
VLukiantsev merged 8 commits into
mainfrom
feat/components-bridge-deletion-protection-b548435797
Aug 31, 2026
Merged

VLukiantsev merged 8 commits into
mainfrom
feat/components-bridge-deletion-protection-b548435797

Conversation

@VLukiantsev
Copy link
Copy Markdown
Collaborator

Summary

Implements deletion protection and dynamic finalizer management for XMC Component custom resources.

Architectural Deliverables

  • Finalizer Management: Manages finalizers.xmc.dev/deletion-protection lifecycle on protected Component CRs.
  • Deletion Protection: Blocks Component teardown and emits DeletionBlocked=True condition when spec.deletionProtection is enabled.
  • Controlled Teardown: Upon unblocked deletion, safely orchestrates 1P/3P workload pruning and calls CMSClient.ExecuteDeletion.
  • Bridge Dynamic Synchronization: Implements syncFinalizers in cmd/xmc-bridge controller loop.
  • Unit Tests: Full test coverage in reconciler_test.go and main_test.go.

TAG=agy

@gemini-code-assist gemini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/components/bridge/reconciler.go Resolved
Comment thread pkg/components/bridge/reconciler.go Resolved
Comment thread cmd/xmc_bridge/main.go Resolved
Comment thread cmd/xmc_bridge/main_test.go Resolved
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from b816548 to 2311dc3 Compare August 26, 2026 08:20
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch 2 times, most recently from 4085971 to 6e3a7b7 Compare August 26, 2026 08:56
@VLukiantsev
Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/components/bridge/reconciler.go Resolved
Comment thread pkg/components/bridge/reconciler.go Resolved
Comment thread cmd/xmc_bridge/main.go Resolved
@VLukiantsev
Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist g 8000 emini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pkg/components/bridge/reconciler.go Resolved
Comment thread cmd/xmc_bridge/main.go Resolved
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch 2 times, most recently from 8b0c0c0 to 3a323f0 Compare August 26, 2026 13:30
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from 3a323f0 to a95f7a6 Compare August 26, 2026 13:53
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from a95f7a6 to 6136cd0 Compare August 26, 2026 14:33
@VLukiantsev
VLukiantsev force-pushed th 8000 e feat/components-bridge-deletion-protection-b548435797 branch from 6136cd0 to 9d9e786 Compare August 26, 2026 14:37
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from 9d9e786 to f706b7f Compare August 26, 2026 14:44
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch 2 times, most recently from 25d2e42 to 6264972 Compare August 26, 2026 21:45
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from 3b3ccc5 to 75708ae Compare August 27, 2026 10:59
AntonTyb
AntonTyb previously approved these changes Aug 27, 2026
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from 75708ae to 808334e Compare August 27, 2026 15:02
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from 808334e to 170b212 Compare August 27, 2026 15:46
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from 170b212 to ee61f81 Compare August 28, 2026 08:34
@VLukiantsev
VLukiantsev requested a review from AntonTyb August 28, 2026 08:43
@VLukiantsev VLukiantsev added the priority::2-high top of mind. should not wait. label Aug 28, 2026
Base automatically changed from feat/components-bridge-core-reconciler-b548435797 to main August 28, 2026 10:31
…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
…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
@VLukiantsev
VLukiantsev force-pushed the feat/components-bridge-deletion-protection-b548435797 branch from ee61f81 to c730eaf Compare August 28, 2026 10:31
Comment thread cmd/xmc_bridge/main.go Resolved
@AntonTyb AntonTyb left a comment
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one logic question about handling sync errors

CCBE
@VLukiantsev
VLukiantsev requested a review from AntonTyb August 28, 2026 13:07
@VLukiantsev
VLukiantsev added this pull request to the merge queue Aug 31, 2026
Merged via the queue into main with commit d92fd4c Aug 31, 2026
12 checks passed
@VLukiantsev
VLukiantsev deleted the feat/components-bridge-deletion-protection-b548435797 branch August 31, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority::2-high top of mind. should not wait.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0