Skip to content

Commit

Permalink
Propagate labels from PipelineRun to TaskRun to Pods
Browse files Browse the repository at this point in the history
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 tektoncd#458
  • Loading branch information
dwnusbaum committed Feb 11, 2019
1 parent 709bc85 commit abde008
Show file tree
Hide file tree
Showing 11 changed files with 407 additions and 166 deletions.
12 changes: 8 additions & 4 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
61 changes: 61 additions & 0 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
23 changes: 11 additions & 12 deletions pkg/reconciler/v1alpha1/taskrun/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 1 addition & 9 deletions pkg/reconciler/v1alpha1/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down
Loading

0 comments on commit abde008

Please sign in to comment.