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

Reference to task A's output param not resolved if task B does not depend on task A #2314

Closed
3 of 4 tasks
thundergolfer opened this issue Feb 26, 2020 · 9 comments · Fixed by #4992
Closed
3 of 4 tasks
Labels

Comments

@thundergolfer
Copy link

thundergolfer commented Feb 26, 2020

Checklist:

  • I've included the version.
  • I've included reproduction steps.
  • I've included the workflow YAML.
  • I've included the logs.

What happened:

A "{{tasks.X.outputs.parameters....}}" reference was left as a literal during workflow execution because the task including that reference did not list X as a dependency.

What you expected to happen:

This I think violates the 'Principle of Least Surprise'. I think this should be a lint error and a runtime execution error, as the task is attempting to reference an output parameter that may not yet exist.

How to reproduce it (as minimally and precisely as possible):

Run this with the #dependencies ... line left commented out and inspect the logs of the workflow.

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: output-parameter-
spec:
  entrypoint: abc
  serviceAccountName: argo
  templates:
  - name: abc
    dag:
      tasks:
        - name: generate-parameter
          template: whalesay
        - name: consume-parameter
          template: print-message
          #dependencies: ["generate-parameter"]
          arguments:
            parameters:
            - name: message
              value: "{{tasks.generate-parameter.outputs.parameters.hello-param}}"
  - name: whalesay
    container:
      image: docker/whalesay:latest
      command: [sh, -c]
      args: ["sleep 1; echo -n hello world > /tmp/hello_world.txt"]
    outputs:
      parameters:
      - name: hello-param
        valueFrom:
          path: /tmp/hello_world.txt

  - name: print-message
    inputs:
      parameters:
      - name: message
    container:
      image: docker/whalesay:latest
      command: [cowsay]
      args: ["{{inputs.parameters.message}}"]

You see:

consume-parameter:	 _________________________________________
consume-parameter:	/ {{tasks.generate-parameter.outputs.para \
consume-parameter:	\ meters.hello-param}}                    /
consume-parameter:	 -----------------------------------------
consume-parameter:	    \
consume-parameter:	     \
consume-parameter:	      \
consume-parameter:	                    ##        .
consume-parameter:	              ## ## ##       ==
consume-parameter:	           ## ## ## ##      ===
consume-parameter:	       /""""""""""""""""___/ ===
consume-parameter:	  ~~~ {~~ ~~~~ ~~~ ~~~~ ~~ ~ /  ===- ~~~
consume-parameter:	       \______ o          __/
consume-parameter:	        \    \        __/
consume-parameter:	          \____\______/

Anything else we need to know?:

Environment:

  • Argo version: We use Nix. Our Nix'd argo version is 2.4.3
  • Kubernetes version :
clientVersion:
  buildDate: "1970-01-01T00:00:01Z"
  compiler: gc
  gitCommit: e7f962ba86f4ce7033828210ca3556393c377bcc
  gitTreeState: archive
  gitVersion: v1.16.6-beta.0
  goVersion: go1.13.6
  major: "1"
  minor: 16+
  platform: darwin/amd64
serverVersion:
  buildDate: "2019-12-22T23:14:11Z"
  compiler: gc
  gitCommit: c0eccca51d7500bb03b2f163dd8d534ffeb2f7a2
  gitTreeState: clean
  gitVersion: v1.14.9-eks-c0eccc
  goVersion: go1.12.12
  major: "1"
  minor: 14+
  platform: linux/amd64

Message from the maintainers:

If you are impacted by this bug please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

@thundergolfer thundergolfer changed the title Task A's output param reference not resolved if task B does not depend on task A Reference to task A's output param not resolved if task B does not depend on task A Feb 26, 2020
@thundergolfer
Copy link
Author

Note that this does not appear to be a regression as doing argo lint on v2.2.1 passes.

@simster7 simster7 self-assigned this Feb 26, 2020
@simster7
Copy link
Member

Coincidentally, I'm looking at the relevant code for another PR. Will take a look at this

@simster7 simster7 added this to the v2.7 milestone Feb 26, 2020
@simster7 simster7 removed their assignment Mar 2, 2020
@alexec alexec removed this from the v2.7 milestone Mar 16, 2020
@stale
Copy link

stale bot commented Aug 1, 2020

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.

@stale stale bot added the wontfix label Aug 1, 2020
@thundergolfer
Copy link
Author

Don't close, I think this bug ought to be addressed if it isn't yet fixed. 🙏

@stale stale bot removed the wontfix label Aug 2, 2020
@stale
Copy link

stale bot commented Oct 1, 2020

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.

@stale stale bot added the wontfix label Oct 1, 2020
@changhc
Copy link
Contributor

changhc commented Oct 1, 2020

Don't close, I've been working on this.

@stale stale bot removed the wontfix label Oct 1, 2020
@changhc
Copy link
Contributor

changhc commented Nov 23, 2020

Hi, sorry I was a bit occupied after my last comment. I've confirmed that this issue is still reproducible with the latest commit.

Initially I tried to fix this, but then I started to wonder if we should really fix this. What if a user really wants to print {{tasks.generate-parameter.outputs.parameters.hello-param}}? Are we going to allow users to escape double brackets?

@thundergolfer
Copy link
Author

What if a user really wants to print

I'd guess that 99/100 times they wouldn't want to print that, and it'd be a mistake in their task dependency specification.

@stale
Copy link

stale bot commented Jan 23, 2021

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.

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