From beda1f879cba8c228c2c39e98673d0e8f191566d Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Fri, 8 Feb 2019 17:34:30 -0500 Subject: [PATCH] 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 --- .../v1alpha1/pipelinerun/pipelinerun.go | 12 +- .../v1alpha1/pipelinerun/pipelinerun_test.go | 61 ++++ .../v1alpha1/taskrun/resources/pod.go | 23 +- pkg/reconciler/v1alpha1/taskrun/taskrun.go | 10 +- .../v1alpha1/taskrun/taskrun_test.go | 280 +++++++++++------- test/builder/build.go | 58 +++- test/builder/build_test.go | 71 +++-- test/builder/owner_reference.go | 28 ++ test/builder/pipeline.go | 10 + test/builder/pipeline_test.go | 10 +- test/builder/task.go | 10 - 11 files changed, 407 insertions(+), 166 deletions(-) create mode 100644 test/builder/owner_reference.go diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index c3edc1759f6..ddbd33e8803 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/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{ diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 1718152de2c..56fcaf62585 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/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) + } +} diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go index 782ccd5c844..c94804a89fd 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go +++ b/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. diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun.go b/pkg/reconciler/v1alpha1/taskrun/taskrun.go index 68a0754edfc..27ba5254058 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun.go +++ b/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 } diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index 4d001163436..758b18e6b1b 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -43,12 +43,20 @@ import ( ) const ( - entrypointLocation = "/tools/entrypoint" - toolsMountName = "tools" + entrypointLocation = "/tools/entrypoint" + toolsMountName = "tools" + buildNameLabelKey = "build.knative.dev/buildName" + taskRunNameLabelKey = "pipeline.knative.dev/taskRun" ) var ( ignoreLastTransitionTime = cmpopts.IgnoreTypes(duckv1alpha1.Condition{}.LastTransitionTime.Inner.Time) + // Pods are created with a random 3-byte (6 hex character) suffix that we want to ignore in our diffs. + ignoreRandomPodNameSuffix = cmp.FilterPath(func(path cmp.Path) bool { + return path.GoString() == "{*v1.Pod}.ObjectMeta.Name" + }, cmp.Comparer(func(name1, name2 string) bool { + return name1[:len(name1)-6] == name2[:len(name2)-6] + })) toolsMount = corev1.VolumeMount{ Name: toolsMountName, @@ -225,10 +233,21 @@ func TestReconcile(t *testing.T) { taskRunWithClusterTask := tb.TaskRun("test-taskrun-with-cluster-task", "foo", tb.TaskRunSpec(tb.TaskRunTaskRef(clustertask.Name, tb.TaskRefKind(v1alpha1.ClusterTaskKind))), ) + + taskRunWithLabels := tb.TaskRun("test-taskrun-with-labels", "foo", + tb.TaskRunLabel("TaskRunLabel", "TaskRunValue"), + tb.TaskRunLabel(buildNameLabelKey, "WillNotBeUsed"), + tb.TaskRunLabel(taskRunNameLabelKey, "WillNotBeUsed"), + tb.TaskRunSpec( + tb.TaskRunTaskRef(simpleTask.Name), + ), + ) + taskruns := []*v1alpha1.TaskRun{ taskRunSuccess, taskRunWithSaSuccess, taskRunTemplating, taskRunInputOutput, taskRunWithTaskSpec, taskRunWithClusterTask, taskRunWithResourceSpecAndTaskSpec, + taskRunWithLabels, } d := test.Data{ @@ -238,132 +257,191 @@ func TestReconcile(t *testing.T) { PipelineResources: []*v1alpha1.PipelineResource{gitResource, anotherGitResource, imageResource}, } for _, tc := range []struct { - name string - taskRun *v1alpha1.TaskRun - wantBuildSpec buildv1alpha1.BuildSpec + name string + taskRun *v1alpha1.TaskRun + wantBuild *buildv1alpha1.Build }{{ name: "success", taskRun: taskRunSuccess, - wantBuildSpec: tb.BuildSpec( - entrypointCopyStep, - tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), - entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + wantBuild: tb.Build("test-taskrun-run-success", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-run-success"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-run-success"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-run-success", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + entrypointCopyStep, + tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), + entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunSuccess.Name)), ), - tb.BuildVolume(getToolsVolume(taskRunSuccess.Name)), ), }, { name: "serviceaccount", taskRun: taskRunWithSaSuccess, - wantBuildSpec: tb.BuildSpec( - tb.BuildServiceAccountName("test-sa"), - entrypointCopyStep, - tb.BuildStep("sa-step", "foo", tb.Command(entrypointLocation), - entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + wantBuild: tb.Build("test-taskrun-with-sa-run-success", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-with-sa-run-success"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-with-sa-run-success", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + tb.BuildServiceAccountName("test-sa"), + entrypointCopyStep, + tb.BuildStep("sa-step", "foo", tb.Command(entrypointLocation), + entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunWithSaSuccess.Name)), ), - tb.BuildVolume(getToolsVolume(taskRunWithSaSuccess.Name)), ), }, { name: "params", taskRun: taskRunTemplating, - wantBuildSpec: tb.BuildSpec( - tb.BuildStep("git-source-git-resource", "override-with-git:latest", - tb.Args("-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), - tb.VolumeMount(corev1.VolumeMount{ - Name: "workspace", - MountPath: workspaceDir, - }), - tb.VolumeMount(implicitBuilderVolumeMounts), - ), - entrypointCopyStep, - tb.BuildStep("mycontainer", "myimage", tb.Command(entrypointLocation), - tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-arg=foo","--my-arg-with-default=bar","--my-arg-with-default2=thedefault","--my-additional-arg=gcr.io/kristoff/sven"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), - tb.VolumeMount(toolsMount), - ), - tb.BuildStep("myothercontainer", "myotherimage", tb.Command(entrypointLocation), - tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-other-arg=https://foo.git"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), - tb.VolumeMount(toolsMount), + wantBuild: tb.Build("test-taskrun-templating", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-templating"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-templating"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-templating", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + tb.BuildStep("git-source-git-resource", "override-with-git:latest", + tb.Args("-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), + tb.VolumeMount(corev1.VolumeMount{ + Name: "workspace", + MountPath: workspaceDir, + }), + tb.VolumeMount(implicitBuilderVolumeMounts), + ), + entrypointCopyStep, + tb.BuildStep("mycontainer", "myimage", tb.Command(entrypointLocation), + tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-arg=foo","--my-arg-with-default=bar","--my-arg-with-default2=thedefault","--my-additional-arg=gcr.io/kristoff/sven"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), + tb.VolumeMount(toolsMount), + ), + tb.BuildStep("myothercontainer", "myotherimage", tb.Command(entrypointLocation), + tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-other-arg=https://foo.git"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), + tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunTemplating.Name)), ), - tb.BuildVolume(getToolsVolume(taskRunTemplating.Name)), ), }, { name: "wrap-steps", taskRun: taskRunInputOutput, - wantBuildSpec: tb.BuildSpec( - tb.BuildStep("create-dir-another-git-resource", "override-with-bash-noop:latest", - tb.Args("-args", "mkdir -p /workspace/another-git-resource"), - ), - tb.BuildStep("source-copy-another-git-resource-0", "override-with-bash-noop:latest", - tb.Args("-args", "cp -r source-folder/. /workspace/another-git-resource"), - tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), - ), - tb.BuildStep("create-dir-git-resource", "override-with-bash-noop:latest", - tb.Args("-args", "mkdir -p /workspace/git-resource"), - ), - tb.BuildStep("source-copy-git-resource-0", "override-with-bash-noop:latest", - tb.Args("-args", "cp -r source-folder/. /workspace/git-resource"), - tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), - ), - entrypointCopyStep, - tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), - entrypointOptionEnvVar, tb.VolumeMount(toolsMount), - ), - tb.BuildStep("source-mkdir-git-resource", "override-with-bash-noop:latest", - tb.Args("-args", "mkdir -p output-folder"), - tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), - ), - tb.BuildStep("source-copy-git-resource", "override-with-bash-noop:latest", - tb.Args("-args", "cp -r /workspace/git-resource/. output-folder"), - tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), + wantBuild: tb.Build("test-taskrun-input-output", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-input-output"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-input-output"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-input-output", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + tb.BuildStep("create-dir-another-git-resource", "override-with-bash-noop:latest", + tb.Args("-args", "mkdir -p /workspace/another-git-resource"), + ), + tb.BuildStep("source-copy-another-git-resource-0", "override-with-bash-noop:latest", + tb.Args("-args", "cp -r source-folder/. /workspace/another-git-resource"), + tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), + ), + tb.BuildStep("create-dir-git-resource", "override-with-bash-noop:latest", + tb.Args("-args", "mkdir -p /workspace/git-resource"), + ), + tb.BuildStep("source-copy-git-resource-0", "override-with-bash-noop:latest", + tb.Args("-args", "cp -r source-folder/. /workspace/git-resource"), + tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), + ), + entrypointCopyStep, + tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), + entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + ), + tb.BuildStep("source-mkdir-git-resource", "override-with-bash-noop:latest", + tb.Args("-args", "mkdir -p output-folder"), + tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), + ), + tb.BuildStep("source-copy-git-resource", "override-with-bash-noop:latest", + tb.Args("-args", "cp -r /workspace/git-resource/. output-folder"), + tb.VolumeMount(corev1.VolumeMount{Name: "test-pvc", MountPath: "/pvc"}), + ), + tb.BuildVolume(getToolsVolume(taskRunInputOutput.Name)), + tb.BuildVolume(resources.GetPVCVolume(taskRunInputOutput.GetPipelineRunPVCName())), ), - tb.BuildVolume(getToolsVolume(taskRunInputOutput.Name)), - tb.BuildVolume(resources.GetPVCVolume(taskRunInputOutput.GetPipelineRunPVCName())), ), }, { name: "taskrun-with-taskspec", taskRun: taskRunWithTaskSpec, - wantBuildSpec: tb.BuildSpec( - tb.BuildStep("git-source-git-resource", "override-with-git:latest", - tb.Args("-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), - tb.VolumeMount(corev1.VolumeMount{ - Name: "workspace", - MountPath: workspaceDir, - }), - tb.VolumeMount(implicitBuilderVolumeMounts), - ), - entrypointCopyStep, - tb.BuildStep("mycontainer", "myimage", tb.Command(entrypointLocation), - tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-arg=foo"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), - tb.VolumeMount(toolsMount), + wantBuild: tb.Build("test-taskrun-with-taskspec", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-with-taskspec"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-with-taskspec"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-with-taskspec", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + tb.BuildStep("git-source-git-resource", "override-with-git:latest", + tb.Args("-url", "https://foo.git", "-revision", "master", "-path", "/workspace/workspace"), + tb.VolumeMount(corev1.VolumeMount{ + Name: "workspace", + MountPath: workspaceDir, + }), + tb.VolumeMount(implicitBuilderVolumeMounts), + ), + entrypointCopyStep, + tb.BuildStep("mycontainer", "myimage", tb.Command(entrypointLocation), + tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["/mycmd","--my-arg=foo"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), + tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunWithTaskSpec.Name)), ), - tb.BuildVolume(getToolsVolume(taskRunWithTaskSpec.Name)), ), }, { name: "success-with-cluster-task", taskRun: taskRunWithClusterTask, - wantBuildSpec: tb.BuildSpec(entrypointCopyStep, - tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), - entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + wantBuild: tb.Build("test-taskrun-with-cluster-task", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-with-cluster-task"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-with-cluster-task"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-with-cluster-task", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec(entrypointCopyStep, + tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), + entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunWithClusterTask.Name)), ), - tb.BuildVolume(getToolsVolume(taskRunWithClusterTask.Name)), ), }, { name: "taskrun-with-resource-spec-task-spec", taskRun: taskRunWithResourceSpecAndTaskSpec, - wantBuildSpec: tb.BuildSpec( - tb.BuildStep("git-source-workspace", "override-with-git:latest", - tb.Args("-url", "github.com/build-pipeline.git", "-revision", "rel-can", "-path", "/workspace/workspace"), - tb.VolumeMount(corev1.VolumeMount{ - Name: "workspace", - MountPath: workspaceDir, - }), - tb.VolumeMount(implicitBuilderVolumeMounts), + wantBuild: tb.Build("test-taskrun-with-resource-spec", "foo", + tb.BuildLabel(buildNameLabelKey, "test-taskrun-with-resource-spec"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-with-resource-spec"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-with-resource-spec", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + tb.BuildStep("git-source-workspace", "override-with-git:latest", + tb.Args("-url", "github.com/build-pipeline.git", "-revision", "rel-can", "-path", "/workspace/workspace"), + tb.VolumeMount(corev1.VolumeMount{ + Name: "workspace", + MountPath: workspaceDir, + }), + tb.VolumeMount(implicitBuilderVolumeMounts), + ), + entrypointCopyStep, + tb.BuildStep("mystep", "ubuntu", tb.Command(entrypointLocation), + tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["mycmd"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), + tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunWithResourceSpecAndTaskSpec.Name)), ), - entrypointCopyStep, - tb.BuildStep("mystep", "ubuntu", tb.Command(entrypointLocation), - tb.EnvVar("ENTRYPOINT_OPTIONS", `{"args":["mycmd"],"process_log":"/tools/process-log.txt","marker_file":"/tools/marker-file.txt"}`), - tb.VolumeMount(toolsMount), + ), + }, { + name: "taskrun-with-labels", + taskRun: taskRunWithLabels, + wantBuild: tb.Build("test-taskrun-with-labels", "foo", + tb.BuildLabel("TaskRunLabel", "TaskRunValue"), + tb.BuildLabel(buildNameLabelKey, "test-taskrun-with-labels"), + tb.BuildLabel(taskRunNameLabelKey, "test-taskrun-with-labels"), + tb.BuildOwnerReference("TaskRun", "test-taskrun-with-labels", + tb.OwnerReferenceAPIVersion("pipeline.knative.dev/v1alpha1")), + tb.BuildSpec( + entrypointCopyStep, + tb.BuildStep("simple-step", "foo", tb.Command(entrypointLocation), + entrypointOptionEnvVar, tb.VolumeMount(toolsMount), + ), + tb.BuildVolume(getToolsVolume(taskRunWithLabels.Name)), ), - tb.BuildVolume(getToolsVolume(taskRunWithResourceSpecAndTaskSpec.Name)), ), }} { t.Run(tc.name, func(t *testing.T) { @@ -413,15 +491,17 @@ func TestReconcile(t *testing.T) { if err != nil { t.Fatalf("Failed to fetch build pod: %v", err) } - wantPod, err := resources.MakePod(&buildv1alpha1.Build{ - Spec: tc.wantBuildSpec, - }, clients.Kube) + // TODO: Using MakePod means that a diff will not catch issues + // specific to the Build to Pod translation (e.g. if labels are + // not propagated in MakePod). To avoid this issue we should create + // a builder for Pods and use that intead. + wantPod, err := resources.MakePod(tc.wantBuild, clients.Kube) if err != nil { t.Fatalf("MakePod: %v", err) } - if d := cmp.Diff(pod.Spec, wantPod.Spec); d != "" { - t.Errorf("pod spec doesn't match, diff: %s", d) + if d := cmp.Diff(pod, wantPod, ignoreRandomPodNameSuffix); d != "" { + t.Errorf("pod doesn't match, diff: %s", d) } if len(clients.Kube.Actions()) == 0 { t.Fatalf("Expected actions to be logged in the kubeclient, got none") @@ -761,11 +841,11 @@ func TestCreateRedirectedBuild(t *testing.T) { tr := tb.TaskRun("tr", "tr", tb.TaskRunSpec( tb.TaskRunServiceAccount("sa"), )) - bs := tb.BuildSpec( + bs := tb.Build("tr", "tr", tb.BuildSpec( tb.BuildStep("foo1", "bar1", tb.Command("abcd"), tb.Args("efgh")), tb.BuildStep("foo2", "bar2", tb.Command("abcd"), tb.Args("efgh")), tb.BuildVolume(corev1.Volume{Name: "v"}), - ) + )).Spec expectedSteps := len(bs.Steps) + 1 expectedVolumes := len(bs.Volumes) + 1 diff --git a/test/builder/build.go b/test/builder/build.go index adc49bb5d17..cf450f83316 100644 --- a/test/builder/build.go +++ b/test/builder/build.go @@ -16,8 +16,12 @@ package builder import ( buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +// BuildOp is an operation which modifies a Build struct. +type BuildOp func(*buildv1alpha1.Build) + // BuildSpecOp is an operation which modify a BuildSpec struct. type BuildSpecOp func(*buildv1alpha1.BuildSpec) @@ -27,14 +31,58 @@ type SourceSpecOp func(*buildv1alpha1.SourceSpec) // ContainerOp is an operation which modify a Container struct. type ContainerOp func(*corev1.Container) +// Build creates a Build with default values. +// Any number of Build modifier can be passed to transform it. +func Build(name, namespace string, ops ...BuildOp) *buildv1alpha1.Build { + build := &buildv1alpha1.Build{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + } + for _, op := range ops { + op(build) + } + return build +} + +func BuildLabel(key, value string) BuildOp { + return func(build *buildv1alpha1.Build) { + if build.ObjectMeta.Labels == nil { + build.ObjectMeta.Labels = map[string]string{} + } + build.ObjectMeta.Labels[key] = value + } +} + +// BuildOwnerReference sets the OwnerReference, with specified kind and name, to the Build. +func BuildOwnerReference(kind, name string, ops ...OwnerReferenceOp) BuildOp { + controller := true + blockOwnerDeletion := true + return func(build *buildv1alpha1.Build) { + o := &metav1.OwnerReference{ + Kind: kind, + Name: name, + Controller: &controller, + BlockOwnerDeletion: &blockOwnerDeletion, + } + for _, op := range ops { + op(o) + } + build.ObjectMeta.OwnerReferences = append(build.ObjectMeta.OwnerReferences, *o) + } +} + // BuildSpec creates a BuildSpec with default values. // Any number of BuildSpec modifier can be passed to transform it. -func BuildSpec(ops ...BuildSpecOp) buildv1alpha1.BuildSpec { - buildSpec := &buildv1alpha1.BuildSpec{} - for _, op := range ops { - op(buildSpec) +func BuildSpec(ops ...BuildSpecOp) BuildOp { + return func(build *buildv1alpha1.Build) { + buildSpec := &build.Spec + for _, op := range ops { + op(buildSpec) + } + build.Spec = *buildSpec } - return *buildSpec } // BuildServiceAccountName sets the service account to the BuildSpec. diff --git a/test/builder/build_test.go b/test/builder/build_test.go index 3de7c1ae2cd..db577ccc4c7 100644 --- a/test/builder/build_test.go +++ b/test/builder/build_test.go @@ -20,9 +20,11 @@ import ( tb "github.com/knative/build-pipeline/test/builder" buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestBuildSpec(t *testing.T) { +func TestBuild(t *testing.T) { + trueB := true toolsMount := corev1.VolumeMount{ Name: "tools-volume", MountPath: "/tools", @@ -31,31 +33,52 @@ func TestBuildSpec(t *testing.T) { Name: "tools-volume", VolumeSource: corev1.VolumeSource{}, } - buildSpec := tb.BuildSpec( - tb.BuildServiceAccountName("sa"), - tb.BuildStep("simple-step", "foo", - tb.Command("/mycmd"), tb.Args("my", "args"), - tb.VolumeMount(toolsMount), + build := tb.Build("build-foo", "foo", + tb.BuildLabel("label", "label-value"), + tb.BuildOwnerReference("TaskRun", "taskrun-foo", + tb.OwnerReferenceAPIVersion("a1")), + tb.BuildSpec( + tb.BuildServiceAccountName("sa"), + tb.BuildStep("simple-step", "foo", + tb.Command("/mycmd"), tb.Args("my", "args"), + tb.VolumeMount(toolsMount), + ), + tb.BuildSource("foo", tb.BuildSourceGit("https://foo.git", "master")), + tb.BuildVolume(volume), ), - tb.BuildSource("foo", tb.BuildSourceGit("https://foo.git", "master")), - tb.BuildVolume(volume), ) - expectedBuildSpec := buildv1alpha1.BuildSpec{ - ServiceAccountName: "sa", - Steps: []corev1.Container{{ - Name: "simple-step", - Image: "foo", - Command: []string{"/mycmd"}, - Args: []string{"my", "args"}, - VolumeMounts: []corev1.VolumeMount{toolsMount}, - }}, - Sources: []buildv1alpha1.SourceSpec{{ - Name: "foo", - Git: &buildv1alpha1.GitSourceSpec{Url: "https://foo.git", Revision: "master"}, - }}, - Volumes: []corev1.Volume{volume}, + expectedBuild := &buildv1alpha1.Build{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "build-foo", + Labels: map[string]string{ + "label": "label-value", + }, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "TaskRun", + Name: "taskrun-foo", + APIVersion: "a1", + Controller: &trueB, + BlockOwnerDeletion: &trueB, + }}, + }, + Spec: buildv1alpha1.BuildSpec{ + ServiceAccountName: "sa", + Steps: []corev1.Container{{ + Name: "simple-step", + Image: "foo", + Command: []string{"/mycmd"}, + Args: []string{"my", "args"}, + VolumeMounts: []corev1.VolumeMount{toolsMount}, + }}, + Sources: []buildv1alpha1.SourceSpec{{ + Name: "foo", + Git: &buildv1alpha1.GitSourceSpec{Url: "https://foo.git", Revision: "master"}, + }}, + Volumes: []corev1.Volume{volume}, + }, } - if d := cmp.Diff(expectedBuildSpec, buildSpec); d != "" { - t.Fatalf("BuildSpec diff -want, +got: %v", d) + if d := cmp.Diff(expectedBuild, build); d != "" { + t.Fatalf("Build diff -want, +got: %v", d) } } diff --git a/test/builder/owner_reference.go b/test/builder/owner_reference.go new file mode 100644 index 00000000000..7ace12aa40f --- /dev/null +++ b/test/builder/owner_reference.go @@ -0,0 +1,28 @@ +/* +Copyright 2018 The Knative Authors +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package builder + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// OwnerReferenceOp is an operation which modifies an OwnerReference struct. +type OwnerReferenceOp func(*metav1.OwnerReference) + +// OwnerReferenceAPIVersion sets the APIVersion to the OwnerReference. +func OwnerReferenceAPIVersion(version string) OwnerReferenceOp { + return func(o *metav1.OwnerReference) { + o.APIVersion = version + } +} diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go index d3886e8f4d6..8a788b78330 100644 --- a/test/builder/pipeline.go +++ b/test/builder/pipeline.go @@ -243,6 +243,16 @@ func PipelineRunSpec(name string, ops ...PipelineRunSpecOp) PipelineRunOp { } } +// PipelineRunLabels adds a label to the PipelineRun. +func PipelineRunLabel(key, value string) PipelineRunOp { + return func(pr *v1alpha1.PipelineRun) { + if pr.ObjectMeta.Labels == nil { + pr.ObjectMeta.Labels = map[string]string{} + } + pr.ObjectMeta.Labels[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) { diff --git a/test/builder/pipeline_test.go b/test/builder/pipeline_test.go index bc13d8b65c4..93ac19231e6 100644 --- a/test/builder/pipeline_test.go +++ b/test/builder/pipeline_test.go @@ -89,9 +89,15 @@ func TestPipelineRun(t *testing.T) { ), tb.PipelineRunStatus(tb.PipelineRunStatusCondition(duckv1alpha1.Condition{ Type: duckv1alpha1.ConditionSucceeded, }), tb.PipelineRunStartTime(startTime), - )) + ), tb.PipelineRunLabel("label-key", "label-value")) expectedPipelineRun := &v1alpha1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{Name: "pear", Namespace: "foo"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "pear", + Namespace: "foo", + Labels: map[string]string{ + "label-key": "label-value", + }, + }, Spec: v1alpha1.PipelineRunSpec{ PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, Trigger: v1alpha1.PipelineTrigger{Type: v1alpha1.PipelineTriggerTypeManual}, diff --git a/test/builder/task.go b/test/builder/task.go index 0a118091c48..98d1e42c4d1 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -68,9 +68,6 @@ type TaskRunOutputsOp func(*v1alpha1.TaskRunOutputs) // ResolvedTaskResourcesOp is an operation which modify a ResolvedTaskResources struct. type ResolvedTaskResourcesOp func(*resources.ResolvedTaskResources) -// OwnerReferenceOp is an operation which modify an OwnerReference struct. -type OwnerReferenceOp func(*metav1.OwnerReference) - // StepStateOp is an operation which modify a StepStep struct. type StepStateOp func(*v1alpha1.StepState) @@ -356,13 +353,6 @@ func TaskRunOwnerReference(kind, name string, ops ...OwnerReferenceOp) TaskRunOp } } -// OwnerReferenceAPIVersion sets the APIVersion to the OwnerReference. -func OwnerReferenceAPIVersion(version string) OwnerReferenceOp { - return func(o *metav1.OwnerReference) { - o.APIVersion = version - } -} - func Controller(o *metav1.OwnerReference) { o.Controller = &trueB }