Propagate annotations from Pipeline/Task to PipelineRun/TaskRun

With this change, annotations are propagated from Pipeline and Task to
PipelineRun and TaskRun, respectively, giving us full annotation propagation
from Pipeline to PipelineRun to TaskRun to Pod and Task to TaskRun to
Pod.

Author: dicarlo2

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go

@@ -216,11 +216,11 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		return err
 	}
 	// Since we are using the status subresource, it is not possible to update
-	// the status and labels simultaneously.
-	if !reflect.DeepEqual(original.ObjectMeta.Labels, pr.ObjectMeta.Labels) {
-		if _, err := c.updateLabels(pr); err != nil {
-			c.Logger.Warn("Failed to update PipelineRun labels", zap.Error(err))
-			c.Recorder.Event(pr, corev1.EventTypeWarning, eventReasonFailed, "PipelineRun failed to update labels")
+	// the status and labels/annotations simultaneously.
+	if !reflect.DeepEqual(original.ObjectMeta.Labels, pr.ObjectMeta.Labels) || !reflect.DeepEqual(original.ObjectMeta.Annotations, pr.ObjectMeta.Annotations) {
+		if _, err := c.updateLabelsAndAnnotations(pr); err != nil {
+			c.Logger.Warn("Failed to update PipelineRun labels/annotations", zap.Error(err))
+			c.Recorder.Event(pr, corev1.EventTypeWarning, eventReasonFailed, "PipelineRun failed to update labels/annotations")
 			return err
 		}
 	}
@@ -281,6 +281,14 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
 	}
 	pr.ObjectMeta.Labels[pipeline.GroupName+pipeline.PipelineLabelKey] = p.Name
 
+	// Propagate annotations from Pipeline to PipelineRun.
+	if pr.ObjectMeta.Annotations == nil {
+		pr.ObjectMeta.Annotations = make(map[string]string, len(p.ObjectMeta.Annotations))
+	}
+	for key, value := range p.ObjectMeta.Annotations {
+		pr.ObjectMeta.Annotations[key] = value
+	}
+
 	pipelineState, err := resources.ResolvePipelineRun(
 		*pr,
 		func(name string) (v1alpha1.TaskInterface, error) {
@@ -455,6 +463,12 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re
 	}
 	labels[pipeline.GroupName+pipeline.PipelineRunLabelKey] = pr.Name
 
+	// Propagate annotations from PipelineRun to TaskRun.
+	annotations := make(map[string]string, len(pr.ObjectMeta.Annotations)+1)
+	for key, val := range pr.ObjectMeta.Annotations {
+		annotations[key] = val
+	}
+
 	tr, _ := c.taskRunLister.TaskRuns(pr.Namespace).Get(rprt.TaskRunName)
 
 	if tr != nil {
@@ -473,6 +487,7 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re
 			Namespace:       pr.Namespace,
 			OwnerReferences: pr.GetOwnerReference(),
 			Labels:          labels,
+			Annotations:     annotations,
 		},
 		Spec: v1alpha1.TaskRunSpec{
 			TaskRef: &v1alpha1.TaskRef{
@@ -524,13 +539,14 @@ func (c *Reconciler) updateStatus(pr *v1alpha1.PipelineRun) (*v1alpha1.PipelineR
 	return newPr, nil
 }
 
-func (c *Reconciler) updateLabels(pr *v1alpha1.PipelineRun) (*v1alpha1.PipelineRun, error) {
+func (c *Reconciler) updateLabelsAndAnnotations(pr *v1alpha1.PipelineRun) (*v1alpha1.PipelineRun, error) {
 	newPr, err := c.pipelineRunLister.PipelineRuns(pr.Namespace).Get(pr.Name)
 	if err != nil {
-		return nil, fmt.Errorf("Error getting PipelineRun %s when updating labels: %s", pr.Name, err)
+		return nil, fmt.Errorf("Error getting PipelineRun %s when updating labels/annotations: %s", pr.Name, err)
 	}
-	if !reflect.DeepEqual(pr.ObjectMeta.Labels, newPr.ObjectMeta.Labels) {
+	if !reflect.DeepEqual(pr.ObjectMeta.Labels, newPr.ObjectMeta.Labels) || !reflect.DeepEqual(pr.ObjectMeta.Annotations, newPr.ObjectMeta.Annotations) {
 		newPr.ObjectMeta.Labels = pr.ObjectMeta.Labels
+		newPr.ObjectMeta.Annotations = pr.ObjectMeta.Annotations
 		return c.PipelineClientSet.TektonV1alpha1().PipelineRuns(pr.Namespace).Update(newPr)
 	}
 	return newPr, nil

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go

@@ -892,3 +892,65 @@ func TestReconcileWithTimeoutAndRetry(t *testing.T) {
 		})
 	}
 }
+
+func TestReconcilePropagateAnnotations(t *testing.T) {
+	names.TestingSeed()
+
+	ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec(
+		tb.PipelineTask("hello-world-1", "hello-world"),
+	))}
+	prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-annotations", "foo",
+		tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"),
+		tb.PipelineRunSpec("test-pipeline",
+			tb.PipelineRunServiceAccount("test-sa"),
+		),
+	)}
+	ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")}
+
+	d := test.Data{
+		PipelineRuns: prs,
+		Pipelines:    ps,
+		Tasks:        ts,
+	}
+
+	// create fake recorder for testing
+	fr := record.NewFakeRecorder(2)
+
+	testAssets := getPipelineRunController(t, d, fr)
+	c := testAssets.Controller
+	clients := testAssets.Clients
+
+	err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-with-annotations")
+	if err != nil {
+		t.Errorf("Did not expect to see error when reconciling completed PipelineRun but saw %s", err)
+	}
+
+	// Check that the PipelineRun was reconciled correctly
+	_, err = clients.Pipeline.Tekton().PipelineRuns("foo").Get("test-pipeline-run-with-annotations", metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err)
+	}
+
+	// Check that the expected TaskRun was created
+	actual := clients.Pipeline.Actions()[0].(ktesting.CreateAction).GetObject().(*v1alpha1.TaskRun)
+	if actual == nil {
+		t.Errorf("Expected a TaskRun to be created, but it wasn't.")
+	}
+	expectedTaskRun := tb.TaskRun("test-pipeline-run-with-annotations-hello-world-1-9l9zj", "foo",
+		tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-with-annotations",
+			tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"),
+			tb.Controller, tb.BlockOwnerDeletion,
+		),
+		tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"),
+		tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-with-annotations"),
+		tb.TaskRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"),
+		tb.TaskRunSpec(
+			tb.TaskRunTaskRef("hello-world"),
+			tb.TaskRunServiceAccount("test-sa"),
+		),
+	)
+
+	if d := cmp.Diff(actual, expectedTaskRun); d != "" {
+		t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRun, d)
+	}
+}

