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

[FEATURE]: allow failure for workflow tasks #6077

Closed
salotz opened this issue May 24, 2024 · 6 comments · Fixed by #6114
Closed

[FEATURE]: allow failure for workflow tasks #6077

salotz opened this issue May 24, 2024 · 6 comments · Fixed by #6114
Labels
feature request help wanted workflows Issues related to Garden Workflows

Comments

@salotz
Copy link

salotz commented May 24, 2024

Feature Request

Background / Motivation

In some cases I want to run a workflow task that I know might fail for various reasons (see #6076 but also others).

What should the user be able to do?

In the workflow step you should be able to specify whether a step is allowed to fail without the entire workflow exiting as failure.

Why do they want to do this? What problem does it solve?

Suggested Implementation(s)

kind: Workflow
steps:
  - name: can-fail
    command: [...]
    allowFailure: true

How important is this feature for you/your team?

🌵 Not having this feature makes using Garden painful

@stefreak
Copy link
Member

Great feature request 👍 Thank you @salotz

@stefreak stefreak removed their assignment May 28, 2024
@alex-kattathra-johnson
Copy link
Contributor

Hey there! I would like to give this a shot!

@alex-kattathra-johnson
Copy link
Contributor

@salotz, @stefreak brought up a good point to clarify. Which behaviour do you think is appropriate? Should subsequent steps after the errored step be skipped unless when: always or when: onError are set? The issue description suggests that the current implementation suffices.
#6114 (comment)

@salotz
Copy link
Author

salotz commented Jun 3, 2024

I think that an allowFailure: true should basically behave as if it succeeded, meaning subsequent steps should proceed as normal. An allowed failure should not stop running onSuccess tasks and onError should not run.

I think the more ideal solution is to distinguish onError and onFailure. onError would trigger when an allowFailure: true task errors is allowed to pass and equally if it isn't allowed to pass. Whereas a onFailure would only trigger if an allowFailure: false step has an error.

But this probably digresses into more systemic design issues of the Workflow type which is beyond the scope here.

@stefreak
Copy link
Member

stefreak commented Jun 4, 2024

I like the idea of distinguishing between onError and onFailure. Another problem that complicates things further is that we should also distinguish between "completed with error, but allowed to fail" and "failed" in the Cloud UI (https://app.garden.io) - but I think it is acceptable in the first iteration of this to solve the problem by making it clear in the logs.

I would be open for implementation of this functionality, marking the allowFailure flags and the onFailure behaviour as experimental for a while so we can monitor the usefulness in practice and can make a breaking change if needed without bumping Garden to 0.14. (Similar how we did it with Cloud Builder by adding something like **Stability: Experimental**. Subject to breaking changes within minor releases. to the docs.

How do you feel about this? @salotz @alex-kattathra-johnson

@salotz
Copy link
Author

salotz commented Jun 6, 2024

That is fine with me. Like I said I'm fine without those extra features and can't say I would even use them immediately, but seems to be the next logical step. If there is going to be some next major version of Workflow features I'd probably have a bunch more things to consider the issue holistically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request help wanted workflows Issues related to Garden Workflows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants