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

Nested steps run with incorrect parameters #1713

Closed
markterm opened this issue Oct 24, 2019 · 12 comments
Closed

Nested steps run with incorrect parameters #1713

markterm opened this issue Oct 24, 2019 · 12 comments
Labels

Comments

@markterm
Copy link
Contributor

markterm commented Oct 24, 2019

Is this a BUG REPORT or FEATURE REQUEST?:
Bug report

What happened:
If you execute the parallelism-nested.yaml example then the pods with a seq-id of b, c or d all have a parallel-id of 1

What you expected to happen:
pods with a seq-id of b, c or d all to have all parallel ids (1, 2, 3 and 4).

How to reproduce it (as minimally and precisely as possible):
argo submit parallelism-nested.yaml
then:
argo get <workflow-id> -o yaml

There will be multiple pods in the Status section with seq-id of b and parallel id of 1, there should only be one.

Anything else we need to know?:
This looks to be related to #1552

storedTemplates contains the following, with seq-id hardcoded:

    /seq-worker:
      arguments: {}
      inputs: {}
      metadata: {}
      name: seq-worker
      outputs: {}
      parallelism: 1
      steps:
      - - arguments:
            parameters:
            - name: parallel-id
              value: "1"
            - name: seq-id
              value: '{{item}}'
          name: seq-step
          template: one-job
          withParam: |
            ["a","b","c","d"]

Environment:

  • Argo version: master
$ argo version
  • Kubernetes version : 1.12
@markterm
Copy link
Contributor Author

@dtaniwaki fyi, I think this relates to your PR

@NirvanaZA
Copy link

Hi sir, When this bug fix will release? Hope soon. Many thanks.

tghartland added a commit to tghartland/masters2-binder that referenced this issue Nov 5, 2019
Latest version has a bug with nested loops which causes input parameters
of jobs to be wrong, and this causes the workflow to fail.

argoproj/argo-workflows#1713
@sarabala1979
Copy link
Member

@dtaniwaki is refactoring the store template feature. This will close most of the bugs.

@NirvanaZA
Copy link

I tried to downgrade the version to 2.4.0, from 2.4.2, and seems the retry feature has some bug as well.

@simster7
Copy link
Member

simster7 commented Nov 5, 2019

@NirvanaZA could you elaborate on the bug with the retry feature?

@samath117
Copy link

samath117 commented Nov 6, 2019

Another minimal example to illustrate what appears to be the same problem:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: steps-
spec:
  entrypoint: steps
  templates:
  - name: print
    inputs:
      parameters:
      - name: message
    container:
      image: alpine:latest
      command: [sh, -c]
      args: ["echo {{inputs.parameters.message}}"]
  - name: print-twice
    inputs:
      parameters:
      - name: message
    steps:
    - - name: print1
        template: print
        arguments:
          parameters:
          - name: message
            value: "{{inputs.parameters.message}}"
    - - name: print2
        template: print
        arguments:
          parameters:
          - name: message
            value: "{{inputs.parameters.message}}"
  - name: steps
    steps:
    - - name: hello
        template: print-twice
        arguments:
          parameters:
          - name: message
            value: hello
    - - name: goodbye
        template: print-twice
        arguments:
          parameters:
          - name: message
            value: goodbye

On Argo 2.4.1, this has the expected behavior. Workflow logs:

print1: hello
print2: hello
print1: goodbye
print2: goodbye

On Argo 2.4.2, the message for the last one is passed incorrectly:

print1: hello
print2: hello
print1: goodbye
print2: hello

@cchanley2003
Copy link

cchanley2003 commented Nov 6, 2019

Just FYI we are also seeing issues with when statements getting the wrong parameters with a nested setup. It works with 2.4.0-rc1 but is broken after that, so pretty certain it is related to that same change that is affected this ticket.

@simonwa7
Copy link

FYI after creating and deploying an argo controller image with #1744 it did fix the issues we were seeing with parameter passing, but now the podSpecPatch does not get applied or even stored with a workflow.

I.E. after submitting a workflow with a task that has a podSpecPatch it is not found anywhere within the workflow obtained by argo get <workflow-ID> -o yaml

@simster7
Copy link
Member

@simonwa7 Could you take a look at @dtaniwaki's comments regarding your issue? #1744 (comment)

epa095 pushed a commit to epa095/gordo-components that referenced this issue Nov 14, 2019
Simplifies the workflow by removing nested templates. I.e. instead of having
a template consisting of building-then-post-service, we add the building
and service-posting as top-level tasks in do-all, and add the builder as a
dependency of the service.

Needed beause of argoproj/argo-workflows#1713
epa095 pushed a commit to epa095/gordo-components that referenced this issue Nov 14, 2019
Simplifies the workflow by removing nested templates. I.e. instead of having
a template consisting of building-then-post-service, we add the building
and service-posting as top-level tasks in do-all, and add the builder as a
dependency of the service.

Needed beause of argoproj/argo-workflows#1713
epa095 pushed a commit to epa095/gordo-components that referenced this issue Nov 14, 2019
Simplifies the workflow by removing nested templates. I.e. instead of having
a template consisting of building-then-post-service, we add the building
and service-posting as top-level tasks in do-all, and add the builder as a
dependency of the service.

Needed beause of argoproj/argo-workflows#1713
epa095 pushed a commit to epa095/gordo-components that referenced this issue Nov 14, 2019
Simplifies the workflow by removing nested templates. I.e. instead of having
a template consisting of building-then-post-service, we add the building
and service-posting as top-level tasks in do-all, and add the builder as a
dependency of the service.

Needed beause of argoproj/argo-workflows#1713
epa095 pushed a commit to equinor/gordo that referenced this issue Nov 14, 2019
Simplifies the workflow by removing nested templates. I.e. instead of having
a template consisting of building-then-post-service, we add the building
and service-posting as top-level tasks in do-all, and add the builder as a
dependency of the service.

Needed beause of argoproj/argo-workflows#1713
@simster7
Copy link
Member

For those blocked by this regression: I would consider building #1744 into an image until the PR goes through review and a release is made hopefully sometime next week.

@simster7 simster7 removed their assignment Nov 26, 2019
@NirvanaZA
Copy link

For those blocked by this regression: I would consider building #1744 into an image until the PR goes through review and a release is made hopefully sometime next week.

👍 Waiting for the new release.

@simster7
Copy link
Member

simster7 commented Dec 6, 2019

Hello everyone v2.4.3 was just released with a fix for this issue, please take a look! Thank you for your patience and thank you to @dtaniwaki for working on this fix!

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.

7 participants