Skip to content

Misleading warnings in AbstractWorkflowExecutor #2167

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

Closed
kosmoz opened this issue Dec 18, 2023 · 3 comments · Fixed by #2170
Closed

Misleading warnings in AbstractWorkflowExecutor #2167

kosmoz opened this issue Dec 18, 2023 · 3 comments · Fixed by #2170
Assignees

Comments

@kosmoz
Copy link

kosmoz commented Dec 18, 2023

Bug Report

When upgrading the Java Operator SDK to 4.6, I noticed some warnings that I did not see before. I believe these warnings are false positives and were introduced on accident when activation conditions were implemented.

What did you do?

  1. Have a reconciler with one managed DR
  2. Start operator
  3. Create resource

What did you expect to see?

No warning.

What did you see instead? Under which circumstances?

The following warning is logged once for each reconciliation of the primary resource:

WARN  i.j.o.p.d.w.WorkflowReconcileExecutor - Notified but still resources under execution. This should not happen.

Environment

Kubernetes cluster type:

vanilla

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

4.6.1

$ java -version

openjdk version "17.0.9" 2023-10-17
OpenJDK Runtime Environment (Red_Hat-17.0.9.0.9-2) (build 17.0.9+9)
OpenJDK 64-Bit Server VM (Red_Hat-17.0.9.0.9-2) (build 17.0.9+9, mixed mode, sharing)

$ kubectl version

Client Version: v1.28.4
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.3

Possible Solution

When activation conditions were implemented, waitForScheduledExecutionsToRun needed to be changed because it had become possible that no executions were scheduled to begin with. To achieve this, the invocation of this.wait() was simply moved to after where noMoreExecutionsScheduled() is checked (see dcbf7d6#diff-42a60986a7e5e09307320e6bb6473cc5e8db0af1fefc502079936802982a8953). The problem, from what I understand, is that it must be expected that noMoreExecutionsScheduled() returns false before wait() has been called at least once. Because it probably makes sense to keep the warning, I think a possible solution would be to keep the contents of the while loop as it was before this change and check noMoreExecutionsScheduled() an additional time before entering the loop (without logging a warning if it returns false).

Additional context

@csviri
Copy link
Collaborator

csviri commented Dec 19, 2023

Exactly @kosmoz , thank you! attached the PR

@kosmoz
Copy link
Author

kosmoz commented Dec 19, 2023

Thanks for the quick solution! 🙂

@csviri
Copy link
Collaborator

csviri commented Dec 19, 2023

Thanks for the quick solution! 🙂

Np, just did what you told me to do :) :P

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

Successfully merging a pull request may close this issue.

2 participants