From eff720bf58e946299980d4fc8e05bc5cd436d2d4 Mon Sep 17 00:00:00 2001 From: Antonio Fernandez Alhambra Date: Wed, 11 Nov 2026 12:46:59 +0100 Subject: [PATCH 1/2] feat: record events about any object and outside reconciliations Signed-off-by: Antonio Fernandez Alhambra --- docs/content/en/docs/documentation/_index.md | 1 + .../docs/documentation/kubernetes-events.md | 115 ++++++++ .../documentation/operations/configuration.md | 3 + .../en/docs/migration/v5-7-migration.md | 25 ++ .../operator/RegisteredController.java | 20 ++ .../api/config/ConfigurationService.java | 5 +- .../event/BoundControllerEventRecorder.java | 45 +++ .../api/event/BoundResourceEventRecorder.java | 52 ++++ .../api/event/ControllerEventRecorder.java | 52 ++++ .../api/event/DefaultEventRecorder.java | 75 ++--- .../operator/api/event/DefaultEventSink.java | 4 +- .../operator/api/event/EventRecorder.java | 63 +++-- .../operator/api/event/EventSink.java | 6 +- .../api/event/ResourceEventRecorder.java | 2 +- .../operator/api/reconciler/Context.java | 21 +- .../api/reconciler/DefaultContext.java | 7 +- .../operator/processing/Controller.java | 13 +- .../ConfigurationServiceOverriderTest.java | 14 +- .../DefaultEventRecorderIntegrationTest.java | 55 ++-- .../api/event/DefaultEventRecorderTest.java | 257 ++++++++++++------ .../event/EventRecorderRegressionTest.java | 54 ++-- .../operator/api/event/EventRecorderTest.java | 101 +++++++ .../api/reconciler/DefaultContextTest.java | 23 ++ .../operator/processing/ControllerTest.java | 49 +++- .../eventrecorder/EventRecorderIT.java | 79 +++++- .../EventRecorderReconciler.java | 17 ++ 26 files changed, 924 insertions(+), 234 deletions(-) create mode 100644 docs/content/en/docs/documentation/kubernetes-events.md create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundControllerEventRecorder.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundResourceEventRecorder.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ControllerEventRecorder.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderTest.java diff --git a/docs/content/en/docs/documentation/_index.md b/docs/content/en/docs/documentation/_index.md index d639b8b9f9..6e215c040e 100644 --- a/docs/content/en/docs/documentation/_index.md +++ b/docs/content/en/docs/documentation/_index.md @@ -25,6 +25,7 @@ This section contains detailed documentation for all Java Operator SDK features - **[Eventing](eventing/)** - Understanding the event-driven model - **[Event filters](event-filters/)** - Filtering events and the default filter behavior - **[Accessing Resources in Caches](working-with-es-caches/)** - How to access resources in caches +- **[Recording Kubernetes Events](kubernetes-events/)** - Surfacing what a controller does in `kubectl describe` - **[Operations](operations/)** - Helm chart, metrics, logging, configurations, leader election - **[Other Features](features/)** - Additional capabilities and integrations diff --git a/docs/content/en/docs/documentation/kubernetes-events.md b/docs/content/en/docs/documentation/kubernetes-events.md new file mode 100644 index 0000000000..d4638633fc --- /dev/null +++ b/docs/content/en/docs/documentation/kubernetes-events.md @@ -0,0 +1,115 @@ +--- +title: Recording Kubernetes Events +weight: 51 +--- + +A controller can record Kubernetes events, so that what it does shows up where users already look +for it: `kubectl describe` on the resource the event is about. Events are informational and best +effort by design, Kubernetes keeps them for a limited time and treats them as supplemental data, so +they complement the status of a resource rather than replace it. + +## Recording from a reconciliation + +Inside a reconciliation the recorder is available from the `Context`, bound to the primary resource: + +```java +@Override +public UpdateControl reconcile(MyResource resource, Context context) { + context.eventRecorder().normal("Reconciled", "resource reconciled"); + context.eventRecorder().warn("SomethingIsOff", "this is a warning about the resource"); + return UpdateControl.noUpdate(); +} +``` + +For more control, build an `EventRecord`, which also carries an optional action, reporting +component, labels and annotations: + +```java +context.eventRecorder().record(EventRecord.builder() + .type(EventType.WARNING) + .reason("DeploymentFailed") + .message("could not scale the deployment") + .action("Scaling") + .build()); +``` + +## Recording about another object + +An event does not have to be about the primary resource. A controller reconciling a child resource +can, for example, record on the parent, so that `kubectl describe parent` shows what happened to its +children. Bind the recorder to the other object instead: + +```java +var parent = context.getSecondaryResource(Parent.class).orElseThrow(); +context.eventRecorder(parent).warn("ChildFailed", "could not reconcile child " + name); +``` + +The event references the given object as its involved object and lives in that object's namespace, +or, for a cluster scoped object, in the namespace configured for such events (see below). It is +still attributed to the controller recording it. Aggregation works per object: two children +recording the same reason and message on one parent count as repeats of a single event on the +parent, while different messages stay separate events. + +## Recording outside of a reconciliation + +From a background task or anywhere else no `Context` is at hand, use the recorder bound to the +controller, available from the `RegisteredController` returned when the controller is registered or +from `Operator#getRegisteredController(String)`. It records about any object: + +```java +RegisteredController registered = operator.register(new MyReconciler()); + +registered.eventRecorder().record(someResource, EventRecord.normal("Noticed", "something happened")); +registered.eventRecorder().forResource(someResource).normal("Noticed", "something happened"); +``` + +## What an event carries + +The recorder fills in everything derivable from the controller and the object: the involved object +reference, the reporting controller (the controller name, or the reporting component set on the +record), the reporting instance (the leader election identity when configured, otherwise the host +name, which in a pod is the pod name), the timestamps and the count. + +Repeats are aggregated, not duplicated. Events are named deterministically, after the object they +are about plus a hash of everything that identifies the event, so recording the same event again +resolves to the event already recorded and counts the occurrence on it. That is what makes `kubectl +describe` report a repeating event once, as `(x12 over 3m)`, instead of listing copies. Use +`EventRecord.Builder#key(...)` to control what counts as the same event, or configure the default +through `DefaultEventRecorder.builder(sink).keyStrategy(...)`, see the javadoc of +`EventKeyStrategy` and `EventNamingStrategy`. + +Events about cluster scoped objects live in the namespace configured through +`withClusterScopedEventNamespace(...)` or the `josdk.events.cluster-scoped-namespace` property, +`default` unless configured otherwise. The API server only accepts `default` and `kube-system`. + +## Failure handling + +Recording is best effort by default: a failure to write the event is logged as a warning and +swallowed, and never fails the caller. To make such a failure fail the reconciliation instead, so +that it goes through the usual error handling and retries, configure the default recorder with +`failOnError(true)`. Outside of a reconciliation the exception then reaches whatever called the +recorder, for example the background task. + +```java +KubernetesClient client = new KubernetesClientBuilder().build(); +Operator operator = new Operator(override -> override + .withKubernetesClient(client) + .withEventRecorder(DefaultEventRecorder.builder(new DefaultEventSink(client)) + .failOnError(true) + .build())); +``` + +## Custom recorders + +To assemble events differently or send them somewhere else entirely, plug in an `EventRecorder` of +your own for the whole operator with `withEventRecorder(...)`. A custom recorder is shared by all +controllers, so it must be stateless and thread safe; it implements a single method, +`record(HasMetadata regarding, EventRecord event, ControllerConfiguration controller)`, and gets +the bound views for free. Extend `DefaultEventRecorder` and override `toEvent(...)` to change how +events are assembled, or implement an `EventSink` to change where the assembled events go. + +## Permissions + +Recording needs `get`, `create` and `patch` on `events` in the core API group in every namespace +events are recorded in, see the +[Events section of the configuration page](operations/configuration.md#events). diff --git a/docs/content/en/docs/documentation/operations/configuration.md b/docs/content/en/docs/documentation/operations/configuration.md index 11d1b7117f..6c78efc4d7 100644 --- a/docs/content/en/docs/documentation/operations/configuration.md +++ b/docs/content/en/docs/documentation/operations/configuration.md @@ -377,6 +377,9 @@ All operator-level keys are prefixed with `josdk.`. #### Events +See [Recording Kubernetes Events](../kubernetes-events.md) for how events are recorded. The key +below configures where events about cluster scoped resources go. + | Key | Type | Description | |---|---|---| | `josdk.events.cluster-scoped-namespace` | `String` | Namespace to record events about cluster scoped resources in (defaults to `default`); the API server only accepts `default` and `kube-system` | diff --git a/docs/content/en/docs/migration/v5-7-migration.md b/docs/content/en/docs/migration/v5-7-migration.md index 9ac3ae40b4..d15d2188be 100644 --- a/docs/content/en/docs/migration/v5-7-migration.md +++ b/docs/content/en/docs/migration/v5-7-migration.md @@ -110,3 +110,28 @@ on one of the following: executor supplier, `null` to have the event source create an executor of its own. - `ExecutorServiceManager.scheduledExecutorService()` is deprecated. The framework doesn't use it anymore, it creates the executor on first use, and shuts it down when the manager is stopped. + +## Event recorder: events about any object + +The event recorder can now record events about objects other than the primary resource, from a +reconciliation with `context.eventRecorder(otherObject)`, and outside of a reconciliation through +`RegisteredController#eventRecorder()`, see +[Recording Kubernetes Events](../documentation/kubernetes-events.md). Reconcilers that only use +`context.eventRecorder()` are not affected. The low level API, which is experimental, changed to +make this possible: + +- A custom `EventRecorder` implements + `record(HasMetadata regarding, EventRecord event, ControllerConfiguration controller)`. + `record(EventRecord, Context)` and `forContext(Context)` are gone, which also means that an + override of either in a subclass of `DefaultEventRecorder` no longer compiles: move its logic to + the new `record`. Callers use `context.eventRecorder()` instead, or + `forResource(context.getPrimaryResource(), context.getControllerConfiguration())` when only the + recorder is at hand. The bound views `forResource(...)` and `forController(...)` are default + methods that delegate to `record`. +- A custom `EventSink` implements `emit(Event, ControllerConfiguration)` instead of + `emit(Event, Context)`. +- A subclass of `DefaultEventRecorder` overrides + `toEvent(HasMetadata regarding, EventRecord record, ControllerConfiguration controller)` + instead of `toEvent(Context, EventRecord)`. +- `Controller#eventRecorder()` returns the `ControllerEventRecorder` bound to the controller + instead of the `EventRecorder` configured for the operator. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java index ac5b7cd468..b917a9761d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java @@ -18,11 +18,31 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; +import io.javaoperatorsdk.operator.api.event.ControllerEventRecorder; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; import io.javaoperatorsdk.operator.health.ControllerHealthInfo; +import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; + public interface RegisteredController

