refactor(controller): move MBC provision logic to domain/usecase/infra layers#277
Merged
Merged
Conversation
ushitora-anqou
force-pushed
the
implement-new-mbc-controller
branch
2 times, most recently
from
22be1d7 to
29b58dc
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the MantleBackupConfig (MBC) “provision” reconciliation path to follow the domain/usecase/infra architecture introduced in #250 and #252, moving controller-owned provisioning logic (PVC/StorageClass cluster-ID responsibility check, finalizer/annotation handling, metrics export, and CronJob create-or-update) out of the controller layer.
Changes:
- Implement
domain.MBCPrimaryReconciler.Provision(...)and represent CronJob create/update as a reconciler operation dispatched by the infra Kubernetes client. - Update the primary MBC usecase to orchestrate provision/finalize, including PVC → StorageClass resolution and operation application.
- Add/expand unit tests for the domain provision logic and envtest-based integration tests for the usecase provision lifecycle; make test unique-name generation concurrency-safe.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/util/util.go | Makes GetUniqueName safe under parallel test execution by using sync.Map. |
| internal/controller/usecase/reconcile_mbc_primary.go | Moves provision orchestration into the usecase (PVC/SC lookup, calling domain provision, applying operations). |
| internal/controller/usecase/reconcile_mbc_primary_test.go | Adds integration tests for full provision lifecycle, cronjob updates, and “not responsible” cases. |
| internal/controller/mantlebackupconfig_controller.go | Removes controller-side provisioning fallback and wires the expanded usecase constructor args. |
| internal/controller/infra/kubernetes_client.go | Adds infra handling for a new “create or update MBC CronJob” domain operation. |
| internal/controller/domain/mbc_primary_reconciler.go | Implements domain-level provision logic and introduces CreateOrUpdateMBCCronJobOperation. |
| internal/controller/domain/mbc_primary_reconciler_test.go | Adds unit tests covering responsibility filtering, finalizer/annotation setup, cronjob op creation, and schedule override. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
satoru-takeuchi
requested changes
May 27, 2026
| // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.16.3/pkg/reconcile | ||
| func (r *MantleBackupConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { | ||
| logger := log.FromContext(ctx) | ||
| _ = log.FromContext(ctx) |
Contributor
There was a problem hiding this comment.
Can we remove this line since we doesn't use the return value.
Contributor
Author
There was a problem hiding this comment.
…a layers Move the MantleBackupConfig provision logic (PVC cluster ID lookup, finalizer/annotation setup, metrics export, CronJob create-or-update) from the controller package into the domain/usecase/infra layers, following the same architecture established in PRs #250 and #252. Remove the ErrNotImplemented fallback pattern and the duplicated createOrUpdateCronJob/getMBCCronJobName from the controller. Add unit tests for domain.Provision and integration tests for the provision path in the usecase layer.
ushitora-anqou
force-pushed
the
implement-new-mbc-controller
branch
from
29b58dc to
3b40e66
Compare
satoru-takeuchi
approved these changes
May 28, 2026
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
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.
Move the MantleBackupConfig provision logic (PVC cluster ID lookup, finalizer/annotation setup, metrics export, CronJob create-or-update) from the controller package into the domain/usecase/infra layers, following the same architecture established in PRs #250 and #252.
Remove the ErrNotImplemented fallback pattern and the duplicated createOrUpdateCronJob/getMBCCronJobName from the controller.
Add unit tests for domain.Provision and integration tests for the provision path in the usecase layer.