feat(controllers): Multi-controller/-finalizer dispatch via `ShouldHandle(TEntity)` by stevefan1999-personal · Pull Request #1084 · dotnet/dotnet-operator-sdk

stevefan1999-personal · 2026-04-11T09:04:21Z

Multi-controller/-finalizer dispatch via `ShouldHandle(TEntity)`

Fixes #909. Also unblocks #803 and similar "dispatch by arbitrary entity predicate" requests. Subsequent PR from #1070 (comment)

Overview

Adds support for registering multiple controllers (and finalizers) for the same Kubernetes entity type, each claiming responsibility for a subset of resources based on any predicate the consumer wants to express in C# — labels, annotations, status conditions, namespace, external lookups, etc.

Per maintainer feedback on the original label-filter PR: instead of parsing a label-selector DSL string, the framework asks each controller ValueTask<bool> ShouldHandle(TEntity) and dispatches to the ones that claim responsibility. This subsumes label filtering and generalises to every future dispatch scenario.

Changes

`IEntityController<TEntity>` — new default method

ValueTask<bool> ShouldHandle(TEntity entity) => ValueTask.FromResult(true);

Controllers that want to scope themselves override it:

public ValueTask<bool> ShouldHandle(V1MyEntity entity) =>
    ValueTask.FromResult(entity.Labels()?.GetValueOrDefault("env") == "prod");

Because it's a default interface method returning true, every existing controller implementation compiles and behaves unchanged.

`IEntityFinalizer<TEntity>` — same new default method

ValueTask<bool> ShouldHandle(TEntity entity) => ValueTask.FromResult(true);

Semantics: ShouldHandle is consulted at auto-attach time. Only finalizers that return true are attached to the entity. Once attached, the finalizer is dispatched by its identifier as usual — ShouldHandle acts as a one-time responsibility claim, not an ongoing gate. This prevents orphaning entities whose attached finalizers later refuse to run.

`OperatorBuilder` — unchanged from the previous PR iteration

AddController still uses AddScoped (not TryAddScoped) so multiple registrations for the same entity type coexist and GetServices<IEntityController<TEntity>>() resolves all of them.

`Reconciler<TEntity>` — async filter via `ShouldHandle`

DispatchToMatchingControllers iterates all registered controllers, awaits ShouldHandle on each, and builds the list of responsible controllers in registration order. On the first failure the chain short-circuits. If no controller claims the entity, the reconciler logs a warning and returns success (preserving backward-compatible behaviour for entities transitioning between operator deployments).

ReconcileEntity (the auto-attach path) now awaits ShouldHandle on each finalizer before calling AddFinalizer, so finalizers only claim entities they're actually responsible for.

Removed

LabelSelectorMatcher.cs — the string-based label-selector parser
LabelSelectorMatcher.Test.cs — its 22 unit tests

The DSL-parsing layer is now the consumer's responsibility (they can reuse KubeOps.KubernetesClient.LabelSelectors or write any other predicate).

What was explicitly NOT changed

IEntityLabelSelector<TEntity> — untouched; still drives the watcher-level server-side label filter
ResourceWatcher<TEntity> — untouched; one watcher per entity type regardless of controller count
AddController<TImplementation, TEntity>() signature — unchanged
All existing controller and finalizer implementations — compile and behave identically

Tests

Reconciler.MultiController.Test.cs — 12 tests: default catch-all ShouldHandle, predicate match/no-match, ordering, failure short-circuit, entity mutation pass-through, DeletedAsync path, async ShouldHandle awaited before dispatch
OperatorBuilder.Test.cs — 3 tests for multi-registration resolution and no-duplicate-watcher (unchanged from previous iteration)
Reconciler.Test.cs — updated existing mocks to stub ShouldHandle (returning true)

All 67 non-integration tests in KubeOps.Operator.Test pass. (Integration tests require a live Kubernetes cluster and are unrelated to this change.)

Example

// Dispatch by labels (the #909 use-case)
public class ProdController : IEntityController<MyEntity>
{
    public ValueTask<bool> ShouldHandle(MyEntity e) =>
        ValueTask.FromResult(e.Labels()?.GetValueOrDefault("env") == "prod");
    // ...
}

// Dispatch by status conditions (the #803-style use-case)
public class DegradedHandler : IEntityController<MyEntity>
{
    public ValueTask<bool> ShouldHandle(MyEntity e) =>
        ValueTask.FromResult(e.Status?.Phase == "Degraded");
    // ...
}

