Skip to content

controller: introduce reconcileResult for MantleBackupReconciler#299

Merged
ushitora-anqou merged 1 commit into
mainfrom
7388-multi-controller
Jul 2, 2026
Merged

controller: introduce reconcileResult for MantleBackupReconciler#299
ushitora-anqou merged 1 commit into
mainfrom
7388-multi-controller

Conversation

@llamerada-jp

@llamerada-jp llamerada-jp commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@llamerada-jp llamerada-jp force-pushed the 7388-multi-controller branch 2 times, most recently from 61079c5 to 1850105 Compare June 26, 2026 02:10
@llamerada-jp llamerada-jp requested a review from Copilot June 26, 2026 02:48

Copilot AI 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.

Pull request overview

This PR refactors the MantleBackup controller’s reconciliation flow to use a unified reconcileResult abstraction and introduces/uses a mantle.cybozu.io/cluster-id label to help ensure MantleBackup resources are processed by the correct controller in multi-controller environments.

Changes:

  • Added reconcileResult helper type to standardize “success / requeue / error” outcomes across controller helper methods.
  • Updated MantleBackup controller logic to return *reconcileResult from many helpers and added checkManagedBackup() gating logic based on cluster identity.
  • Updated controller tests to adapt to the new reconcileResult return style.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
internal/controller/reconcile_result.go New reconcileResult type and helpers to convert internal outcomes to (ctrl.Result, error).
internal/controller/mantlebackup_controller.go Refactors reconciliation to use reconcileResult; adds cluster-id label usage and checkManagedBackup() filtering.
internal/controller/mantlebackup_controller_test.go Updates tests to work with reconcileResult (getResult() calls).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/controller/mantlebackup_controller.go Outdated
Comment thread internal/controller/mantlebackup_controller.go Outdated
Comment thread internal/controller/reconcile_result.go Outdated
Comment thread internal/controller/mantlebackup_controller.go
Comment thread internal/controller/mantlebackup_controller.go Outdated
@llamerada-jp llamerada-jp force-pushed the 7388-multi-controller branch 4 times, most recently from bf66549 to 7363ce7 Compare June 29, 2026 01:21
@llamerada-jp llamerada-jp changed the title filter resources correctly in a multi-controller environment controller: introduce reconcileResult for MantleBackupReconciler Jun 29, 2026
@llamerada-jp llamerada-jp marked this pull request as ready for review June 29, 2026 01:23
@llamerada-jp llamerada-jp requested a review from a team as a code owner June 29, 2026 01:23
@llamerada-jp llamerada-jp requested review from ushitora-anqou and removed request for a team June 29, 2026 01:23
@llamerada-jp llamerada-jp marked this pull request as draft June 29, 2026 02:08
@llamerada-jp llamerada-jp changed the title controller: introduce reconcileResult for MantleBackupReconciler filter resources correctly in a multi-controller environment Jun 29, 2026
@llamerada-jp llamerada-jp force-pushed the 7388-multi-controller branch from afc4900 to 753e09c Compare June 29, 2026 03:07
@llamerada-jp llamerada-jp force-pushed the 7388-multi-controller branch from 2a50a7d to 2e3c5f8 Compare June 29, 2026 08:55
@llamerada-jp llamerada-jp requested a review from Copilot June 29, 2026 08:56

Copilot AI 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.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comment thread internal/testutil/resources.go Outdated
Comment thread internal/controller/mantlebackup_controller.go Outdated
Comment thread internal/controller/mantlebackup_controller.go
@llamerada-jp llamerada-jp force-pushed the 7388-multi-controller branch 5 times, most recently from 0417992 to 41d7edc Compare June 30, 2026 03:10
@llamerada-jp llamerada-jp marked this pull request as ready for review June 30, 2026 04:01
Comment thread internal/controller/mantlebackup_controller.go
Comment thread internal/controller/reconcile_result.go Outdated
Comment thread internal/controller/reconcile_result.go Outdated
Comment thread internal/controller/reconcile_result.go Outdated
Comment thread internal/controller/mantlebackup_controller.go Outdated
@llamerada-jp llamerada-jp changed the title filter resources correctly in a multi-controller environment controller: introduce reconcileResult for MantleBackupReconciler Jul 1, 2026
Replace the (ctrl.Result, error) return convention used throughout
MantleBackupReconciler's helper methods with a single *reconcile.Result
value. reconcile.Result encodes exactly one of four outcomes: success,
requeue, error, or "continue" (nil), which makes the control flow of
each step explicit and removes the need to juggle ctrl.Result and error
side by side at every call site.

Signed-off-by: Yuji Ito <llamerada.jp@gmail.com>
@llamerada-jp llamerada-jp force-pushed the 7388-multi-controller branch from 41d7edc to 19fdf2b Compare July 1, 2026 07:00
@ushitora-anqou ushitora-anqou merged commit 345f1d8 into main Jul 2, 2026
15 of 16 checks passed
@ushitora-anqou ushitora-anqou deleted the 7388-multi-controller branch July 2, 2026 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants