Fix Pipeline error with ClusterTasks

It is possible to use `ClusterTask` in a Pipeline, for that you specify that the `Task` you want to use is of type `ClusterTask` (default being `Task`). But, before this change, the `PipelineRun` using this `Pipeline` would fail as the `Kind` information is not passed from the `PipelineTask.TaskRef` to the actual `TaskRun`. This fixes that by passing the `Kind` of a `TaskRef` when resolving the `Task`/`TaskRun` in the `PipelineRun` reconcilier. A yaml test is added as a regression test. Signed-off-by: Vincent Demeester <[email protected]>
tektoncd · Jun 26, 2019 · cf2ecd5 · cf2ecd5
1 parent 2a5a263
commit cf2ecd5
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 11 deletions.
diff --git a/examples/pipelineruns/clustertask-pipelinerun.yaml b/examples/pipelineruns/clustertask-pipelinerun.yaml
@@ -0,0 +1,32 @@
+apiVersion: tekton.dev/v1alpha1
+kind: ClusterTask
+metadata:
+  name: cluster-task-pipeline-4
+spec:
+  steps:
+  - name: task-two-step-one
+    image: ubuntu
+    command: ["/bin/bash"]
+    args: ['-c', 'echo success']
+---
+apiVersion: tekton.dev/v1alpha1
+kind: Pipeline
+metadata:
+  name: sample-pipeline-cluster-task-4
+spec:
+  tasks:
+  - name: cluster-task-pipeline-4
+    taskRef:
+      name: cluster-task-pipeline-4
+      kind: ClusterTask
+---
+apiVersion: tekton.dev/v1alpha1
+kind: PipelineRun
+metadata:
+  name: demo-pipeline-run-4
+spec:
+  pipelineRef:
+    name: sample-pipeline-cluster-task-4
+  trigger:
+    type: manual
+  serviceAccount: 'default'
diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
@@ -504,6 +504,7 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re
 		Spec: v1alpha1.TaskRunSpec{
 			TaskRef: &v1alpha1.TaskRef{
 				Name: rprt.ResolvedTaskResources.TaskName,
+				Kind: rprt.ResolvedTaskResources.Kind,
 			},
 			Inputs: v1alpha1.TaskRunInputs{
 				Params: rprt.PipelineTask.Params,
@@ -513,7 +514,8 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re
 			NodeSelector:   pr.Spec.NodeSelector,
 			Tolerations:    pr.Spec.Tolerations,
 			Affinity:       pr.Spec.Affinity,
-		}}
+		},
+	}
 
 	resources.WrapSteps(&tr.Spec, rprt.PipelineTask, rprt.ResolvedTaskResources.Inputs, rprt.ResolvedTaskResources.Outputs, storageBasePath)
 

diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go
@@ -246,7 +246,7 @@ func ResolvePipelineRun(
 		}
 
 		spec := t.TaskSpec()
