From 2a910a3dea3e69d31770a7e8b4199a3a915b8b85 Mon Sep 17 00:00:00 2001 From: Jerop Date: Fri, 6 Aug 2021 14:13:42 -0500 Subject: [PATCH 1/2] Propagate Pipeline name label to PipelineRun Currently, a `PipelineRun` started in a `Cancelled` or `Pending` state doesn't receive the `Pipeline` name label. As such, a user cannot find all the `PipelineRuns` related to a given `Pipeline`. This is a challenge for a user searching for associated `PipelineRuns` in `Pending` state in order to start them. In this change, we ensure that the `Pipeline` name label is propagated from the `Pipeline` to the `PipelineRun`. Fixes https://github.com/tektoncd/pipeline/issues/3903. --- pkg/reconciler/pipelinerun/pipelinerun.go | 20 +++++ .../pipelinerun/pipelinerun_test.go | 88 +++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 8d5574bbf3a..31246a9b43d 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -208,6 +208,11 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) return c.finishReconcileUpdateEmitEvents(ctx, pr, before, nil) } + if err := propagatePipelineNameLabelToPipelineRun(pr); err != nil { + logger.Errorf("Failed to propagate pipeline name label to pipelinerun %s: %v", pr.Name, err) + return c.finishReconcileUpdateEmitEvents(ctx, pr, before, err) + } + // If the pipelinerun is cancelled, cancel tasks and update status if pr.IsCancelled() { err := cancelPipelineRun(ctx, logger, pr, c.PipelineClientSet) @@ -932,6 +937,21 @@ func getTaskrunAnnotations(pr *v1beta1.PipelineRun) map[string]string { return annotations } +func propagatePipelineNameLabelToPipelineRun(pr *v1beta1.PipelineRun) error { + if pr.ObjectMeta.Labels == nil { + pr.ObjectMeta.Labels = make(map[string]string) + } + switch { + case pr.Spec.PipelineRef != nil && pr.Spec.PipelineRef.Name != "": + pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pr.Spec.PipelineRef.Name + case pr.Spec.PipelineSpec != nil: + pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pr.Name + default: + return fmt.Errorf("pipelineRun %s not providing PipelineRef or PipelineSpec", pr.Name) + } + return nil +} + func getTaskrunLabels(pr *v1beta1.PipelineRun, pipelineTaskName string, includePipelineLabels bool) map[string]string { // Propagate labels from PipelineRun to TaskRun. labels := make(map[string]string, len(pr.ObjectMeta.Labels)+1) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 31474920d5f..295bdd9a9cf 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -2555,9 +2555,14 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { tb.PipelineRunStartTime(time.Now()), ), )} + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask("hello-world-1", "hello-world"), + tb.PipelineTask("hello-world-2", "hello-world"), + ))} d := test.Data{ PipelineRuns: prs, + Pipelines: ps, } testAssets, cancel := getPipelineRunController(t, d) @@ -2581,6 +2586,13 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { t.Fatalf("Somehow had error getting reconciled run out of fake client: %s", err) } + if val, ok := reconciledRun.GetLabels()[pipeline.PipelineLabelKey]; !ok { + t.Fatalf("expected pipeline label") + if d := cmp.Diff("test-pipelines", val); d != "" { + t.Errorf("expected to see pipeline label. Diff %s", diff.PrintWantGot(d)) + } + } + // The PipelineRun should not be cancelled b/c we couldn't cancel the TaskRun condition := reconciledRun.Status.GetCondition(apis.ConditionSucceeded) if !condition.IsUnknown() { @@ -2699,6 +2711,82 @@ func TestReconcilePropagateLabels(t *testing.T) { } } +func TestReconcilePropagateLabelsPending(t *testing.T) { + names.TestingSeed() + taskName := "hello-world-1" + + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask(taskName, "hello-world"), + ))} + prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-labels", tb.PipelineRunNamespace("foo"), + tb.PipelineRunLabel("PipelineRunLabel", "PipelineRunValue"), + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccountName("test-sa"), + tb.PipelineRunPending, + ), + )} + ts := []*v1beta1.Task{tb.Task("hello-world", tb.TaskNamespace("foo"))} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + _, clients := prt.reconcileRun("foo", "test-pipeline-run-with-labels", []string{}, false) + + reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get(prt.TestAssets.Ctx, "test-pipeline-run-with-labels", metav1.GetOptions{}) + if err != nil { + t.Fatalf("unexpected error when updating status: %v", err) + } + + want := "test-pipeline" + got := reconciledRun.ObjectMeta.Labels["tekton.dev/pipeline"] + if d := cmp.Diff(want, got); d != "" { + t.Errorf("expected to see label %v created. Diff %s", want, diff.PrintWantGot(d)) + } +} + +func TestReconcilePropagateLabelsCancelled(t *testing.T) { + names.TestingSeed() + taskName := "hello-world-1" + + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask(taskName, "hello-world"), + ))} + prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-labels", tb.PipelineRunNamespace("foo"), + tb.PipelineRunLabel("PipelineRunLabel", "PipelineRunValue"), + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccountName("test-sa"), + tb.PipelineRunCancelled, + ), + )} + ts := []*v1beta1.Task{tb.Task("hello-world", tb.TaskNamespace("foo"))} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + _, clients := prt.reconcileRun("foo", "test-pipeline-run-with-labels", []string{}, false) + + reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get(prt.TestAssets.Ctx, "test-pipeline-run-with-labels", metav1.GetOptions{}) + if err != nil { + t.Fatalf("unexpected error when updating status: %v", err) + } + + want := "test-pipeline" + got := reconciledRun.ObjectMeta.Labels["tekton.dev/pipeline"] + if d := cmp.Diff(want, got); d != "" { + t.Errorf("expected to see label %v created. Diff %s", want, diff.PrintWantGot(d)) + } +} + func TestReconcileWithDifferentServiceAccounts(t *testing.T) { names.TestingSeed() From 66bacd73ffbcf0f0023c5c8373af51bbd5f1006c Mon Sep 17 00:00:00 2001 From: Scott Date: Thu, 19 Aug 2021 09:32:29 -0400 Subject: [PATCH 2/2] Patch pipeline name label with GroupName to avoid back compat change in 0.27 The pipeline name label propagation change in 2a910a3 was merged after a slight change in the way we construct some publicly exported label constants. Bringing in the change to public constants would have technically been breaking for a patch release. This reintroduces GroupName to the label construction when propagating the pipeline name label to workaround the conflict. --- pkg/reconciler/pipelinerun/pipelinerun.go | 4 ++-- pkg/reconciler/pipelinerun/pipelinerun_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 31246a9b43d..b08cbf5279b 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -943,9 +943,9 @@ func propagatePipelineNameLabelToPipelineRun(pr *v1beta1.PipelineRun) error { } switch { case pr.Spec.PipelineRef != nil && pr.Spec.PipelineRef.Name != "": - pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pr.Spec.PipelineRef.Name + pr.ObjectMeta.Labels[pipeline.GroupName+pipeline.PipelineLabelKey] = pr.Spec.PipelineRef.Name case pr.Spec.PipelineSpec != nil: - pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pr.Name + pr.ObjectMeta.Labels[pipeline.GroupName+pipeline.PipelineLabelKey] = pr.Name default: return fmt.Errorf("pipelineRun %s not providing PipelineRef or PipelineSpec", pr.Name) } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 295bdd9a9cf..06b2ea4feab 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -2586,7 +2586,7 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { t.Fatalf("Somehow had error getting reconciled run out of fake client: %s", err) } - if val, ok := reconciledRun.GetLabels()[pipeline.PipelineLabelKey]; !ok { + if val, ok := reconciledRun.GetLabels()[pipeline.GroupName+pipeline.PipelineLabelKey]; !ok { t.Fatalf("expected pipeline label") if d := cmp.Diff("test-pipelines", val); d != "" { t.Errorf("expected to see pipeline label. Diff %s", diff.PrintWantGot(d))