// Catch-all auditing controller (no override needed)
public class AuditController : IEntityController<MyEntity> { /* default ShouldHandle returns true */ }

builder.AddController<ProdController, MyEntity>();
builder.AddController<DegradedHandler, MyEntity>();
builder.AddController<AuditController, MyEntity>();

Backward compatibility

The default implementation of ShouldHandle returns true, so every existing controller and finalizer behaves identically to before. No existing code requires changes.

Design decisions

"No controller claims the entity" → Success + warning (not failure). Rationale: transitional systems (entity exists before its owning operator is deployed) are legitimate; making this a reconcile failure would spam error alerts during rollouts.
Finalizer ShouldHandle is consulted at attach time, not dispatch time. Rationale: once a finalizer is written into entity metadata, it's a contract with the API server. Re-evaluating at dispatch risks orphaning entities with finalizers that now refuse to run.
ShouldHandle is called on every reconcile pass with no memoisation. Rationale: simplest, no cache-invalidation complexity. Consumers are responsible for keeping it fast; they can memoise internally if needed.

…ndle(TEntity)`

Copilot

Pull request overview

Adds predicate-based multi-controller and multi-finalizer dispatch for the same Kubernetes entity type by introducing ShouldHandle(TEntity) on controllers/finalizers and updating reconciliation to dispatch to all matching registrations.

Changes:

Add default ShouldHandle(TEntity) methods to IEntityController<TEntity> and IEntityFinalizer<TEntity> (defaulting to true).
Update OperatorBuilder.AddController to allow multiple scoped controller registrations for the same entity type.
Update Reconciler<TEntity> to filter controllers/finalizers via ShouldHandle and add/adjust unit tests for multi-controller dispatch.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/KubeOps.Abstractions/Reconciliation/Controller/IEntityController{TEntity}.cs	Introduces `ShouldHandle(TEntity)` default interface method for controller responsibility claims.
src/KubeOps.Abstractions/Reconciliation/Finalizer/IEntityFinalizer{TEntity}.cs	Introduces `ShouldHandle(TEntity)` default interface method for finalizer responsibility claims (used at auto-attach time).
src/KubeOps.Operator/Builder/OperatorBuilder.cs	Switches controller registration to `AddScoped` to allow multiple controllers per entity type.
src/KubeOps.Operator/Reconciliation/Reconciler.cs	Dispatches to all controllers matching `ShouldHandle` and filters finalizers during auto-attach.
test/KubeOps.Operator.Test/Builder/OperatorBuilder.Test.cs	Adds tests validating multi-controller registrations and watcher non-duplication.
test/KubeOps.Operator.Test/LeaderElector/LeaderElectionBackgroundService.Test.cs	Improves retry test synchronization to reduce flakiness.
test/KubeOps.Operator.Test/Reconciliation/Reconciler.Test.cs	Updates mocks to account for new `ShouldHandle` and enumerable controller resolution.
test/KubeOps.Operator.Test/Reconciliation/Reconciler.MultiController.Test.cs	Adds multi-controller dispatch tests around `ShouldHandle`, ordering, and short-circuit behavior.

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

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

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

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

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

Copilot · 2026-04-11T09:33:27Z

+        foreach (var controller in responsibleControllers)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            result = await operation(controller, result.Entity, cancellationToken);
+            if (!result.IsSuccess) return result;
+        }
+
+        return result;


In the multi-controller loop, any RequeueAfter requested by an earlier controller can be overwritten/dropped by a later controller that returns success with RequeueAfter == null, because result is replaced each iteration. This can change behavior when adding an additional controller (e.g., an auditing controller) and prevent intended requeues. Consider aggregating RequeueAfter across all successful controller results (e.g., keep the earliest/non-null) while still passing the latest Entity through the chain.

Suggested change

foreach (var controller in responsibleControllers)

{

cancellationToken.ThrowIfCancellationRequested();

result = await operation(controller, result.Entity, cancellationToken);

if (!result.IsSuccess) return result;

}

return result;

var requeueAfter = result.RequeueAfter;

foreach (var controller in responsibleControllers)

{

cancellationToken.ThrowIfCancellationRequested();

result = await operation(controller, result.Entity, cancellationToken);

if (!result.IsSuccess) return result;

if (result.RequeueAfter is not null &&

(requeueAfter is null || result.RequeueAfter < requeueAfter))

{

requeueAfter = result.RequeueAfter;

}

}

return ReconciliationResult<TEntity>.Success(result.Entity, requeueAfter);

Copilot · 2026-04-11T09:33:28Z

+        var responsibleControllers = new List<IEntityController<TEntity>>();
+        foreach (var controller in registeredControllers)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            if (await controller.ShouldHandle(entity))
+            {
+                responsibleControllers.Add(controller);
+            }
+        }
+
+        if (responsibleControllers.Count == 0)
+        {
+            logger.LogWarning(
+                """No responsible controller found for "{Kind}/{Name}". Skipping.""",
+                entity.Kind,
+                entity.Name());
+            return ReconciliationResult<TEntity>.Success(entity);
+        }
+
+        ReconciliationResult<TEntity> result = ReconciliationResult<TEntity>.Success(entity);
+        foreach (var controller in responsibleControllers)
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+            result = await operation(controller, result.Entity, cancellationToken);
+            if (!result.IsSuccess) return result;


ShouldHandle is evaluated for all controllers against the initial entity before any controller runs, but the dispatched controllers receive the potentially mutated result.Entity. This means a controller can be invoked even if it would return false for the entity state it actually receives. Consider evaluating ShouldHandle just-in-time per controller using the current entity (or document that ShouldHandle is evaluated only on the pre-dispatch entity).

Copilot · 2026-04-11T09:33:28Z

+        var registeredControllers = services.GetServices<IEntityController<TEntity>>().ToList();
+        if (registeredControllers.Count == 0)
+        {
+            return ReconciliationResult<TEntity>.Failure(
+                entity,
+                $"No IEntityController<{typeof(TEntity).Name}> registered. Did you forget to call AddController<T, TEntity>() on the operator builder?");
+        }


Cancellation is checked only after resolving/enumerating controllers via GetServices(...).ToList(). If cancellation is already requested, this still constructs all scoped controllers (and could run their constructors) unnecessarily. Consider calling cancellationToken.ThrowIfCancellationRequested() before resolving controllers (and similarly before enumerating finalizers) to avoid work after cancellation.

Copilot · 2026-04-11T09:33:28Z

+        var provider = _builder.Services.BuildServiceProvider();
+        var controllers = provider
+            .GetServices<IEntityController<V1OperatorIntegrationTestEntity>>()
+            .ToList();
+
+        controllers.Should().HaveCount(2);
+        controllers.Should().ContainItemsAssignableTo<IEntityController<V1OperatorIntegrationTestEntity>>();
+        controllers.Select(c => c.GetType()).Should().Contain(typeof(TestController));
+        controllers.Select(c => c.GetType()).Should().Contain(typeof(SecondTestController));


This test resolves scoped IEntityController<T> instances directly from the root ServiceProvider. In production this is typically done from an IServiceScope (and can fail if ValidateScopes is enabled). Consider creating a scope (provider.CreateScope()) and resolving controllers from scope.ServiceProvider to better reflect runtime behavior.

…on timing

Copilot

Pull request overview

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

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

kimpenhaus

hey @stevefan1999-personal thanks for the new pull request.

I've added some comments - in general I think the pattern should still be that only 1 controller/finalizer at a time should be responsible for the reconciliation of an entity but dispatching is more fine grained to that component (by ShouldHandle).
I will close the old pull request - as this will replace it
I think the docs should also get an update 😄

cheers

kimpenhaus · 2026-04-12T08:58:24Z

+    /// <param name="entity">The entity the reconciler is about to dispatch.</param>
+    /// <param name="cancellationToken">The token used to signal cancellation of the operation.</param>
+    /// <returns>A <see cref="ValueTask{Boolean}"/> that resolves to <c>true</c> if this controller should reconcile the entity.</returns>
+    ValueTask<bool> ShouldHandle(TEntity entity, CancellationToken cancellationToken = default) => ValueTask.FromResult(true);


@stevefan1999-personal would prefer this not to be = default by default but mandatory

kimpenhaus · 2026-04-12T08:58:43Z

+    /// <param name="entity">The entity the reconciler is considering for this finalizer.</param>
+    /// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
+    /// <returns>A <see cref="ValueTask{Boolean}"/> that resolves to <c>true</c> if this finalizer should claim the entity.</returns>
+    ValueTask<bool> ShouldHandle(TEntity entity, CancellationToken cancellationToken = default) => ValueTask.FromResult(true);


same here (mandatory not = default)

kimpenhaus · 2026-04-12T08:59:11Z

        where TEntity : IKubernetesObject<V1ObjectMeta>
    {
-        Services.TryAddScoped<IEntityController<TEntity>, TImplementation>();
+        // TryAddEnumerable dedupes by (ServiceType, ImplementationType), so calling AddController


not sure this AI comment is really needed 😄

kimpenhaus · 2026-04-12T09:00:48Z


        if (operatorSettings.AutoAttachFinalizers)
        {
+            cancellationToken.ThrowIfCancellationRequested();


can't see any benefit in this line

kimpenhaus · 2026-04-12T09:01:50Z

+        Func<IEntityController<TEntity>, TEntity, CancellationToken, Task<ReconciliationResult<TEntity>>> operation,
+        CancellationToken cancellationToken)
+    {
+        cancellationToken.ThrowIfCancellationRequested();


same here - please remove

kimpenhaus · 2026-04-12T09:07:03Z

+    /// returns <c>Success(entity)</c> never erases a requeue requested by an earlier controller.
+    /// </para>
+    /// </summary>
+    private async Task<ReconciliationResult<TEntity>> DispatchToMatchingControllers(


to be honest I am pretty unsure about the possibility to have more than 1 controller being responsible for an entity - shouldn't the pattern be more like:

if there == 1 controller -> dispatch

if there == 0 -> finish with not dispatch

if there > 1 -> error

kimpenhaus · 2026-04-12T09:10:36Z

+            foreach (var finalizer in finalizers)
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+                if (!await finalizer.ShouldHandle(entity, cancellationToken))


I think this is the same as with the controllers - this could lead to being > 1 finalizers being responsible for an entity which might lead to weired results.

feat(controllers): Multi-controller/-finalizer dispatch via `ShouldHa…

