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

Enhanced depends logic not working when task is omitted #8654

Closed
3 tasks done
luke-hemisphere opened this issue May 6, 2022 · 6 comments · Fixed by #8672 or argoproj/argo-helm#1297
Closed
3 tasks done

Enhanced depends logic not working when task is omitted #8654

luke-hemisphere opened this issue May 6, 2022 · 6 comments · Fixed by #8672 or argoproj/argo-helm#1297

Comments

@luke-hemisphere
Copy link

Checklist

  • Double-checked my configuration.
  • Tested using the latest version.
  • Used the Emissary executor.

Summary

What happened?
When a task is Skipped/Omitted because a dependent task is skipped you can't use enhanced depends on that task.

  O
 /  \
A    B <- B is skipped
|    |
C    D <- D gets omitted
|   /
E <- if C.Succeeded && (D.Succeeded || D.Skipped)*

*doesn't work

What you expected to happen?
If a task is omitted/skipped due a dependent predecessor task being skipped, you should be able to use depends.Skipped on that task in another task.

What version are you running? 3.3.1

Diagnostics

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: broken-depends
  namespace: argo
spec:
  serviceAccountName: argo-workflow-sa
  entrypoint: diamond
  arguments:
    parameters:
      - name: input_var
        default: "true"
  templates:
    - name: diamond
      inputs:
        parameters:
          - name: input_var
      dag:
        tasks:
          - name: A
            template: pass
          - name: B
            template: pass
            when: "{{ inputs.parameters.input_var }} == false"
          - name: C
            template: pass
            depends: B.Succeeded
          - name: D
            template: pass
            depends: A.Succeeded
          - name: E
            template: pass
            depends: D.Succeeded && C
    - name: pass
      container:
        image: alpine:3.7
        command:
          - sh
          - -c
          - exit 0
    - name: fail
      container:
        image: alpine:3.7
        command:
          - sh
          - -c
          - exit 1
# Logs from the workflow controller:
time="2022-05-06T05:20:29.162Z" level=info msg="Processing workflow" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.176Z" level=info msg="Updated phase  -> Running" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.198Z" level=info msg="DAG node broken-depends-pn58z initialized Running" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.199Z" level=info msg="All of node broken-depends-pn58z.B dependencies [] completed" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.199Z" level=info msg="Skipped node broken-depends-pn58z-2614862280 initialized Skipped (message: when 'true == false' evaluated false)" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.199Z" level=info msg="Skipped node broken-depends-pn58z-2631639899 initialized Omitted (message: omitted: depends condition not met)" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.200Z" level=info msg="All of node broken-depends-pn58z.A dependencies [] completed" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.200Z" level=info msg="Pod node broken-depends-pn58z-2665195137 initialized Pending" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.228Z" level=info msg="Created pod: broken-depends-pn58z.A (broken-depends-pn58z-2665195137)" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.229Z" level=info msg="TaskSet Reconciliation" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.229Z" level=info msg=reconcileAgentPod namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:29.267Z" level=info msg="Workflow update successful" namespace=argo phase=Running resourceVersion=359615338 workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.234Z" level=info msg="Processing workflow" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.235Z" level=info msg="node changed" new.message= new.phase=Succeeded new.progress=0/1 nodeID=broken-depends-pn58z-2665195137 old.message= old.phase=Pending old.progress=0/1
time="2022-05-06T05:20:39.236Z" level=info msg="All of node broken-depends-pn58z.D dependencies [A] completed" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.241Z" level=info msg="Pod node broken-depends-pn58z-2715527994 initialized Pending" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.262Z" level=info msg="Created pod: broken-depends-pn58z.D (broken-depends-pn58z-2715527994)" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.263Z" level=info msg="TaskSet Reconciliation" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.263Z" level=info msg=reconcileAgentPod namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.278Z" level=info msg="Workflow update successful" namespace=argo phase=Running resourceVersion=359615414 workflow=broken-depends-pn58z
time="2022-05-06T05:20:39.283Z" level=info msg="cleaning up pod" action=labelPodCompleted key=argo/broken-depends-pn58z-2665195137/labelPodCompleted
time="2022-05-06T05:20:49.267Z" level=info msg="Processing workflow" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.269Z" level=info msg="node changed" new.message= new.phase=Succeeded new.progress=0/1 nodeID=broken-depends-pn58z-2715527994 old.message= old.phase=Pending old.progress=0/1
time="2022-05-06T05:20:49.270Z" level=info msg="Outbound nodes of broken-depends-pn58z set to [broken-depends-pn58z-2631639899 broken-depends-pn58z-2715527994]" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="node broken-depends-pn58z phase Running -> Succeeded" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="node broken-depends-pn58z finished: 2022-05-06 05:20:49.270813223 +0000 UTC" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="Checking daemoned children of broken-depends-pn58z" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="TaskSet Reconciliation" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg=reconcileAgentPod namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="Updated phase Running -> Succeeded" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="Marking workflow completed" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.270Z" level=info msg="Doesn't match with archive label selector. Skipping Archive" namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.272Z" level=info msg="Checking daemoned children of " namespace=argo workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.277Z" level=info msg="cleaning up pod" action=deletePod key=argo/broken-depends-pn58z-1340600742-agent/deletePod
time="2022-05-06T05:20:49.288Z" level=info msg="Workflow update successful" namespace=argo phase=Succeeded resourceVersion=359615487 workflow=broken-depends-pn58z
time="2022-05-06T05:20:49.300Z" level=info msg="cleaning up pod" action=labelPodCompleted key=argo/broken-depends-pn58z-2715527994/labelPodCompleted

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

