Adding pipeline results #2042

othomann · 2020-02-13T02:56:42Z

Changes

Adding the ability to reuse task results into other tasks within the same pipeline

Fixes #1978

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

Reviewer Notes

Release Notes

Add pipeline results. This makes it possible to a user to use results from another task as parameters to a task within the same pipeline.

tekton-robot · 2020-02-13T02:59:01Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

vdemeester · 2020-02-13T07:39:25Z

/area api
/kind api-change
/assign

tekton-robot · 2020-02-13T14:38:29Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

tekton-robot · 2020-02-13T14:40:17Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

tekton-robot · 2020-02-13T14:41:46Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

othomann · 2020-02-13T17:09:06Z

/retest

tekton-robot · 2020-02-13T17:39:35Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

tekton-robot · 2020-02-13T17:40:10Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

ghost · 2020-02-13T19:00:59Z

Something that occurs to me reading through this: It looks like we allow the user to only use a single isolated variable substitution in a given parameter. In other words they can't do something like:

params:
- name: ephemeral-env-name
  value: "ci-$(tasks.git-push.results.short-commit-sha)-ephemeral"

Where the expected value here would end up looking something like "ci-991f807-ephemeral" and in this example would be the name of an ephemeral namespace or something that is deployed to when a PR is being tested.

I need to check what variable substitution looks like with other fields. I have a hunch that this kind of thing is supported with others and we'd probably want the same kind of support here as well. Will look into it and update this PR with a comment.

ghost

Looks good so far! I've left some comments from an initial read-through and I want to get your feedback before I comment on the code any more.

pkg/apis/pipeline/v1alpha2/param_types.go

pkg/reconciler/pipelinerun/pipelinerun.go

eddie4941 · 2020-02-13T19:46:53Z

params:

name: ephemeral-env-name
value: "ci-$(tasks.git-push.results.short-commit-sha)-ephemeral"
Where the expected value here would end up looking something like "ci-991f807-ephemeral" and > in this example would be the name of an ephemeral namespace or something that is deployed to > when a PR is being tested.

Oh yeah. Didn't think about that. I don't think it would be too hard to implement that behavior. though theres something to think about that is the desired behavior.

Since results are taken verbatim form the contents of the result file that the task lays on disk, we might run into a bunch of issues with newlines when performing the substitutions in embedded cases like the one you mentioned.

In that example, if the task laid down the SHA like so (newline at the end):

f3af3eddd5047d2e7eb795affac54cb6952d22b9\n

"ci-$(tasks.git-push.results.short-commit-sha)-ephemeral"

would turn into

ci-f3af3eddd5047d2e7eb795affac54cb6952d22b9\n-ephemeral

We could strip newline to prevent this, but that might be a bad idea as we likely don't want to alter the users output.

skaegi · 2020-02-13T23:08:12Z

@sbwsg I'm trying to rework all the variable substitution right now to use the jsonpath.Expand func from PR #1951 everywhere so that kind of logic isn't reimplemented a zillion times. I just hadn't realized it was such a huge job. @eddie4941 we're so close to the beta that if it's easy to implement in the current way it would be good even if we re-implement a little bit later.
fwiw -- I believe Expand would output...

ci-f3af3eddd5047d2e7eb795affac54cb6952d22b9\n-ephemeral

...as the value of the JSON string which I believe is what we want. Meaning the new line lives on when it's extracted to a regular string. Going the other way that means you don't have to encode the new-line as it is a legitimate String character (DM me if that's confusing as hell)

ghost · 2020-02-14T14:46:00Z

I need to check what variable substitution looks like with other fields. I have a hunch that this kind of thing is supported with others

OK, I've run a test this morning to see what happens if you wrap a pipeline's param into a larger string being passed into a Task's param and it looks like it operates as mentioned. The pipeline's param is interpolated into the rest of the string contents. [1]

we might run into a bunch of issues with newlines when performing the substitutions in embedded cases like the one you mentioned.

Great point. I had not thought about newlines being included here. My feeling is we should clearly document the behaviour initially and start considering changes such as trimming the whitespace only if users really struggle with it. My guess right now is that so long as we are passing the result values around verbatim that users will quickly learn to remove extra whitespace from the result values they emit from their tasks. This also sounds like a good thing to watch out for when we start updating tasks in the Catalog to emit their results.

[1] Here's the quick example pipeline I wrote up to check the params behaviour:

apiVersion: tekton.dev/v1alpha1
kind: Task
metadata:
  name: print-message
spec:
  inputs:
    params:
    - name: message
      type: string
      description: "The message to print"
      default: "hello world"
  steps:
  - name: print
    image: ubuntu
    script: |
      #!/usr/bin/env bash
      echo "$(inputs.params.message)"
---
apiVersion: tekton.dev/v1alpha1
kind: PipelineRun
metadata:
  name: add-octopus-arms
spec:
  params:
  - name: message
    value: "o.O"
  pipelineSpec:
    params:
    - name: message
      type: string
      description: "The figure to wrap in octopus arms"
    tasks:
    - name: add-octopus-arms
      taskRef:
        name: print-message
      params:
      - name: message
        value: "~~$(params.message)~~"

ghost · 2020-02-14T20:29:31Z

This also sounds like a good thing to watch out for when we start updating tasks in the Catalog to emit their results.

Example from a catalog entry of trimming newline when storing to a result here: https://github.com/tektoncd/catalog/pull/182/files#diff-1239eb0a6238e01666691e444b6367b7R55

It would be cool if this were handled for the user but it's not a huuuuge headache as it stands. Just echo -n $FOO > $(results.foo.path)

tekton-robot · 2020-02-20T18:50:57Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.1%	-3.6
test/builder/task.go	81.1%	80.4%	-0.7

tekton-robot · 2020-02-20T21:48:17Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha2/param_types.go	94.7%	91.3%	-3.4
test/builder/task.go	81.1%	80.4%	-0.7

othomann · 2020-03-02T15:20:18Z

I am doing a rebase on top of the v1beta1 change. This will require a lgtmtag again once I am done.

tekton-robot · 2020-03-02T15:53:13Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

tekton-robot · 2020-03-02T17:11:12Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

othomann · 2020-03-02T17:25:12Z

/retest

othomann · 2020-03-02T17:42:43Z

/test tekton-pipeline-unit-tests

othomann · 2020-03-02T18:45:16Z

unit tests are passing locally, but they fail when run as automated tests. Investigating.

tekton-robot · 2020-03-02T19:04:37Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

tekton-robot · 2020-03-02T19:21:20Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

tekton-robot · 2020-03-02T19:22:27Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

tekton-robot · 2020-03-02T19:27:47Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

othomann · 2020-03-02T19:39:01Z

Since the move to v1beta1, the test github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun.TestReconcileWithTaskResults no longer works when run as part of the automated tests, but works perfectly fine locally. If someone has any clue how to debug that, please let me know.

tekton-robot · 2020-03-02T19:47:38Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

tekton-robot · 2020-03-02T19:49:49Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

tekton-robot · 2020-03-02T20:20:16Z

The following is the coverage report on pkg/.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/pipeline_types.go	82.4%	76.2%	-6.2
pkg/apis/pipeline/v1alpha1/pipeline_validation.go	95.9%	96.2%	0.2
pkg/apis/pipeline/v1beta1/param_types.go	94.7%	34.0%	-60.8
test/builder/task.go	70.2%	69.7%	-0.5

othomann · 2020-03-02T20:22:00Z

ok problem came from the validation of names. Fix pushed.

ghost · 2020-03-02T20:30:11Z

re-adding the earlier lgtm

/lgtm

🤞

ghost · 2020-03-02T20:50:17Z

🎉

tekton-robot requested review from abayer and vdemeester February 13, 2020 02:56

googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Feb 13, 2020

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 13, 2020

tekton-robot assigned vdemeester Feb 13, 2020

tekton-robot added area/api Indicates an issue or PR that deals with the API. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Feb 13, 2020

othomann force-pushed the pipeline_results branch from 39b676b to 8d96f29 Compare February 13, 2020 14:38

othomann force-pushed the pipeline_results branch from 2947409 to 40054a7 Compare February 13, 2020 17:37

othomann requested a review from a user February 13, 2020 18:15

ghost reviewed Feb 13, 2020

View reviewed changes

pkg/apis/pipeline/v1alpha2/param_types.go Outdated Show resolved Hide resolved

pkg/reconciler/pipelinerun/pipelinerun.go Outdated Show resolved Hide resolved

ghost mentioned this pull request Feb 14, 2020

Update the git fetch task to include all params used in the Git PipelineResource tektoncd/catalog#182

Merged

3 tasks

othomann force-pushed the pipeline_results branch from 40054a7 to 88c637c Compare February 20, 2020 18:48

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 20, 2020

ghost removed this from the Pipelines 0.11 🐱 milestone Mar 2, 2020

ghost added this to the Pipelines 0.11 🐱 milestone Mar 2, 2020

othomann force-pushed the pipeline_results branch from 20244f9 to 2a86059 Compare March 2, 2020 19:17

othomann force-pushed the pipeline_results branch from ca3bb5c to 9d410d4 Compare March 2, 2020 19:24

othomann force-pushed the pipeline_results branch from 57881d6 to 3e8c9b7 Compare March 2, 2020 19:45

Adding pipeline results

38ec513

othomann force-pushed the pipeline_results branch from f45a541 to 38ec513 Compare March 2, 2020 20:17

tekton-robot assigned ghost Mar 2, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 2, 2020

tekton-robot merged commit c317d64 into tektoncd:master Mar 2, 2020

danielhelfand mentioned this pull request Mar 2, 2020

Adding Documentation on PipelineTask Timeout #2130

Merged

3 tasks

vdemeester mentioned this pull request Mar 3, 2020

Add support for result in Pipeline/Task (and Runs) tektoncd/cli#766

Closed

bobcatfish mentioned this pull request Mar 16, 2020

Support referencing task results from task params in a pipeline #1978

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding pipeline results #2042

Adding pipeline results #2042

othomann commented Feb 13, 2020 •

edited by bobcatfish

Loading

tekton-robot commented Feb 13, 2020

vdemeester commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

othomann commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

ghost commented Feb 13, 2020

ghost left a comment

eddie4941 commented Feb 13, 2020

skaegi commented Feb 13, 2020 •

edited

Loading

ghost commented Feb 14, 2020

ghost commented Feb 14, 2020

tekton-robot commented Feb 20, 2020

tekton-robot commented Feb 20, 2020

othomann commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

othomann commented Mar 2, 2020

othomann commented Mar 2, 2020

othomann commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

othomann commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

othomann commented Mar 2, 2020

ghost commented Mar 2, 2020

ghost commented Mar 2, 2020

Adding pipeline results #2042

Adding pipeline results #2042

Conversation

othomann commented Feb 13, 2020 • edited by bobcatfish Loading

Changes

Submitter Checklist

Reviewer Notes

Release Notes

tekton-robot commented Feb 13, 2020

vdemeester commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

othomann commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

tekton-robot commented Feb 13, 2020

ghost commented Feb 13, 2020

ghost left a comment

Choose a reason for hiding this comment

eddie4941 commented Feb 13, 2020

skaegi commented Feb 13, 2020 • edited Loading

ghost commented Feb 14, 2020

ghost commented Feb 14, 2020

tekton-robot commented Feb 20, 2020

tekton-robot commented Feb 20, 2020

othomann commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

othomann commented Mar 2, 2020

othomann commented Mar 2, 2020

othomann commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

othomann commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

tekton-robot commented Mar 2, 2020

othomann commented Mar 2, 2020

ghost commented Mar 2, 2020

ghost commented Mar 2, 2020

othomann commented Feb 13, 2020 •

edited by bobcatfish

Loading

skaegi commented Feb 13, 2020 •

edited

Loading