pkg/reconciler/v1alpha1/taskrun/resources/pod.go

@@ -231,14 +231,6 @@ func TryGetPod(taskRunStatus v1alpha1.TaskRunStatus, gp GetPod) (*corev1.Pod, er
 // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified
 // by the supplied CRD.
 func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient kubernetes.Interface, cache *entrypoint.Cache, logger *zap.SugaredLogger) (*corev1.Pod, error) {
-	// Copy annotations on the build through to the underlying pod to allow users
-	// to specify pod annotations.
-	annotations := map[string]string{}
-	for key, val := range taskRun.Annotations {
-		annotations[key] = val
-	}
-	annotations["sidecar.istio.io/inject"] = "false"
-
 	cred, secrets, err := makeCredentialInitializer(taskRun.Spec.ServiceAccount, taskRun.Namespace, kubeclient)
 	if err != nil {
 		return nil, err
@@ -329,7 +321,7 @@ func MakePod(taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient k
 			OwnerReferences: []metav1.OwnerReference{
 				*metav1.NewControllerRef(taskRun, groupVersionKind),
 			},
-			Annotations: annotations,
+			Annotations: makeAnnotations(taskRun),
 			Labels:      makeLabels(taskRun),
 		},
 		Spec: corev1.PodSpec{
@@ -355,6 +347,16 @@ func makeLabels(s *v1alpha1.TaskRun) map[string]string {
 	return labels
 }
 
+// makeAnnotations constructs the annotations we will propagate from TaskRuns to Pods.
+func makeAnnotations(s *v1alpha1.TaskRun) map[string]string {
+	annotations := make(map[string]string, len(s.ObjectMeta.Annotations)+1)
+	for k, v := range s.ObjectMeta.Annotations {
+		annotations[k] = v
+	}
+	annotations["sidecar.istio.io/inject"] = "false"
+	return annotations
+}
+
 // zeroNonMaxResourceRequests zeroes out the container's cpu, memory, or
 // ephemeral storage resource requests if the container does not have the
 // largest request out of all containers in the pod. This is done because Tekton

pkg/reconciler/v1alpha1/taskrun/taskrun.go

@@ -197,10 +197,10 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 		return err
 	}
 	// Since we are using the status subresource, it is not possible to update
-	// the status and labels simultaneously.
+	// the status and labels/annotations simultaneously.
 	if !reflect.DeepEqual(original.ObjectMeta.Labels, tr.ObjectMeta.Labels) {
-		if _, err := c.updateLabels(tr); err != nil {
-			c.Logger.Warn("Failed to update TaskRun labels", zap.Error(err))
+		if _, err := c.updateLabelsAndAnnotations(tr); err != nil {
+			c.Logger.Warn("Failed to update TaskRun labels/annotations", zap.Error(err))
 			return err
 		}
 	}
@@ -264,6 +264,14 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error
 		tr.ObjectMeta.Labels[pipeline.GroupName+pipeline.TaskLabelKey] = taskMeta.Name
 	}
 
+	// Propagate annotations from Task to TaskRun.
+	if tr.ObjectMeta.Annotations == nil {
+		tr.ObjectMeta.Annotations = make(map[string]string, len(taskMeta.Annotations))
+	}
+	for key, value := range taskMeta.Annotations {
+		tr.ObjectMeta.Annotations[key] = value
+	}
+
 	// Check if the TaskRun has timed out; if it is, this will set its status
 	// accordingly.
 	if timedOut, err := c.checkTimeout(tr, taskSpec, c.KubeClientSet.CoreV1().Pods(tr.Namespace).Delete); err != nil {
@@ -491,13 +499,14 @@ func (c *Reconciler) updateStatus(taskrun *v1alpha1.TaskRun) (*v1alpha1.TaskRun,
 	return newtaskrun, nil
 }
 
-func (c *Reconciler) updateLabels(tr *v1alpha1.TaskRun) (*v1alpha1.TaskRun, error) {
+func (c *Reconciler) updateLabelsAndAnnotations(tr *v1alpha1.TaskRun) (*v1alpha1.TaskRun, error) {
 	newTr, err := c.taskRunLister.TaskRuns(tr.Namespace).Get(tr.Name)
 	if err != nil {
-		return nil, fmt.Errorf("Error getting TaskRun %s when updating labels: %s", tr.Name, err)
+		return nil, fmt.Errorf("Error getting TaskRun %s when updating labels/annotations: %s", tr.Name, err)
 	}
-	if !reflect.DeepEqual(tr.ObjectMeta.Labels, newTr.ObjectMeta.Labels) {
+	if !reflect.DeepEqual(tr.ObjectMeta.Labels, newTr.ObjectMeta.Labels) || !reflect.DeepEqual(tr.ObjectMeta.Annotations, newTr.ObjectMeta.Annotations) {
 		newTr.ObjectMeta.Labels = tr.ObjectMeta.Labels
+		newTr.ObjectMeta.Annotations = tr.ObjectMeta.Annotations
 		return c.PipelineClientSet.TektonV1alpha1().TaskRuns(tr.Namespace).Update(newTr)
 	}
 	return newTr, nil

pkg/reconciler/v1alpha1/taskrun/taskrun_test.go

@@ -304,6 +304,13 @@ func TestReconcile(t *testing.T) {
 		),
 	)
 
+	taskRunWithAnnotations := tb.TaskRun("test-taskrun-with-annotations", "foo",
+		tb.TaskRunAnnotation("TaskRunAnnotation", "TaskRunValue"),
+		tb.TaskRunSpec(
+			tb.TaskRunTaskRef(simpleTask.Name),
+		),
+	)
+
 	taskRunWithResourceRequests := tb.TaskRun("test-taskrun-with-resource-requests", "foo",
 		tb.TaskRunSpec(
 			tb.TaskRunTaskSpec(
@@ -347,7 +354,7 @@ func TestReconcile(t *testing.T) {
 		taskRunSuccess, taskRunWithSaSuccess,
 		taskRunTemplating, taskRunInputOutput,
 		taskRunWithTaskSpec, taskRunWithClusterTask, taskRunWithResourceSpecAndTaskSpec,
-		taskRunWithLabels, taskRunWithResourceRequests, taskRunTaskEnv, taskRunWithPod,
+		taskRunWithLabels, taskRunWithAnnotations, taskRunWithResourceRequests, taskRunTaskEnv, taskRunWithPod,
 	}
 
 	d := test.Data{
@@ -845,6 +852,42 @@ func TestReconcile(t *testing.T) {
 				),
 			),
 		),
+	}, {
+		name:    "taskrun-with-annotations",
+		taskRun: taskRunWithAnnotations,
+		wantPod: tb.Pod("test-taskrun-with-annotations-pod-123456", "foo",
+			tb.PodAnnotation("sidecar.istio.io/inject", "false"),
+			tb.PodAnnotation("TaskRunAnnotation", "TaskRunValue"),
+			tb.PodLabel(taskNameLabelKey, "test-task"),
+			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-annotations"),
+			tb.PodOwnerReference("TaskRun", "test-taskrun-with-annotations",
+				tb.OwnerReferenceAPIVersion(currentApiVersion)),
+			tb.PodSpec(
+				tb.PodVolumes(toolsVolume, workspaceVolume, homeVolume),
+				tb.PodRestartPolicy(corev1.RestartPolicyNever),
+				getCredentialsInitContainer("9l9zj"),
+				getPlaceToolsInitContainer(),
+				tb.PodContainer("build-step-simple-step", "foo",
+					tb.Command(entrypointLocation),
+					tb.Args("-wait_file", "", "-post_file", "/builder/tools/0", "-entrypoint", "/mycmd", "--"),
+					tb.WorkingDir(workspaceDir),
+					tb.EnvVar("HOME", "/builder/home"),
+					tb.VolumeMount("tools", "/builder/tools"),
+					tb.VolumeMount("workspace", workspaceDir),
+					tb.VolumeMount("home", "/builder/home"),
+					tb.Resources(tb.Requests(
+						tb.CPU("0"),
+						tb.Memory("0"),
+						tb.EphemeralStorage("0"),
+					)),
+				),
+				tb.PodContainer("nop", "override-with-nop:latest",
+					tb.Command("/builder/tools/entrypoint"),
+					tb.Args("-wait_file", "/builder/tools/0", "-post_file", "/builder/tools/1", "-entrypoint", "/ko-app/nop", "--"),
+					tb.VolumeMount(entrypoint.MountName, entrypoint.MountPoint),
+				),
+			),
+		),
 	}, {
 		name:    "task-env",
 		taskRun: taskRunTaskEnv,

test/builder/pipeline.go

@@ -270,6 +270,16 @@ func PipelineRunLabel(key, value string) PipelineRunOp {
 	}
 }
 
+// PipelineRunAnnotations adds a annotation to the PipelineRun.
+func PipelineRunAnnotation(key, value string) PipelineRunOp {
+	return func(pr *v1alpha1.PipelineRun) {
+		if pr.ObjectMeta.Annotations == nil {
+			pr.ObjectMeta.Annotations = map[string]string{}
+		}
+		pr.ObjectMeta.Annotations[key] = value
+	}
+}
+
 // PipelineRunResourceBinding adds bindings from actual instances to a Pipeline's declared resources.
 func PipelineRunResourceBinding(name string, ops ...PipelineResourceBindingOp) PipelineRunSpecOp {
 	return func(prs *v1alpha1.PipelineRunSpec) {

test/builder/task.go

@@ -266,8 +266,9 @@ func ParamDefault(value string) TaskParamOp {
 func TaskRun(name, namespace string, ops ...TaskRunOp) *v1alpha1.TaskRun {
 	tr := &v1alpha1.TaskRun{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace: namespace,
-			Name:      name,
+			Namespace:   namespace,
+			Name:        name,
+			Annotations: map[string]string{},
 		},
 	}
 
@@ -395,6 +396,15 @@ func TaskRunLabel(key, value string) TaskRunOp {
 	}
 }
 
+func TaskRunAnnotation(key, value string) TaskRunOp {
+	return func(tr *v1alpha1.TaskRun) {
+		if tr.ObjectMeta.Annotations == nil {
+			tr.ObjectMeta.Annotations = map[string]string{}
+		}
+		tr.ObjectMeta.Annotations[key] = value
+	}
+}
+
 // TaskRunSpec sets the specified spec of the TaskRun.
 // Any number of TaskRunSpec modifier can be passed to transform it.
 func TaskRunSpec(ops ...TaskRunSpecOp) TaskRunOp {

test/builder/task_test.go

@@ -117,7 +117,7 @@ func TestClusterTask(t *testing.T) {
 	}
 }
 
-func TestTaskRunWitTaskRef(t *testing.T) {
+func TestTaskRunWithTaskRef(t *testing.T) {
 	var trueB = true
 	taskRun := tb.TaskRun("test-taskrun", "foo",
 		tb.TaskRunOwnerReference("PipelineRun", "test",
@@ -164,7 +164,8 @@ func TestTaskRunWitTaskRef(t *testing.T) {
 				Controller:         &trueB,
 				BlockOwnerDeletion: &trueB,
 			}},
-			Labels: map[string]string{"label": "label-value"},
+			Labels:      map[string]string{"label": "label-value"},
+			Annotations: map[string]string{},
 		},
 		Spec: v1alpha1.TaskRunSpec{
 			Inputs: v1alpha1.TaskRunInputs{
@@ -223,6 +224,7 @@ func TestTaskRunWithTaskSpec(t *testing.T) {
 	expectedTaskRun := &v1alpha1.TaskRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-taskrun", Namespace: "foo",
+			Annotations: map[string]string{},
 		},
 		Spec: v1alpha1.TaskRunSpec{
 			TaskSpec: &v1alpha1.TaskSpec{