argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
14.98k stars 3.19k forks source link

Add `.Complete` to Enhanced Depends #4527

Open JamesLaverack opened 3 years ago

JamesLaverack commented 3 years ago

Summary

Currently using the directed-acyclic graph workflow template and the enhanced depends logic it's possible to declare that a step should run if a specific previous step failed.

To give a simple example, here if A is successful then B will run. If A fails then C will run instead.

- name: A
- name: B
  depends: "A"
- name: C
  depends: "A.Failed"

Along with the logical or (||) operator, it is possible to declare a step should run after another step, irrespective of if that step succeeds or fails.

- name: A
- name: B
  depends: "A || A.Failed"

However this only works if A was run at all. If instead it is at the bottom of a longer chain of dependencies with a failure higher up then it is not possible to execute B.

For example

- name: A
- name: B
  depends: "A"
- name: C
  depends: "B"
- name: D
  depends: "C || C.Failed"

In this chan if C fails then D will run, but if A or B fail then D will not run. The only way to specify that D should always run, but only after C is to set D's depends to C || C.Failed || B.Failed || A.Failed. This is workable but becomes difficult to maintain if there is a long chain of steps with a "always run" step at the bottom, and critically is easy to miss a step.

Changing existing behaviour is likely a bad idea, but a new syntax to support this case could work. For example step.Complete could mean "step completed in any way. Including not run at all." but other language may be less confusing.

Use Cases

A common use case is to perform some cleanup after a series of steps. To give a more complete real-world example:

- name: download-terraform
- name: create-test-environment
  depends: "download-terraform"
  arguments:
    artifacts:
    - name: terraform
      from: '{{tasks.download-terraform.outputs.artifacts.terraform}}'
- name: configure-test-environment
  depends: "create-test-environment"
- name: install-project
  depends: "configure-test-environment"
- name: run-e2e-tests
  depends: "install-project"
- name: destroy-test-environment
  depends: "create-test-environment && (run-e2e-tests || run-e2e-tests.Failed || install-project.Failed || configure-test-environment.Failed)"
  arguments:
    artifacts:
    - name: terraform
      from: '{{tasks.download-terraform.outputs.artifacts.terraform}}'

Here we create a test environment, perform a number of steps and ultimately run some e2e tests on our project. Then we want to destroy the test environment. But even if any of the intermediate environment configuration steps fail we want to still destroy the test environment, so long as the original create-test-environment step ran. So we'd like to simply that depends down from create-test-environment && (run-e2e-tests || run-e2e-tests.Failed || install-project.Failed || configure-test-environment.Failed) to something more like create-test-environment && run-e2e-tests.Complete.


Message from the maintainers:

Impacted by this bug? Give it a πŸ‘. We prioritise the issues with the most πŸ‘.

JamesLaverack commented 3 years ago

For background, a related thread in Argo project Slack: https://argoproj.slack.com/archives/C8J6SGN12/p1605206561485900

tahiraha commented 1 year ago

For background, a related thread in Argo project Slack: https://argoproj.slack.com/archives/C8J6SGN12/p1605206561485900

@JamesLaverack This link is outdated, any chance you have an updated link? This issue is relevant to my current problem and I'd like to look at the thread. Thanks!