extends NamespaceChangeable { ControllerConfiguration

getConfiguration(); ControllerHealthInfo getControllerHealthInfo(); + + /** + * Returns an event recorder bound to this controller, to record Kubernetes events about any + * object outside of a reconciliation, for example from a background task. Within a + * reconciliation, use {@link io.javaoperatorsdk.operator.api.reconciler.Context#eventRecorder()} + * or {@link io.javaoperatorsdk.operator.api.reconciler.Context#eventRecorder(HasMetadata)} + * instead. + * + * @return an event recorder bound to this controller + * @throws UnsupportedOperationException if the implementation does not provide an event recorder + */ + @Experimental(API_MIGHT_CHANGE) + default ControllerEventRecorder eventRecorder() { + throw new UnsupportedOperationException( + "This implementation of RegisteredController does not provide a ControllerEventRecorder"); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 844af80ce2..5fff91eccd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -408,8 +408,9 @@ default String clusterScopedEventNamespace() { * *

When empty, which is the default, every controller gets a {@link DefaultEventRecorder} of * its own. A recorder configured here is shared by all controllers of the operator instead, which - * is why the reconciliation an event is recorded from is passed to it per call rather than - * configured on it: implementations have to be stateless and thread safe. + * is why the object an event is about and the configuration of the controller recording it are + * passed to it per call rather than configured on it: implementations have to be stateless and + * thread safe. * * @return the event recorder to use for the whole operator, or an empty optional to let each * controller use its own default one diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundControllerEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundControllerEventRecorder.java new file mode 100644 index 0000000000..e1ed9bab5a --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundControllerEventRecorder.java @@ -0,0 +1,45 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.event; + +import java.util.Objects; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; + +/** + * A {@link ControllerEventRecorder} that records through an {@link EventRecorder} on behalf of a + * fixed controller. + */ +record BoundControllerEventRecorder(EventRecorder delegate, ControllerConfiguration controller) + implements ControllerEventRecorder { + + BoundControllerEventRecorder { + Objects.requireNonNull(delegate, "the event recorder to record through must not be null"); + Objects.requireNonNull( + controller, "the configuration of the controller recording events must not be null"); + } + + @Override + public void record(HasMetadata regarding, EventRecord event) { + delegate.record(regarding, event, controller); + } + + @Override + public ResourceEventRecorder forResource(HasMetadata regarding) { + return delegate.forResource(regarding, controller); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundResourceEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundResourceEventRecorder.java new file mode 100644 index 0000000000..55d38745b6 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/BoundResourceEventRecorder.java @@ -0,0 +1,52 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.event; + +import java.util.Objects; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; + +/** + * A {@link ResourceEventRecorder} that records through an {@link EventRecorder} about a fixed + * object on behalf of a fixed controller. + */ +record BoundResourceEventRecorder( + EventRecorder delegate, HasMetadata regarding, ControllerConfiguration controller) + implements ResourceEventRecorder { + + BoundResourceEventRecorder { + Objects.requireNonNull(delegate, "the event recorder to record through must not be null"); + Objects.requireNonNull(regarding, "the object events are recorded about must not be null"); + Objects.requireNonNull( + controller, "the configuration of the controller recording events must not be null"); + } + + @Override + public void normal(String reason, String message) { + record(EventRecord.normal(reason, message)); + } + + @Override + public void warn(String reason, String message) { + record(EventRecord.warning(reason, message)); + } + + @Override + public void record(EventRecord event) { + delegate.record(regarding, event, controller); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ControllerEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ControllerEventRecorder.java new file mode 100644 index 0000000000..35f658ded1 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ControllerEventRecorder.java @@ -0,0 +1,52 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.event; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; + +import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; + +/** + * An {@link EventRecorder} bound to a controller: events recorded through it are attributed to that + * controller and assembled from its configuration, and can be about any object. This is the form to + * use outside of a reconciliation, for example from a background task, see {@link + * io.javaoperatorsdk.operator.RegisteredController#eventRecorder()}. Within a reconciliation, + * prefer {@link io.javaoperatorsdk.operator.api.reconciler.Context#eventRecorder()} for events + * about the primary resource and {@link + * io.javaoperatorsdk.operator.api.reconciler.Context#eventRecorder(HasMetadata)} for events about + * another object. + */ +@Experimental(API_MIGHT_CHANGE) +public interface ControllerEventRecorder { + + /** + * Records an event about the given object, attributed to the bound controller. + * + * @param regarding the object the event is about, referenced as its involved object + * @param event the event to record + */ + void record(HasMetadata regarding, EventRecord event); + + /** + * Returns a view of this recorder bound to the given object as well, so that the object doesn't + * have to be passed for every event. + * + * @param regarding the object subsequent events will be about + * @return a recorder bound to the controller and to {@code regarding} + */ + ResourceEventRecorder forResource(HasMetadata regarding); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java index 248972cd01..552a09ff7d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorder.java @@ -37,15 +37,15 @@ import io.fabric8.kubernetes.api.model.MicroTime; import io.fabric8.kubernetes.api.model.ObjectReference; import io.fabric8.kubernetes.api.model.ObjectReferenceBuilder; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.Context; import static java.util.Objects.requireNonNullElse; /** - * Default {@link EventRecorder}. Assembles events from an {@link EventRecord} plus the context the - * controller already knows about (the involved object reference, the reporting controller and - * instance), then hands them to an {@link EventSink}. + * Default {@link EventRecorder}. Assembles events from an {@link EventRecord} plus what is known + * about the object the event is about (the involved object reference) and the controller recording + * it (the reporting controller and instance), then hands them to an {@link EventSink}. * *

Events for cluster scoped objects have to live in some namespace: by default the {@value * #CLUSTER_SCOPED_EVENT_NAMESPACE} namespace is used, following the Kubernetes convention, but it @@ -63,6 +63,10 @@ * and the caller carries on. A recorder built with {@link Builder#failOnError(boolean)} propagates * the failure to the caller instead, so that a reconciliation that cannot record its events fails * and is retried. + * + *

An event can be about any object, not only the primary resource of a reconciliation: the + * object is passed per call, along with the configuration of the controller the event is attributed + * to, see {@link EventRecorder}. */ public class DefaultEventRecorder implements EventRecorder { @@ -218,13 +222,18 @@ private static String resolve() { } @Override - public void record(EventRecord event, Context context) { - Objects.requireNonNull(context, "the context of the reconciliation must not be null"); + public void record( + HasMetadata regarding, EventRecord event, ControllerConfiguration controller) { + Objects.requireNonNull(regarding, "the object the event is about must not be null"); + Objects.requireNonNull( + regarding.getMetadata(), "the object the event is about must have metadata"); Objects.requireNonNull(event, "event must not be null"); + Objects.requireNonNull( + controller, "the configuration of the controller recording the event must not be null"); Event assembled = null; try { - assembled = toEvent(context, event); - sink.emit(assembled, context); + assembled = toEvent(regarding, event, controller); + sink.emit(assembled, controller); } catch (Exception e) { if (failOnError) { throw e; @@ -233,28 +242,23 @@ public void record(EventRecord event, Context context) { // reconcile because it could not write an event is usually worse off than one that records // nothing log.warn( - "Could not record {} event named {} with reason {} for resource {} in namespace {}", + "Could not record {} event named {} with reason {} about {} {} in namespace {}", event.type(), assembled != null ? assembled.getMetadata().getName() : "unknown", event.reason(), - context.getPrimaryResource().getMetadata().getName(), - context.getPrimaryResource().getMetadata().getNamespace(), + regarding.getKind(), + regarding.getMetadata().getName(), + regarding.getMetadata().getNamespace(), e); } } - @Override - public ResourceEventRecorder forContext(Context context) { - Objects.requireNonNull(context, "the context events will be recorded from must not be null"); - return new BoundEventRecorder(this, context); - } - - protected Event toEvent(Context context, EventRecord record) { - var controllerName = context.getControllerConfiguration().getName(); - var regarding = context.getPrimaryResource(); + protected Event toEvent( + HasMetadata regarding, EventRecord record, ControllerConfiguration controller) { + var controllerName = controller.getName(); var now = Instant.now().truncatedTo(ChronoUnit.SECONDS).toString(); var involvedObject = objectReferenceFor(regarding); - var namespace = eventNamespace(regarding, context); + var namespace = eventNamespace(regarding, controller); var builder = new EventBuilder() .withNewMetadata() @@ -272,8 +276,7 @@ protected Event toEvent(Context context, EventRecord record) { .withCount(1) .withReportingComponent(record.reportingComponent().orElse(controllerName)) .withReportingInstance( - context - .getControllerConfiguration() + controller .getConfigurationService() .getLeaderElectionConfiguration() .flatMap(LeaderElectionConfiguration::getIdentity) @@ -321,16 +324,13 @@ private static boolean requiresEventTime(HasMetadata regarding, String eventName && KUBE_SYSTEM_NAMESPACE.equals(eventNamespace); } - private String eventNamespace(HasMetadata regarding, Context context) { + private String eventNamespace(HasMetadata regarding, ControllerConfiguration controller) { var namespace = regarding.getMetadata().getNamespace(); if (namespace != null) { return namespace; } return requireNonNullElse( - context - .getControllerConfiguration() - .getConfigurationService() - .clusterScopedEventNamespace(), + controller.getConfigurationService().clusterScopedEventNamespace(), CLUSTER_SCOPED_EVENT_NAMESPACE); } @@ -443,23 +443,4 @@ private ObjectReference objectReferenceFor(HasMetadata resource) { .withResourceVersion(resource.getMetadata().getResourceVersion()) .build(); } - - private record BoundEventRecorder(EventRecorder delegate, Context context) - implements ResourceEventRecorder { - - @Override - public void normal(String reason, String message) { - delegate.record(EventRecord.normal(reason, message), context); - } - - @Override - public void warn(String reason, String message) { - delegate.record(EventRecord.warning(reason, message), context); - } - - @Override - public void record(EventRecord event) { - delegate.record(event, context); - } - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java index 4d86aa0728..ec15129542 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/DefaultEventSink.java @@ -18,7 +18,7 @@ import io.fabric8.kubernetes.api.model.Event; import io.fabric8.kubernetes.api.model.EventBuilder; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import static java.util.Objects.requireNonNullElse; @@ -52,7 +52,7 @@ public DefaultEventSink(KubernetesClient client) { } @Override - public void emit(Event event, Context context) { + public void emit(Event event, ControllerConfiguration controller) { var events = client.v1().events().inNamespace(event.getMetadata().getNamespace()); var name = event.getMetadata().getName(); var existing = events.withName(name).get(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventRecorder.java index e752cff939..272cca0943 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventRecorder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventRecorder.java @@ -15,7 +15,8 @@ */ package io.javaoperatorsdk.operator.api.event; -import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Experimental; import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; @@ -24,37 +25,63 @@ * Records Kubernetes events on behalf of a controller. * *

This is the unbound form of the API: an instance is shared by all the controllers of the - * operator, and everything that varies between them - the primary resource an event is about, the - * controller the event is attributed to, and the configuration the event is assembled from - is - * passed per call, as the {@link Context} of the reconciliation recording the event. - * Implementations are therefore expected to be stateless and thread safe. To record events through - * an implementation of your own, see {@link - * io.javaoperatorsdk.operator.api.config.ConfigurationService#eventRecorder()}. Within a - * reconciliation, prefer {@link + * operator, and everything that varies between them - the object an event is about and the + * configuration of the controller the event is attributed to and assembled from - is passed per + * call. Implementations are therefore expected to be stateless and thread safe, and only have to + * implement {@link #record(HasMetadata, EventRecord, ControllerConfiguration)}: the views bound to + * an object or to a controller are default methods that delegate to it. To record events through an + * implementation of your own, see {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationService#eventRecorder()}. + * + *

Within a reconciliation, use {@link * io.javaoperatorsdk.operator.api.reconciler.Context#eventRecorder()}, which is already bound to - * the context. + * the primary resource, or {@link + * io.javaoperatorsdk.operator.api.reconciler.Context#eventRecorder(HasMetadata)} for an event about + * another object. Outside of a reconciliation, use {@link + * io.javaoperatorsdk.operator.RegisteredController#eventRecorder()}, which is bound to the + * controller. * *

Whether a failure to record an event fails the caller is up to the implementation, see {@link * DefaultEventRecorder} for the default. */ @Experimental(API_MIGHT_CHANGE) +@FunctionalInterface public interface EventRecorder { /** - * Records an event about the primary resource of the given reconciliation. + * Records an event about the given object, attributed to the controller with the given + * configuration and assembled from it. * + * @param regarding the object the event is about, referenced as the involved object of the + * resulting event * @param event the event to record - * @param context the context of the reconciliation recording the event; the event is about its - * primary resource and is attributed to its controller + * @param controller the configuration of the controller recording the event + */ + void record(HasMetadata regarding, EventRecord event, ControllerConfiguration controller); + + /** + * Returns a view of this recorder bound to the given object and controller, so that neither has + * to be passed for every event. The controller bound view returned by {@link + * #forController(ControllerConfiguration)} obtains its object bound views through this method, so + * overriding it changes those as well. + * + * @param regarding the object subsequent events will be about + * @param controller the configuration of the controller recording them + * @return a recorder bound to {@code regarding} and {@code controller} */ - void record(EventRecord event, Context context); + default ResourceEventRecorder forResource( + HasMetadata regarding, ControllerConfiguration controller) { + return new BoundResourceEventRecorder(this, regarding, controller); + } /** - * Returns a view of this recorder bound to the given reconciliation, so that the context doesn't - * have to be passed for every event. + * Returns a view of this recorder bound to the given controller, to record events about any + * object on its behalf, typically outside of a reconciliation. * - * @param context the context subsequent events will be recorded from - * @return a recorder bound to {@code context} + * @param controller the configuration of the controller subsequent events are recorded for + * @return a recorder bound to {@code controller} */ - ResourceEventRecorder forContext(Context context); + default ControllerEventRecorder forController(ControllerConfiguration controller) { + return new BoundControllerEventRecorder(this, controller); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventSink.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventSink.java index 93c58dcf54..48ecec61f2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventSink.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/EventSink.java @@ -16,7 +16,7 @@ package io.javaoperatorsdk.operator.api.event; import io.fabric8.kubernetes.api.model.Event; -import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Experimental; import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; @@ -35,10 +35,10 @@ public interface EventSink { * * @param event the event to deliver, fully assembled: everything the event says is already built * into it - * @param context the context of the reconciliation the event was recorded from, for + * @param controller the configuration of the controller the event is recorded for, for * implementations that route the event based on it rather than on its contents. Ignored by * {@link DefaultEventSink}. */ @Experimental(API_MIGHT_CHANGE) - void emit(Event event, Context context); + void emit(Event event, ControllerConfiguration controller); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ResourceEventRecorder.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ResourceEventRecorder.java index dfb88dc7fd..ee3c7ba667 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ResourceEventRecorder.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/event/ResourceEventRecorder.java @@ -21,7 +21,7 @@ /** * An {@link EventRecorder} bound to a single object, typically the primary resource of the current - * reconciliation. + * reconciliation, and to the controller recording events about it. */ @Experimental(API_MIGHT_CHANGE) public interface ResourceEventRecorder { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java index 5a87659246..9d25281779 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java @@ -217,7 +217,8 @@ default Optional getSecondaryResource( /** * Returns a {@link ResourceEventRecorder} bound to this context, to record Kubernetes events * about the primary resource. To record them through an implementation of your own, see {@link - * io.javaoperatorsdk.operator.api.config.ConfigurationService#eventRecorder()}. + * io.javaoperatorsdk.operator.api.config.ConfigurationService#eventRecorder()}. For events about + * another object, see {@link #eventRecorder(HasMetadata)}. * * @return an event recorder bound to the primary resource * @throws UnsupportedOperationException if the implementation does not provide an event recorder @@ -227,6 +228,24 @@ default ResourceEventRecorder eventRecorder() { "This implementation of Context does not provide a ResourceEventRecorder"); } + /** + * Returns a {@link ResourceEventRecorder} bound to the given object, to record Kubernetes events + * about an object other than the primary resource from this reconciliation, for example about the + * parent of the primary resource. The events are attributed to the controller of this + * reconciliation and live in the namespace of the given object, or, for a cluster scoped object, + * in the namespace configured for such events, see {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationService#clusterScopedEventNamespace()}. + * + * @param regarding the object subsequent events will be about + * @return an event recorder bound to {@code regarding} + * @throws UnsupportedOperationException if the implementation does not provide an event recorder + */ + @Experimental(Experimental.API_MIGHT_CHANGE) + default ResourceEventRecorder eventRecorder(HasMetadata regarding) { + throw new UnsupportedOperationException( + "This implementation of Context does not provide a ResourceEventRecorder"); + } + /** ExecutorService initialized by framework for workflows. Used for workflow standalone mode. */ ExecutorService getWorkflowExecutorService(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 50e61526c4..8792a3aad0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -212,7 +212,12 @@ public ResourceOperations

resourceOperations() { @Override public ResourceEventRecorder eventRecorder() { - return controller.eventRecorder().forContext(this); + return controller.eventRecorder().forResource(primaryResource); + } + + @Override + public ResourceEventRecorder eventRecorder(HasMetadata regarding) { + return controller.eventRecorder().forResource(regarding); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 4e0ce0447d..6dec27eee3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -39,6 +39,7 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec; +import io.javaoperatorsdk.operator.api.event.ControllerEventRecorder; import io.javaoperatorsdk.operator.api.event.DefaultEventRecorder; import io.javaoperatorsdk.operator.api.event.DefaultEventSink; import io.javaoperatorsdk.operator.api.event.EventRecorder; @@ -99,7 +100,7 @@ public class Controller

private final EventProcessor

eventProcessor; private final ControllerHealthInfo controllerHealthInfo; private final EventSourceContext

eventSourceContext; - private final EventRecorder eventRecorder; + private final ControllerEventRecorder eventRecorder; public Controller( Reconciler

reconciler, @@ -116,7 +117,8 @@ public Controller( this.eventRecorder = configurationService .eventRecorder() - .orElseGet(() -> new DefaultEventRecorder(new DefaultEventSink(kubernetesClient))); + .orElseGet(() -> new DefaultEventRecorder(new DefaultEventSink(kubernetesClient))) + .forController(configuration); contextInitializer = reconciler instanceof ContextInitializer; isCleaner = reconciler instanceof Cleaner; @@ -352,12 +354,13 @@ public ControllerHealthInfo getControllerHealthInfo() { } /** - * The {@link EventRecorder} this controller records its Kubernetes events through, either the one - * configured for the operator, see {@link + * The event recorder this controller records its Kubernetes events through, bound to it: either + * the {@link EventRecorder} configured for the operator, see {@link * io.javaoperatorsdk.operator.api.config.ConfigurationService#eventRecorder()}, or a {@link * DefaultEventRecorder} of its own. */ - public EventRecorder eventRecorder() { + @Override + public ControllerEventRecorder eventRecorder() { return eventRecorder; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java index 9c356b55f7..f0ee39d507 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java @@ -30,11 +30,8 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Secret; import io.javaoperatorsdk.operator.api.event.DefaultEventRecorder; -import io.javaoperatorsdk.operator.api.event.EventRecord; import io.javaoperatorsdk.operator.api.event.EventRecorder; -import io.javaoperatorsdk.operator.api.event.ResourceEventRecorder; import io.javaoperatorsdk.operator.api.monitoring.Metrics; -import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; import io.javaoperatorsdk.operator.api.reconciler.dependent.DesiredStateAspect; @@ -213,16 +210,7 @@ void retryAndRescheduleExecutorDefaultsToADaemonPoolOfTheConfiguredSize() { @Test void eventRecorderIsNotConfiguredByDefaultAndCanBeOverridden() { - final var eventRecorder = - new EventRecorder() { - @Override - public void record(EventRecord event, Context context) {} - - @Override - public ResourceEventRecorder forContext(Context context) { - return null; - } - }; + final EventRecorder eventRecorder = (regarding, event, controller) -> {}; assertThat(config.eventRecorder()).isEmpty(); assertThat( diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java index 9c60ac6b4e..08fcbc8325 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderIntegrationTest.java @@ -30,7 +30,6 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.Context; import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder.aLeaderElectionConfiguration; import static org.assertj.core.api.Assertions.assertThat; @@ -49,10 +48,10 @@ class DefaultEventRecorderIntegrationTest { @Test void countsRepeatedEventsAboutANamespacedResource() { var configMap = configMap("test1"); - var context = context(configMap, "default"); + var controller = controller("default"); - recorder().record(EventRecord.normal("Ready", "all good"), context); - recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(configMap, EventRecord.normal("Ready", "all good"), controller); + recorder().record(configMap, EventRecord.normal("Ready", "all good"), controller); var events = eventsAbout(configMap, configMap.getMetadata().getNamespace()); assertThat(events).hasSize(1); @@ -62,10 +61,10 @@ void countsRepeatedEventsAboutANamespacedResource() { @Test void countsRepeatedEventsAboutAClusterScopedResourceInTheDefaultNamespace() { var namespace = clusterScoped(); - var context = context(namespace, "default"); + var controller = controller("default"); - recorder().record(EventRecord.normal("Ready", "all good"), context); - recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(namespace, EventRecord.normal("Ready", "all good"), controller); + recorder().record(namespace, EventRecord.normal("Ready", "all good"), controller); var events = eventsAbout(namespace, "default"); assertThat(events).hasSize(1); @@ -76,10 +75,10 @@ void countsRepeatedEventsAboutAClusterScopedResourceInTheDefaultNamespace() { @Test void countsRepeatedEventsAboutAClusterScopedResourceInTheKubeSystemNamespace() { var namespace = clusterScoped(); - var context = context(namespace, "kube-system"); + var controller = controller("kube-system"); - recorder().record(EventRecord.normal("Ready", "all good"), context); - recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(namespace, EventRecord.normal("Ready", "all good"), controller); + recorder().record(namespace, EventRecord.normal("Ready", "all good"), controller); var events = eventsAbout(namespace, "kube-system"); assertThat(events).hasSize(1); @@ -96,10 +95,10 @@ void clusterScopedEventsInOtherNamespacesAreRejectedByTheApiServer() { new NamespaceBuilder().withNewMetadata().withName("operator-ns").endMetadata().build()) .serverSideApply(); var namespace = clusterScoped(); - var context = context(namespace, "operator-ns"); - var event = recorder().toEvent(context, EventRecord.normal("Ready", "all good")); + var controller = controller("operator-ns"); + var event = recorder().toEvent(namespace, EventRecord.normal("Ready", "all good"), controller); - assertThatThrownBy(() -> new DefaultEventSink(client).emit(event, context)) + assertThatThrownBy(() -> new DefaultEventSink(client).emit(event, controller)) .isInstanceOf(KubernetesClientException.class); assertThat(eventsAbout(namespace, "operator-ns")).isEmpty(); } @@ -107,18 +106,19 @@ void clusterScopedEventsInOtherNamespacesAreRejectedByTheApiServer() { @Test void aggregatingOntoANamedEventTakesTheTypeReasonAndActionOfTheLatestOccurrence() { var configMap = configMap("test2"); - var context = context(configMap, "default"); + var controller = controller("default"); var recorder = namedRecorder("named-event-" + UUID.randomUUID()); recorder.record( + configMap, EventRecord.builder() .type(EventType.NORMAL) .reason("Ready") .message("all good") .action("Create") .build(), - context); - recorder.record(EventRecord.warning("Degraded", "replica down"), context); + controller); + recorder.record(configMap, EventRecord.warning("Degraded", "replica down"), controller); var events = eventsAbout(configMap, "default"); assertThat(events).hasSize(1); @@ -133,11 +133,11 @@ void aggregatingOntoANamedEventTakesTheTypeReasonAndActionOfTheLatestOccurrence( @Test void aggregatingOntoANamedEventWithAnEventTimeTakesTheTypeReasonAndActionOfTheLatestOccurrence() { var namespace = clusterScoped(); - var context = context(namespace, "kube-system"); + var controller = controller("kube-system"); var recorder = namedRecorder("named-event-" + UUID.randomUUID()); - recorder.record(EventRecord.normal("Ready", "all good"), context); - recorder.record(EventRecord.warning("Degraded", "replica down"), context); + recorder.record(namespace, EventRecord.normal("Ready", "all good"), controller); + recorder.record(namespace, EventRecord.warning("Degraded", "replica down"), controller); var events = eventsAbout(namespace, "kube-system"); assertThat(events).hasSize(1); @@ -151,7 +151,7 @@ void aggregatingOntoANamedEventWithAnEventTimeTakesTheTypeReasonAndActionOfTheLa @Test void longSuppliedNamesAreAcceptedAndStayDistinct() { var configMap = configMap("test3"); - var context = context(configMap, "default"); + var controller = controller("default"); var prefix = "a".repeat(219) + "-"; var recorder = DefaultEventRecorder.builder(new DefaultEventSink(client)) @@ -159,8 +159,8 @@ void longSuppliedNamesAreAcceptedAndStayDistinct() { (regarding, record) -> Optional.of(prefix + record.reason().toLowerCase() + "x")) .build(); - recorder.record(EventRecord.normal("First", "first"), context); - recorder.record(EventRecord.normal("Second", "second"), context); + recorder.record(configMap, EventRecord.normal("First", "first"), controller); + recorder.record(configMap, EventRecord.normal("Second", "second"), controller); var events = eventsAbout(configMap, "default"); assertThat(events).hasSize(2); @@ -171,9 +171,9 @@ void longSuppliedNamesAreAcceptedAndStayDistinct() { @Test void generatedNamesShortenedAtASeparatorAreAccepted() { var configMap = configMap("a".repeat(219) + "-" + "b".repeat(10)); - var context = context(configMap, "default"); + var controller = controller("default"); - recorder().record(EventRecord.normal("Ready", "all good"), context); + recorder().record(configMap, EventRecord.normal("Ready", "all good"), controller); assertThat(eventsAbout(configMap, "default")).hasSize(1); } @@ -218,7 +218,7 @@ private static HasMetadata clusterScoped() { } @SuppressWarnings({"rawtypes", "unchecked"}) - private static Context context(HasMetadata primaryResource, String clusterScopedNamespace) { + private static ControllerConfiguration controller(String clusterScopedNamespace) { var configurationService = mock(ConfigurationService.class); when(configurationService.getLeaderElectionConfiguration()) .thenReturn( @@ -227,9 +227,6 @@ private static Context context(HasMetadata primaryResource, String clusterSco ControllerConfiguration controllerConfiguration = mock(ControllerConfiguration.class); when(controllerConfiguration.getName()).thenReturn("testcontroller"); when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); - Context context = mock(Context.class); - when(context.getPrimaryResource()).thenReturn(primaryResource); - when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); - return context; + return controllerConfiguration; } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java index 12d1f65907..cb31629205 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/DefaultEventRecorderTest.java @@ -24,17 +24,17 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.Event; -import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.api.model.SecretBuilder; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.Context; import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder.aLeaderElectionConfiguration; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatNullPointerException; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -50,7 +50,8 @@ class DefaultEventRecorderTest { @Test void fillsInEverythingDerivableFromTheControllerAndTheInvolvedObject() { - recorder.record(EventRecord.warning("Failed", "could not do the thing"), context(configMap())); + recorder.record( + configMap(), EventRecord.warning("Failed", "could not do the thing"), controller()); assertThat(emitted).hasSize(1); var event = emitted.get(0); @@ -73,11 +74,11 @@ void fillsInEverythingDerivableFromTheControllerAndTheInvolvedObject() { } @Test - void takesTheReportingControllerFromTheControllerConfigurationOfTheContext() { - var context = context(configMap()); - when(context.getControllerConfiguration().getName()).thenReturn("othercontroller"); + void takesTheReportingControllerFromTheControllerConfiguration() { + var controller = controller(); + when(controller.getName()).thenReturn("othercontroller"); - recorder.record(EventRecord.normal("Created", "created"), context); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller); assertThat(emitted.get(0).getReportingComponent()).isEqualTo("othercontroller"); assertThat(emitted.get(0).getSource().getComponent()).isEqualTo("othercontroller"); @@ -85,14 +86,11 @@ void takesTheReportingControllerFromTheControllerConfigurationOfTheContext() { @Test void fallsBackToTheHostNameWhenLeaderElectionConfiguresNoIdentity() { - var context = context(configMap()); - when(context - .getControllerConfiguration() - .getConfigurationService() - .getLeaderElectionConfiguration()) + var controller = controller(); + when(controller.getConfigurationService().getLeaderElectionConfiguration()) .thenReturn(Optional.empty()); - recorder.record(EventRecord.normal("Created", "created"), context); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller); assertThat(emitted.get(0).getReportingInstance()) .isEqualTo(DefaultEventRecorder.defaultReportingInstance()); @@ -100,7 +98,7 @@ void fallsBackToTheHostNameWhenLeaderElectionConfiguresNoIdentity() { @Test void createsTheEventInTheNamespaceOfTheInvolvedObject() { - recorder.record(EventRecord.normal("Created", "created"), context(configMap())); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller()); assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("ns1"); assertThat(emitted.get(0).getMetadata().getName()).startsWith("test1."); @@ -108,7 +106,7 @@ void createsTheEventInTheNamespaceOfTheInvolvedObject() { @Test void recordsEventsForClusterScopedObjectsInTheDefaultNamespace() { - recorder.record(EventRecord.normal("Created", "created"), context(clusterScoped())); + recorder.record(clusterScoped(), EventRecord.normal("Created", "created"), controller()); assertThat(emitted.get(0).getMetadata().getNamespace()) .isEqualTo(DefaultEventRecorder.CLUSTER_SCOPED_EVENT_NAMESPACE); @@ -118,14 +116,15 @@ void recordsEventsForClusterScopedObjectsInTheDefaultNamespace() { @Test void clusterScopedEventNamespaceCanBeOverridden() { recorder.record( - EventRecord.normal("Created", "created"), context(clusterScoped(), "operator-ns")); + clusterScoped(), EventRecord.normal("Created", "created"), controller("operator-ns")); assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("operator-ns"); } @Test void anOverriddenClusterScopedNamespaceDoesNotAffectNamespacedResources() { - recorder.record(EventRecord.normal("Created", "created"), context(configMap(), "operator-ns")); + recorder.record( + configMap(), EventRecord.normal("Created", "created"), controller("operator-ns")); assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("ns1"); } @@ -133,13 +132,14 @@ void anOverriddenClusterScopedNamespaceDoesNotAffectNamespacedResources() { @Test void perEventReportingComponentOverridesTheControllerName() { recorder.record( + configMap(), EventRecord.builder() .reason("Submitted") .message("submitted") .reportingComponent("JobManagerDeployment") .action("Submit") .build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getReportingComponent()).isEqualTo("JobManagerDeployment"); assertThat(emitted.get(0).getSource().getComponent()).isEqualTo("JobManagerDeployment"); @@ -151,13 +151,14 @@ void perEventReportingComponentOverridesTheControllerName() { @Test void passesLabelsAndAnnotationsThrough() { recorder.record( + configMap(), EventRecord.builder() .reason("Scaling") .message("scaling up") .label("group", "autoscaler") .annotation("recommendation", "4") .build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getLabels()).containsEntry("group", "autoscaler"); assertThat(emitted.get(0).getMetadata().getAnnotations()).containsEntry("recommendation", "4"); @@ -172,7 +173,8 @@ void aFailingSinkNeverFailsTheCaller() { }); assertThatCode( - () -> failing.record(EventRecord.normal("Created", "created"), context(configMap()))) + () -> + failing.record(configMap(), EventRecord.normal("Created", "created"), controller())) .doesNotThrowAnyException(); } @@ -188,7 +190,8 @@ void aFailingSinkFailsTheCallerWhenTheRecorderIsBuiltToFailOnError() { .build(); assertThatThrownBy( - () -> failing.record(EventRecord.normal("Created", "created"), context(configMap()))) + () -> + failing.record(configMap(), EventRecord.normal("Created", "created"), controller())) .isSameAs(failure); } @@ -202,24 +205,25 @@ void aFailingSinkNeverFailsTheCallerWhenTheRecorderIsBuiltWithDefaults() { .build(); assertThatCode( - () -> failing.record(EventRecord.normal("Created", "created"), context(configMap()))) + () -> + failing.record(configMap(), EventRecord.normal("Created", "created"), controller())) .doesNotThrowAnyException(); } @Test - void passesTheContextOnToTheSink() { - var contexts = new ArrayList>(); - var recording = new DefaultEventRecorder((event, context) -> contexts.add(context)); - var context = context(configMap()); + void passesTheControllerConfigurationOnToTheSink() { + var controllers = new ArrayList>(); + var recording = new DefaultEventRecorder((event, controller) -> controllers.add(controller)); + var controller = controller(); - recording.record(EventRecord.normal("Created", "created"), context); + recording.record(configMap(), EventRecord.normal("Created", "created"), controller); - assertThat(contexts).containsExactly(context); + assertThat(controllers).containsExactly(controller); } @Test - void boundRecorderRecordsAboutThePrimaryResourceOfTheBoundContext() { - var bound = recorder.forContext(context(configMap())); + void boundRecorderRecordsAboutTheObjectItIsBoundTo() { + var bound = recorder.forResource(configMap(), controller()); bound.normal("Created", "created"); bound.warn("Failed", "failed"); @@ -231,6 +235,88 @@ void boundRecorderRecordsAboutThePrimaryResourceOfTheBoundContext() { assertThat(emitted.get(1).getType()).isEqualTo("Warning"); } + @Test + void recordsEventsAboutAnObjectOtherThanThePrimaryResource() { + var other = + new ConfigMapBuilder() + .withNewMetadata() + .withName("parent") + .withNamespace("ns2") + .withUid("uid-parent") + .endMetadata() + .build(); + + recorder.record(other, EventRecord.warning("ChildFailed", "child failed"), controller()); + + assertThat(emitted).hasSize(1); + var event = emitted.get(0); + assertThat(event.getInvolvedObject().getKind()).isEqualTo("ConfigMap"); + assertThat(event.getInvolvedObject().getName()).isEqualTo("parent"); + assertThat(event.getInvolvedObject().getNamespace()).isEqualTo("ns2"); + assertThat(event.getInvolvedObject().getUid()).isEqualTo("uid-parent"); + assertThat(event.getMetadata().getNamespace()).isEqualTo("ns2"); + assertThat(event.getMetadata().getName()).startsWith("parent."); + assertThat(event.getReportingComponent()).isEqualTo(CONTROLLER); + assertThat(event.getReportingInstance()).isEqualTo(INSTANCE); + } + + @Test + void recordsEventsAboutClusterScopedObjectsOtherThanThePrimaryInTheConfiguredNamespace() { + recorder.record( + clusterScoped(), EventRecord.normal("Noticed", "noticed"), controller("kube-system")); + + assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("kube-system"); + assertThat(emitted.get(0).getInvolvedObject().getKind()).isEqualTo("Namespace"); + } + + @Test + void theSameRecordAboutDifferentObjectsSharingANameResolvesToDifferentEvents() { + var record = EventRecord.normal("Noticed", "noticed"); + var controller = controller(); + var secret = + new SecretBuilder() + .withNewMetadata() + .withName("test1") + .withNamespace("ns1") + .withUid("uid-secret") + .endMetadata() + .build(); + + recorder.record(configMap(), record, controller); + recorder.record(secret, record, controller); + + assertThat(emitted).hasSize(2); + assertThat(emitted).allSatisfy(e -> assertThat(e.getMetadata().getName()).startsWith("test1.")); + assertThat(emitted.get(0).getMetadata().getName()) + .isNotEqualTo(emitted.get(1).getMetadata().getName()); + } + + @Test + void rejectsAnObjectWithoutMetadata() { + assertThatNullPointerException() + .isThrownBy( + () -> + recorder.record( + new ConfigMap(), EventRecord.normal("Noticed", "noticed"), controller())) + .withMessageContaining("metadata"); + assertThat(emitted).isEmpty(); + } + + @Test + void theControllerBoundRecorderRecordsAboutTheObjectItIsGiven() { + var bound = recorder.forController(controller()); + + bound.record(configMap(), EventRecord.normal("Created", "created")); + bound.forResource(clusterScoped()).warn("Failed", "failed"); + + assertThat(emitted).hasSize(2); + assertThat(emitted.get(0).getInvolvedObject().getName()).isEqualTo("test1"); + assertThat(emitted.get(1).getInvolvedObject().getName()).isEqualTo("ns1"); + assertThat(emitted.get(1).getInvolvedObject().getKind()).isEqualTo("Namespace"); + assertThat(emitted) + .allSatisfy(e -> assertThat(e.getReportingComponent()).isEqualTo(CONTROLLER)); + } + @Test void truncatesTheNameOfTheInvolvedObjectToStayWithinTheKubernetesNameLimit() { var longName = "a".repeat(253); @@ -242,7 +328,7 @@ void truncatesTheNameOfTheInvolvedObjectToStayWithinTheKubernetesNameLimit() { .endMetadata() .build(); - recorder.record(EventRecord.normal("Created", "created"), context(configMap)); + recorder.record(configMap, EventRecord.normal("Created", "created"), controller()); var name = emitted.get(0).getMetadata().getName(); assertThat(name).hasSizeLessThanOrEqualTo(253); @@ -259,7 +345,7 @@ void reasonIsRequired() { @Test void alwaysDerivesTheSameDefaultNameForTheSameEvent() { - recorder.record(EventRecord.normal("Created", "created"), context(configMap())); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller()); assertThat(emitted.get(0).getMetadata().getName()) .isEqualTo("test1.3c699548f37ff9cd6d2a786f64a27228"); @@ -267,7 +353,7 @@ void alwaysDerivesTheSameDefaultNameForTheSameEvent() { @Test void namesEventsWithADnsSafeHashSuffix() { - recorder.record(EventRecord.normal("Created", "created"), context(configMap())); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller()); assertThat(emitted.get(0).getMetadata().getName()).matches("test1\\.[0-9a-f]{32}"); } @@ -280,8 +366,8 @@ void givesEventsWhoseMessagesCollideUnderStringHashCodeDistinctNames() { // name and the sink would take the second for a repeat of the first and drop it. assertThat("Aa".hashCode()).isEqualTo("BB".hashCode()); - recorder.record(EventRecord.warning("Failed", "Aa"), context(configMap())); - recorder.record(EventRecord.warning("Failed", "BB"), context(configMap())); + recorder.record(configMap(), EventRecord.warning("Failed", "Aa"), controller()); + recorder.record(configMap(), EventRecord.warning("Failed", "BB"), controller()); assertThat(emitted).hasSize(2); assertThat(emitted.get(0).getMetadata().getName()) @@ -294,10 +380,10 @@ void takesTheMessageOutOfTheEventIdentityWithADefaultKeyStrategy() { DefaultEventRecorder.builder((event, context) -> emitted.add(event)) .keyStrategy(EventKeyStrategy.byReason()) .build(); - var context = context(configMap()); + var controller = controller(); - recorder.record(EventRecord.warning("Failed", "first message"), context); - recorder.record(EventRecord.warning("Failed", "second message"), context); + recorder.record(configMap(), EventRecord.warning("Failed", "first message"), controller); + recorder.record(configMap(), EventRecord.warning("Failed", "second message"), controller); assertThat(emitted.get(0).getMetadata().getName()) .isEqualTo(emitted.get(1).getMetadata().getName()); @@ -309,17 +395,18 @@ void prefersThePerRecordKeyOverTheDefaultKeyStrategy() { DefaultEventRecorder.builder((event, context) -> emitted.add(event)) .keyStrategy(EventKeyStrategy.byReason()) .build(); - var context = context(configMap()); + var controller = controller(); - recorder.record(EventRecord.warning("Failed", "message"), context); + recorder.record(configMap(), EventRecord.warning("Failed", "message"), controller); recorder.record( + configMap(), EventRecord.builder() .type(EventType.WARNING) .reason("Failed") .message("message") .key("another aggregate") .build(), - context); + controller); assertThat(emitted.get(0).getMetadata().getName()) .isNotEqualTo(emitted.get(1).getMetadata().getName()); @@ -327,10 +414,10 @@ void prefersThePerRecordKeyOverTheDefaultKeyStrategy() { @Test void keepsTheMessageInTheEventIdentityWithoutADefaultKeyStrategy() { - var context = context(configMap()); + var controller = controller(); - recorder.record(EventRecord.warning("Failed", "first message"), context); - recorder.record(EventRecord.warning("Failed", "second message"), context); + recorder.record(configMap(), EventRecord.warning("Failed", "first message"), controller); + recorder.record(configMap(), EventRecord.warning("Failed", "second message"), controller); assertThat(emitted.get(0).getMetadata().getName()) .isNotEqualTo(emitted.get(1).getMetadata().getName()); @@ -342,10 +429,10 @@ void keepsEventsWithTheSameReasonButDifferentTypesApartUnderByReason() { DefaultEventRecorder.builder((event, context) -> emitted.add(event)) .keyStrategy(EventKeyStrategy.byReason()) .build(); - var context = context(configMap()); + var controller = controller(); - recorder.record(EventRecord.normal("Flipped", "message"), context); - recorder.record(EventRecord.warning("Flipped", "message"), context); + recorder.record(configMap(), EventRecord.normal("Flipped", "message"), controller); + recorder.record(configMap(), EventRecord.warning("Flipped", "message"), controller); assertThat(emitted.get(0).getMetadata().getName()) .isNotEqualTo(emitted.get(1).getMetadata().getName()); @@ -358,7 +445,7 @@ void setsTheOwnerReferenceToTheInvolvedObjectWhenOwningEventsByRegarding() { .ownerReference(true) .build(); - recorder.record(EventRecord.normal("Created", "created"), context(configMap())); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller()); assertThat(emitted.get(0).getMetadata().getOwnerReferences()) .singleElement() @@ -373,7 +460,7 @@ void setsTheOwnerReferenceToTheInvolvedObjectWhenOwningEventsByRegarding() { @Test void carriesNoOwnerReferenceByDefault() { - recorder.record(EventRecord.normal("Created", "created"), context(configMap())); + recorder.record(configMap(), EventRecord.normal("Created", "created"), controller()); assertThat(emitted.get(0).getMetadata().getOwnerReferences()).isEmpty(); } @@ -386,8 +473,9 @@ void letsARecordOptOutOfTheRecorderLevelOwnerReference() { .build(); recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").ownedByRegarding(false).build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getOwnerReferences()).isEmpty(); } @@ -395,8 +483,9 @@ void letsARecordOptOutOfTheRecorderLevelOwnerReference() { @Test void letsARecordOptIntoTheOwnerReferenceOnItsOwn() { recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").ownedByRegarding(true).build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getOwnerReferences()).hasSize(1); } @@ -407,8 +496,9 @@ void setsNoOwnerReferenceWhenTheRegardingObjectHasNoUidYet() { withoutUid.getMetadata().setUid(null); recorder.record( + withoutUid, EventRecord.builder().reason("Created").message("created").ownedByRegarding(true).build(), - context(withoutUid)); + controller()); assertThat(emitted.get(0).getMetadata().getOwnerReferences()).isEmpty(); } @@ -416,8 +506,9 @@ void setsNoOwnerReferenceWhenTheRegardingObjectHasNoUidYet() { @Test void namesTheEventAfterThePerRecordNameWhenOneIsSet() { recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name("my-event-name").build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).isEqualTo("my-event-name"); } @@ -431,8 +522,8 @@ void namesEventsThroughTheNamingStrategy() { Optional.of(regarding.getMetadata().getName() + "-" + record.reason())) .build(); - recorder.record(EventRecord.warning("failed", "first"), context(configMap())); - recorder.record(EventRecord.warning("failed", "second"), context(configMap())); + recorder.record(configMap(), EventRecord.warning("failed", "first"), controller()); + recorder.record(configMap(), EventRecord.warning("failed", "second"), controller()); assertThat(emitted) .allSatisfy(event -> assertThat(event.getMetadata().getName()).isEqualTo("test1-failed")); @@ -445,7 +536,7 @@ void fallsBackToTheIdentityHashNameWhenTheNamingStrategyResolvesNothing() { .namingStrategy((regarding, record) -> Optional.empty()) .build(); - recorder.record(EventRecord.warning("Failed", "message"), context(configMap())); + recorder.record(configMap(), EventRecord.warning("Failed", "message"), controller()); assertThat(emitted.get(0).getMetadata().getName()) .startsWith("test1.") @@ -455,8 +546,9 @@ void fallsBackToTheIdentityHashNameWhenTheNamingStrategyResolvesNothing() { @Test void truncatesSuppliedNamesToTheKubernetesNameLengthLimit() { recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name("a".repeat(300)).build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).hasSize(253); } @@ -464,19 +556,21 @@ void truncatesSuppliedNamesToTheKubernetesNameLengthLimit() { @Test void stripsTrailingSeparatorsFromTruncatedNames() { recorder.record( + configMap(), EventRecord.builder() .reason("Created") .message("created") .name("a".repeat(219) + "." + "b".repeat(47)) .build(), - context(configMap())); + controller()); recorder.record( + configMap(), EventRecord.builder() .reason("Created") .message("created") .name("b".repeat(219) + "-" + "c".repeat(47)) .build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).matches("a{219}\\.[0-9a-f]{32}"); assertThat(emitted.get(1).getMetadata().getName()).matches("b{219}\\.[0-9a-f]{32}"); @@ -485,19 +579,21 @@ void stripsTrailingSeparatorsFromTruncatedNames() { @Test void keepsLongSuppliedNamesSharingTheirPrefixDistinct() { recorder.record( + configMap(), EventRecord.builder() .reason("Created") .message("created") .name("a".repeat(300) + "-one") .build(), - context(configMap())); + controller()); recorder.record( + configMap(), EventRecord.builder() .reason("Created") .message("created") .name("a".repeat(300) + "-two") .build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).hasSize(253); assertThat(emitted.get(0).getMetadata().getName()) @@ -512,8 +608,9 @@ void treatsABlankNameAsUnset() { .build(); recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name("").build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).isEqualTo("strategy-name"); } @@ -526,8 +623,9 @@ void fallsBackToTheIdentityHashNameWhenEveryNameIsBlank() { .build(); recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name("").build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).matches("test1\\.[0-9a-f]{32}"); } @@ -535,8 +633,9 @@ void fallsBackToTheIdentityHashNameWhenEveryNameIsBlank() { @Test void fallsBackToTheIdentityHashNameWhenTruncationLeavesNothing() { recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name(".".repeat(300)).build(), - context(configMap())); + controller()); assertThat(emitted.get(0).getMetadata().getName()).matches("test1\\.[0-9a-f]{32}"); } @@ -544,11 +643,13 @@ void fallsBackToTheIdentityHashNameWhenTruncationLeavesNothing() { @Test void fallsBackToTheIdentityHashNameWhenTheSuppliedNameIsInvalid() { recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name("Status").build(), - context(configMap())); + controller()); recorder.record( + configMap(), EventRecord.builder().reason("Created").message("created").name("status_name").build(), - context(configMap())); + controller()); assertThat(emitted) .allSatisfy( @@ -566,23 +667,25 @@ void aFailingNamingStrategyNeverFailsTheCaller() { .build(); assertThatCode( - () -> recorder.record(EventRecord.normal("Created", "created"), context(configMap()))) + () -> + recorder.record( + configMap(), EventRecord.normal("Created", "created"), controller())) .doesNotThrowAnyException(); assertThat(emitted).isEmpty(); } - Context context(HasMetadata primaryResource) { - return context(primaryResource, DefaultEventRecorder.CLUSTER_SCOPED_EVENT_NAMESPACE); + ControllerConfiguration controller() { + return controller(DefaultEventRecorder.CLUSTER_SCOPED_EVENT_NAMESPACE); } /** - * The recorder reads everything it does not get from the {@link EventRecord} off the context: the - * primary resource the event is about, the name of the controller the event is attributed to, and - * the configuration service the reporting instance and the cluster scoped event namespace come - * from. + * The recorder reads everything it does not get from the {@link EventRecord} or the object the + * event is about off the controller configuration: the name of the controller the event is + * attributed to, and the configuration service the reporting instance and the cluster scoped + * event namespace come from. */ @SuppressWarnings("rawtypes") - Context context(HasMetadata primaryResource, String clusterScopedEventNamespace) { + ControllerConfiguration controller(String clusterScopedEventNamespace) { var configurationService = mock(ConfigurationService.class); when(configurationService.getLeaderElectionConfiguration()) .thenReturn( @@ -593,11 +696,7 @@ Context context(HasMetadata primaryResource, String clusterScopedEventNamespa ControllerConfiguration controllerConfiguration = mock(ControllerConfiguration.class); when(controllerConfiguration.getName()).thenReturn(CONTROLLER); when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); - - Context context = mock(Context.class); - when(context.getPrimaryResource()).thenReturn(primaryResource); - when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); - return context; + return controllerConfiguration; } ConfigMap configMap() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java index 582b379859..eb21cf0dbc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderRegressionTest.java @@ -37,7 +37,6 @@ import io.fabric8.kubernetes.client.dsl.V1APIGroupDSL; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.Context; import static io.javaoperatorsdk.operator.api.config.LeaderElectionConfigurationBuilder.aLeaderElectionConfiguration; import static org.assertj.core.api.Assertions.assertThat; @@ -62,7 +61,7 @@ void clusterScopedEventsInTheKubeSystemNamespaceCarryAnEventTimeAndAnAction() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); recorder.record( - EventRecord.normal("Created", "created"), context(clusterScoped("ns1"), "kube-system")); + clusterScoped("ns1"), EventRecord.normal("Created", "created"), controller("kube-system")); var event = emitted.get(0); assertThat(event.getInvolvedObject().getNamespace()).isNull(); @@ -77,13 +76,14 @@ void aSuppliedActionIsKeptOnClusterScopedEventsInTheKubeSystemNamespace() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); recorder.record( + clusterScoped("ns1"), EventRecord.builder() .type(EventType.NORMAL) .reason("Created") .message("created") .action("Create") .build(), - context(clusterScoped("ns1"), "kube-system")); + controller("kube-system")); assertThat(emitted.get(0).getAction()).isEqualTo("Create"); } @@ -93,7 +93,7 @@ void clusterScopedEventsInTheDefaultNamespaceDoNotCarryAnEventTime() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); recorder.record( - EventRecord.normal("Created", "created"), context(clusterScoped("ns1"), "default")); + clusterScoped("ns1"), EventRecord.normal("Created", "created"), controller("default")); assertThat(emitted.get(0).getEventTime()).isNull(); assertThat(emitted.get(0).getMetadata().getNamespace()).isEqualTo("default"); @@ -116,10 +116,11 @@ void aggregatingOntoANamedEventTakesTheTypeAndReasonOfTheLatestOccurrence() { (regarding, record) -> Optional.of(regarding.getMetadata().getName() + "-status-report")) .build(); - var context = context(configMap("test1"), "default"); + var controller = controller("default"); - recorder.record(EventRecord.normal("Ready", "all good"), context); - recorder.record(EventRecord.warning("Degraded", "replica down"), context); + recorder.record(configMap("test1"), EventRecord.normal("Ready", "all good"), controller); + recorder.record( + configMap("test1"), EventRecord.warning("Degraded", "replica down"), controller); assertThat(patched.get()).isNotNull(); assertThat(patched.get().getCount()).isEqualTo(2); @@ -137,10 +138,11 @@ void aggregatingOntoAnEventWithAnEventTimeUpdatesTypeReasonAndAction() { DefaultEventRecorder.builder(sink) .namingStrategy((regarding, record) -> Optional.of("fixed-name")) .build(); - var context = context(clusterScoped("ns1"), "kube-system"); + var controller = controller("kube-system"); - recorder.record(EventRecord.normal("Ready", "all good"), context); - recorder.record(EventRecord.warning("Degraded", "replica down"), context); + recorder.record(clusterScoped("ns1"), EventRecord.normal("Ready", "all good"), controller); + recorder.record( + clusterScoped("ns1"), EventRecord.warning("Degraded", "replica down"), controller); assertThat(stored.get().getEventTime()).isNotNull(); assertThat(patched.get().getCount()).isEqualTo(2); @@ -158,17 +160,19 @@ void aggregatingOntoANamedEventWithoutAnActionClearsTheActionOfTheEarlierOccurre DefaultEventRecorder.builder(sink) .namingStrategy((regarding, record) -> Optional.of("fixed-name")) .build(); - var context = context(configMap("test1"), "default"); + var controller = controller("default"); recorder.record( + configMap("test1"), EventRecord.builder() .type(EventType.NORMAL) .reason("Created") .message("created") .action("Create") .build(), - context); - recorder.record(EventRecord.warning("Degraded", "replica down"), context); + controller); + recorder.record( + configMap("test1"), EventRecord.warning("Degraded", "replica down"), controller); assertThat(patched.get().getAction()).isNull(); } @@ -194,7 +198,7 @@ void aggregatingOntoAnEventWithAnEventTimeWithoutAnActionFallsBackToTheReason() .build(); recorder.record( - EventRecord.warning("Degraded", "replica down"), context(configMap("test1"), "default")); + configMap("test1"), EventRecord.warning("Degraded", "replica down"), controller("default")); assertThat(patched.get().getEventTime()).isNotNull(); assertThat(patched.get().getAction()).isEqualTo("Degraded"); @@ -205,8 +209,9 @@ void shortenedGeneratedNamesDoNotEndInASeparatorBeforeTheHash() { var recorder = new DefaultEventRecorder((event, context) -> emitted.add(event)); recorder.record( + configMap("a".repeat(219) + "-" + "b".repeat(10)), EventRecord.normal("Created", "created"), - context(configMap("a".repeat(219) + "-" + "b".repeat(10)), "default")); + controller("default")); assertThat(emitted.get(0).getMetadata().getName()).matches("a{219}\\.[0-9a-f]{32}"); } @@ -224,10 +229,16 @@ void longSuppliedNamesStayDistinct() { Optional.of( regarding.getMetadata().getName() + "-" + record.reason().toLowerCase())) .build(); - var context = context(configMap("a".repeat(245)), "default"); + var controller = controller("default"); - recorder.record(EventRecord.warning("BackoffLimitExceeded", "job failed"), context); - recorder.record(EventRecord.warning("BackoffStarted", "job retrying"), context); + recorder.record( + configMap("a".repeat(245)), + EventRecord.warning("BackoffLimitExceeded", "job failed"), + controller); + recorder.record( + configMap("a".repeat(245)), + EventRecord.warning("BackoffStarted", "job retrying"), + controller); assertThat(emitted.get(0).getMetadata().getName()) .isNotEqualTo(emitted.get(1).getMetadata().getName()); @@ -272,7 +283,7 @@ private KubernetesClient mockClient( } @SuppressWarnings("rawtypes") - private Context context(HasMetadata primaryResource, String clusterScopedEventNamespace) { + private ControllerConfiguration controller(String clusterScopedEventNamespace) { var configurationService = mock(ConfigurationService.class); when(configurationService.getLeaderElectionConfiguration()) .thenReturn( @@ -282,10 +293,7 @@ private Context context(HasMetadata primaryResource, String clusterScopedEven ControllerConfiguration controllerConfiguration = mock(ControllerConfiguration.class); when(controllerConfiguration.getName()).thenReturn("testcontroller"); when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); - Context context = mock(Context.class); - when(context.getPrimaryResource()).thenReturn(primaryResource); - when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); - return context; + return controllerConfiguration; } private HasMetadata clusterScoped(String name) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderTest.java new file mode 100644 index 0000000000..cee277f734 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/event/EventRecorderTest.java @@ -0,0 +1,101 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.event; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNullPointerException; +import static org.mockito.Mockito.mock; + +/** + * The views an {@link EventRecorder} offers are default methods, so that an implementation only has + * to record. This exercises them on a minimal implementation that only records what it was asked. + */ +class EventRecorderTest { + + private record Recording( + HasMetadata regarding, EventRecord event, ControllerConfiguration controller) {} + + private final List recordings = new ArrayList<>(); + private final EventRecorder recorder = + (regarding, event, controller) -> recordings.add(new Recording(regarding, event, controller)); + + private final ControllerConfiguration controller = mock(ControllerConfiguration.class); + private final ConfigMap primary = configMap("primary"); + private final ConfigMap other = configMap("other"); + + @Test + void theViewBoundToAnObjectRecordsAboutThatObjectForTheController() { + var bound = recorder.forResource(other, controller); + + bound.normal("Created", "created"); + bound.warn("Failed", "failed"); + bound.record(EventRecord.builder().reason("Noticed").build()); + + assertThat(recordings).hasSize(3); + assertThat(recordings).allSatisfy(r -> assertThat(r.regarding()).isSameAs(other)); + assertThat(recordings).allSatisfy(r -> assertThat(r.controller()).isSameAs(controller)); + assertThat(recordings) + .extracting(r -> r.event().type()) + .containsExactly(EventType.NORMAL, EventType.WARNING, EventType.NORMAL); + assertThat(recordings) + .extracting(r -> r.event().reason()) + .containsExactly("Created", "Failed", "Noticed"); + } + + @Test + void theViewBoundToAControllerRecordsAboutAnyObject() { + var bound = recorder.forController(controller); + var event = EventRecord.normal("Noticed", "noticed"); + + bound.record(other, event); + bound.forResource(primary).warn("Failed", "failed"); + + assertThat(recordings).hasSize(2); + assertThat(recordings.get(0)).isEqualTo(new Recording(other, event, controller)); + assertThat(recordings.get(1).regarding()).isSameAs(primary); + assertThat(recordings.get(1).event().reason()).isEqualTo("Failed"); + assertThat(recordings.get(1).controller()).isSameAs(controller); + } + + @Test + void viewsRejectMissingArguments() { + assertThatNullPointerException().isThrownBy(() -> recorder.forResource(null, controller)); + assertThatNullPointerException().isThrownBy(() -> recorder.forResource(other, null)); + assertThatNullPointerException().isThrownBy(() -> recorder.forController(null)); + assertThatNullPointerException() + .isThrownBy(() -> recorder.forController(controller).forResource(null)); + } + + private static ConfigMap configMap(String name) { + return new ConfigMapBuilder() + .withNewMetadata() + .withName(name) + .withNamespace("ns1") + .withUid("uid-" + name) + .endMetadata() + .build(); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java index 7b9658f98d..1ca282c08c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java @@ -31,6 +31,8 @@ import io.fabric8.kubernetes.api.model.PodBuilder; import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.api.model.SecretBuilder; +import io.javaoperatorsdk.operator.api.event.ControllerEventRecorder; +import io.javaoperatorsdk.operator.api.event.ResourceEventRecorder; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException; @@ -403,4 +405,25 @@ void latestDistinctComparesNumericVersionsCorrectly() { HasMetadata resultPod = result.iterator().next(); assertThat(resultPod.getMetadata().getResourceVersion()).isEqualTo("1000"); } + + @Test + void eventRecorderIsBoundToThePrimaryResource() { + var controllerRecorder = mock(ControllerEventRecorder.class); + var bound = mock(ResourceEventRecorder.class); + when(mockController.eventRecorder()).thenReturn(controllerRecorder); + when(controllerRecorder.forResource(context.getPrimaryResource())).thenReturn(bound); + + assertThat(context.eventRecorder()).isSameAs(bound); + } + + @Test + void eventRecorderCanBeBoundToAnotherObject() { + var other = new ConfigMapBuilder().withNewMetadata().withName("parent").endMetadata().build(); + var controllerRecorder = mock(ControllerEventRecorder.class); + var bound = mock(ResourceEventRecorder.class); + when(mockController.eventRecorder()).thenReturn(controllerRecorder); + when(controllerRecorder.forResource(other)).thenReturn(bound); + + assertThat(context.eventRecorder(other)).isSameAs(bound); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index de582f593d..80ec5ac246 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -15,20 +15,24 @@ */ package io.javaoperatorsdk.operator.processing; +import java.util.ArrayList; import java.util.Optional; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Secret; +import io.fabric8.kubernetes.api.model.SecretBuilder; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec; -import io.javaoperatorsdk.operator.api.event.DefaultEventRecorder; +import io.javaoperatorsdk.operator.api.event.EventRecord; import io.javaoperatorsdk.operator.api.event.EventRecorder; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; @@ -45,6 +49,7 @@ import static io.javaoperatorsdk.operator.processing.Controller.CLEANER_NOT_SUPPORTED_ON_ALL_EVENT_ERROR_MESSAGE; import static io.javaoperatorsdk.operator.processing.Controller.MANAGED_WORKFLOWS_NOT_SUPPORTED_TRIGGER_RECONCILER_ON_ALL_EVENT_ERROR_MESSAGE; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -115,17 +120,33 @@ void doesNotNotifyMetricsWhenEventProcessorNotStarted() { @Test void recordsEventsThroughTheEventRecorderConfiguredForTheOperator() { final var client = MockKubernetesClient.client(Secret.class); - final var eventRecorder = mock(EventRecorder.class); + final var recorded = new ArrayList(); + final var recordedFor = new ArrayList>(); + final EventRecorder eventRecorder = + (regarding, event, controller) -> { + recorded.add(regarding); + recordedFor.add(controller); + }; final var configurationService = ConfigurationService.newOverriddenConfigurationService( new BaseConfigurationService(), o -> o.withEventRecorder(eventRecorder).withKubernetesClient(client)); final var configuration = MockControllerConfiguration.forResource(Secret.class, configurationService); - final var controller = new Controller(reconciler, configuration, client); - - assertThat(controller.eventRecorder()).isSameAs(eventRecorder); + final var secret = + new SecretBuilder() + .withNewMetadata() + .withName("s") + .withNamespace("ns") + .endMetadata() + .build(); + + controller.eventRecorder().record(secret, EventRecord.normal("Created", "created")); + + assertThat(recorded).containsExactly(secret); + assertThat(recordedFor).hasSize(1); + assertThat(recordedFor.get(0)).isSameAs(configuration); } @Test @@ -136,10 +157,22 @@ void recordsEventsThroughAnEventRecorderOfItsOwnWhenNoneIsConfigured() { Secret.class, ConfigurationService.newOverriddenConfigurationService( new BaseConfigurationService(), o -> o.withKubernetesClient(client))); - final var controller = new Controller(reconciler, configuration, client); - - assertThat(controller.eventRecorder()).isInstanceOf(DefaultEventRecorder.class); + final var secret = + new SecretBuilder() + .withNewMetadata() + .withName("s") + .withNamespace("ns") + .endMetadata() + .build(); + + // the default recorder is best effort: the mock client cannot write the event, and the caller + // must not notice + assertThatCode( + () -> + controller.eventRecorder().record(secret, EventRecord.normal("Created", "created"))) + .doesNotThrowAnyException(); + verify(client).v1(); } @Test diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderIT.java index 5c1319d884..1a05b72878 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderIT.java @@ -21,9 +21,11 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.Event; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.annotation.Sample; +import io.javaoperatorsdk.operator.api.event.EventRecord; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import static org.assertj.core.api.Assertions.assertThat; @@ -37,11 +39,14 @@ recorder available from the reconciliation context. Verifies that both normal and warning \ events reach the cluster, refer to the primary resource as their involved object, and are \ attributed to the reporting controller, and that recording the same event again counts the \ - occurrence on the event already recorded instead of recording it a second time. + occurrence on the event already recorded instead of recording it a second time. Also \ + records an event about another object from the reconciliation, and one about the primary \ + resource from outside of any reconciliation through the registered controller. """) class EventRecorderIT { public static final String TEST_RESOURCE_NAME = "test1"; + public static final String OUTSIDE_REASON = "NoticedFromOutside"; private final EventRecorderReconciler reconciler = new EventRecorderReconciler(); @@ -106,17 +111,87 @@ void countsRepeatedOccurrencesOnTheEventAlreadyRecorded() { }); } + @Test + void recordsEventsAboutAnotherObjectFromTheReconciliation() { + extension.create( + new ConfigMapBuilder() + .withNewMetadata() + .withName(EventRecorderReconciler.OTHER_OBJECT_NAME) + .endMetadata() + .build()); + extension.create(testResource()); + + await() + .untilAsserted( + () -> { + var events = eventsAbout(EventRecorderReconciler.OTHER_OBJECT_NAME); + assertThat(events) + .extracting(Event::getReason) + .contains(EventRecorderReconciler.OTHER_OBJECT_REASON); + var event = + events.stream() + .filter( + e -> EventRecorderReconciler.OTHER_OBJECT_REASON.equals(e.getReason())) + .findFirst() + .orElseThrow(); + assertThat(event.getInvolvedObject().getKind()).isEqualTo("ConfigMap"); + assertThat(event.getInvolvedObject().getName()) + .isEqualTo(EventRecorderReconciler.OTHER_OBJECT_NAME); + assertThat(event.getInvolvedObject().getUid()).isNotBlank(); + assertThat(event.getMessage()) + .isEqualTo("child " + TEST_RESOURCE_NAME + " reconciled"); + assertThat(event.getReportingComponent()).isEqualTo(controllerName()); + }); + } + + @Test + void recordsEventsOutsideOfAReconciliation() { + var resource = extension.create(testResource()); + await().untilAsserted(() -> assertThat(reconciler.getNumberOfExecutions()).isPositive()); + + extension + .getRegisteredControllerForReconcile(EventRecorderReconciler.class) + .eventRecorder() + .record( + resource, EventRecord.normal(OUTSIDE_REASON, "recorded outside of a reconciliation")); + + await() + .untilAsserted( + () -> { + var event = + eventsForTestResource().stream() + .filter(e -> OUTSIDE_REASON.equals(e.getReason())) + .findFirst(); + assertThat(event).isPresent(); + assertThat(event.get().getInvolvedObject().getKind()) + .isEqualTo("EventRecorderCustomResource"); + assertThat(event.get().getInvolvedObject().getName()).isEqualTo(TEST_RESOURCE_NAME); + assertThat(event.get().getReportingComponent()).isEqualTo(controllerName()); + }); + } + private List eventsForTestResource() { + return eventsAbout(TEST_RESOURCE_NAME); + } + + private List eventsAbout(String involvedObjectName) { return extension .getKubernetesClient() .v1() .events() .inNamespace(extension.getNamespace()) - .withField("involvedObject.name", TEST_RESOURCE_NAME) + .withField("involvedObject.name", involvedObjectName) .list() .getItems(); } + private String controllerName() { + return extension + .getRegisteredControllerForReconcile(EventRecorderReconciler.class) + .getConfiguration() + .getName(); + } + EventRecorderCustomResource testResource() { var resource = new EventRecorderCustomResource(); resource.setMetadata(new ObjectMetaBuilder().withName(TEST_RESOURCE_NAME).build()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderReconciler.java index 8108eb4031..57692733da 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/eventrecorder/EventRecorderReconciler.java @@ -27,6 +27,8 @@ public class EventRecorderReconciler implements Reconciler reconcile( numberOfExecutions.incrementAndGet(); context.eventRecorder().normal(NORMAL_REASON, "resource reconciled"); context.eventRecorder().warn(WARNING_REASON, "this is a warning about the resource"); + + // the test that exercises events about another object creates this config map first; the + // other tests have no such object and record nothing about it + var other = + context + .getClient() + .configMaps() + .inNamespace(resource.getMetadata().getNamespace()) + .withName(OTHER_OBJECT_NAME) + .get(); + if (other != null) { + context + .eventRecorder(other) + .normal(OTHER_OBJECT_REASON, "child " + resource.getMetadata().getName() + " reconciled"); + } return UpdateControl.noUpdate(); } From 46d0b2182edcbf0cd9bbe7c708947f8c3c9a8938 Mon Sep 17 00:00:00 2001 From: Antonio Fernandez Alhambra Date: Wed, 11 Nov 2026 13:15:16 +0100 Subject: [PATCH 2/2] test: wait for the reconcile before deleting in the cleaner ITs Signed-off-by: Antonio Fernandez Alhambra --- .../cleanerforreconciler/CleanerForReconcilerIT.java | 3 +++ .../CleanerForManagedDependentResourcesOnlyIT.java | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cleanerforreconciler/CleanerForReconcilerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cleanerforreconciler/CleanerForReconcilerIT.java index 4a28425e54..269366cffe 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cleanerforreconciler/CleanerForReconcilerIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cleanerforreconciler/CleanerForReconcilerIT.java @@ -63,6 +63,9 @@ void addsFinalizerAndCallsCleanupIfCleanerImplemented() { .getMetadata() .getFinalizers() .isEmpty()); + // adding the finalizer does not reconcile, the update event it produces does: deleting before + // that event is processed turns the pending execution into a cleanup and no reconcile ever runs + await().until(() -> reconciler.getNumberOfExecutions() == 1); operator.delete(testResource); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/cleanermanageddependent/CleanerForManagedDependentResourcesOnlyIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/cleanermanageddependent/CleanerForManagedDependentResourcesOnlyIT.java index bd3dd9802e..175c11a16c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/cleanermanageddependent/CleanerForManagedDependentResourcesOnlyIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/cleanermanageddependent/CleanerForManagedDependentResourcesOnlyIT.java @@ -46,6 +46,8 @@ class CleanerForManagedDependentResourcesOnlyIT { @Test void addsFinalizerAndCallsCleanupIfCleanerImplemented() { + CleanerForManagedDependentTestReconciler reconciler = + (CleanerForManagedDependentTestReconciler) operator.getFirstReconciler(); var testResource = createTestResource(); operator.create(testResource); @@ -57,6 +59,9 @@ void addsFinalizerAndCallsCleanupIfCleanerImplemented() { .getMetadata() .getFinalizers() .isEmpty()); + // adding the finalizer does not reconcile, the update event it produces does: deleting before + // that event is processed turns the pending execution into a cleanup and no reconcile ever runs + await().until(() -> reconciler.getNumberOfExecutions() == 1); operator.delete(testResource); @@ -66,9 +71,6 @@ void addsFinalizerAndCallsCleanupIfCleanerImplemented() { operator.get(CleanerForManagedDependentCustomResource.class, TEST_RESOURCE_NAME) == null); - CleanerForManagedDependentTestReconciler reconciler = - (CleanerForManagedDependentTestReconciler) operator.getFirstReconciler(); - assertThat(reconciler.getNumberOfExecutions()).isEqualTo(1); assertThat(ConfigMapDependentResource.getNumberOfCleanupExecutions()).isEqualTo(1); }