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

templateDefaults has inconsistent behavior and does not allow command line parameter overwrite #6880

Closed
apiwoni opened this issue Oct 6, 2021 · 3 comments · Fixed by #6887
Labels

Comments

@apiwoni
Copy link

apiwoni commented Oct 6, 2021

Summary

What happened/what you expected to happen?
I have declared templateDefaults.inputs.parameters.value|valueFrom.configMapKeyRef in workflow template referenced by templates via workflow steps and the following issues are observed:

  1. templates.[name].inputs.parameters.[name] is not enough and requires value or default etc. value and default are not required in other cases
  2. templateDefaults.inputs.parameters.value does not overwrite templates.[name].inputs.parameters.[name].value|default
  3. but templateDefaults.inputs.parameters.valueFrom.configMapKeyRefdoes overwrite value or default. This is inconsistent
  4. parameter passed on command line does not overwrite any of templateDefaults.inputs.parameters.value|valueFrom.configMapKeyRef

What version of Argo Workflows are you running?
argo: v3.2.0-rc5

Diagnostics

Either a workflow that reproduces the bug, or paste you whole workflow YAML, including status, something like:

apiVersion: v1
kind: ConfigMap
metadata:
  name: message-parameters
  labels:
    workflows.argoproj.io/configmap-type: Parameter
data:
  message_1: "first message from config map"
  message_2: "second message from config map"
  last_message: "last message from config map"
-------------------------------------------------

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: wf-template-defaults
spec:
  entrypoint: main
  templateDefaults:
    inputs:
      parameters:
        - name: message1
          valueFrom:
            configMapKeyRef:
              name: message-parameters
              key: message_1
              optional: false
        - name: message2
          # value unlike valueFrom is ignored
          value: "value from templateDefaults"
        - name: lastmessage
          valueFrom:
            configMapKeyRef:
              name: message-parameters
              key: last_message
              optional: true
  templates:
    - name: main
      steps:
        - - name: first-step
            template: first-step
        - - name: second-step
            template: second-step
        - - name: last-step
            template: last-step
    - name: first-step
      inputs:
        parameters:
          - name: message1
            # Why default is required here but not in other cases
            default: ""
      container:
        image: docker/whalesay:latest
        command: [cowsay]
        args: ["{{inputs.parameters.message1}}"]

    - name: second-step
      inputs:
        parameters:
          - name: message2
            default: ""
      container:
        image: docker/whalesay:latest
        command: [cowsay]
        args: ["{{inputs.parameters.message2}}"]

    - name: last-step
      inputs:
        parameters:
          - name: lastmessage
            default: ""
      container:
        image: docker/whalesay:latest
        command: [cowsay]
        args: ["{{inputs.parameters.lastmessage}}"]
argo submit --from workflowtemplate/wf-template-defaults -p message1=firstMessageFromCommandLine

Expected Output:

"firstMessageFromCommandLine"
"value from templateDefaults"
"last message from config map"

Actual Output:

"first message from config map"
""
"last message from config map"

I'm at a loss here. I'm working on translator from Oozie to Argo workflows but I cannot see consistent pattern for paremeters propagation and order of preference!?


Message from the maintainers:

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

@apiwoni
Copy link
Author

apiwoni commented Oct 7, 2021

@sarabala1979 #6887 Does this fix resolve 1, 2, 3 and 4?

@sarabala1979
Copy link
Member

@apiwoni #1 by design all input.parameter should have value or default. but you use workflow argument if you don;t want to specify.
#3 and #4 have precedence orders. I think #3 is as expected. But if we want to address #4, then #3 will not work.

I think Value should take precedence with others. This will solve the #3 and #4

@apiwoni
Copy link
Author

apiwoni commented Oct 8, 2021

@sarabala Thank you for shedding some light on this.

#1 by design all input.parameter should have value or default. but you use workflow argument if you don;t want to specify.

Indeed when I specified workflow argument I could have declared templates[].input.parameters without default or value. However, I expected templateDefaults.inputs.parameters with any type of value to fulfill this requirement.
Use case is when I only want to provide local input parameters to all template workflows only via templateDefaults. To ensure value is provided could be done either by validating value is provided in templateDefaults and checking optional is set totrue for ConfigMap as opposed to requiring each template to provide fake "" default value. I think it is much cleaner.

#3 is no longer an issue now that you fixed #2 since both behave consistently.

#4 Is the biggest issue because command line parameters should always take precedence and that is not the case only when using templateDefaults, so it is also inconsistent with other cases where command line parameters can overwrite regular arguments and inputs.

I would say that templateDefaults are really great idea because I don't want to copy and paste multiple parameters required by multiple, possibly nested, templates. However, as with everything these are only defaults that I should be able to overwrite from command line, especially if my use case calls for submitting workflows only from workflow templates with some arguments that need to be supplied by caller.

While I think #4 should be very high on priority to be fixed, I'm starting to wonder if I should define workflow template with argument.parameters.valueFrom.configMapKeyRef instead of templateDefaults and reference then as globals {{workflow.parameters}} in all templates within workflow template? This does not require, I think, to declare inputs for each template within a workflow. Regardless of approach I still think command line parameters should overwrite anything, including templateDefaults

Again, thanks for fixing #2. I just tested it and it works.

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.

2 participants