Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ namespace KubeOps.Abstractions.Reconciliation.Controller;
public interface IEntityController<TEntity>
where TEntity : IKubernetesObject<V1ObjectMeta>
{
/// <summary>
/// Returns <c>true</c> when this controller is responsible for the given entity.
/// The default implementation returns <c>true</c>, preserving single-controller backward-compatible behaviour.
///
/// When multiple controllers are registered for the same entity type the reconciler asks every
/// controller whether it <see cref="ShouldHandle"/> the entity and dispatches to all that claim
/// responsibility in registration order. Typical use cases: filtering by labels, annotations,
/// namespace, status conditions, or any other entity-derived predicate the consumer needs.
/// </summary>
/// <param name="entity">The entity the reconciler is about to dispatch.</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) => ValueTask.FromResult(true);
Comment thread
stevefan1999-personal marked this conversation as resolved.
Outdated

/// <summary>
/// Reconciles the state of the specified entity with the desired state.
/// This method is triggered for `added` and `modified` events from the watcher.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ namespace KubeOps.Abstractions.Reconciliation.Finalizer;
public interface IEntityFinalizer<TEntity>
where TEntity : IKubernetesObject<V1ObjectMeta>
{
/// <summary>
/// Returns <c>true</c> when this finalizer is responsible for the given entity.
/// The default implementation returns <c>true</c>, preserving backward-compatible behaviour.
///
/// When <see cref="KubeOps.Abstractions.Builder.OperatorSettings.AutoAttachFinalizers"/> is enabled,
/// only finalizers that return <c>true</c> from <see cref="ShouldHandle"/> are attached to the entity.
/// Once attached, the finalizer is dispatched by its identifier as usual — <see cref="ShouldHandle"/>
/// acts as a one-time responsibility claim at attach time, not an ongoing gate.
/// </summary>
/// <param name="entity">The entity the reconciler is considering for this finalizer.</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) => ValueTask.FromResult(true);

/// <summary>
Comment thread
stevefan1999-personal marked this conversation as resolved.
/// Finalize an entity that is pending for deletion.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/KubeOps.Operator/Builder/OperatorBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public IOperatorBuilder AddController<TImplementation, TEntity>()
where TImplementation : class, IEntityController<TEntity>
where TEntity : IKubernetesObject<V1ObjectMeta>
{
Services.TryAddScoped<IEntityController<TEntity>, TImplementation>();
Services.AddScoped<IEntityController<TEntity>, TImplementation>();
Comment thread
stevefan1999-personal marked this conversation as resolved.
Outdated
Services.TryAddSingleton<IReconciler<TEntity>, Reconciler<TEntity>>();
Comment thread
stevefan1999-personal marked this conversation as resolved.

// Requeue
Expand Down
69 changes: 61 additions & 8 deletions src/KubeOps.Operator/Reconciliation/Reconciler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using k8s.Models;

using KubeOps.Abstractions.Builder;
using KubeOps.Abstractions.Entities;
using KubeOps.Abstractions.Reconciliation;
using KubeOps.Abstractions.Reconciliation.Controller;
using KubeOps.Abstractions.Reconciliation.Finalizer;
Expand Down Expand Up @@ -115,8 +116,11 @@ await entityQueue
cancellationToken);

await using var scope = serviceProvider.CreateAsyncScope();
var controller = scope.ServiceProvider.GetRequiredService<IEntityController<TEntity>>();
var result = await controller.DeletedAsync(reconciliationContext.Entity, cancellationToken);
var result = await DispatchToMatchingControllers(
scope.ServiceProvider,
reconciliationContext.Entity,
(ctrl, entity, ct) => ctrl.DeletedAsync(entity, ct),
cancellationToken);

if (result.IsSuccess)
{
Expand All @@ -139,19 +143,68 @@ await entityQueue
{
var finalizers = scope.ServiceProvider.GetKeyedServices<IEntityFinalizer<TEntity>>(KeyedService.AnyKey);

var anyFinalizerAdded = finalizers
.Aggregate(
false,
(changed, finalizer) => entity.AddFinalizer(finalizer.GetIdentifierName(entity)) || changed);
var anyFinalizerAdded = false;
foreach (var finalizer in finalizers)
{
if (!await finalizer.ShouldHandle(entity))
{
continue;
}

anyFinalizerAdded = entity.AddFinalizer(finalizer.GetIdentifierName(entity)) || anyFinalizerAdded;
}
Comment thread
stevefan1999-personal marked this conversation as resolved.

if (anyFinalizerAdded)
{
entity = await client.UpdateAsync(entity, cancellationToken);
}
}

var controller = scope.ServiceProvider.GetRequiredService<IEntityController<TEntity>>();
return await controller.ReconcileAsync(entity, cancellationToken);
return await DispatchToMatchingControllers(
scope.ServiceProvider,
entity,
(ctrl, e, ct) => ctrl.ReconcileAsync(e, ct),
cancellationToken);
}

/// <summary>
/// Gets all <see cref="IEntityController{TEntity}"/> registrations whose <see cref="IEntityController{TEntity}.ShouldHandle"/>
/// returns <c>true</c> for the given entity, then calls <paramref name="operation"/> on each in registration order.
/// On the first failure the chain is short-circuited and that failure result is returned.
/// If no controller claims responsibility, a success result is returned and a warning is logged.
/// </summary>
private async Task<ReconciliationResult<TEntity>> DispatchToMatchingControllers(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

IServiceProvider services,
TEntity entity,
Func<IEntityController<TEntity>, TEntity, CancellationToken, Task<ReconciliationResult<TEntity>>> operation,
CancellationToken cancellationToken)
{
var responsibleControllers = new List<IEntityController<TEntity>>();
foreach (var controller in services.GetServices<IEntityController<TEntity>>())
{
if (await controller.ShouldHandle(entity))
{
responsibleControllers.Add(controller);
}
}
Comment thread
stevefan1999-personal marked this conversation as resolved.
Outdated

if (responsibleControllers.Count == 0)
{
logger.LogWarning(
"""No responsible controller found for "{Kind}/{Name}". Skipping.""",
entity.Kind,
entity.Name());
return ReconciliationResult<TEntity>.Success(entity);
}
Comment thread
stevefan1999-personal marked this conversation as resolved.
Outdated

ReconciliationResult<TEntity> result = ReconciliationResult<TEntity>.Success(entity);
foreach (var controller in responsibleControllers)
{
result = await operation(controller, result.Entity, cancellationToken);
if (!result.IsSuccess) return result;
}

return result;
}

private async Task<ReconciliationResult<TEntity>> ReconcileFinalizersSequential(TEntity entity, CancellationToken cancellationToken)
Expand Down
56 changes: 56 additions & 0 deletions test/KubeOps.Operator.Test/Builder/OperatorBuilder.Test.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,53 @@ public void Should_Add_Leader_Elector()
s.Lifetime == ServiceLifetime.Singleton);
}

