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

Expression results are printed incorrectly at random #6673

Closed
terrytangyuan opened this issue Sep 4, 2021 · 23 comments · Fixed by #6786
Closed

Expression results are printed incorrectly at random #6673

terrytangyuan opened this issue Sep 4, 2021 · 23 comments · Fixed by #6786
Assignees
Labels
Milestone

Comments

@terrytangyuan
Copy link
Member

terrytangyuan commented Sep 4, 2021

Summary

The container prints out the result correctly (see the printed "1" below).

 ___ 
< 1 >
 --- 
    \
     \
      \     
                    ##        .            
              ## ## ##       ==            
           ## ## ## ##      ===            
       /""""""""""""""""___/ ===        
  ~~~ {~~ ~~~~ ~~~ ~~~~ ~~ ~ /  ===- ~~~   
       \______ o          __/            
        \    \        __/             
          \____\______/  

However, it sometimes (1 out of 5 times) prints out the raw expression instead (see {{=jsonpath(workflow.parameters.config, \ \ '$.a')} below):


 _________________________________________ 
/ {{=jsonpath(workflow.parameters.config, \
\ '$.a')}}                                /
 ----------------------------------------- 
    \
     \
      \     
                    ##        .            
              ## ## ##       ==            
           ## ## ## ##      ===            
       /""""""""""""""""___/ ===        
  ~~~ {~~ ~~~~ ~~~ ~~~~ ~~ ~ /  ===- ~~~   
       \______ o          __/            
        \    \        __/             
          \____\______/  

Reproducible workflow YAML:

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: hello-world-template
spec:
  serviceAccountName: argo
  templates:
    - name: hello-world
      inputs:
        parameters:
          - name: a
            value: "{{=jsonpath(workflow.parameters.config, '$.a')}}"
          - name: b
            value: "{{=jsonpath(workflow.parameters.config, '$.b')}}"
          - name: c
            value: "{{=jsonpath(workflow.parameters.config, '$.c')}}"
      container:
        image: docker/whalesay
        command: [cowsay]
        args: ["{{inputs.parameters.a}}"]
---
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: hello-world-
spec:
  serviceAccountName: argo
  entrypoint: whalesay
  arguments:
    parameters:
      - name: config
        value: '{"a": "1", "b": "2", "c": "3"}'
  templates:
    - name: whalesay
      steps:
        - - name: hello-world
            templateRef:
              name: hello-world-template
              template: hello-world

Diagnostics

👀 Yes! We need all of your diagnostics, please make sure you add it all, otherwise we'll go around in circles asking you for it:

What Kubernetes provider are you using?

k3d

What version of Argo Workflows are you running?

v3.2.0-rc1

What executor are you running? Docker/K8SAPI/Kubelet/PNS/Emissary

PNS

Did this work in a previous version? I.e. is it a regression?

Haven't checked.


Message from the maintainers:

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

@terrytangyuan terrytangyuan changed the title Expression results are incorrect randomly Expression results are printed incorrectly at random Sep 4, 2021
@alexec alexec self-assigned this Sep 7, 2021
@alexec
Copy link
Contributor

alexec commented Sep 7, 2021

Well, this has me flummoxed. This is some more minimal YAML:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: hello-world
spec:
  entrypoint: main
  arguments:
    parameters:
      - name: config
        value: '{"a": "1"}'
  templates:
    - name: main
      container:
        image: docker/whalesay
        command:
          - cowsay
        args: [ "{{=jsonpath(workflow.parameters.config, '$.a')}}" ]

All code around marshalling parameters is nasty, just the investigation has taken me 2h and I've really gotten nowhere. I'm going to leave this unfixed.

@alexec alexec added the wontfix label Sep 7, 2021
@alexec alexec removed their assignment Sep 7, 2021
@stale stale bot removed the wontfix label Sep 7, 2021
@alexec
Copy link
Contributor

alexec commented Sep 7, 2021

Ah - one question - i could not reproduce this without using jsonpath - have you seen any example with it that fails?

@alexec alexec added the wontfix label Sep 7, 2021
@terrytangyuan
Copy link
Member Author

I only observed this issue with jsonpath so it might be something specific to jsonpath then.

@stale stale bot removed the wontfix label Sep 8, 2021
@crenshaw-dev
Copy link
Member

This works consistently (sample size ~10):

        args: [ "{{=jsonpath(workflow.parameters['config'], '$.a')}}" ]

I bet sometimes the value of workflow.parameters.config is being literally inserted before the expression is evaluated. So expr is seeing this:

jsonpath({"a": "1"}, '$.a')

Since that's not proper expr, it's failing. That's my theory anyway.

@alexec
Copy link
Contributor

alexec commented Sep 8, 2021

I did wonder about that. Quoting is nasty in expressions. It's basically broken and we have many random calls to quote/unquote.

@alexec
Copy link
Contributor

alexec commented Sep 8, 2021

This might be fixed by this:

#6442

@alexec
Copy link
Contributor

alexec commented Sep 8, 2021

@terrytangyuan would you like to try #6442?

@crenshaw-dev
Copy link
Member

Tested against that branch. Reproduced the issue 1/20 times. Soooo 🤷

@alexec
Copy link
Contributor

alexec commented Sep 8, 2021

I was thinking if it is only jsonpath, then we can get rid of it and use something like dataflow:

object(input.parameters.config).a

@terrytangyuan
Copy link
Member Author

terrytangyuan commented Sep 9, 2021

Here's another more interesting example (note that {{=workflow.parameters.key}} does not use jsonpath).

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: hello-world-template
spec:
  serviceAccountName: argo
  templates:
    - name: hello-world
      inputs:
        parameters:
          - name: memory
            value: "{{=workflow.parameters.key}} == 'dev' ? {{=jsonpath(workflow.parameters.config, '$.dev')}} : {{=jsonpath(workflow.parameters.config, '$.prod')}}"
      container:
        image: docker/whalesay
        command: [cowsay]
        args: ["{{inputs.parameters.memory}}"]
---
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: hello-world-
spec:
  serviceAccountName: argo
  entrypoint: whalesay
  arguments:
    parameters:
      - name: config
        value: '{"dev": "100Mi", "prod": "200Mi"}'
      - name: key
        value: dev
  templates:
    - name: whalesay
      steps:
        - - name: hello-world
            templateRef:
              name: hello-world-template
              template: hello-world

Results are different for every workflow:

 ______________________________ 
< dev == 'dev' ? 100Mi : 200Mi >
 ------------------------------ 
 _________________________________________ 
/ dev == 'dev' ?                          \
| {{=jsonpath(workflow.parameters.config, |
\ '$.dev')}} : 200Mi                      /
 ----------------------------------------- 
 ________________________________________ 
/ {{=workflow.parameters.key}} == 'dev' ? \
\ 100Mi : 200Mi                          /
 ---------------------------------------- 

So I doubt that it has something to do with jsonpath since even {{=workflow.parameters.key}} is not being evaluated sometimes. So the problem is that not all the expressions are evaluated.

@alexec
Copy link
Contributor

alexec commented Sep 9, 2021

Ok. So not just jsonpath. What about, is this only expression tag templates? Does it affect the old style ones?

@alexec
Copy link
Contributor

alexec commented Sep 9, 2021

Also, does it happen for examples without quotes?

@terrytangyuan
Copy link
Member Author

Ok. So not just jsonpath. What about, is this only expression tag templates? Does it affect the old style ones?

So far I only see it in expression tag templates.

Also, does it happen for examples without quotes?

For the example above, looks like if I replace the value by "{{=workflow.parameters.key}} == dev ? dev : prod}}", the raw expression is printed out (the conditional statement is not evaluated - not sure if this is related to this particular issue):

 _____________________________ 
< dev == dev ? dev : prod}} >
 -----------------------------