terrytangyuan added a commit to terrytangyuan/argo-workflows that referenced this issue May 6, 2022
…roj#8654

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
terrytangyuan added a commit that referenced this issue May 6, 2022
…8672)

* feat: Handle omitted nodes in DAG enhanced depends logic. Fixes #8654

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

* fix: test

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
@terrytangyuan
Copy link
Member

This is fixed in #8672. Thanks for reporting!

@yavulan
Copy link

yavulan commented May 14, 2022

@terrytangyuan I'm using Omitted with dag, and sensor bumps into this error https://github.com/argoproj/argo-workflows/blob/master/workflow/common/ancestry.go#L102

@terrytangyuan
Copy link
Member

@terrytangyuan I'm using Omitted with dag, and sensor bumps into this error https://github.com/argoproj/argo-workflows/blob/master/workflow/common/ancestry.go#L102

Thank you! I am fixing this in #8776.

@sarabala1979 sarabala1979 mentioned this issue May 25, 2022
14 tasks
sarabala1979 pushed a commit that referenced this issue May 25, 2022
…8672)

* feat: Handle omitted nodes in DAG enhanced depends logic. Fixes #8654

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>

* fix: test

Signed-off-by: Yuan Tang <terrytangyuan@gmail.com>
jmeridth added a commit to jmeridth/argo-helm that referenced this issue May 26, 2022
[Release Notes](https://github.com/argoproj/argo-workflows/releases/tag/v3.3.6)

Includes 1 fix:
- eat: Handle omitted nodes in DAG enhanced depends logic. Fixes [#8654](argoproj/argo-workflows#8654)

Signed-off-by: jmeridth <jmeridth@gmail.com>
mbevc1 pushed a commit to argoproj/argo-helm that referenced this issue May 26, 2022
[Release Notes](https://github.com/argoproj/argo-workflows/releases/tag/v3.3.6)

Includes 1 fix:
- eat: Handle omitted nodes in DAG enhanced depends logic. Fixes [#8654](argoproj/argo-workflows#8654)

Signed-off-by: jmeridth <jmeridth@gmail.com>

Co-authored-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>
foxtel-temujincabigao pushed a commit to foxtel-temujincabigao/argo-helm that referenced this issue May 30, 2022
[Release Notes](https://github.com/argoproj/argo-workflows/releases/tag/v3.3.6)

Includes 1 fix:
- eat: Handle omitted nodes in DAG enhanced depends logic. Fixes [#8654](argoproj/argo-workflows#8654)

Signed-off-by: jmeridth <jmeridth@gmail.com>

Co-authored-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>
Signed-off-by: foxtel-temujincabigao <86087373+foxtel-temujincabigao@users.noreply.github.com>
@mhehle
Copy link

mhehle commented Jul 14, 2022

@terrytangyuan @luke-hemisphere

in version 3.3.8 this issue still exists.
Its impossible to launch/create a workflow with depends on .Omitted step.
You get an error: Internal Server Error: templates.main.tasks.E task result 'Omitted' for task 'D' is invalid

Example Workflow:

apiVersion: argoproj.io/v1alpha1
metadata:
  name: broken-depends
  generateName: broken-depends-
  namespace: argo
spec:
  serviceAccountName: workflow
  entrypoint: main
  templates:
    - name: main
      inputs:
        parameters:
          - name: input_var
            default: 'true'
      dag:
        tasks:
          - name: A
            template: pass
          - name: B
            template: pass
            when: '{{ inputs.parameters.input_var }} == false'
          - name: C
            template: pass
            depends: A.Succeeded
          - name: D
            template: pass
            depends: B.Succeeded
          - name: E
            template: pass
            depends: (D.Succeeded || D.Omitted) && C
    - name: pass
      container:
        image: 'alpine:3.7'
        command:
          - sh
          - '-c'
          - exit 0
    - name: fail
      container:
        image: 'alpine:3.7'
        command:
          - sh
          - '-c'
          - exit 1

@SiddharthMalhotra
Copy link

Facing the same issue as @mhehle

terrych0u pushed a commit to terrych0u/argo-helm that referenced this issue Aug 4, 2022
[Release Notes](https://github.com/argoproj/argo-workflows/releases/tag/v3.3.6)

Includes 1 fix:
- eat: Handle omitted nodes in DAG enhanced depends logic. Fixes [#8654](argoproj/argo-workflows#8654)

Signed-off-by: jmeridth <jmeridth@gmail.com>

Co-authored-by: Marco Kilchhofer <mkilchhofer@users.noreply.github.com>
@mhehle
Copy link

mhehle commented Aug 16, 2022

the commit 961f731 with the fix was only picked for 3.3.9 release. So this issue should be resolved since version 3.3.9.

CherryPick list for 3.3.9
#9262

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