[Fact]
public void Should_Allow_Multiple_Controllers_For_Same_Entity_Type()
{
_builder.AddController<TestController, V1OperatorIntegrationTestEntity>();
_builder.AddController<SecondTestController, V1OperatorIntegrationTestEntity>();

var registrations = _builder.Services
.Where(s =>
s.ServiceType == typeof(IEntityController<V1OperatorIntegrationTestEntity>) &&
s.Lifetime == ServiceLifetime.Scoped)
.ToList();

registrations.Should().HaveCount(2);
registrations.Should().Contain(s => s.ImplementationType == typeof(TestController));
registrations.Should().Contain(s => s.ImplementationType == typeof(SecondTestController));
}

[Fact]
public void Should_Resolve_All_Controllers_For_Same_Entity_Type()
{
_builder.AddController<TestController, V1OperatorIntegrationTestEntity>();
_builder.AddController<SecondTestController, V1OperatorIntegrationTestEntity>();

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));

Copilot AI Apr 11, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}

[Fact]
public void Should_Not_Register_Duplicate_ResourceWatcher_For_Multiple_Controllers()
{
_builder.AddController<TestController, V1OperatorIntegrationTestEntity>();
_builder.AddController<SecondTestController, V1OperatorIntegrationTestEntity>();

_builder.Services
.Where(s =>
s.ServiceType == typeof(IHostedService) &&
s.ImplementationType == typeof(ResourceWatcher<V1OperatorIntegrationTestEntity>))
.Should().HaveCount(1);
}

