Support way to differentiant API resume from timeout resume (supplied ValueFrom is ignoring default)

[CodingOrca]

Discussed in https://github.com/argoproj/argo-workflows/discussions/7905

I am using argo workflows v3.3.0-rc4, and need a way to distinguish after a suspend step if its output parameter was supplied with a value per API and then resumed, or if the configured duration expired. Currently this is not possible:

^{Originally posted by **CodingOrca** February 17, 2022} I try to implement an approve - release workflow. I need a timeout on the approve step, and need to be able to react in a sub-sequent step if the timeout has expired. The documentation of the [SupppliedValueFrom](https://argoproj.github.io/argo-workflows/fields/#suppliedvaluefrom) shows an example without a duration: https://github.com/argoproj/argo-workflows/blob/master/examples/suspend-template-outputs.yaml Is is working, but it is not of practical use if I have no duration set on the suspend Template. So I have modified it according to the documentation found here: https://argoproj.github.io/argo-workflows/executor_swagger/#valuefrom This is only the changed part of the example: ``` - name: approve suspend: duration: "60" outputs: parameters: - name: message valueFrom: default: "some default value" supplied: {} ``` If I run the workflow, and do not supply a value from outside, after 60 seconds the output printed by the "release" step is: `{{steps.approve.outputs.parameters.message}}` But I am expecting the "release" step to print: `some default value` If I feed a value to the parameter, as described in the example, e.g. "Approved", and then resume the workflow, the "release" step is printing the expected value: `Approved` So basically I need the example to be extended with a "duration" of the suspend template and a way to detect in a subsequent step if the suspend was resumed per API or due to the timout.

[alexec]

{} is not a string. Have you tried setting supplied to the default value?

[CodingOrca]

You mean, something like this?

    outputs:
      parameters:
        - name: message
          valueFrom:
            supplied: "default"

If I do this, and try to create such a workflow, I get this error: Error: Could not create Workflow: . . . . v1alpha1.Parameter.ValueFrom: v1alpha1.ValueFrom.Supplied: skipObjectDecoder: expect object or null, error found in #10 byte of ...|supplied\":\"default\"}|..., bigger context ...|eters\":[{\"name\":\"message\",\"valueFrom\":{\"supplied\":\"default\"}}]},\"suspend\":{\"duration\":\"6\"}}],\"ttlStr|..."}

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[swafa]

@CodingOrca Did you manage to get around this somehow? I have a very similar use case and this is currently blocking me, too.

@alexec Any updates or remarks on @CodingOrca last comment? Would you please, provide a simple example how to provide defaults for the supplied value?

I also tried:

supplied: {"foo": "bar"}

But unfortunately, it didn't work either. It even got ignored from the manifest. When I check the manifest from the UI after submitting using kubectl apply, I see this:

supplied: {}

(as if anything between the curly braces is completely ignored)

[alexec]

OK. I was wrong. You cannot use supplied.

[alexec]

I don't think this is a bug, it is an enhancement.

If anyone is interesting submitting a PR, I give pointers .

[swafa]

@alexec Any tips in the meantime to circumvent the issue of distinguishing whether a previous suspend step's output has been supplied? I am under a bit of a time pressure :D

[swafa]

I also posted a question here: https://github.com/argoproj/argo-workflows/discussions/8248

[swafa]

I don't think this is a bug, it is an enhancement.

If anyone is interesting submitting a PR, I give pointers .

@alexec I am sorry but I super respectfully disagree. I see this as a bug because if the default value is supported for the output parameters, then it should work for all kinds of output parameters, not just some of them. It would be an enhancement if we can live without it or if there's any workaround to the topic of distinguishing whether the workflow has resumed automatically or not. Your call though of course. I trust your judgement and I might be mistaken.

[CodingOrca]

@alexec Any tips in the meantime to circumvent the issue of distinguishing whether a previous suspend step's output has been supplied? I am under a bit of a time pressure :D

Okay, I know I am not alexec... but I use as work around another suspend, with delay, but with no output, from where I trigger the resume of the my own workflow... so that my "approve" suspend gets resumed. If it is resumed like this, the configured default is taken:

. . .
  templates:
    - name: main
      dag:
        tasks:
          - name: waitForSeconds
            template: waitForSeconds
            arguments:
              parameters:
                - name: delay
                  value: 60

          - name: resumeWorkflow
            dependencies: [waitForSeconds]
            template: resumeWorkflow

        - name: approve
           template: approve

. . .

    - name: waitForSeconds
      inputs:
        parameters:
          - name: delay
      suspend:
        duration: "{{inputs.parameters.delay}}"

    - name: resumeWorkflow
      http:
        method: "PUT"
        url: "http://argo-server.argo-workflows:2746/api/v1/workflows/kondor-workflow/{{workflow.name}}/resume"

    - name: approve
        suspend: {}
        outputs:
          parameters:
            - name: message
              valueFrom:
                default: "some default value"
                supplied: {}

[swafa]

@CodingOrca Thanks for the hint. I tried your example and it did indeed take the defaults. I started wondering why is it that it works in that example (but not when I tried resuming through the API call from Postman, for example). I just managed to isolate the problem just now. For me, I noticed that the default value gets sets only when all of the following is true:

• We resume via API call. Here it does not matter from another node or from an outside call via Postman, for instance (When the node gets auto-resumed after the duration time is up, the default still gets ignored)
• Defaults are set for all output parameters of that node being resumed (it's possible to have more than one) and not just some of them
• There is no nodeFieldSelector in the resume request!!!!

Adding the nodeFieldSelector to the resume request breaks it and the default gets ignored. This one really surprised me.

@alexec This information might be useful if someone ever attempts to fix this. Just FYI.

[alexec]

I think I can point to where the bug is:

https://github.com/argoproj/argo-workflows/blob/78f01f2b9f24a89db15a119885dfe8eb6420c70d/workflow/util/util.go#L369

ResumeWorkflow call a different func if there is a nodeFieldSelector. If there is not one, it takes into account defaults:

https://github.com/argoproj/argo-workflows/blob/78f01f2b9f24a89db15a119885dfe8eb6420c70d/workflow/util/util.go#L397

But if there is one, they are not taken into account:

https://github.com/argoproj/argo-workflows/blob/78f01f2b9f24a89db15a119885dfe8eb6420c70d/workflow/util/util.go#L529

I think the block of code doing defaults should be copied and pasted into the second location.

Would anyone like to submit a PR for this? It's by far the fastest way to fix bugs. The core team won't look at this anytime soon.

[fhera]

Hello, i am exactly same issue. Any advance to fix it?

[kush-work]

@alexec I will try to submit a PR to fix this since I am right now blocked on this.