-		rtr, err := resources.ResolveTaskResources(&spec, t.TaskMetadata().Name, inputs, outputs, getResource)
+		rtr, err := resources.ResolveTaskResources(&spec, t.TaskMetadata().Name, pt.TaskRef.Kind, inputs, outputs, getResource)
 		if err != nil {
 			return nil, &ResourceNotFoundError{Msg: err.Error()}
 		}

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/taskresourceresolution.go b/pkg/reconciler/v1alpha1/taskrun/resources/taskresourceresolution.go
@@ -25,6 +25,7 @@ import (
 // the TaskRun: the TaskRun, it's Task and the PipelineResources it needs.
 type ResolvedTaskResources struct {
 	TaskName string
+	Kind     v1alpha1.TaskKind
 	TaskSpec *v1alpha1.TaskSpec
 	// Inputs is a map from the name of the input required by the Task
 	// to the actual Resource to use for it
@@ -40,10 +41,11 @@ type GetResource func(string) (*v1alpha1.PipelineResource, error)
 // ResolveTaskResources looks up PipelineResources referenced by inputs and outputs and returns
 // a structure that unites the resolved references and the Task Spec. If referenced PipelineResources
 // can't be found, an error is returned.
-func ResolveTaskResources(ts *v1alpha1.TaskSpec, taskName string, inputs []v1alpha1.TaskResourceBinding, outputs []v1alpha1.TaskResourceBinding, gr GetResource) (*ResolvedTaskResources, error) {
+func ResolveTaskResources(ts *v1alpha1.TaskSpec, taskName string, kind v1alpha1.TaskKind, inputs []v1alpha1.TaskResourceBinding, outputs []v1alpha1.TaskResourceBinding, gr GetResource) (*ResolvedTaskResources, error) {
 	rtr := ResolvedTaskResources{
 		TaskName: taskName,
 		TaskSpec: ts,
+		Kind:     kind,
 		Inputs:   map[string]*v1alpha1.PipelineResource{},
 		Outputs:  map[string]*v1alpha1.PipelineResource{},
 	}

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/taskresourceresolution_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/taskresourceresolution_test.go
@@ -61,6 +61,7 @@ func TestResolveTaskRun(t *testing.T) {
 	}}
 
 	taskName := "orchestrate"
+	kind := v1alpha1.NamespacedTaskKind
 	taskSpec := v1alpha1.TaskSpec{
 		Steps: []corev1.Container{{
 			Name: "step1",
@@ -90,7 +91,7 @@ func TestResolveTaskRun(t *testing.T) {
 		return r, nil
 	}
 
-	rtr, err := ResolveTaskResources(&taskSpec, taskName, inputs, outputs, gr)
+	rtr, err := ResolveTaskResources(&taskSpec, taskName, kind, inputs, outputs, gr)
 	if err != nil {
 		t.Fatalf("Did not expect error trying to resolve TaskRun: %s", err)
 	}
@@ -170,7 +171,7 @@ func TestResolveTaskRun_missingOutput(t *testing.T) {
 		}}}
 
 	gr := func(n string) (*v1alpha1.PipelineResource, error) { return nil, xerrors.New("nope") }
-	_, err := ResolveTaskResources(&v1alpha1.TaskSpec{}, "orchestrate", []v1alpha1.TaskResourceBinding{}, outputs, gr)
+	_, err := ResolveTaskResources(&v1alpha1.TaskSpec{}, "orchestrate", v1alpha1.NamespacedTaskKind, []v1alpha1.TaskResourceBinding{}, outputs, gr)
 	if err == nil {
 		t.Fatalf("Expected to get error because output resource couldn't be resolved")
 	}
@@ -184,7 +185,7 @@ func TestResolveTaskRun_missingInput(t *testing.T) {
 		}}}
 	gr := func(n string) (*v1alpha1.PipelineResource, error) { return nil, xerrors.New("nope") }
 
-	_, err := ResolveTaskResources(&v1alpha1.TaskSpec{}, "orchestrate", inputs, []v1alpha1.TaskResourceBinding{}, gr)
+	_, err := ResolveTaskResources(&v1alpha1.TaskSpec{}, "orchestrate", v1alpha1.NamespacedTaskKind, inputs, []v1alpha1.TaskResourceBinding{}, gr)
 	if err == nil {
 		t.Fatalf("Expected to get error because output resource couldn't be resolved")
 	}
@@ -198,7 +199,7 @@ func TestResolveTaskRun_noResources(t *testing.T) {
 
 	gr := func(n string) (*v1alpha1.PipelineResource, error) { return &v1alpha1.PipelineResource{}, nil }
 
-	rtr, err := ResolveTaskResources(&taskSpec, "orchestrate", []v1alpha1.TaskResourceBinding{}, []v1alpha1.TaskResourceBinding{}, gr)
+	rtr, err := ResolveTaskResources(&taskSpec, "orchestrate", v1alpha1.NamespacedTaskKind, []v1alpha1.TaskResourceBinding{}, []v1alpha1.TaskResourceBinding{}, gr)
 	if err != nil {
 		t.Fatalf("Did not expect error trying to resolve TaskRun: %s", err)
 	}

diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun.go b/pkg/reconciler/v1alpha1/taskrun/taskrun.go
@@ -193,8 +193,9 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 	return err
 }
 
-func (c *Reconciler) getTaskFunc(tr *v1alpha1.TaskRun) resources.GetTask {
+func (c *Reconciler) getTaskFunc(tr *v1alpha1.TaskRun) (resources.GetTask, v1alpha1.TaskKind) {
 	var gtFunc resources.GetTask
+	kind := v1alpha1.NamespacedTaskKind
 	if tr.Spec.TaskRef != nil && tr.Spec.TaskRef.Kind == v1alpha1.ClusterTaskKind {
 		gtFunc = func(name string) (v1alpha1.TaskInterface, error) {
 			t, err := c.clusterTaskLister.Get(name)
@@ -203,6 +204,7 @@ func (c *Reconciler) getTaskFunc(tr *v1alpha1.TaskRun) resources.GetTask {
 			}
 			return t, nil
 		}
+		kind = v1alpha1.ClusterTaskKind
 	} else {
 		gtFunc = func(name string) (v1alpha1.TaskInterface, error) {
 			t, err := c.taskLister.Tasks(tr.Namespace).Get(name)
@@ -212,7 +214,7 @@ func (c *Reconciler) getTaskFunc(tr *v1alpha1.TaskRun) resources.GetTask {
 			return t, nil
 		}
 	}
-	return gtFunc
+	return gtFunc, kind
 }
 
 func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error {
@@ -225,7 +227,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error
 		return err
 	}
 
-	getTaskFunc := c.getTaskFunc(tr)
+	getTaskFunc, kind := c.getTaskFunc(tr)
 	taskMeta, taskSpec, err := resources.GetTaskData(tr, getTaskFunc)
 	if err != nil {
 		c.Logger.Errorf("Failed to determine Task spec to use for taskrun %s: %v", tr.Name, err)
@@ -265,7 +267,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error
 		return nil
 	}
 
-	rtr, err := resources.ResolveTaskResources(taskSpec, taskMeta.Name, tr.Spec.Inputs.Resources, tr.Spec.Outputs.Resources, c.resourceLister.PipelineResources(tr.Namespace).Get)
+	rtr, err := resources.ResolveTaskResources(taskSpec, taskMeta.Name, kind, tr.Spec.Inputs.Resources, tr.Spec.Outputs.Resources, c.resourceLister.PipelineResources(tr.Namespace).Get)
 	if err != nil {
 		c.Logger.Errorf("Failed to resolve references for taskrun %s: %v", tr.Name, err)
 		tr.Status.SetCondition(&apis.Condition{