Error deleting a custom resources

[jalonsomagnolia]

Bug Report

After upgrading to the java-operator-sdk 5.0, we have the following error when we try to delete a custom resource:

09:43:21 ERROR [io.ja.op.pr.ev.EventProcessor] (ReconcilerExecutor-subscriptionreconciler-46) Error during event processing ExecutionScope{ resource id: ResourceID{name='postman', namespace='paas-admin-api'}, version: 861762903}: io.javaoperatorsdk.operator.OperatorException: io.javaoperatorsdk.operator.OperatorException: java.lang.UnsupportedOperationException: Implement this
	at io.javaoperatorsdk.operator.processing.Controller.cleanup(Controller.java:216)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleCleanup(ReconciliationDispatcher.java:245)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleDispatch(ReconciliationDispatcher.java:91)
	at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleExecution(ReconciliationDispatcher.java:66)
	at io.javaoperatorsdk.operator.processing.event.EventProcessor$ReconcilerExecutor.run(EventProcessor.java:444)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: io.javaoperatorsdk.operator.OperatorException: java.lang.UnsupportedOperationException: Implement this
	at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.lambda$timeControllerExecution$0(MicrometerMetrics.java:151)
	at io.micrometer.core.instrument.composite.CompositeTimer.record(CompositeTimer.java:69)
	at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.timeControllerExecution(MicrometerMetrics.java:147)
	at io.javaoperatorsdk.operator.processing.Controller.cleanup(Controller.java:164)
	... 7 more
Caused by: java.lang.UnsupportedOperationException: Implement this
	at io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.cleanup(Workflow.java:20)
	at io.javaoperatorsdk.operator.processing.Controller$2.execute(Controller.java:199)
	at io.javaoperatorsdk.operator.processing.Controller$2.execute(Controller.java:165)
	at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.lambda$timeControllerExecution$0(MicrometerMetrics.java:149)
	... 10 more

Everything worked fine before moving to 5.0.

Our controller is configured with something like this:

@Slf4j
@RequiredArgsConstructor
@ControllerConfiguration(
    informer = @Informer(
        namespaces = WATCH_ALL_NAMESPACES,
    ),
    generationAwareEventProcessing = false
)
public class SubscriptionReconciler implements Reconciler<K8Subscription>, Cleaner<K8Subscription> {

       //// more code here

      @Override
    public DeleteControl cleanup(K8Subscription subscription, Context<K8Subscription> context) {
        log.info("cleanup {}", subscription);
   }
}

The code fails before calling our cleanup.

Checking the controller code, where the error is being thrown, I can see

// The cleanup is called also when explicit invocation is true, but the cleaner is not
              // implemented
              if (managedWorkflow.hasCleaner() || !explicitWorkflowInvocation) {
                workflowCleanupResult = managedWorkflow.cleanup(resource, context);
              }

Shouldn't be insteadif (managedWorkflow.hasCleaner() || explicitWorkflowInvocation) { (so only when explicitWorkflowInvocation = true). By default @Workflow has explicitWorkflowInvocation = false, so I understand that unless you want to implement a workflow, this code shouldn't be called.

[csviri]

Hi @jalonsomagnolia , thx for reporting, taking a look!

[csviri]

@jalonsomagnolia could you pls provide a simple reproducer?

this works correctly in integration tests like this

That logic is actually right, since in this case it should be basically an empty no-op workflow there.

[jalonsomagnolia]

Thanks for the response @csviri . I'll provide you the reproducer asap. But in the meantime, let me understand something:

// The cleanup is called also when explicit invocation is true, but the cleaner is not
              // implemented
              if (managedWorkflow.hasCleaner() || !explicitWorkflowInvocation) {
                workflowCleanupResult = managedWorkflow.cleanup(resource, context);
              }