@alexec
Copy link
Contributor

alexec commented Sep 9, 2021

I'm not sure what you can do with {{..}} inside {{..}}

@alexec
Copy link
Contributor

alexec commented Sep 9, 2021

Does it also happen when the values do not also contain { or }?

@terrytangyuan
Copy link
Member Author

Does it also happen when the values do not also contain { or }?

I haven't observed related issues yet. The majority of the use cases involves using curly braces though.

@alexec
Copy link
Contributor

alexec commented Sep 14, 2021

OK. I think if we see a case where curly braces are not involved, we discus then.

@alexec alexec added solution/workaround There's a workaround, might not be great, but exists invalid labels Sep 21, 2021
@stale stale bot removed wontfix labels Sep 21, 2021
@alexec alexec added wontfix and removed solution/workaround There's a workaround, might not be great, but exists invalid labels Sep 21, 2021
@alexec alexec added this to the v3.1 milestone Sep 21, 2021
@alexec alexec self-assigned this Sep 22, 2021
@alexec
Copy link
Contributor

alexec commented Sep 22, 2021

More minimal repo:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: hello-world-
spec:
  entrypoint: main
  arguments:
    parameters:
      - name: a
        # quotes seem to be important
        value: "v"
  templates:
    - name: main
      container:
        image: docker/whalesay
        command:
          - cowsay
        args: [ '{{=workflow.parameters.a}}' ]

@alexec
Copy link
Contributor

alexec commented Sep 22, 2021

Could be related to #6285.

@alexec
Copy link
Contributor

alexec commented Sep 22, 2021

Could be fixed by #6442, if this is quoting issue.

@alexec
Copy link
Contributor

alexec commented Sep 22, 2021

I think I've found the issue:

doublerebel/bellows#8

@alexec
Copy link
Contributor

alexec commented Sep 22, 2021

@samene @ThSchrad @terrytangyuan I think I've tracked this issue down. I'll create a PR today and see if we can merge to master later on for testing.

alexec added a commit that referenced this issue Sep 22, 2021
Signed-off-by: Alex Collins <alex_collins@intuit.com>
alexec added a commit that referenced this issue Sep 22, 2021
Signed-off-by: Alex Collins <alex_collins@intuit.com>
@terrytangyuan
Copy link
Member Author

@alexec Nice! Thanks for tracking it down!

@sarabala1979 sarabala1979 mentioned this issue Sep 28, 2021
36 tasks
sarabala1979 pushed a commit that referenced this issue Sep 28, 2021
Signed-off-by: Alex Collins <alex_collins@intuit.com>
sarabala1979 pushed a commit that referenced this issue Sep 28, 2021
Signed-off-by: Alex Collins <alex_collins@intuit.com>
kriti-sc pushed a commit to kriti-sc/argo-workflows that referenced this issue Oct 24, 2021
…proj#6786)

Signed-off-by: Alex Collins <alex_collins@intuit.com>
Signed-off-by: kriti-sc <kathuriakriti1@gmail.com>
@sarabala1979 sarabala1979 mentioned this issue Nov 9, 2021
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants