Argo: Make output parameters easily accessible from code

[0xdevalias]

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

What you expected to happen:

I've been playing around with running argo workflows from my own golang code, and have been pondering the best way to 'extract data' from the workflow.

This lead to me looking at a few options, including:

• the 'artifact' features (likely too heavyweight/overengineered for my needs)
• parsing logs (hacky)
• using a shared volume between my code and the workflow (potential)
• output parameters

The output parameters seemed of particular interest, but when I raised the question on Slack, @jessesuen implied that it might be annoying to parse them from the 'result json' in its current form:

output parameters might work for you — the value will be stored in the workflow resource object (JSON) for a consumer to parse. The problem is that the outputs would be nested into some arbitrary node’s outputs which is not obvious to parse and extract the value. There is a case to be made for having workflow outputs as a first class citizen. Then the workflow’s outputs could be extracted via the json field: myworkflow.outputs.parameters.myparam Can you file a feature request and we can take the discussion there?

As mentioned above, if these outputs were treated as 'first class citizens', then we could access them in a format similar to myworkflow.outputs.parameters.myparam, which would be nice.

Opening here for discussion.

Environment:

• Argo version:

$ argo version
  argo: v2.0.0-beta1
  BuildDate: 2018-01-18T22:04:00Z
  GitCommit: 549870c1ee08138b20b8a4b0c026569cf1e6c19a
  GitTreeState: clean
  GitTag: v2.0.0-beta1
  GoVersion: go1.9.1
  Compiler: gc
  Platform: darwin/amd64

• Kubernetes version :

$ kubectl version -o yaml
clientVersion:
  buildDate: 2018-01-18T21:11:08Z
  compiler: gc
  gitCommit: 5fa2db2bd46ac79e5e00a4e6ed24191080aa463b
  gitTreeState: clean
  gitVersion: v1.9.2
  goVersion: go1.9.2
  major: "1"
  minor: "9"
  platform: darwin/amd64
serverVersion:
  buildDate: 2018-01-26T19:04:38Z
  compiler: gc
  gitCommit: 925c127ec6b946659ad0fd596fa959be43f0cc05
  gitTreeState: clean
  gitVersion: v1.9.0
  goVersion: go1.9.1
  major: ""
  minor: ""
  platform: linux/amd64

[0xdevalias]

Hacked together some PoC code for my usecase (overwrites clashing keys in the final map, but otherwise should do the trick):

// Extract the output parameters from the workflow (clashing keys will be overwritten)
func GetOutputParams(wf *wfv1.Workflow) (map[string]wfv1.Parameter, error) {
    if wf.Status.FinishedAt.IsZero() {
        return nil, errors.New("Workflow not finished")
    } else if wf.Status.Phase != wfv1.NodeSucceeded {
        return nil, errors.New("Workflow not Succeeded")
    }

    outputParams := make(map[string]wfv1.Parameter)

    for _, node := range wf.Status.Nodes {
        for _, p := range node.Outputs.Parameters {
            // TODO: This will overwrite 'clashing' keys, find a better solution
            outputParams[p.Name] = p
        }
    }

    return outputParams, nil
}

// Extract existing output parameter values from the workflow (clashing keys will be overwritten)
func GetOutputParamValues(wf *wfv1.Workflow) (map[string]string, error) {
    params, err := GetOutputParams(wf)
    if err != nil {
        return nil, err
    }

    outputParams := make(map[string]string)

    for _, p := range params {
        // TODO: This will overwrite 'clashing' keys, find a better solution
        if p.Value != nil {
            // TODO: Should we notify somehow if this was nil?
            outputParams[p.Name] = *p.Value
        }
    }

    return outputParams, nil
}

[jessesuen]

Any suggestions on syntax? I'm thinking something like:

  - name: whalesay
    container:
      image: docker/whalesay:latest
      command: [sh, -c]
      args: ["echo -n hello world > /tmp/hello_world.txt"]
    outputs:
      parameters:
      - name: hello-param
        valueFrom:
          path: /tmp/hello_world.txt
        globalOutputName: my-wf-output

Which would result in:

status:
  outputs:
    parameters:
    - name: my-wf-output
      value: hello world

[0xdevalias]

No strong feels on syntax, but wondering if there is a need to have a difference between name: hello-param and globalOutputName: my-wf-output

Would make more logical sense to me for them to be the same name.

If globalOutputName is there to mark things that will be output globally (with the default being not to), then maybe just have global: true or similar?

[jessesuen]

I started my thinking with global: true, but felt it might be limiting. I'm not sure if there is a real use-case of having the global output name be different than just the output name, or if I'm just overthinking this. I do like the simplicity of just having global: true. Let me get some opinions from folks here, as well as the slack channel.

[jessesuen]

OK, I now remember why I went this route, and why global: true would be too limiting. I feel it is important to support something like this

    outputs:
      parameters:
      - name: hello-param
        valueFrom:
          path: /tmp/hello_world.txt
        globalOutputName: "{{inputs.parameters.someval}}"

This keeps the template composable, but enables it to produce different global outputs with different names. With global: true, it would not be possible.

[0xdevalias]

Would that not also be possible with the following?

    outputs:
      parameters:
      - name: "{{inputs.parameters.someval}}"
        valueFrom:
          path: /tmp/hello_world.txt
        global: "true"

Or by composable, do you mean internally other things could refer to it byhello-param (in your example), and externally I can define whatever I want? If so, I can see the logic.. but wonder if there is much benefit over adding a few more template params around the place.

Ultimately I don't think it makes a huge distinction either way, so happy to go with consensus.

[jessesuen]

Right, that is what I meant. We also currently don't support variables in the name field, and I fear that supporting it may complicate the implementation to support the name: "{{inputs.parameters.someval}}".

BTW, globalOutputName is a bit verbose, so I think globalName is a bit better.

[0xdevalias]

Agree RE: globalName. The 'output' part is already implied by the context.

[jessesuen]

Commit a45bf1b7558b3eb60ec65d02c166c306e7797a79 adds support for global outputs. examples/global-outputs.yaml is provided as an example.

Exported parameters and artifacts will also show up in the CLI:

$ argo get global-outputs-jrpsd
Name:                global-outputs-jrpsd
Namespace:           default
ServiceAccount:      default
Status:              Succeeded
Created:             Wed Mar 07 00:50:46 -0800 (14 seconds ago)
Started:             Wed Mar 07 00:50:46 -0800 (14 seconds ago)
Finished:            Wed Mar 07 00:50:54 -0800 (6 seconds ago)
Duration:            8 seconds
Output Parameters:
  my-global-param:   hello world
Output Artifacts:
  my-global-art:     http://argo-artifacts-minio-svc.default:9000/my-bucket/global-outputs-jrpsd/global-outputs-jrpsd-1571663520/hello-art.tgz

STEP                            PODNAME                          DURATION  MESSAGE
 ✔ global-outputs-jrpsd
 └---✔ generate                 global-outputs-jrpsd-1571663520  3s

 ✔ global-outputs-jrpsd.onExit  global-outputs-jrpsd-2910728254  3s

[0xdevalias]

Awesome! Thanks!