Skip to content

Commit

Permalink
feat: Combine resource events created,updated,notmodified. (#293)
Browse files Browse the repository at this point in the history
This closes #292.

BREAKING CHANGE: This removes the separat methods
for "created", "updated", and "not modified" events.
Those events are combined into one event "reconcile".
According to https://en.wikipedia.org/wiki/Control_loop,
this is considered good practice. To migrate, remove
all references to the mentioned events and replace
them with one call to "reconcileasync".
  • Loading branch information
buehler committed Sep 20, 2021
1 parent cb73e20 commit acde9e8
Show file tree
Hide file tree
Showing 13 changed files with 63 additions and 156 deletions.
11 changes: 5 additions & 6 deletions docs/docs/controller.md
Expand Up @@ -30,11 +30,9 @@ public class FooCtrl : IResourceController<MyCustomEntity>
// Implement the needed methods here.
// The interface provides default implementation which do a NOOP.
// Possible overwrites:
// "Created" (i.e. when the operator sees the entity for the first time),
// "Updated" (i.e. when the operator knows the entity and it was updated),
// "NotModified" (i.e. when nothing changed but a timed requeue happend),
// "StatusModified" (i.e. when only the status was updated),
// "Deleted" (i.e. when the entity was deleted and all finalizers are done)
// "ReconcileAsync": when the operator sees the entity for the first time, it was modified or just fired an event,
// "StatusModifiedAsync" (i.e. when only the status was updated),
// "DeletedAsync" (i.e. when the entity was deleted and all finalizers are done)
}
```

Expand Down Expand Up @@ -87,7 +85,8 @@ which takes a list of api groups, resources, versions and a selection of

## Requeue

The controller's methods have a return value of <xref:KubeOps.Operator.Controller.Results.ResourceControllerResult>.
The controller's methods (reconcile) have
a return value of <xref:KubeOps.Operator.Controller.Results.ResourceControllerResult>.
There are multiple ways how a result of a controller can be created:

- `null`: The controller will not requeue your entity / event.
Expand Down
4 changes: 1 addition & 3 deletions docs/docs/features.md
Expand Up @@ -9,9 +9,7 @@ As of now, the operator sdk supports - roughly - the following features:
- Normal entities
- Multi version entities
- Controller with all operations of an entity
- Created
- Updated
- NotModified
- Reconcile
- StatusModified
- Deleted
- Finalizers for entities
Expand Down
30 changes: 16 additions & 14 deletions src/KubeOps/Operator/Caching/CacheComparisonResult.cs
Expand Up @@ -6,16 +6,23 @@
internal enum CacheComparisonResult
{
/// <summary>
/// The resource is new to the cache
/// and was never seen.
/// <para>
/// The resource is either:
/// <list type="bullet">
/// <item>
/// <term>New to the cache</term>
/// </item>
/// <item>
/// <term>Modified</term>
/// </item>
/// <item>
/// <term>Not Modified</term>
/// </item>
/// </list>
/// </para>
/// <para>But not status or finalizer modified. This is used to reconcile objects.</para>
/// </summary>
New,

/// <summary>
/// The resource was in the cache and some
/// properties changed (but not resourceVersion).
/// </summary>
Modified,
Other,

/// <summary>
/// The resource has changed, but only the "status" of it.
Expand All @@ -26,10 +33,5 @@ internal enum CacheComparisonResult
/// The resource has changed, but only the "finalizers" list.
/// </summary>
FinalizersModified,

/// <summary>
/// The resource stayed the same.
/// </summary>
NotModified,
}
}
6 changes: 3 additions & 3 deletions src/KubeOps/Operator/Caching/ResourceCache{TEntity}.cs
Expand Up @@ -84,14 +84,14 @@ private CacheComparisonResult CompareCache(TEntity resource)
{
if (!Exists(resource))
{
return CacheComparisonResult.New;
return CacheComparisonResult.Other;
}

var cacheObject = _cache[resource.Metadata.Uid];
var comparison = _compare.Compare(resource, cacheObject);
if (comparison.AreEqual)
{
return CacheComparisonResult.NotModified;
return CacheComparisonResult.Other;
}

if (comparison.Differences.All(d => d.PropertyName.Split('.')[0] == Status))
Expand All @@ -104,7 +104,7 @@ private CacheComparisonResult CompareCache(TEntity resource)
return CacheComparisonResult.FinalizersModified;
}

return CacheComparisonResult.Modified;
return CacheComparisonResult.Other;
}

private bool Exists(TEntity resource) => _cache.ContainsKey(resource.Metadata.Uid);
Expand Down
30 changes: 3 additions & 27 deletions src/KubeOps/Operator/Controller/IResourceController{TEntity}.cs
Expand Up @@ -15,39 +15,15 @@ public interface IResourceController<in TEntity>
where TEntity : IKubernetesObject<V1ObjectMeta>
{
/// <summary>
/// Called for <see cref="ResourceEventType.Created"/> events for a given entity.
/// Called for <see cref="ResourceEventType.Reconcile"/> events for a given entity.
/// </summary>
/// <param name="entity">The entity that fired the created event.</param>
/// <param name="entity">The entity that fired the reconcile event.</param>
/// <returns>
/// A task with an optional <see cref="ResourceControllerResult"/>.
/// Use the static constructors on the <see cref="ResourceControllerResult"/> class
/// to create your controller function result.
/// </returns>
Task<ResourceControllerResult?> CreatedAsync(TEntity entity) =>
Task.FromResult<ResourceControllerResult?>(null);

/// <summary>
/// Called for <see cref="ResourceEventType.Updated"/> events for a given entity.
/// </summary>
/// <param name="entity">The entity that fired the updated event.</param>
/// <returns>
/// A task with an optional <see cref="ResourceControllerResult"/>.
/// Use the static constructors on the <see cref="ResourceControllerResult"/> class
/// to create your controller function result.
/// </returns>
Task<ResourceControllerResult?> UpdatedAsync(TEntity entity) =>
Task.FromResult<ResourceControllerResult?>(null);

/// <summary>
/// Called for <see cref="ResourceEventType.NotModified"/> events for a given entity.
/// </summary>
/// <param name="entity">The entity that fired the not-modified event.</param>
/// <returns>
/// A task with an optional <see cref="ResourceControllerResult"/>.
/// Use the static constructors on the <see cref="ResourceControllerResult"/> class
/// to create your controller function result.
/// </returns>
Task<ResourceControllerResult?> NotModifiedAsync(TEntity entity) =>
Task<ResourceControllerResult?> ReconcileAsync(TEntity entity) =>
Task.FromResult<ResourceControllerResult?>(null);

/// <summary>
Expand Down
Expand Up @@ -224,17 +224,9 @@ protected async Task HandleResourceEvent(QueuedEvent? data)
{
switch (@event)
{
case ResourceEventType.Created:
result = await controller.CreatedAsync(resource);
_metrics.CreatedEvents.Inc();
break;
case ResourceEventType.Updated:
result = await controller.UpdatedAsync(resource);
_metrics.UpdatedEvents.Inc();
break;
case ResourceEventType.NotModified:
result = await controller.NotModifiedAsync(resource);
_metrics.NotModifiedEvents.Inc();
case ResourceEventType.Reconcile:
result = await controller.ReconcileAsync(resource);
_metrics.ReconciledEvents.Inc();
break;
case ResourceEventType.Deleted:
await controller.DeletedAsync(resource);
Expand Down Expand Up @@ -355,18 +347,12 @@ await scope.ServiceProvider.GetRequiredService<IFinalizerManager<TEntity>>()

switch (state)
{
case CacheComparisonResult.New when resource.Metadata.DeletionTimestamp != null:
case CacheComparisonResult.Modified when resource.Metadata.DeletionTimestamp != null:
case CacheComparisonResult.NotModified when resource.Metadata.DeletionTimestamp != null:
case CacheComparisonResult.Other when resource.Metadata.DeletionTimestamp != null:
return (ResourceEventType.Finalizing, resource);
case CacheComparisonResult.New:
return (ResourceEventType.Created, resource);
case CacheComparisonResult.Modified:
return (ResourceEventType.Updated, resource);
case CacheComparisonResult.Other:
return (ResourceEventType.Reconcile, resource);
case CacheComparisonResult.StatusModified:
return (ResourceEventType.StatusUpdated, resource);
case CacheComparisonResult.NotModified:
return (ResourceEventType.NotModified, resource);
case CacheComparisonResult.FinalizersModified:
return (ResourceEventType.FinalizerModified, resource);
default:
Expand Down
26 changes: 4 additions & 22 deletions src/KubeOps/Operator/DevOps/ResourceControllerMetrics{TEntity}.cs
Expand Up @@ -57,24 +57,10 @@ public ResourceControllerMetrics(OperatorSettings settings)
Labels)
.WithLabels(labelValues);

CreatedEvents = Metrics
ReconciledEvents = Metrics
.CreateCounter(
"operator_resource_controller_created_events",
"The count of total 'created' events reconciled by the controller",
Labels)
.WithLabels(labelValues);

UpdatedEvents = Metrics
.CreateCounter(
"operator_resource_controller_updated_events",
"The count of total 'updated' events reconciled by the controller",
Labels)
.WithLabels(labelValues);

NotModifiedEvents = Metrics
.CreateCounter(
"operator_resource_controller_not_modified_events",
"The count of total 'not modified' events reconciled by the controller",
"operator_resource_controller_reconciled_events",
"The count of total events reconciled by the controller",
Labels)
.WithLabels(labelValues);

Expand Down Expand Up @@ -108,14 +94,10 @@ public ResourceControllerMetrics(OperatorSettings settings)

public Counter.Child ErroredEvents { get; }

public Counter.Child CreatedEvents { get; }

public Counter.Child UpdatedEvents { get; }
public Counter.Child ReconciledEvents { get; }

public Counter.Child DeletedEvents { get; }

public Counter.Child NotModifiedEvents { get; }

public Counter.Child StatusUpdatedEvents { get; }

public Counter.Child FinalizingEvents { get; }
Expand Down
14 changes: 2 additions & 12 deletions src/KubeOps/Operator/Kubernetes/ResourceEventType.cs
Expand Up @@ -6,25 +6,15 @@
public enum ResourceEventType
{
/// <summary>
/// Fired when a resource (even requeued) is new to the system.
/// Fired when a resource (even requeued) is catched by the watcher and needs to be reconciled.
/// </summary>
Created,

/// <summary>
/// Fired when a resource (even requeued) has changed in the system.
/// </summary>
Updated,
Reconcile,

/// <summary>
/// Fired when a resource was removed from the system.
/// </summary>
Deleted,

/// <summary>
/// Fired when a resource (even requeued) has not changed.
/// </summary>
NotModified,

/// <summary>
/// Fired when the status part of a resource changed but nothing else.
/// </summary>
Expand Down
12 changes: 6 additions & 6 deletions tests/KubeOps.Test/Operator/Caching/ResourceCache.Test.cs
Expand Up @@ -55,7 +55,7 @@ public void Should_Remove_Object()
{
null,
new TestStatusEntity { Metadata = new V1ObjectMeta { Uid = "test" } },
CacheComparisonResult.New,
CacheComparisonResult.Other,
},
new object?[]
{
Expand All @@ -69,13 +69,13 @@ public void Should_Remove_Object()
Metadata = new V1ObjectMeta { Uid = "test2" },
Spec = new TestStatusEntitySpec { SpecString = "test" },
},
CacheComparisonResult.New,
CacheComparisonResult.Other,
},
new object?[]
{
new TestStatusEntity { Metadata = new V1ObjectMeta { Uid = "test" } },
new TestStatusEntity { Metadata = new V1ObjectMeta { Uid = "test" } },
CacheComparisonResult.NotModified,
CacheComparisonResult.Other,
},
new object?[]
{
Expand All @@ -89,7 +89,7 @@ public void Should_Remove_Object()
Metadata = new V1ObjectMeta { Uid = "test" },
Spec = new TestStatusEntitySpec { SpecString = "test" },
},
CacheComparisonResult.NotModified,
CacheComparisonResult.Other,
},
new object?[]
{
Expand All @@ -103,7 +103,7 @@ public void Should_Remove_Object()
Metadata = new V1ObjectMeta { Uid = "test" },
Spec = new TestStatusEntitySpec { SpecString = "test2" },
},
CacheComparisonResult.Modified,
CacheComparisonResult.Other,
},
new object?[]
{
Expand All @@ -117,7 +117,7 @@ public void Should_Remove_Object()
Metadata = new V1ObjectMeta { Uid = "test" },
Status = new TestStatusEntityStatus { StatusString = "status" },
},
CacheComparisonResult.NotModified,
CacheComparisonResult.Other,
},
new object?[]
{
Expand Down
24 changes: 12 additions & 12 deletions tests/KubeOps.TestOperator.Test/TestController.Test.cs
Expand Up @@ -24,10 +24,10 @@ public async Task Test_If_Manager_Created_Is_Called()
_factory.Run();
var mock = _factory.Services.GetRequiredService<Mock<IManager>>();
mock.Reset();
mock.Setup(o => o.Created(It.IsAny<V1TestEntity>()));
mock.Verify(o => o.Created(It.IsAny<V1TestEntity>()), Times.Never);
await _factory.EnqueueEvent(ResourceEventType.Created, new V1TestEntity());
mock.Verify(o => o.Created(It.IsAny<V1TestEntity>()), Times.Once);
mock.Setup(o => o.Reconciled(It.IsAny<V1TestEntity>()));
mock.Verify(o => o.Reconciled(It.IsAny<V1TestEntity>()), Times.Never);
await _factory.EnqueueEvent(ResourceEventType.Reconcile, new V1TestEntity());
mock.Verify(o => o.Reconciled(It.IsAny<V1TestEntity>()), Times.Once);
}

[Fact]
Expand All @@ -36,10 +36,10 @@ public async Task Test_If_Manager_Updated_Is_Called()
_factory.Run();
var mock = _factory.Services.GetRequiredService<Mock<IManager>>();
mock.Reset();
mock.Setup(o => o.Updated(It.IsAny<V1TestEntity>()));
mock.Verify(o => o.Updated(It.IsAny<V1TestEntity>()), Times.Never);
await _factory.EnqueueEvent(ResourceEventType.Updated, new V1TestEntity());
mock.Verify(o => o.Updated(It.IsAny<V1TestEntity>()), Times.Once);
mock.Setup(o => o.Reconciled(It.IsAny<V1TestEntity>()));
mock.Verify(o => o.Reconciled(It.IsAny<V1TestEntity>()), Times.Never);
await _factory.EnqueueEvent(ResourceEventType.Reconcile, new V1TestEntity());
mock.Verify(o => o.Reconciled(It.IsAny<V1TestEntity>()), Times.Once);
}

[Fact]
Expand All @@ -48,10 +48,10 @@ public async Task Test_If_Manager_NotModified_Is_Called()
_factory.Run();
var mock = _factory.Services.GetRequiredService<Mock<IManager>>();
mock.Reset();
mock.Setup(o => o.NotModified(It.IsAny<V1TestEntity>()));
mock.Verify(o => o.NotModified(It.IsAny<V1TestEntity>()), Times.Never);
await _factory.EnqueueEvent(ResourceEventType.NotModified, new V1TestEntity());
mock.Verify(o => o.NotModified(It.IsAny<V1TestEntity>()), Times.Once);
mock.Setup(o => o.Reconciled(It.IsAny<V1TestEntity>()));
mock.Verify(o => o.Reconciled(It.IsAny<V1TestEntity>()), Times.Never);
await _factory.EnqueueEvent(ResourceEventType.Reconcile, new V1TestEntity());
mock.Verify(o => o.Reconciled(It.IsAny<V1TestEntity>()), Times.Once);
}

[Fact]
Expand Down
16 changes: 2 additions & 14 deletions tests/KubeOps.TestOperator/Controller/TestController.cs
Expand Up @@ -17,21 +17,9 @@ public TestController(IManager manager)
_manager = manager;
}

public Task<ResourceControllerResult> CreatedAsync(V1TestEntity entity)
public Task<ResourceControllerResult> ReconcileAsync(V1TestEntity entity)
{
_manager.Created(entity);
return Task.FromResult<ResourceControllerResult>(null);
}

public Task<ResourceControllerResult> UpdatedAsync(V1TestEntity entity)
{
_manager.Updated(entity);
return Task.FromResult<ResourceControllerResult>(null);
}

public Task<ResourceControllerResult> NotModifiedAsync(V1TestEntity entity)
{
_manager.NotModified(entity);
_manager.Reconciled(entity);
return Task.FromResult<ResourceControllerResult>(null);
}

Expand Down

0 comments on commit acde9e8

Please sign in to comment.