Tasks that claim to emit results but don't should fail

[bobcatfish]

Expected Behavior

If I create a Task which claims to emit a result, but it doesn't, the TaskRun should fail, i.e. I should be able to rely on the interface a Task claims to provide.

Actual Behavior

This will only be considered a failure if something in the Pipeline tries to use the result; if nothing tries to use the result, there will be no error, but if something does try to use the result you will get an error like:

tkn pr list
NAME                     STARTED          DURATION    STATUS
sum-three-pipeline-run   10 minutes ago   5 seconds   Failed(InvalidTaskResultReference)

Steps to Reproduce the Problem

I took this example pipeline with results and made a few changes:

• add-task no longer emits a result, tho it claims to
• the pipeline no longer refers to results from add-task

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: add-task
spec:
  params:
    - name: first
      description: the first operand
    - name: second
      description: the second operand
  results:
    - name: sum
      description: the sum of the first and second operand
  steps:
    - name: add
      image: alpine
      env:
        - name: OP1
          value: $(params.first)
        - name: OP2
          value: $(params.second)
      command: ["/bin/sh", "-c"]
      args:
        - echo -n $((${OP1}+${OP2}))
---
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: sum-three-pipeline
spec:
  params:
    - name: first
      description: the first operand
    - name: second
      description: the second operand
    - name: third
      description: the third operand
  tasks:
    - name: first-add
      taskRef:
        name: add-task
      params:
        - name: first
          value: $(params.first)
        - name: second
          value: $(params.second)
    - name: second-add
      taskRef:
        name: add-task
      params:
        - name: first
          value: "5"
        - name: second
          value: $(params.third)
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: sum-three-pipeline-run
spec:
  pipelineRef:
    name: sum-three-pipeline
  params:
    - name: first
      value: "2"
    - name: second
      value: "10"
    - name: third
      value: "10"

If you apply this, you will see that it runs successfully. If you add any references to the results, the run will fail.

Additional Info

• Kubernetes version: v1.16.13-gke.401

• Tekton Pipeline version: HEAD @ 7b5b2fa

FYI in #3472 we're removing the behaviour where a PipelineRun is put into a failed state due to a Task dropping a result. Feedback welcome!

Edit for posterity: I was wrong - a PipelineRun was never put into a failed state due to a Task dropping a result. Only if the PipelineRun couldn't fetch the assocaited PipelineSpec while trying to resolve those result references. Ignore this message!

[pritidesai]

thanks @bobcatfish, #2701 has some discussion as well

[afrittoli]

I feel like this depends on what we define with "producing a result". I think with the current code today is:

• no file created -> result not produced
• file exists, empty -> empty result is produced (which will be an empty string)

That said, it is probably good to continue to force tasks to produce a result like they do today (unless we introduce support for default values, and a task specifies a default), as this would help task authors and users to avoid silent failures from being ignored.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

@bobcatfish do you still want to pursue this one or should we leave it for the time being?