Skip to content
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

Context.getSecondaryResource(A.class) should return Optional.empty when activationCondition not met #2198

Open
Javatar81 opened this issue Jan 11, 2024 · 2 comments

Comments

@Javatar81
Copy link

Bug Report

What did you do?

I've created a class ADependent extends CRUDKubernetesDependentResource<A, B>. This class is registered at a reconciler class via annotation:

@Dependent(activationCondition = AlwaysFalseActivationCondition.class, type = ADependent.class)

Then I've called context.getSecondaryResource(A.class) in the reconcile method.

What did you expect to see?

When I try to get the dependent resource via context.getSecondaryResource(A.class), I would expect an Optional.empty as return value.

What did you see instead? Under which circumstances?

When I try to get the dependent resource via context.getSecondaryResource(A.class) I always get the following exception:

io.javaoperatorsdk.operator.OperatorException: io.javaoperatorsdk.operator.AggregatedOperatorException: Exception(s) during workflow execution. Details:
 - io.devjoy.operator.project.k8s.SourceRepositoryDependentResource -> java.lang.IllegalArgumentException: There is no event source found for class:io.devjoy.operator.environment.k8s.build.BuildEventListenerDependentResource
        at io.javaoperatorsdk.operator.processing.event.EventSources.get(EventSources.java:127)
        at io.javaoperatorsdk.operator.processing.event.EventSourceManager.getResourceEventSourceFor(EventSourceManager.java:273)
        at io.javaoperatorsdk.operator.api.reconciler.DefaultContext.getSecondaryResource(DefaultContext.java:59)
        at io.javaoperatorsdk.operator.api.reconciler.Context.getSecondaryResource(Context.java:20)
        at io.devjoy.operator.project.k8s.SourceRepositoryDependentResource.desired(SourceRepositoryDependentResource.java:58)
        at io.devjoy.operator.project.k8s.SourceRepositoryDependentResource.desired(SourceRepositoryDependentResource.java:27)
        at io.javaoperatorsdk.operator.processing.dependent.AbstractDependentResource.reconcile(AbstractDependentResource.java:59)
        at io.javaoperatorsdk.operator.processing.dependent.SingleDependentResourceReconciler.reconcile(SingleDependentResourceReconciler.java:19)
        at io.javaoperatorsdk.operator.processing.dependent.AbstractDependentResource.reconcile(AbstractDependentResource.java:52)
        at io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileExecutor$NodeReconcileExecutor.doRun(WorkflowReconcileExecutor.java:141)
        at io.javaoperatorsdk.operator.processing.dependent.workflow.NodeExecutor.run(NodeExecutor.java:22)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
        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:1623)

        at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.lambda$timeControllerExecution$0(MicrometerMetrics.java:165)
        at io.micrometer.core.instrument.composite.CompositeTimer.record(CompositeTimer.java:69)
        at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.timeControllerExecution(MicrometerMetrics.java:161)
        at io.javaoperatorsdk.operator.processing.Controller.reconcile(Controller.java:110)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.reconcileExecution(ReconciliationDispatcher.java:140)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleReconcile(ReconciliationDispatcher.java:121)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleDispatch(ReconciliationDispatcher.java:91)
        at io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.handleExecution(ReconciliationDispatcher.java:64)
        at io.javaoperatorsdk.operator.processing.event.EventProcessor$ReconcilerExecutor.run(EventProcessor.java:417)
        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:1623)
