Propagate labels from PipelineRun to TaskRun to Pods

Previously, labels were propagated from TaskRun to Build, but not from
Build to Pod. This PR fixes that, and also propagates labels from
PipelineRun to TaskRun, so that users are more easily able to filter,
identify, and query Pods created by Build Pipeline.

In all cases, if a user specifies a label whose key overlaps with one
of the labels that we set automatically, the user's label is ignored.

As mentioned in taskrun_test.go, the current tests do not fully
demonstrate that the TaskRun to Pod propagation works correctly since
they use Build as an intermediate form for comparison.

Fixes #458

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go

@@ -360,15 +360,19 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re
 		taskRunTimeout = nil
 	}
 
+	labels := make(map[string]string, len(pr.ObjectMeta.Labels)+2)
+	for key, val := range pr.ObjectMeta.Labels {
+		labels[key] = val
+	}
+	labels[pipeline.GroupName+pipeline.PipelineLabelKey] = pr.Spec.PipelineRef.Name
+	labels[pipeline.GroupName+pipeline.PipelineRunLabelKey] = pr.Name
+
 	tr := &v1alpha1.TaskRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            rprt.TaskRunName,
 			Namespace:       pr.Namespace,
 			OwnerReferences: pr.GetOwnerReference(),
-			Labels: map[string]string{
-				pipeline.GroupName + pipeline.PipelineLabelKey:    pr.Spec.PipelineRef.Name,
-				pipeline.GroupName + pipeline.PipelineRunLabelKey: pr.Name,
-			},
+			Labels:          labels,
 		},
 		Spec: v1alpha1.TaskRunSpec{
 			TaskRef: &v1alpha1.TaskRef{

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go

@@ -586,3 +586,64 @@ func TestReconcileWithTimeout(t *testing.T) {
 		t.Errorf("TaskRun timeout %s should be less than or equal to PipelineRun timeout %s", actual.Spec.Timeout.Duration.String(), prs[0].Spec.Timeout.Duration.String())
 	}
 }
+
+func TestReconcilePropagateLabels(t *testing.T) {
+	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-labels", "foo",
+		tb.PipelineRunLabel("PipelineRunLabel", "PipelineRunValue"),
+		tb.PipelineRunLabel("pipeline.knative.dev/pipeline", "WillNotBeUsed"),
+		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(d, fr)
+	c := testAssets.Controller
+	clients := testAssets.Clients
+
+	err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-with-labels")
+	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.Pipeline().PipelineRuns("foo").Get("test-pipeline-run-with-labels", 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-labels-hello-world-1", "foo",
+		tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-with-labels",
+			tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1"),
+			tb.Controller, tb.BlockOwnerDeletion,
+		),
+		tb.TaskRunLabel("pipeline.knative.dev/pipeline", "test-pipeline"),
+		tb.TaskRunLabel("pipeline.knative.dev/pipelineRun", "test-pipeline-run-with-labels"),
+		tb.TaskRunLabel("PipelineRunLabel", "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

@@ -30,7 +30,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 
 	"github.com/knative/build-pipeline/pkg/credentials"
@@ -220,6 +219,14 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po
 	}
 	annotations["sidecar.istio.io/inject"] = "false"
 
+	labels := map[string]string{}
+	for key, val := range build.ObjectMeta.Labels {
+		labels[key] = val
+	}
+	// TODO: Redundant with TaskRun label set in `taskrun.makeLabels`. Should
+	// probably be removed once we translate from TaskRun to Pod directly.
+	labels[buildNameLabelKey] = build.Name
+
 	cred, secrets, err := makeCredentialInitializer(build, kubeclient)
 	if err != nil {
 		return nil, err
@@ -298,18 +305,10 @@ func MakePod(build *v1alpha1.Build, kubeclient kubernetes.Interface) (*corev1.Po
 			// is deleted and re-created with the same name.
 			// We don't use GenerateName here because k8s fakes don't support it.
 			Name: fmt.Sprintf("%s-pod-%s", build.Name, gibberish),
-			// If our parent Build is deleted, then we should be as well.
-			OwnerReferences: []metav1.OwnerReference{
-				*metav1.NewControllerRef(build, schema.GroupVersionKind{
-					Group:   v1alpha1.SchemeGroupVersion.Group,
-					Version: v1alpha1.SchemeGroupVersion.Version,
-					Kind:    "Build",
-				}),
-			},
+			// If our parent TaskRun is deleted, then we should be as well.
+			OwnerReferences: build.OwnerReferences,
 			Annotations: annotations,
-			Labels: map[string]string{
-				buildNameLabelKey: build.Name,
-			},
+			Labels: labels,
 		},
 		Spec: corev1.PodSpec{
 			// If the build fails, don't restart it.

pkg/reconciler/v1alpha1/taskrun/taskrun.go

@@ -430,14 +430,6 @@ func (c *Reconciler) createBuildPod(ctx context.Context, tr *v1alpha1.TaskRun, t
 		return nil, fmt.Errorf("translating Build to Pod: %v", err)
 	}
 
-	// Rewrite the pod's OwnerRef to point the TaskRun, instead of a
-	// non-existent build.
-	// TODO(jasonhall): Just set this directly when creating a Pod from a
-	// TaskRun.
-	pod.OwnerReferences = []metav1.OwnerReference{
-		*metav1.NewControllerRef(tr, groupVersionKind),
-	}
-
 	return c.KubeClientSet.CoreV1().Pods(tr.Namespace).Create(pod)
 }
 
@@ -488,10 +480,10 @@ func createRedirectedBuild(ctx context.Context, bs *buildv1alpha1.BuildSpec, tr
 // makeLabels constructs the labels we will apply to TaskRun resources.
 func makeLabels(s *v1alpha1.TaskRun) map[string]string {
 	labels := make(map[string]string, len(s.ObjectMeta.Labels)+1)
-	labels[pipeline.GroupName+pipeline.TaskRunLabelKey] = s.Name
 	for k, v := range s.ObjectMeta.Labels {
 		labels[k] = v
 	}
+	labels[pipeline.GroupName+pipeline.TaskRunLabelKey] = s.Name
 	return labels
 }