Skip to content

feat: remove automatic observed generation handling #2382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ It makes it easy to implement best practices and patterns for an Operator. Featu
* Handling dependent resources, related events, and caching.
* Automatic Retries
* Smart event scheduling
* Handling Observed Generations automatically
* Easy to use Error Handling
* ... and everything that a batteries included framework needs

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package {{groupId}};

import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;

public class {{artifactClassId}}Status extends ObservedGenerationAwareStatus {
public class {{artifactClassId}}Status {

}
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
package io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope;

import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;

public class BoundedCacheTestStatus extends ObservedGenerationAwareStatus {
public class BoundedCacheTestStatus {
}
55 changes: 1 addition & 54 deletions docs/documentation/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,57 +186,6 @@ annotation. If you do not specify a finalizer name, one will be automatically ge

From v5 by default finalizer is added using Served Side Apply. See also UpdateControl in docs.

## Automatic Observed Generation Handling

Having an `.observedGeneration` value on your resources' status is a best practice to
indicate the last generation of the resource which was successfully reconciled by the controller.
This helps users / administrators diagnose potential issues.

In order to have this feature working:

- the **status class** (not the resource itself) must implement the
[`ObservedGenerationAware`](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAware.java)
interface. See also
the [`ObservedGenerationAwareStatus`](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/ObservedGenerationAwareStatus.java)
convenience implementation that you can extend in your own status class implementations.
- The other condition is that the `CustomResource.getStatus()` method should not return `null`.
So the status should be instantiated when the object is returned using the `UpdateControl`.

If these conditions are fulfilled and generation awareness is activated, the observed generation
is automatically set by the framework after the `reconcile` method is called.

When using SSA based patches, the observed generation is only updated when `UpdateControl.patchStatus` or
`UpdateControl.patchResourceAndStatus` is returned. In case the of non-SSA based patches
the observed generation is also updated even when `UpdateControl.noUpdate()` is returned from the
reconciler.
See this feature at work in the [WebPage example](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java#L5).
See turning off an on the SSA based patching at [`ConfigurationServcice.useSSAToPatchPrimaryResource()`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java#L385-L385).

```java
public class WebPageStatus extends ObservedGenerationAwareStatus {
// omitted code
}
```

Initializing status automatically on custom resource could be done by overriding the `initStatus` method
of `CustomResource`. However, this is NOT advised, since breaks the status patching if you use:
`UpdateControl.patchStatus`. See
also [javadocs](https://github.com/java-operator-sdk/java-operator-sdk/blob/3994f5ffc1fb000af81aa198abf72a5f75fd3e97/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java#L41-L42)
.

```java
@Group("sample.javaoperatorsdk")
@Version("v1")
public class WebPage extends CustomResource<WebPageSpec, WebPageStatus>
implements Namespaced {

@Override
protected WebPageStatus initStatus() {
return new WebPageStatus();
}
}
```

## Generation Awareness and Event Filtering

A best practice when an operator starts up is to reconcile all the associated resources because
Expand All @@ -254,9 +203,7 @@ To turn off this feature, set `generationAwareEventProcessing` to `false` for th
## Support for Well Known (non-custom) Kubernetes Resources

A Controller can be registered for a non-custom resource, so well known Kubernetes resources like (
`Ingress`, `Deployment`,...). Note that automatic observed generation handling is not supported
for these resources, though, in this case, the handling of the observed generation is probably
handled by the primary controller.
`Ingress`, `Deployment`,...).

See
the [integration test](https://github.com/java-operator-sdk/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/deployment/DeploymentReconciler.java)
Expand Down
11 changes: 6 additions & 5 deletions docs/documentation/v5-0-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ permalink: /docs/v5-0-migration
where it is demonstrated. Also, the related part of
a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).

Related automatic observed generation handling changes:
Automated Observed Generation (see features in docs), is automatically handled for non-SSA, even if
the status sub-resource is not instructed to be updated. This is not true for SSA, observed generation is updated
only when patch status is instructed by `UpdateControl`.

6. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.
7. `ResourceDiscriminator` was removed. In most of the cases you can just delete the discriminator, everything should
Expand All @@ -57,3 +52,9 @@ permalink: /docs/v5-0-migration
8. `ConfigurationService.getTerminationTimeoutSeconds` and associated overriding mechanism have been removed,
use `Operator.stop(Duration)` instead.
9. `Operator.installShutdownHook()` has been removed, use `Operator.installShutdownHook(Duration)` instead
10. Automated observed generation handling feature was removed (`ObservedGenerationAware` interface
and `ObservedGenerationAwareStatus` class were deleted). Manually handling observed generation is fairly easy to do
in your reconciler, however, it cannot be done automatically when using SSA. We therefore removed the feature since
it would have been confusing to have a different behavior for SSA and non-SSA cases. For an example of how to do
observed generation handling manually in your reconciler, see
[this sample](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/manualobservedgeneration/ManualObservedGenerationReconciler.java).

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() {
/**
* {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can
* either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for
* adding finalizers, managing observed generation, patching resources and status.
* adding finalizers, patching resources and status.
*
* @return {@code true} if Server-Side Apply (SSA) should be used when patching the primary
* resources, {@code false} otherwise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
import io.fabric8.kubernetes.api.model.KubernetesResourceList;
import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.dsl.base.PatchContext;
import io.fabric8.kubernetes.client.dsl.base.PatchType;
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
import io.javaoperatorsdk.operator.api.config.Cloner;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.BaseControl;
Expand Down Expand Up @@ -170,13 +168,8 @@ private PostExecutionControl<P> reconcileExecution(ExecutionScope<P> executionSc
}
}

// check if status also needs to be updated
final var updateObservedGeneration = updateControl.isNoUpdate()
? shouldUpdateObservedGenerationAutomatically(resourceForExecution)
: shouldUpdateObservedGenerationAutomatically(updatedCustomResource);
// if using SSA the observed generation is updated only if user instructs patching the status
if (updateControl.isPatchStatus() || (updateObservedGeneration && !useSSA)) {
updatedCustomResource = patchStatusGenerationAware(toUpdate, originalResource);
if (updateControl.isPatchStatus()) {
customResourceFacade.patchStatus(toUpdate, originalResource);
}
return createPostExecutionControl(updatedCustomResource, updateControl);
}
Expand Down Expand Up @@ -206,9 +199,8 @@ public boolean isLastAttempt() {

P updatedResource = null;
if (errorStatusUpdateControl.getResource().isPresent()) {
updatedResource =
patchStatusGenerationAware(errorStatusUpdateControl.getResource().orElseThrow(),
originalResource);
updatedResource = customResourceFacade
.patchStatus(errorStatusUpdateControl.getResource().orElseThrow(), originalResource);
}
if (errorStatusUpdateControl.isNoRetry()) {
PostExecutionControl<P> postExecutionControl;
Expand All @@ -234,38 +226,9 @@ private boolean isErrorStatusHandlerPresent() {
}

private P patchStatusGenerationAware(P resource, P originalResource) {
updateStatusObservedGenerationIfRequired(resource);
return customResourceFacade.patchStatus(resource, originalResource);
}

@SuppressWarnings("rawtypes")
private boolean shouldUpdateObservedGenerationAutomatically(P resource) {
if (configuration().isGenerationAware() && resource instanceof CustomResource<?, ?>) {
var customResource = (CustomResource) resource;
var status = customResource.getStatus();
// Note that if status is null we won't update the observed generation.
if (status instanceof ObservedGenerationAware) {
var observedGen = ((ObservedGenerationAware) status).getObservedGeneration();
var currentGen = resource.getMetadata().getGeneration();
return !currentGen.equals(observedGen);
}
}
return false;
}

@SuppressWarnings("rawtypes")
private void updateStatusObservedGenerationIfRequired(P resource) {
if (configuration().isGenerationAware() && resource instanceof CustomResource<?, ?>) {
var customResource = (CustomResource) resource;
var status = customResource.getStatus();
// Note that if status is null we won't update the observed generation.
if (status instanceof ObservedGenerationAware) {
((ObservedGenerationAware) status)
.setObservedGeneration(resource.getMetadata().getGeneration());
}
}
}

private PostExecutionControl<P> createPostExecutionControl(P updatedCustomResource,
UpdateControl<P> updateControl) {
PostExecutionControl<P> postExecutionControl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,29 +435,6 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() {
.isEqualTo(1000L);
}

@Test
void setObservedGenerationForStatusIfNeeded() throws Exception {
var observedGenResource = createObservedGenCustomResource();

Reconciler<ObservedGenCustomResource> reconciler = mock(Reconciler.class);
ControllerConfiguration<ObservedGenCustomResource> config =
MockControllerConfiguration.forResource(ObservedGenCustomResource.class);
CustomResourceFacade<ObservedGenCustomResource> facade = mock(CustomResourceFacade.class);
var dispatcher = init(observedGenResource, reconciler, config, facade, true);

when(config.isGenerationAware()).thenReturn(true);

when(reconciler.reconcile(any(), any()))
.thenReturn(UpdateControl.patchStatus(observedGenResource));
when(facade.patchStatus(eq(observedGenResource), any())).thenReturn(observedGenResource);

PostExecutionControl<ObservedGenCustomResource> control = dispatcher.handleExecution(
executionScopeWithCREvent(observedGenResource));
assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional"))
.getStatus().getObservedGeneration())
.isEqualTo(1L);
}

@Test
void doesNotUpdatesObservedGenerationIfStatusIsNotPatchedWhenUsingSSA() throws Exception {
var observedGenResource = createObservedGenCustomResource();
Expand All @@ -476,28 +453,6 @@ void doesNotUpdatesObservedGenerationIfStatusIsNotPatchedWhenUsingSSA() throws E
assertThat(control.getUpdatedCustomResource()).isEmpty();
}

@Test
void patchObservedGenerationOnCustomResourcePatchIfNoSSA() throws Exception {
var observedGenResource = createObservedGenCustomResource();

Reconciler<ObservedGenCustomResource> reconciler = mock(Reconciler.class);
final var config = MockControllerConfiguration.forResource(ObservedGenCustomResource.class);
CustomResourceFacade<ObservedGenCustomResource> facade = mock(CustomResourceFacade.class);
when(config.isGenerationAware()).thenReturn(true);
when(reconciler.reconcile(any(), any()))
.thenReturn(UpdateControl.patchResource(observedGenResource));
when(facade.patchResource(any(), any())).thenReturn(observedGenResource);
when(facade.patchStatus(eq(observedGenResource), any())).thenReturn(observedGenResource);
initConfigService(false);
var dispatcher = init(observedGenResource, reconciler, config, facade, true);

PostExecutionControl<ObservedGenCustomResource> control = dispatcher.handleExecution(
executionScopeWithCREvent(observedGenResource));
assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional"))
.getStatus().getObservedGeneration())
.isEqualTo(1L);
}

@Test
void doesNotPatchObservedGenerationOnCustomResourcePatch() throws Exception {
var observedGenResource = createObservedGenCustomResource();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package io.javaoperatorsdk.operator.sample.observedgeneration;

import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;

public class ObservedGenStatus extends ObservedGenerationAwareStatus {
public class ObservedGenStatus {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package io.javaoperatorsdk.operator;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
import io.javaoperatorsdk.operator.sample.manualobservedgeneration.ManualObservedGenerationCustomResource;
import io.javaoperatorsdk.operator.sample.manualobservedgeneration.ManualObservedGenerationReconciler;
import io.javaoperatorsdk.operator.sample.manualobservedgeneration.ManualObservedGenerationSpec;

import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

public class ManualObservedGenerationIT {

public static final String RESOURCE_NAME = "test1";
@RegisterExtension
LocallyRunOperatorExtension extension =
LocallyRunOperatorExtension.builder().withReconciler(new ManualObservedGenerationReconciler())
.build();

@Test
void observedGenerationUpdated() {
extension.create(testResource());

await().untilAsserted(() -> {
var r = extension.get(ManualObservedGenerationCustomResource.class, RESOURCE_NAME);
assertThat(r).isNotNull();
assertThat(r.getStatus().getObservedGeneration()).isEqualTo(1);
assertThat(r.getStatus().getObservedGeneration()).isEqualTo(r.getMetadata().getGeneration());
});

var changed = testResource();
changed.getSpec().setValue("changed value");
extension.replace(changed);

await().untilAsserted(() -> {
var r = extension.get(ManualObservedGenerationCustomResource.class, RESOURCE_NAME);
assertThat(r.getStatus().getObservedGeneration()).isEqualTo(2);
assertThat(r.getStatus().getObservedGeneration()).isEqualTo(r.getMetadata().getGeneration());
});
}

ManualObservedGenerationCustomResource testResource() {
var res = new ManualObservedGenerationCustomResource();
res.setMetadata(new ObjectMetaBuilder()
.withName(RESOURCE_NAME)
.build());
res.setSpec(new ManualObservedGenerationSpec());
res.getSpec().setValue("Initial Value");
return res;
}
}
Loading
Loading