refactor(multigrescluster): split controller, fix deletion, and update API#112
Merged
fernando-villalba merged 2 commits intomainfrom Jan 6, 2026
Merged
Conversation
…e API Refactors the MultigresCluster controller for better maintainability and fixes critical issues with deletion and default resolution. Key Changes: - **Controller Refactor:** Split the monolithic `MultigresCluster` controller into logical files (`reconcile_global.go`, `reconcile_cells.go`, `reconcile_databases.go`, `status.go`). - **Active Deletion:** Implemented "Active Deletion" logic in `checkChildrenDeleted`. The controller now explicitly deletes child resources (`Cells`, `TableGroups`) instead of relying solely on Garbage Collection. This fixes hanging tests in `envtest` environments. - **Smart Defaulting Fix:** Updated `PopulateClusterDefaults` in the Resolver to propagate the Cluster's `Cells` to the default Shard's `MultiOrch` spec. This resolves the "MultiOrch has no cells" error in minimal configurations. - **API Update:** Changed `GlobalTopoServer` and `MultiAdmin` fields in `MultigresClusterSpec` to pointers. This allows proper detection of "missing" vs "empty" fields for lazy configuration. - **Tests:** Split unit and integration tests into focused files matching the controller structure. Updated test expectations to align with the new active deletion error messages and default injection logic. - **Build:** Updated `Makefile` to use `kubectl apply --server-side` for installing CRDs to avoid size limit issues.
This comment has been minimized.
This comment has been minimized.
🔬 Go Test Coverage ReportSummary
StatusDetailShow New Coverage |
Collaborator
Author
|
Merging this to fix some critical issues on the cluster handler, resolver and also update the API so that the linter can pass on CI. The test coverage is currently 100%. Also additional smoke testing and integration testing was done to verify proper functionality. I will do another PR straight after this one to ensure the go module references point to the correct API version and we don't get the linter issues. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactors the MultigresCluster controller for better maintainability and fixes critical issues with deletion and default resolution.
Key Changes:
MultigresClustercontroller into logical files (reconcile_global.go,reconcile_cells.go,reconcile_databases.go,status.go).checkChildrenDeleted. The controller now explicitly deletes child resources (Cells,TableGroups) instead of relying solely on Garbage Collection. This fixes hanging tests inenvtestenvironments.PopulateClusterDefaultsin the Resolver to propagate the Cluster'sCellsto the default Shard'sMultiOrchspec. This resolves the "MultiOrch has no cells" error in minimal configurations.GlobalTopoServerandMultiAdminfields inMultigresClusterSpecto pointers. This allows proper detection of "missing" vs "empty" fields for lazy configuration.Makefileto usekubectl apply --server-sidefor installing CRDs to avoid size limit issues.