Skip to content

Commit

Permalink
Propagate Labels and Annotations from Pipeline to PipelineRun
Browse files Browse the repository at this point in the history 
Currently, a `PipelineRun` started in a `Cancelled` or `Pending` state
doesn't receive labels and annotations from the `Pipeline`. As such, a
user cannot find all the `PipelineRuns` related to a given `Pipeline`.
This is especially a challenge for a user searching for associated
`PipelineRuns` in `Pending` state in order to start them.

In this change, we ensure that the `Labels` and `Annotations` are
propagated from the `Pipeline` to the `PipelineRun`. This involves
refactoring the order of execution in the reconcile loop. In addition,
we fix a test case that was meant to test for cancellation but wasn't
configured correctly (was missing the `Pipeline` in test).

Fixes #3903.
  • Loading branch information
@jerop
jerop committed Aug 17, 2021
1 parent f0a2f00 commit 8158130
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 33 deletions.
72 changes: 39 additions & 33 deletions pkg/reconciler/pipelinerun/pipelinerun.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,23 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, nil)
}

pipelineMeta, pipelineSpec, err := resources.GetPipelineData(ctx, pr, getPipelineFunc)
if err != nil {
logger.Errorf("Failed to determine Pipeline spec to use for pipelinerun %s: %v", pr.Name, err)
pr.Status.MarkFailed(ReasonCouldntGetPipeline,
"Error retrieving pipeline for pipelinerun %s/%s: %s",
pr.Namespace, pr.Name, err)
return c.finishReconcileUpdateEmitEvents(ctx, pr, before, controller.NewPermanentError(err))
}

// Store the fetched PipelineSpec on the PipelineRun for auditing
if err := storePipelineSpec(ctx, pr, pipelineSpec); err != nil {
logger.Errorf("Failed to store PipelineSpec on PipelineRun.Status for pipelinerun %s: %v", pr.Name, err)
}

propagateLabelsFromPipelineToPipelineRun(pr, pipelineMeta)
propagateAnnotationsFromPipelineToPipelineRun(pr, pipelineMeta)

// If the pipelinerun is cancelled, cancel tasks and update status
if pr.IsCancelled() {
err := cancelPipelineRun(ctx, logger, pr, c.PipelineClientSet)
Expand All @@ -222,7 +239,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)

// Reconcile this copy of the pipelinerun and then write back any status or label
// updates regardless of whether the reconciliation errored out.
if err = c.reconcile(ctx, pr, getPipelineFunc); err != nil {
if err = c.reconcile(ctx, pr, pipelineMeta, pipelineSpec); err != nil {
logger.Errorf("Reconcile error: %v", err.Error())
}

Expand Down Expand Up @@ -312,7 +329,7 @@ func (c *Reconciler) resolvePipelineState(
return pst, nil
}

func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, getPipelineFunc resources.GetPipeline) error {
func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, pipelineMeta *metav1.ObjectMeta, pipelineSpec *v1beta1.PipelineSpec) error {
logger := logging.FromContext(ctx)
cfg := config.FromContextOrDefaults(ctx)
// We may be reading a version of the object that was stored at an older version
Expand All @@ -330,37 +347,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
return nil
}

pipelineMeta, pipelineSpec, err := resources.GetPipelineData(ctx, pr, getPipelineFunc)
if err != nil {
logger.Errorf("Failed to determine Pipeline spec to use for pipelinerun %s: %v", pr.Name, err)
pr.Status.MarkFailed(ReasonCouldntGetPipeline,
"Error retrieving pipeline for pipelinerun %s/%s: %s",
pr.Namespace, pr.Name, err)
return controller.NewPermanentError(err)
}

// Store the fetched PipelineSpec on the PipelineRun for auditing
if err := storePipelineSpec(ctx, pr, pipelineSpec); err != nil {
logger.Errorf("Failed to store PipelineSpec on PipelineRun.Status for pipelinerun %s: %v", pr.Name, err)
}

// Propagate labels from Pipeline to PipelineRun.
if pr.ObjectMeta.Labels == nil {
pr.ObjectMeta.Labels = make(map[string]string, len(pipelineMeta.Labels)+1)
}
for key, value := range pipelineMeta.Labels {
pr.ObjectMeta.Labels[key] = value
}
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pipelineMeta.Name

// Propagate annotations from Pipeline to PipelineRun.
if pr.ObjectMeta.Annotations == nil {
pr.ObjectMeta.Annotations = make(map[string]string, len(pipelineMeta.Annotations))
}
for key, value := range pipelineMeta.Annotations {
pr.ObjectMeta.Annotations[key] = value
}

d, err := dag.Build(v1beta1.PipelineTaskList(pipelineSpec.Tasks), v1beta1.PipelineTaskList(pipelineSpec.Tasks).Deps())
if err != nil {
// This Run has failed, so we need to mark it as failed and stop reconciling it
Expand Down Expand Up @@ -931,6 +917,26 @@ func getTaskrunAnnotations(pr *v1beta1.PipelineRun) map[string]string {
return annotations
}

func propagateLabelsFromPipelineToPipelineRun(pr *v1beta1.PipelineRun, pipelineMeta *metav1.ObjectMeta) {
if pr.ObjectMeta.Labels == nil {
pr.ObjectMeta.Labels = make(map[string]string, len(pipelineMeta.Labels)+1)
}
for key, value := range pipelineMeta.Labels {
pr.ObjectMeta.Labels[key] = value
}
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pipelineMeta.Name
}

func propagateAnnotationsFromPipelineToPipelineRun(pr *v1beta1.PipelineRun, pipelineMeta *metav1.ObjectMeta) {
if pr.ObjectMeta.Annotations == nil {
pr.ObjectMeta.Annotations = make(map[string]string, len(pipelineMeta.Annotations))
}
for key, value := range pipelineMeta.Annotations {
pr.ObjectMeta.Annotations[key] = value
}

}

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)
Expand Down
88 changes: 88 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2557,9 +2557,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)
Expand All @@ -2583,6 +2588,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() {
Expand Down Expand Up @@ -2701,6 +2713,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()

Expand Down

0 comments on commit 8158130

Please sign in to comment.