We don't want to invoke this managedWorkflow.cleanup, because the result is the error we are seeing. Then, the condition has to be false. managedWorkflow.hasCleaner()is false, which it's ok. And explicitWorkflowInvocation is false, which makes the if condition true, executing the code that produces the error.

default WorkflowCleanupResult cleanup(P primary, Context<P> context) {
    throw new UnsupportedOperationException("Implement this");
  }

Then, if you say that the ìf` condition is correct, then maybe the initialiser is incorrect:

explicitWorkflowInvocation =
        configuration.getWorkflowSpec().map(WorkflowSpec::isExplicitInvocation)
            .orElse(false);

In our case, configuration.getWorkflowSpec() is null, making explicitWorkflowInvocation = false. Should the default value be true then?

Finally, notice that the code in version 4.9.7, which works fine, was like this:

Controller.this.initContextIfNeeded(resource, context);
                    WorkflowCleanupResult workflowCleanupResult = null;
                    if (Controller.this.managedWorkflow.hasCleaner()) {
                        workflowCleanupResult = Controller.this.managedWorkflow.cleanup(resource, context);
                    }

The change is adding that new explicitWorkflowInvocation variable. To me it looks that something is wrong related to that variable.

[csviri]

Hi right there is an issue, your right, it is a bit more complex than it first seems, pls take a look on attached PR if makes sense.
#2680

Will add unit tests soon.
However would be nice to have a reproducer since there are other reasons that this should not failed nevertheless. thx

[jalonsomagnolia]

Good morning @csviri . Here it is the reproducer project. As you can see it's a really simple operator, but if you create a CR and then tries to delete, you'll have this exception

2025-02-04 17:20:06,899 ERROR [io.jav.ope.pro.eve.EventProcessor] (ReconcilerExecutor-mgnlsimplereconciler-166) Error during event processing ExecutionScope{ resource id: ResourceID{name='test', namespace='test'}, version: 861967378}: io.javaoperatorsdk.operator.OperatorException: java.lang.UnsupportedOperationException: Implement this
        at io.javaoperatorsdk.operator.processing.Controller.cleanup(Controller.java:216)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleCleanup(ReconciliationDispatcher.java:245)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleDispatch(ReconciliationDispatcher.java:91)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleExecution(ReconciliationDispatcher.java:66)
        at io.javaoperatorsdk.operator.processing.event.EventProcessor$ReconcilerExecutor.run(EventProcessor.java:444)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
        at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.UnsupportedOperationException: Implement this
        at io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.cleanup(Workflow.java:20)
        at io.javaoperatorsdk.operator.processing.Controller$2.execute(Controller.java:199)
        at io.javaoperatorsdk.operator.processing.Controller$2.execute(Controller.java:165)
        at io.javaoperatorsdk.operator.api.monitoring.Metrics.timeControllerExecution(Metrics.java:165)
        at io.javaoperatorsdk.operator.processing.Controller.cleanup(Controller.java:164)
        ... 7 more

[csviri]

@jalonsomagnolia the issue is that this exception is not thrown in any of our samples or any integration tests; those are spinning up real controllers, so I'm not able to reproduce. Would be nice to have a sample operator provided to use (within a github repo)

But nevertheless I fixes the issue with that expression. It would be a help if you could build a local copy from that branch or after merged to main, and try you operator with that if the problem disappears. Thank you!

[jalonsomagnolia]

Oh, sorry. I forgot to include the link 🤦 . Here it is -> https://gitlab.magnolia-platform.com/micro-services/public/simple-operator

[csviri]

Ahh, I checked the sample just now, it is in Quarkus Operator SDK, that is a completely different story, probably this fix will help on that, but the workflow is created in a different way there.

cc @metacosm @xstefank you might want to check that, and add an integration / e2e test for this use case.
We can release this patch after this is verified.

(I tested locally manually, it seems that the pr fixed the issue)

[jalonsomagnolia]

Thanks for the fix. I can confirm it works like a charm

[csviri]

Thank you for reporting @jalonsomagnolia !!