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

Missing workflowExecutionResult during error handling with Managed Workflow #2551

Open
murillio4 opened this issue Oct 12, 2024 · 7 comments · May be fixed by #2552
Open

Missing workflowExecutionResult during error handling with Managed Workflow #2551

murillio4 opened this issue Oct 12, 2024 · 7 comments · May be fixed by #2552
Assignees

Comments

@murillio4
Copy link

murillio4 commented Oct 12, 2024

Bug Report

What did you do?

I attempted to handle errored dependents within managed workflow. I implemented ErrorStatusHandler in my reconciler and configured updateErrorStatus to update the CRD status with a list of failed dependents (including error messages) and successful dependents.

What did you expect to see?

The CRD status should display a list of failed dependents (with error messages) and successful dependents.

What did you see instead? Under which circumstances?

The CRD status was not updated because getWorkflowReconcileResult() returned null.

Environment

AKS kubernetes version: 1.30.4

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

$ java -version

openjdk 21 2023-09-19
OpenJDK Runtime Environment (build 21+35-2513)
OpenJDK 64-Bit Server VM (build 21+35-2513, mixed mode, sharing)

$ kubectl version

Client Version: v1.30.1
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.30.4

Possible Explenation

I seems both Controller.execute and DefaultWorkflow.reconcile are designed to handle workflow errors via throwAggregateExceptionIfErrorsPresent. The DefaultWorkflowhandles errors only if throwExceptionAutomatically is set to true, which it is by default (THROW_EXCEPTION_AUTOMATICALLY_DEFAULT is true).

However, the issue is that Controller.execute is responsible for setting the workflowExecutionResult. Since DefaultWorkflow.reconcile throws an exception when handling errors with throwAggregateExceptionIfErrorsPresent, the error handling in Controller.execute never hits, and workflowExecutionResult is never set.

@murillio4 murillio4 changed the title Missing workflowExecutionResult in Managed Workflow Error Handling Missing workflowExecutionResult During Error Handling with Managed Workflow Oct 12, 2024
@murillio4 murillio4 changed the title Missing workflowExecutionResult During Error Handling with Managed Workflow Missing workflowExecutionResult during error handling with Managed Workflow Oct 12, 2024
@metacosm metacosm self-assigned this Oct 12, 2024
@metacosm
Copy link
Collaborator

Indeed, there's an issue there. Thank you for the report. Can this wait for version 5 so that we can address this in a proper fashion as this would be rather complex to address this for version 4 of the SDK while maintaining backwards compatibility? The easiest solution would be to change the default value to not throw exceptions automatically but this would break the existing behavior.

@murillio4
Copy link
Author

While I had hoped this would be addressed in version 4, I understand that fixing it now would break backward compatibility. Addressing it in version 5 makes sense.

Do you know when v5 is scheduled for release?

@murillio4
Copy link
Author

murillio4 commented Oct 13, 2024

I saw in your response to issue #2545 that you're waiting for the fabric8 client v7, which answers my question.

Do you expect this issue to be resolved in the initial v5 release, or will it come in a later patch?

@metacosm
Copy link
Collaborator

I will see if I can work something out for v4. It will definitely be in the first v5 release.

@murillio4
Copy link
Author

Super! Maybe making throwExceptionAutomatically configurable through the ConfigurationService. Then default can be true making it backward compatible.

@metacosm
Copy link
Collaborator

See linked PR, let me know if this addresses the issue.

metacosm added a commit that referenced this issue Oct 14, 2024
metacosm added a commit that referenced this issue Oct 15, 2024
@murillio4
Copy link
Author

Seems to work 🙂 workflowExecutionResult are now available in updateErrorStatus.

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