Canceled PipelineRun shouldn't succeed for failed-to-Cancel TaskRuns with minimal EmbeddedStatus

[JeromeJu]

Expected Behavior

For the case where the TaskRuns were unsuccessfully canceled, the pipelineRun should not be canceled with Succedded state.

Thanks to Lee's comment as below:

It seems like this test is expecting that the taskruns referenced in the pipelineruns aren't able to be canceled successfully, and therefore the PipelineRun can't be canceled. However, what's actually happening is that the PipelineRun is canceled successfully. It sounds like our cancelation logic doesn't handle childReferences correctly.

Originally posted by @lbernick in #5934 (comment)

Actual Behavior

The TaskRun was cancelled while the PipelineRun was succeedded under minimal EmbeddedStatus.

→ tkn tr list
NAME                                      STARTED          DURATION   STATUS
test-fetch-secure-data                    12 minutes ago   11s        Cancelled(TaskRunCancelled)`

 → tkn pr list
NAME                  STARTED         DURATION   STATUS
test                  2 minutes ago   11s        Cancelled(Cancelled)

Steps to Reproduce the Problem

1. Get a pipelineRun started with Minimal EmbeddedStatus
2. Cancel it with tkn pr <pr name> cancel

Additional Info

[JeromeJu]

Related TEP:
#4954

[lbernick]

@JeromeJu I think you originally based this bug reproducer on the test case TestReconcileCancelledFailsTaskRunCancellation? This test case fakes a kube apiserver error to prevent the taskruns from being canceled, which is difficult (impossible??) to reproduce on your own cluster. I think the behavior pasted above is expected.

I tried to recreate one of the other test failures from your PR, TestReconcileOnCancelledRunFinallyPipelineRunWithRunningFinalTask, using the following definitions:

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: hello-world
  namespace: issue-5993
spec:
  steps:
  - image: busybox
    script: "echo hello world"
---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: sleep
  namespace: issue-5993
spec:
  steps:
  - image: busybox
    script: "sleep 100000"
---
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: test-pipeline
  namespace: issue-5993
spec:
  finally:
  - name: final-task-1
    taskRef:
      name: sleep
  tasks:
  - name: hello-world-1
    taskRef:
      name: hello-world
---
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  name: test-pipelinerun
  namespace: issue-5993
spec:
  pipelineRef:
    name: test-pipeline

I waited until the taskrun was complete, and then updated the PipelineRun status with "CancelledRunFinally". With both full and minimal embedded-status, the PipelineRun ended up with the correct taskruns/childrefs in its status.

So I checked out your changes in #5934 and was able to fix the test cases by changing the TaskRuns in the PipelineRun status in the test cases to ChildRefs. I think this may be an issue with the PR rather than a bug-- sorry I got this one wrong. If you do find a reliable reproducer, definitely feel free to reopen this issue!