Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ namespace KubeOps.Abstractions.Reconciliation.Controller;
public interface IEntityController<TEntity>
where TEntity : IKubernetesObject<V1ObjectMeta>
{
/// <summary>
/// An optional Kubernetes label selector expression (e.g. <c>app in (foo,bar),env in (prod)</c>) that
/// restricts which entities this controller handles. When <c>null</c> (the default) the controller
/// handles all entities of the given type, preserving backward-compatible behaviour.
///
/// When multiple controllers are registered for the same entity type the reconciler evaluates every
/// controller's <see cref="LabelFilter"/> against the entity's labels and dispatches to all that
/// match, allowing fine-grained fan-out without touching the watcher or DI registration plumbing.
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

The doc comment mentions fan-out works "without touching the watcher or DI registration plumbing", but enabling multiple controllers for the same entity type does require DI registration changes (e.g., TryAddScoped -> AddScoped). Reword this to avoid misleading readers (e.g., emphasize no watcher changes / no signature changes instead).

Suggested change
/// match, allowing fine-grained fan-out without touching the watcher or DI registration plumbing.
/// match, allowing fine-grained fan-out without changing the watcher logic or controller method signatures.

Copilot uses AI. Check for mistakes.
/// </summary>
string? LabelFilter => null;

/// <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
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>();
Services.TryAddSingleton<IReconciler<TEntity>, Reconciler<TEntity>>();

// Requeue
Expand Down
108 changes: 108 additions & 0 deletions src/KubeOps.Operator/Reconciliation/LabelSelectorMatcher.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information.

namespace KubeOps.Operator.Reconciliation;

/// <summary>
/// Evaluates a Kubernetes label-selector expression against an entity's label dictionary.
/// Supports the set-based formats emitted by KubeOps.KubernetesClient label-selector types:
/// <list type="bullet">
/// <item><c>key in (v1,v2)</c> – EqualsSelector</item>
/// <item><c>key notin (v1,v2)</c> – NotEqualsSelector</item>
/// <item><c>key</c> – ExistsSelector</item>
/// <item><c>!key</c> – NotExistsSelector</item>
Comment on lines +9 to +14
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

The XML doc says this matcher "supports the set-based formats emitted" by the KubeOps label-selector types, but the implementation also supports key=value and key!=value clauses. Either document these additional supported operators or remove them to keep behavior and docs aligned.

Suggested change
/// Supports the set-based formats emitted by KubeOps.KubernetesClient label-selector types:
/// <list type="bullet">
/// <item><c>key in (v1,v2)</c> – EqualsSelector</item>
/// <item><c>key notin (v1,v2)</c> – NotEqualsSelector</item>
/// <item><c>key</c> – ExistsSelector</item>
/// <item><c>!key</c> – NotExistsSelector</item>
/// Supports the set-based formats emitted by KubeOps.KubernetesClient label-selector types, as well as equality-based clauses:
/// <list type="bullet">
/// <item><c>key in (v1,v2)</c> – EqualsSelector</item>
/// <item><c>key notin (v1,v2)</c> – NotEqualsSelector</item>
/// <item><c>key</c> – ExistsSelector</item>
/// <item><c>!key</c> – NotExistsSelector</item>
/// <item><c>key=value</c> – equality match</item>
/// <item><c>key!=value</c> – inequality match</item>

Copilot uses AI. Check for mistakes.
/// </list>
/// Multiple clauses joined by commas are evaluated as AND.
/// </summary>
internal static class LabelSelectorMatcher
{
internal static bool Matches(string? selector, IReadOnlyDictionary<string, string>? entityLabels)
{
if (selector is null) return true;
entityLabels ??= new Dictionary<string, string>();

return SplitTopLevel(selector).All(clause => MatchClause(clause, entityLabels));
}
Comment on lines +20 to +26
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

Matches treats only null as a catch-all, but an empty/whitespace selector string currently matches nothing (because it becomes an empty clause). LabelSelector[].ToExpression() returns an empty string for an empty selector list, and Kubernetes semantics for an empty selector are "match all". Consider handling string.IsNullOrWhiteSpace(selector) as true and/or skipping empty clauses in SplitTopLevel (also covers trailing commas). Add a unit test for the empty-string case to prevent regressions.

Copilot uses AI. Check for mistakes.

// Splits "key in (a,b),other notin (c)" at top-level commas only (ignores commas inside parens).
private static IEnumerable<string> SplitTopLevel(string selector)
{
int depth = 0, start = 0;
for (int i = 0; i < selector.Length; i++)
{
switch (selector[i])
{
case '(':
depth++;
break;
case ')':
depth--;
break;
case ',' when depth == 0:
yield return selector[start..i].Trim();
start = i + 1;
break;
}
}

if (start < selector.Length)
{
yield return selector[start..].Trim();
}
}

private static bool MatchClause(string clause, IReadOnlyDictionary<string, string> labels)
{
const string inOp = " in (";
const string notinOp = " notin (";

// "key in (v1,v2)"
int idx = clause.IndexOf(inOp, StringComparison.Ordinal);
if (idx >= 0 && clause.EndsWith(')'))
{
var key = clause[..idx].Trim();
var values = ParseValues(clause[(idx + inOp.Length)..^1]);
return labels.TryGetValue(key, out var v) && values.Contains(v);
}

// "key notin (v1,v2)"
idx = clause.IndexOf(notinOp, StringComparison.Ordinal);
if (idx >= 0 && clause.EndsWith(')'))
{
var key = clause[..idx].Trim();
var values = ParseValues(clause[(idx + notinOp.Length)..^1]);
return !labels.TryGetValue(key, out var v) || !values.Contains(v);
}

// "key!=value"
int neqIdx = clause.IndexOf("!=", StringComparison.Ordinal);
if (neqIdx >= 0)
{
var key = clause[..neqIdx].Trim();
var value = clause[(neqIdx + 2)..].Trim();
return !labels.TryGetValue(key, out var v) || v != value;
}

// "key=value"
int eqIdx = clause.IndexOf('=');
if (eqIdx >= 0)
{
var key = clause[..eqIdx].Trim();
var value = clause[(eqIdx + 1)..].Trim();
return labels.TryGetValue(key, out var v) && v == value;
}

// "!key"
if (clause.StartsWith('!'))
{
return !labels.ContainsKey(clause[1..].Trim());
}

// "key"
return labels.ContainsKey(clause);
}

private static HashSet<string> ParseValues(string csv) =>
csv.Split(',').Select(v => v.Trim()).ToHashSet(StringComparer.Ordinal);
}
54 changes: 50 additions & 4 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 Down Expand Up @@ -150,8 +154,50 @@ await entityQueue
}
}

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}.LabelFilter"/>
/// matches the given entity's labels, 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 matches, a success result is returned and a warning is logged.
/// </summary>
private async Task<ReconciliationResult<TEntity>> DispatchToMatchingControllers(
IServiceProvider services,
TEntity entity,
Func<IEntityController<TEntity>, TEntity, CancellationToken, Task<ReconciliationResult<TEntity>>> operation,
CancellationToken cancellationToken)
{
var entityLabels = (IReadOnlyDictionary<string, string>?)entity.Labels()
?? new Dictionary<string, string>();

var controllers = services
.GetServices<IEntityController<TEntity>>()
.Where(c => LabelSelectorMatcher.Matches(c.LabelFilter, entityLabels))
.ToList();

if (controllers.Count == 0)
{
logger.LogWarning(
"""No controller matched labels for "{Kind}/{Name}". Skipping.""",
entity.Kind,
entity.Name());
return ReconciliationResult<TEntity>.Success(entity);
Comment on lines +184 to +190
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

Logging a Warning for every event where no controller matches can create very noisy logs in the intended use-case (multiple scoped controllers, no catch-all). Since "no match" can be an expected outcome, consider lowering this to Information/Debug, or otherwise rate-limiting/aggregating it (e.g., include a counter, log once per kind, or gate behind a setting).

Copilot uses AI. Check for mistakes.
}

ReconciliationResult<TEntity> result = ReconciliationResult<TEntity>.Success(entity);
foreach (var controller in controllers)
{
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));
}

[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
Loading
Loading