e304e28

…ndle(TEntity)`

Copilot AI review requested due to automatic review settings April 11, 2026 09:04

Copilot started reviewing on behalf of stevefan1999-personal April 11, 2026 09:05 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

stevefan1999-personal added 2 commits April 11, 2026 17:10

Merge branch 'main' into feat/reconcile-status-update

acdbe13

fix(reconciler): don't make it pass if no controllers are registered

1a87f17

stevefan1999-personal requested a review from Copilot April 11, 2026 09:20

Copilot started reviewing on behalf of stevefan1999-personal April 11, 2026 09:21 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Comment thread src/KubeOps.Operator/Builder/OperatorBuilder.cs Outdated

fix(operator): TryAddEnumerable instead of AddScoped

e93746d

stevefan1999-personal requested a review from Copilot April 11, 2026 09:29

Copilot started reviewing on behalf of stevefan1999-personal April 11, 2026 09:29 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

fix(reconciler): more robust handling on requeing and object evaluati…

93314b2

…on timing

stevefan1999-personal requested a review from Copilot April 11, 2026 09:44

Copilot started reviewing on behalf of stevefan1999-personal April 11, 2026 09:44 View session

Copilot AI reviewed Apr 11, 2026

View reviewed changes

Comment thread src/KubeOps.Operator/Builder/OperatorBuilder.cs

Comment thread src/KubeOps.Abstractions/Reconciliation/Controller/IEntityController{TEntity}.cs Outdated

Comment thread src/KubeOps.Abstractions/Reconciliation/Finalizer/IEntityFinalizer{TEntity}.cs

fix(reconciler): pass cancellation token down the line

5fabcd5

kimpenhaus reviewed Apr 12, 2026

View reviewed changes

kimpenhaus mentioned this pull request Apr 12, 2026

feat(controllers): Enable multiple controllers with different label selectors for the same entity type (V2) #1070

Closed

Merge branch 'main' into feat/reconcile-status-update

49f82e5

Conversation

stevefan1999-personal commented Apr 11, 2026

Multi-controller/-finalizer dispatch via ShouldHandle(TEntity)

Overview

Changes

IEntityController<TEntity> — new default method

IEntityFinalizer<TEntity> — same new default method

OperatorBuilder — unchanged from the previous PR iteration

Reconciler<TEntity> — async filter via ShouldHandle

Removed

What was explicitly NOT changed

Tests

Example

Backward compatibility

Design decisions

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kimpenhaus left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

kimpenhaus Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

Multi-controller/-finalizer dispatch via `ShouldHandle(TEntity)`

`IEntityController<TEntity>` — new default method

`IEntityFinalizer<TEntity>` — same new default method

`OperatorBuilder` — unchanged from the previous PR iteration

`Reconciler<TEntity>` — async filter via `ShouldHandle`

kimpenhaus left a comment •

edited

Loading

kimpenhaus Apr 12, 2026 •

edited

Loading

kimpenhaus Apr 12, 2026 •

edited

Loading