Caused by: io.javaoperatorsdk.operator.AggregatedOperatorException: Exception(s) during workflow execution. Details:
 - io.devjoy.operator.project.k8s.SourceRepositoryDependentResource -> java.lang.IllegalArgumentException: There is no event source found for class:io.devjoy.operator.environment.k8s.build.BuildEventListenerDependentResource
        at io.javaoperatorsdk.operator.processing.event.EventSources.get(EventSources.java:127)
        at io.javaoperatorsdk.operator.processing.event.EventSourceManager.getResourceEventSourceFor(EventSourceManager.java:273)
        at io.javaoperatorsdk.operator.api.reconciler.DefaultContext.getSecondaryResource(DefaultContext.java:59)
        at io.javaoperatorsdk.operator.api.reconciler.Context.getSecondaryResource(Context.java:20)
        at io.devjoy.operator.project.k8s.SourceRepositoryDependentResource.desired(SourceRepositoryDependentResource.java:58)
        at io.devjoy.operator.project.k8s.SourceRepositoryDependentResource.desired(SourceRepositoryDependentResource.java:27)
        at io.javaoperatorsdk.operator.processing.dependent.AbstractDependentResource.reconcile(AbstractDependentResource.java:59)
        at io.javaoperatorsdk.operator.processing.dependent.SingleDependentResourceReconciler.reconcile(SingleDependentResourceReconciler.java:19)
        at io.javaoperatorsdk.operator.processing.dependent.AbstractDependentResource.reconcile(AbstractDependentResource.java:52)
        at io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileExecutor$NodeReconcileExecutor.doRun(WorkflowReconcileExecutor.java:141)
        at io.javaoperatorsdk.operator.processing.dependent.workflow.NodeExecutor.run(NodeExecutor.java:22)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:577)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
        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:1623)

        at io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowResult.throwAggregateExceptionIfErrorsPresent(WorkflowResult.java:41)
        at io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult.throwAggregateExceptionIfErrorsPresent(WorkflowReconcileResult.java:9)
        at io.javaoperatorsdk.operator.processing.dependent.workflow.DefaultWorkflow.reconcile(DefaultWorkflow.java:95)
        at io.javaoperatorsdk.operator.processing.Controller$1.execute(Controller.java:148)
        at io.javaoperatorsdk.operator.processing.Controller$1.execute(Controller.java:111)
        at io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics.lambda$timeControllerExecution$0(MicrometerMetrics.java:163)
        ... 11 more

Environment

Kubernetes cluster type:

OpenShift Local 4.14.7

$ Mention java-operator-sdk version from pom.xml file

4.6.1

$ java -version

openjdk version "20.0.1" 2023-04-18
OpenJDK Runtime Environment Temurin-20.0.1+9 (build 20.0.1+9)
OpenJDK 64-Bit Server VM Temurin-20.0.1+9 (build 20.0.1+9, mixed mode)

$ kubectl version

Client Version: v1.28.1
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.8+4fab27b

Possible Solution

Alternatively I could use the activationCondition class to check the return value of the isMet() method, however to instantiate the condition class, I would need to pass the instance of the dependentResource that would only be directly available if I would use standalone dependent resources. Maybe I could also get it via context.managedDependentResourceContext()? Still, this would only be a workaround.

Additional context

For my use case the activationCondition is not always false and I need to delete the resource (if it is active) in my reconciler cleanup method. The most intuitive way to accomplish this would be: context.getSecondaryResource(A.class).ifPresent(a -> client.resource(a).delete()); However, this leads to an exception as described above.

@csviri csviri added this to the 5.0 milestone Jan 11, 2024
@csviri
Copy link
Collaborator

csviri commented Jan 11, 2024

Hi @Javatar81 ,

the background of this until we very recently introduced Activation condition, but also generic dynamic event source registration, accessing a resource with getSecondaryResource which did not have an event source was clearly an issue with the implementation. Now it is not the case anymore, and I agree that it would be nicer to in general return an empty Optional if no event source is present or there is no resource yet in event sources.

However it could be also misleading, since it could be checked that the resource does not exists, however if there were just (maybe as a bug?) an event source not registered for it, this would be false alarm.

So maybe we could add a flag to the gerSecondaryResource if we expect that the event source is registered dynamically.

Note that now you can not just catch the exception. Maybe create your own wrapper utility method.

But returning all the cases an Optional might not be a good idea in my opinion.

cc @metacosm

@Javatar81
Copy link
Author

I just add my perspective as a user of JOSDK. For me dynamic event source registration and event sources in general are internal details most of the time. Ideally, I can just use the annotations and from this perspective the getSecondaryResource method will either return the resource or not (which I would expect to be Optional.empty because its return type is Optional). There could be several reasons why I would not get the resource and thus the methods should return empty:

  1. I passed a class as expectedType parameter that is not known by the reconciler at all, e.g. by mistake. (btw. all classes are allowed and I often have to look up in the docs whether it expects the resource type itself or the one for the CRUDKubernetesDependentResource but AFAIK this is because we can manage a variety of resources not only Kubernetes resources)

  2. I passed a class as expectedType parameter that is generally known by the reconciler because the respective CRUDKubernetesDependentResource exists but it is not activated because the activationCondition holds false

Btw. currently both cases currently lead to the described java.lang.IllegalArgumentException which makes unclear for when getSecondaryResource actually returns an empty optional at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants