Repository navigation
refactor(reconciler): run batcher and kueuesync as manager runnables - #1024
Merged
VLukiantsev merged 1 commit intoOct 2, 2026
Merged
VLukiantsev merged 1 commit into
VLukiantsev merged 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the registration of background runnables (the update batcher and kueue sync) by registering them directly with the controller-runtime manager, allowing them to start and stop in sync with the controllers. Feedback suggests replacing the deprecated manager.RunnableFunc with a custom runnable adapter to ensure compatibility with newer versions of the controller-runtime library.
VLukiantsev
force-pushed
the
refactor/reconciler-manager-runnables
branch
from
September 29, 2026 09:58
7d4a3fb to
b97a72f
Compare
VLukiantsev
force-pushed
the
refactor/reconciler-manager-runnables
branch
from
September 29, 2026 10:12
b97a72f to
32cad5e
Compare
VLukiantsev
force-pushed
the
refactor/reconciler-manager-runnables
branch
2 times, most recently
from
September 29, 2026 11:10
53e2ff8 to
95512c9
Compare
VLukiantsev
marked this pull request as ready for review
September 29, 2026 11:16
janzak-dev
previously approved these changes
Sep 29, 2026
VLukiantsev
force-pushed
the
refactor/reconciler-manager-runnables
branch
from
September 29, 2026 12:40
95512c9 to
bffd7a7
Compare
VLukiantsev
force-pushed
the
refactor/reconciler-manager-runnables
branch
from
September 29, 2026 12:59
bffd7a7 to
0683b7d
Compare
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit bffd7a7.
Background loops were started in two ad-hoc places: the JobSet controller's SetupWithManager registered the update batcher as a runnable (coupling an unrelated shared component to one controller), and Manager.Start spawned the kueuesync position-in-queue loop in a bare goroutine outside the controller-runtime lifecycle, with its own cancellation handling. Register both through a single addBackgroundRunnables step in registerControllers via manager.RunnableFunc, so they start after the caches sync and stop together with the controllers on shutdown. registerControllers now returns only the batcher, the Manager no longer tracks the kueuesync reconciler, and Manager.Start simply delegates to the controller-runtime manager. Tests: fakeControllerManager records added runnables; the new TestAddBackgroundRunnables covers the success path (two runnables), kueuesync construction failure, and manager Add failure. TAG=agy CONV=01d33a7b-38ec-4d7e-bfc6-b0e56c70a294
VLukiantsev
force-pushed
the
refactor/reconciler-manager-runnables
branch
from
October 2, 2026 10:08
0683b7d to
2124674
Compare
There was a problem hiding this comment.
🤖 Incremental PR Approval
Clean rebase detected with zero semantic patch lines changed.
Preserving prior approval by @janzak-dev on commit bffd7a7.
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.
Summary
Registers the update batcher and the kueuesync position-in-queue loop as controller-runtime manager runnables, so both start after caches sync and stop with the controllers on shutdown.
Stack: Part 4/5 of stack; depends on #1023.
Motivation & Context
Background loops were started in two ad-hoc places: the JobSet controller's
SetupWithManagerregistered the batcher (coupling a shared component to one controller), andManager.Startspawned kueuesync in a bare goroutine outside the controller-runtime lifecycle with its own cancellation handling. A single, manager-owned lifecycle removes that duplication and ordering risk.Key Changes
addBackgroundRunnablesstep inregisterControllersadds both loops viamanager.RunnableFunc.registerControllersreturns only the batcher;Managerno longer tracks the kueuesync reconciler;Manager.Startdelegates to the controller-runtime manager.fakeControllerManagerrecords added runnables;TestAddBackgroundRunnablescovers success (two runnables), kueuesync construction failure and managerAddfailure.Assumptions
bazel coverageyields empty reports forcompat_go_*targets (pre-existing tooling limitation).Verification & Testing
bazel build //cmd/reconciler/...,bazel test //cmd/reconciler/...,.githooks/pre-commit.batcher_*metrics served, and the E2E WorkUnit reachedCOMPLETED.Related Issues / Bugs
N/A
TAG=agy
CONV=3b97e4d5-890c-4d84-b18b-da955eeb96cd