WIP of reworked embedded status implementation

docs/install.md

@@ -389,7 +389,7 @@ features](#alpha-features) to be used.
 - `embedded-status`: set this flag to "full" to enable full embedding of `TaskRun` and `Run` statuses in the 
  `PipelineRun` status. Set it to "minimal" to populate the `ChildReferences` field in the `PipelineRun` status with
  name, kind, and API version information for each `TaskRun` and `Run` in the `PipelineRun` instead. Set it to "both" to 
- do both. For more information, see [Configuring usage of `TaskRun` and `Run` embedded statuses](pipelineruns.md#configuring-usage-of-taskrun-and-run-embedded-statuses). **NOTE**: This functionality is not yet active.
+ do both. For more information, see [Configuring usage of `TaskRun` and `Run` embedded statuses](pipelineruns.md#configuring-usage-of-taskrun-and-run-embedded-statuses).
 
 For example:
 

go.mod

@@ -10,6 +10,7 @@ require (
  github.com/google/go-containerregistry v0.8.1-0.20220216220642-00c59d91847c
  github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20220328141311-efc62d802606
  github.com/google/uuid v1.3.0
+ github.com/hashicorp/errwrap v1.0.0
  github.com/hashicorp/go-multierror v1.1.1
  github.com/hashicorp/golang-lru v0.5.4
  github.com/jenkins-x/go-scm v1.10.10
@@ -90,7 +91,6 @@ require (
  github.com/google/gofuzz v1.2.0 // indirect
  github.com/googleapis/gnostic v0.5.5 // indirect
  github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
- github.com/hashicorp/errwrap v1.0.0
  github.com/imdario/mergo v0.3.12 // indirect
  github.com/jmespath/go-jmespath v0.4.0 // indirect
  github.com/josharian/intern v1.0.0 // indirect

pkg/apis/pipeline/v1beta1/pipelinerun_types.go

@@ -415,6 +415,21 @@ type ChildStatusReference struct {
  WhenExpressions []WhenExpression `json:"whenExpressions,omitempty"`
 }
 
+// GetConditionChecksAsMap returns a map representation of this ChildStatusReference's ConditionChecks, in the same form
+// as PipelineRunTaskRunStatus.ConditionChecks.
+func (cr ChildStatusReference) GetConditionChecksAsMap() map[string]*PipelineRunConditionCheckStatus {
+ if len(cr.ConditionChecks) == 0 {
+ return nil
+ }
+ ccMap := make(map[string]*PipelineRunConditionCheckStatus)
+
+ for _, cc := range cr.ConditionChecks {
+ ccMap[cc.ConditionCheckName] = &cc.PipelineRunConditionCheckStatus
+ }
+
+ return ccMap
+}
+
 // PipelineRunStatusFields holds the fields of PipelineRunStatus' status.
 // This is defined separately and inlined so that other types can readily
 // consume these fields via duck typing.

pkg/reconciler/pipelinerun/cancel.go

@@ -99,23 +99,48 @@ func cancelPipelineRun(ctx context.Context, logger *zap.SugaredLogger, pr *v1bet
 func cancelPipelineTaskRuns(ctx context.Context, logger *zap.SugaredLogger, pr *v1beta1.PipelineRun, clientSet clientset.Interface) []string {
  errs := []string{}
 
- // Loop over the TaskRuns in the PipelineRun status.
- // If a TaskRun is not in the status yet we should not cancel it anyways.
- for taskRunName := range pr.Status.TaskRuns {
- logger.Infof("cancelling TaskRun %s", taskRunName)
-
- if _, err := clientSet.TektonV1beta1().TaskRuns(pr.Namespace).Patch(ctx, taskRunName, types.JSONPatchType, cancelTaskRunPatchBytes, metav1.PatchOptions{}, ""); err != nil {
- errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %s", taskRunName, err).Error())
- continue
+ // If pr.Status.ChildReferences is populated, use that as source of truth for TaskRun and Run names.
+ if len(pr.Status.ChildReferences) > 0 {
+ // Loop over the ChildReferences in the PipelineRun status.
+ for _, cr := range pr.Status.ChildReferences {
+ switch cr.Kind {
+ case "TaskRun":
+ logger.Infof("cancelling TaskRun %s", cr.Name)
+
+ if _, err := clientSet.TektonV1beta1().TaskRuns(pr.Namespace).Patch(ctx, cr.Name, types.JSONPatchType, cancelTaskRunPatchBytes, metav1.PatchOptions{}, ""); err != nil {
+ errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %s", cr.Name, err).Error())
+ continue
+ }
+ case "Run":
+ logger.Infof("cancelling Run %s", cr.Name)
+
+ if err := cancelRun(ctx, cr.Name, pr.Namespace, clientSet); err != nil {
+ errs = append(errs, fmt.Errorf("Failed to patch Run `%s` with cancellation: %s", cr.Name, err).Error())
+ continue
+ }
+ default:
+ errs = append(errs, fmt.Errorf("unknown or unsupported kind `%s` for cancellation", cr.Kind).Error())
+ }
  }
- }
- // Loop over the Runs in the PipelineRun status.
- for runName := range pr.Status.Runs {
- logger.Infof("cancelling Run %s", runName)
-
- if err := cancelRun(ctx, runName, pr.Namespace, clientSet); err != nil {
- errs = append(errs, fmt.Errorf("Failed to patch Run `%s` with cancellation: %s", runName, err).Error())
- continue
+ } else {
+ // Loop over the TaskRuns in the PipelineRun status.
+ // If a TaskRun is not in the status yet we should not cancel it anyways.
+ for taskRunName := range pr.Status.TaskRuns {
+ logger.Infof("cancelling TaskRun %s", taskRunName)
+
+ if _, err := clientSet.TektonV1beta1().TaskRuns(pr.Namespace).Patch(ctx, taskRunName, types.JSONPatchType, cancelTaskRunPatchBytes, metav1.PatchOptions{}, ""); err != nil {
+ errs = append(errs, fmt.Errorf("Failed to patch TaskRun `%s` with cancellation: %s", taskRunName, err).Error())
+ continue
+ }
+ }
+ // Loop over the Runs in the PipelineRun status.
+ for runName := range pr.Status.Runs {
+ logger.Infof("cancelling Run %s", runName)
+
+ if err := cancelRun(ctx, runName, pr.Namespace, clientSet); err != nil {
+ errs = append(errs, fmt.Errorf("Failed to patch Run `%s` with cancellation: %s", runName, err).Error())
+ continue
+ }
  }
  }
 

pkg/reconciler/pipelinerun/cancel_test.go

@@ -20,6 +20,8 @@ import (
  "context"
  "testing"
 
+ "k8s.io/apimachinery/pkg/runtime"
+
  "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
  "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
  _ "github.com/tektoncd/pipeline/pkg/pipelinerunmetrics/fake" // Make sure the pipelinerunmetrics are setup
@@ -36,6 +38,7 @@ func TestCancelPipelineRun(t *testing.T) {
  pipelineRun *v1beta1.PipelineRun
  taskRuns []*v1beta1.TaskRun
  runs []*v1alpha1.Run
+ wantErr bool
  }{{
  name: "no-resolved-taskrun",
  pipelineRun: &v1beta1.PipelineRun{
@@ -104,10 +107,67 @@ func TestCancelPipelineRun(t *testing.T) {
  Status: v1beta1.PipelineRunSpecStatusCancelledDeprecated,
  },
  },
+ }, {
+ name: "child-references",
+ pipelineRun: &v1beta1.PipelineRun{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
+ Spec: v1beta1.PipelineRunSpec{
+ Status: v1beta1.PipelineRunSpecStatusCancelled,
+ },
+ Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
+ ChildReferences: []v1beta1.ChildStatusReference{
+ {
+ TypeMeta: runtime.TypeMeta{Kind: "TaskRun"},
+ Name: "t1",
+ PipelineTaskName: "task-1",
+ },
+ {
+ TypeMeta: runtime.TypeMeta{Kind: "TaskRun"},
+ Name: "t2",
+ PipelineTaskName: "task-2",
+ },
+ {
+ TypeMeta: runtime.TypeMeta{Kind: "Run"},
+ Name: "r1",
+ PipelineTaskName: "run-1",
+ },
+ {
+ TypeMeta: runtime.TypeMeta{Kind: "Run"},
+ Name: "r2",
+ PipelineTaskName: "run-2",
+ },
+ },
+ }},
+ },
+ taskRuns: []*v1beta1.TaskRun{
+ {ObjectMeta: metav1.ObjectMeta{Name: "t1"}},
+ {ObjectMeta: metav1.ObjectMeta{Name: "t2"}},
+ },
+ runs: []*v1alpha1.Run{
+ {ObjectMeta: metav1.ObjectMeta{Name: "r1"}},
+ {ObjectMeta: metav1.ObjectMeta{Name: "r2"}},
+ },
+ }, {
+ name: "unknown-kind-on-child-references",
+ pipelineRun: &v1beta1.PipelineRun{
+ ObjectMeta: metav1.ObjectMeta{Name: "test-pipeline-run-cancelled"},
+ Spec: v1beta1.PipelineRunSpec{
+ Status: v1beta1.PipelineRunSpecStatusCancelled,
+ },
+ Status: v1beta1.PipelineRunStatus{PipelineRunStatusFields: v1beta1.PipelineRunStatusFields{
+ ChildReferences: []v1beta1.ChildStatusReference{{
+ TypeMeta: runtime.TypeMeta{Kind: "InvalidKind"},
+ Name: "t1",
+ PipelineTaskName: "task-1",
+ }},
+ }},
+ },
+ wantErr: true,
  }}
  for _, tc := range testCases {
  tc := tc
  t.Run(tc.name, func(t *testing.T) {
+
  d := test.Data{
  PipelineRuns: []*v1beta1.PipelineRun{tc.pipelineRun},
  TaskRuns: tc.taskRuns,
@@ -117,33 +177,41 @@ func TestCancelPipelineRun(t *testing.T) {
  ctx, cancel := context.WithCancel(ctx)
  defer cancel()
  c, _ := test.SeedTestData(t, ctx, d)
- if err := cancelPipelineRun(ctx, logtesting.TestLogger(t), tc.pipelineRun, c.Pipeline); err != nil {
- t.Fatal(err)
- }
- // This PipelineRun should still be complete and false, and the status should reflect that
- cond := tc.pipelineRun.Status.GetCondition(apis.ConditionSucceeded)
- if cond.IsTrue() {
- t.Errorf("Expected PipelineRun status to be complete and false, but was %v", cond)
- }
- if tc.taskRuns != nil {
- l, err := c.Pipeline.TektonV1beta1().TaskRuns("").List(ctx, metav1.ListOptions{})
+
+ err := cancelPipelineRun(ctx, logtesting.TestLogger(t), tc.pipelineRun, c.Pipeline)
+ if tc.wantErr {
+ if err == nil {
+ t.Error("expected an error, but did not get one")
+ }
+ } else {
  if err != nil {
  t.Fatal(err)
  }
- for _, tr := range l.Items {
-  if tr.Spec.Status != v1beta1.TaskRunSpecStatusCancelled {
-  t.Errorf("expected task %q to be marked as cancelled, was %q", tr.Name, tr.Spec.Status)
- }
+ // This PipelineRun should still be complete and false, and the status should reflect that
+ cond := tc.pipelineRun.Status.GetCondition(apis.ConditionSucceeded)
+ if cond.IsTrue() {
+ t.Errorf("Expected PipelineRun status to be complete and false, but was %v", cond)
  }
- }
- if tc.runs != nil {
- l, err := c.Pipeline.TektonV1alpha1().Runs("").List(ctx, metav1.ListOptions{})
- if err != nil {
- t.Fatal(err)
+ if tc.taskRuns != nil {
+ for _, expectedTR := range tc.taskRuns {
+ tr, err := c.Pipeline.TektonV1beta1().TaskRuns("").Get(ctx, expectedTR.Name, metav1.GetOptions{})
+ if err != nil {
+ t.Fatalf("couldn't get expected TaskRun %s, got error %s", expectedTR.Name, err)
+ }
+ if tr.Spec.Status != v1beta1.TaskRunSpecStatusCancelled {
+ t.Errorf("expected task %q to be marked as cancelled, was %q", tr.Name, tr.Spec.Status)
+ }
+ }
  }
- for _, r := range l.Items {
- if r.Spec.Status != v1alpha1.RunSpecStatusCancelled {
- t.Errorf("expected Run %q to be marked as cancelled, was %q", r.Name, r.Spec.Status)
+ if tc.runs != nil {
+ for _, expectedRun := range tc.runs {
+ r, err := c.Pipeline.TektonV1alpha1().Runs("").Get(ctx, expectedRun.Name, metav1.GetOptions{})
+ if err != nil {
+ t.Fatalf("couldn't get expected Run %s, got error %s", expectedRun.Name, err)
+ }
+ if r.Spec.Status != v1alpha1.RunSpecStatusCancelled {
+ t.Errorf("expected task %q to be marked as cancelled, was %q", r.Name, r.Spec.Status)
+ }
  }
  }
  }