[REGRESSION] Unable to cancel pipeline run - error PipelineRunCouldntCancel reported on a pending custom task

[rafalbigaj]

Expected Behavior

Pipeline run with unscheduled custom test should be cancellable.

Regression: This scenario works correctly on v0.33.1.

Actual Behavior

When cancelling a pipeline run, which contains unscheduled custom test, the error PipelineRunCouldntCancel occurs.

Steps to Reproduce the Problem

1. Install wait-task custom task controller.
2. Run a sample pipeline run with tow steps, one depended on the other:

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  generateName: cancel-test-
spec:
  pipelineSpec:
    tasks:
    - name: wait-1
      taskSpec:
        apiVersion: example.dev/v0
        kind: Wait
      params:
        - name: duration
          value: 1h
    - name: wait-2
      runAfter:
        - wait-1
      taskSpec:
        apiVersion: example.dev/v0
        kind: Wait
      params:
        - name: duration
          value: 10s
  serviceAccountName: pipeline-runner

3. Cancel the pipeline run setting spec.status to "Cancelled". The final PipelineRun status is:

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  creationTimestamp: "2022-08-08T09:09:13Z"
  generateName: cancel-test-
  generation: 2
  labels:
    tekton.dev/pipeline: cancel-test-qwndq
  name: cancel-test-qwndq
  namespace: wait-task
  resourceVersion: "10224"
  uid: 5cb9a96f-fbfe-4103-961f-81ff775c0a33
spec:
  pipelineSpec:
    tasks:
    - name: wait-1
      params:
      - name: duration
        value: 1h
      taskSpec:
        apiVersion: example.dev/v0
        kind: Wait
        metadata: {}
        spec: null
    - name: wait-2
      params:
      - name: duration
        value: 10s
      runAfter:
      - wait-1
      taskSpec:
        apiVersion: example.dev/v0
        kind: Wait
        metadata: {}
        spec: null
  serviceAccountName: pipeline-runner
  status: Cancelled
  timeout: 1h0m0s
status:
  conditions:
  - lastTransitionTime: "2022-08-08T09:10:18Z"
    message: 'PipelineRun "cancel-test-qwndq" was cancelled but had errors trying
      to cancel TaskRuns and/or Runs: Failed to patch Run `cancel-test-qwndq-wait-2`
      with cancellation: runs.tekton.dev "cancel-test-qwndq-wait-2" not found'
    reason: PipelineRunCouldntCancel
    status: Unknown
    type: Succeeded
  pipelineSpec:
    tasks:
    - name: wait-1
      params:
      - name: duration
        value: 1h
      taskSpec:
        apiVersion: example.dev/v0
        kind: Wait
        metadata: {}
        spec: null
    - name: wait-2
      params:
      - name: duration
        value: 10s
      runAfter:
      - wait-1
      taskSpec:
        apiVersion: example.dev/v0
        kind: Wait
        metadata: {}
        spec: null
  runs:
    cancel-test-qwndq-wait-1:
      pipelineTaskName: wait-1
      status:
        extraFields: null
    cancel-test-qwndq-wait-2:
      pipelineTaskName: wait-2
  startTime: "2022-08-08T09:09:13Z"

Additional Info

• Kubernetes version:

  Output of kubectl version:

Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.0", GitCommit:"4ce5a8954017644c5420bae81d72b09b735c21f0", GitTreeState:"clean", BuildDate:"2022-05-03T13:36:49Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"darwin/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.3", GitCommit:"816c97ab8cff8a1c72eccca1026f7820e93e0d25", GitTreeState:"clean", BuildDate:"2022-01-25T21:19:12Z", GoVersion:"go1.17.6", Compiler:"gc", Platform:"linux/amd64"}

• Tekton Pipeline version:

Client version: 0.25.0
Pipeline version: v0.38.2

[afrittoli]

Thank you for the bug report @rafalbigaj - it sounds like we don't have test coverage for this.
Perhaps we should consider installing a simple custom run controller in our e2e tests so we may test these scenarios.
/cc @abayer @lbernick

[afrittoli]

@rafalbigaj Does the cancel happen right away?
Both runs are in the status, which is why the controller attempts to cancel both and fails. It could be that that is the source of the issue.
Are you using minimal embedded status in your setup? Any other alpha flag enabled?

[abayer]

Perhaps we should consider installing a simple custom run controller in our e2e tests so we may test these scenarios

👍 Agreed!

[dibyom]

cc #5120

[pritidesai]

Perhaps we should consider installing a simple custom run controller in our e2e tests so we may test these scenarios

Would this require changing pluming to deploy wait task controller or can it be done via e2e-common (similar to pipeline controller)? Should we bring wait task controller into pipeline repo to decouple from whats available in experimental repo?

[afrittoli]

I wrote a reproducer unit test, so we can test this even without e2e tests, nonetheless it would be good to install the wait controller.

/*
Copyright 2022 The Tekton Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package pipelinerun

import (
	"fmt"
	"testing"

	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
	"github.com/tektoncd/pipeline/test"
	"github.com/tektoncd/pipeline/test/parse"
	corev1 "k8s.io/api/core/v1"
	logtesting "knative.dev/pkg/logging/testing"

	_ "knative.dev/pkg/system/testing" // Setup system.Namespace()
)

func TestReconcileTwoCustomTasks(t *testing.T) {
	pipelineRunName := "cancel-test-run"
	prs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, `metadata:
  name: cancel-test-run
  namespace: foo
spec:
  pipelineSpec:
    tasks:
      - name: wait-1
        taskSpec:
          apiVersion: example.dev/v0
          kind: Wait
          params:
            - name: duration
              value: 1h
      - name: wait-2
        runAfter:
          - wait-1
        taskSpec:
          apiVersion: example.dev/v0
          kind: Wait
          params:
            - name: duration
              value: 10s
`)}

	cms := []*corev1.ConfigMap{withCustomTasks(newFeatureFlagsConfigMap())}

	d := test.Data{
		PipelineRuns: prs,
		ConfigMaps:   cms,
	}
	prt := newPipelineRunTest(d, t)
	defer prt.Cancel()

	pr, clients := prt.reconcileRun("foo", pipelineRunName, []string{}, false)
	fmt.Printf("%v", pr)
	err := cancelPipelineRun(prt.TestAssets.Ctx, logtesting.TestLogger(t), pr, clients.Pipeline)
	if err != nil {
		t.Fatalf("Error found: %v", err)
	}
}

I used git bisect to find the first failing commit 9d60d0a

$ git bisect start v0.37.0 v0.36.0 --
$ git bisect run go test -v github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun -run TestReconcileTwoCustomTasks
(...)
9d60d0adf41e74f78029a0eaab73140fa4f7206a is the first bad commit

On the surface it looks unrelated, I need to dig in to understand what happened.

[afrittoli]

The issue happens only with full status - with minimal status it works fine - i.e. in the test, using:

	cms := []*corev1.ConfigMap{withCustomTasks(withEmbeddedStatus(newFeatureFlagsConfigMap(), config.MinimalEmbeddedStatus))}

Works on main.