[Fact]
public void Should_Add_LeaderAwareResourceWatcher()
{
Expand All @@ -152,6 +199,15 @@ public Task<ReconciliationResult<V1OperatorIntegrationTestEntity>> DeletedAsync(
Task.FromResult(ReconciliationResult<V1OperatorIntegrationTestEntity>.Success(entity));
}

private sealed class SecondTestController : IEntityController<V1OperatorIntegrationTestEntity>
{
public Task<ReconciliationResult<V1OperatorIntegrationTestEntity>> ReconcileAsync(V1OperatorIntegrationTestEntity entity, CancellationToken cancellationToken) =>
Task.FromResult(ReconciliationResult<V1OperatorIntegrationTestEntity>.Success(entity));

public Task<ReconciliationResult<V1OperatorIntegrationTestEntity>> DeletedAsync(V1OperatorIntegrationTestEntity entity, CancellationToken cancellationToken) =>
Task.FromResult(ReconciliationResult<V1OperatorIntegrationTestEntity>.Success(entity));
}

private sealed class TestFinalizer : IEntityFinalizer<V1OperatorIntegrationTestEntity>
{
public Task<ReconciliationResult<V1OperatorIntegrationTestEntity>> FinalizeAsync(V1OperatorIntegrationTestEntity entity, CancellationToken cancellationToken) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public async Task Elector_Throws_Should_Retry()

var electionLock = Mock.Of<ILock>();

var electionLockSubsequentCallEvent = new AutoResetEvent(false);
var subsequentCallTcs = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
bool hasElectionLockThrown = false;
Mock.Get(electionLock)
.Setup(el => el.GetAsync(It.IsAny<CancellationToken>()))
Expand All @@ -34,7 +34,7 @@ public async Task Elector_Throws_Should_Retry()
if (hasElectionLockThrown)
{
// Signal to the test that a subsequent call has been made.
electionLockSubsequentCallEvent.Set();
subsequentCallTcs.TrySetResult(true);

// Delay returning for a long time, allowing the test to stop the background service, in turn cancelling the cancellation token.
await Task.Delay(TimeSpan.FromSeconds(10), cancellationToken);
Expand All @@ -54,8 +54,9 @@ public async Task Elector_Throws_Should_Retry()
await leaderElectionBackgroundService.StartAsync(CancellationToken.None);

// Starting the background service should result in the lock attempt throwing, and then a subsequent attempt being made.
// Wait for the subsequent event to be signalled, if we time out the test fails. The retry delay requires us to wait at least 3 seconds.
electionLockSubsequentCallEvent.WaitOne(TimeSpan.FromMilliseconds(3100)).Should().BeTrue();
// Wait for the retry to be signalled; use a generous timeout so CI scheduling jitter doesn't cause false failures.
var completed = await Task.WhenAny(subsequentCallTcs.Task, Task.Delay(TimeSpan.FromSeconds(10), TestContext.Current.CancellationToken));
completed.Should().Be(subsequentCallTcs.Task, "the leader elector should retry after throwing");

await leaderElectionBackgroundService.StopAsync(CancellationToken.None);
}
Expand Down
Loading
Loading