Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate annotations from Pipeline/Task to PipelineRun/TaskRun #894

Merged
merged 1 commit into from
May 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand Down
62 changes: 62 additions & 0 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
20 changes: 11 additions & 9 deletions pkg/reconciler/v1alpha1/taskrun/resources/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down
21 changes: 15 additions & 6 deletions pkg/reconciler/v1alpha1/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
45 changes: 44 additions & 1 deletion pkg/reconciler/v1alpha1/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions test/builder/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
14 changes: 12 additions & 2 deletions test/builder/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
},
}

Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions test/builder/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
Loading