When workflow level retry expression evaluated False, workflow message is always `retryStrategy.expression evaluated to false` regardless of actual failure

tczhao commented 2 weeks ago

Pre-requisites

[X] I have double-checked my configuration
[X] I have tested with the :latest image tag (i.e. quay.io/argoproj/workflow-controller:latest) and can confirm the issue still exists on :latest. If not, I have explained why, in detail, in my description below.
[X] I have searched existing issues and could not find a match for this bug
[ ] I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

When workflow level retryExpression is used. regardless of workflow succeed or failure, the message at workflow level is always retryStrategy.expression evaluated to false

when no expression is used

workflow succeed -> no workflow message
workflow fail -> no more retry left

when expression is used

workflow succeed -> retryStrategy.expression evaluated to false
workflow fail (expression evaluated False) -> retryStrategy.expression evaluated to false
workflow fail (expression evaluated True) -> no more retry left

This is problematic if one relies on the workflow message for metrc, analytics or debug. We expect when expression is used, the workflow level message follows the same pattern as normal retry

Version

v3.5.2, latest (https://github.com/argoproj/argo-workflows/commit/71f1d860b2c665ad87e8c313c4f865b829d07626)

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: dag-nested-
  namespace: argo
spec:
  templates:
    - name: echo
      outputs: {}
      metadata: {}
      container:
        name: ''
        image: alpine:3.7
        command:
          - exit
          - '1'
        resources: {}
    - name: nested-diamond
      outputs: {}
      metadata: {}
      dag:
        tasks:
          - name: A
            template: echo
          - name: B
            template: echo
            depends: A
          - name: C
            template: echo
            depends: A
          - name: D
            template: echo
            depends: B && C
  entrypoint: diamond
  arguments: {}
  retryStrategy:
    retryPolicy: OnError
    expression: >-
      lastRetry.status == 'Errored'

Logs from the workflow controller

kubectl logs -n argo deploy/workflow-controller | grep ${workflow}

Logs from in your workflow's wait container

kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded

agilgur5 commented 2 weeks ago

v3.5.8

Did you mean v3.4.8? or v3.5.6? Because the version you wrote doesn't exist right now

tczhao commented 2 weeks ago

Sorry, I was meant to add the commit hash for latest. Nothing to do with x.x.8 Updated

argoproj / argo-workflows