Repository: kubeflow/kfp-tekton (language: python)

Issue:
bug(controllers) PipelineLoop doesn't support podTemplate and taskRunSpecs
/kind bug

**What steps did you take and what happened:**
Add a valid `podTemplate` and `taskRunSpecs` entry to any loop example. Typical use-case would be to add:
```yaml
apiVersion: custom.tekton.dev/v1alpha1
kind: PipelineLoop
metadata:
  name: pipeline-abc
spec:
  pipelineSpec:
    (...)
    tasks:
    - name: task-a
      (...)
    - name: task-b
      (...)
    - name: task-c
      (...)
    - name: task-that-needs-account-token
      (...)
  podTemplate:
    automountServiceAccountToken: false
  taskRunSpecs:
  - pipelineTaskName: task-that-needs-account-token
    taskPodTemplate:
      podTemplate:
        automountServiceAccountToken: true
---
apiVersion: custom.tekton.dev/v1alpha1
kind: PipelineRun
metadata: (...)
spec:
  pipelineSpec:
    (...)
    tasks:
    - name: task-abc
      taskRef:
        apiVersion: custom.tekton.dev/v1alpha1
        kind: PipelineLoop
        name: pipeline-abc
    - name: task-def
      (...)
  podTemplate:
    automountServiceAccountToken: false
```

It fails on Tekton validation of that field, calling those fields unknown.


**What did you expect to happen:**
PipelineLoop to accepting any fields that PipelineRun would.

**Additional information:**
In PipelineLoop pkg I can see:
```go
// PipelineLoopSpec defines the desired state of the PipelineLoop
type PipelineLoopSpec struct {
	// TaskRef is a reference to a task definition.
	// +optional
	// TaskRef     *v1beta1.TaskRef     `json:"taskRef,omitempty"`
	PipelineRef *v1beta1.PipelineRef `json:"pipelineRef,omitempty"`

	// TaskSpec is a specification of a task
	// +optional
	PipelineSpec *v1beta1.PipelineSpec `json:"pipelineSpec,omitempty"`

	// IterateParam is the name of the task parameter that is iterated upon.
	IterateParam string `json:"iterateParam"`

	// The separator for IterateParam if the IterateParam is a strings with separator char, this field is optional.
	// +optional
	IterateParamSeparator string `json:"iterateParamStringSeparator,omitempty"`

	IterateNumeric string `json:"iterateNumeric"`

	// Time after which the TaskRun times out.
	// +optional
	Timeout *metav1.Duration `json:"timeout,omitempty"`

	// Parallelism represents how many pipelines can be triggered simutaniously by the loop.
	// +optional
	Parallelism int `json:"parallelism,omitempty"`

	// Retries represents how many times a task should be retried in case of task failure.
	// +optional
	Retries int `json:"retries,omitempty"`

	// Workspace to a volume mapping to be consumed by a PipelineRun.
	// +optional
	Workspaces []v1beta1.WorkspaceBinding `json:"workspaces,omitempty"`
}
```
i.e. contains `Workspaces` but not `PodTempalte` or `TaskRunSpecs`, as in `PipelineRunSpec`:
```go
// PipelineRunSpec defines the desired state of PipelineRun
type PipelineRunSpec struct {
	// +optional
	PipelineRef *PipelineRef `json:"pipelineRef,omitempty"`
	// +optional
	PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"`
	// Resources is a list of bindings specifying which actual instances of
	// PipelineResources to use for the resources the Pipeline has declared
	// it needs.
	Resources []PipelineResourceBinding `json:"resources,omitempty"`
	// Params is a list of parameter names and values.
	Params []Param `json:"params,omitempty"`
	// +optional
	ServiceAccountName string `json:"serviceAccountName,omitempty"`

	// Deprecated: use taskRunSpecs.ServiceAccountName instead
	// +optional
	ServiceAccountNames []PipelineRunSpecServiceAccountName `json:"serviceAccountNames,omitempty"`
	// Used for cancelling a pipelinerun (and maybe more later on)
	// +optional
	Status PipelineRunSpecStatus `json:"status,omitempty"`
	// This is an alpha field. You must set the "enable-api-fields" feature flag to "alpha"
	// for this field to be supported.
	//
	// Time after which the Pipeline times out.
	// Currently three keys are accepted in the map
	// pipeline, tasks and finally
	// with Timeouts.pipeline >= Timeouts.tasks + Timeouts.finally
	// +optional
	Timeouts *TimeoutFields `json:"timeouts,omitempty"`
	// Time after which the Pipeline times out. Defaults to never.
	// Refer to Go's ParseDuration documentation for expected format: https://golang.org/pkg/time/#ParseDuration
	// +optional
	Timeout *metav1.Duration `json:"timeout,omitempty"`
	// PodTemplate holds pod specific configuration
	PodTemplate *PodTemplate `json:"podTemplate,omitempty"`
	// Workspaces holds a set of workspace bindings that must match names
	// with those declared in the pipeline.
	// +optional
	Workspaces []WorkspaceBinding `json:"workspaces,omitempty"`
	// TaskRunSpecs holds a set of runtime specs
	// +optional
	TaskRunSpecs []PipelineTaskRunSpec `json:"taskRunSpecs,omitempty"`
}
```

I can see that the source code does mention some of those as "TODO":
```go
func (c *Reconciler) createPipelineRun(ctx context.Context, logger *zap.SugaredLogger, tls *pipelineloopv1alpha1.PipelineLoopSpec, run *v1alpha1.Run, iteration int, iterationElements []interface{}) (*v1beta1.PipelineRun, error) {

	// Create name for PipelineRun from Run name plus iteration number.
	prName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("%s-%s", run.Name, fmt.Sprintf("%05d", iteration)))
	pipelineRunAnnotations := getPipelineRunAnnotations(run)
	currentIndex := iteration - 1
	if currentIndex > len(iterationElements) {
		currentIndex = len(iterationElements) - 1
	}
	currentIterationItemBytes, _ := json.Marshal(iterationElements[currentIndex])
	pipelineRunAnnotations[pipelineloop.GroupName+pipelineLoopCurrentIterationItemAnnotationKey] = string(currentIterationItemBytes)

	pr := &v1beta1.PipelineRun{
		ObjectMeta: metav1.ObjectMeta{
			Name:      prName,
			Namespace: run.Namespace,
			OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(run,
				schema.GroupVersionKind{Group: "tekton.dev", Version: "v1alpha1", Kind: "Run"})},
			Labels:      getPipelineRunLabels(run, strconv.Itoa(iteration)),
			Annotations: pipelineRunAnnotations,
		},
		Spec: v1beta1.PipelineRunSpec{
			Params:             getParameters(run, tls, iteration),
			Timeout:            tls.Timeout,
			ServiceAccountName: "",  // TODO: Implement service account name
			PodTemplate:        nil, // TODO: Implement pod template
			Workspaces:         tls.Workspaces,
		}}

	if tls.PipelineRef != nil {
		pr.Spec.PipelineRef = &v1beta1.PipelineRef{
			Name: tls.PipelineRef.Name,
			// Kind: tls.PipelineRef.Kind,
		}
	} else if tls.PipelineSpec != nil {
		pr.Spec.PipelineSpec = tls.PipelineSpec
	}

	logger.Infof("Creating a new PipelineRun object %s", prName)
	return c.pipelineClientSet.TektonV1beta1().PipelineRuns(run.Namespace).Create(ctx, pr, metav1.CreateOptions{})

}
```


**Environment:**

* Python Version (use `python --version`): irrelevant
* SDK Version: irrelevant
* Tekton Version (use `tkn version`): v0.30.0
* Kubernetes Version (use `kubectl version`): v1.21.10+IKS
* OS (e.g. from `/etc/os-release`): irrelevant



Discussion / hints:
@Udiknedormin 
Thanks for reporting this. Gang and Jinchi no longer working on this pipelineloop controller. We can take on these TODOs and make it feature complete. 

Fix patch:
diff --git a/tekton-catalog/pipeline-loops/pkg/apis/pipelineloop/v1alpha1/pipelineloop_types.go b/tekton-catalog/pipeline-loops/pkg/apis/pipelineloop/v1alpha1/pipelineloop_types.go
index 1f267cd7c3..5d55f01d30 100644
--- a/tekton-catalog/pipeline-loops/pkg/apis/pipelineloop/v1alpha1/pipelineloop_types.go
+++ b/tekton-catalog/pipeline-loops/pkg/apis/pipelineloop/v1alpha1/pipelineloop_types.go
@@ -69,9 +69,20 @@ type PipelineLoopSpec struct {
 	// +optional
 	Retries int `json:"retries,omitempty"`
 
+	// PodTemplate holds pod specific configuration
+	// +optional
+	PodTemplate *v1beta1.PodTemplate `json:"podTemplate,omitempty"`
+
+	// +optional
+	ServiceAccountName string `json:"serviceAccountName,omitempty"`
+
 	// Workspace to a volume mapping to be consumed by a PipelineRun.
 	// +optional
 	Workspaces []v1beta1.WorkspaceBinding `json:"workspaces,omitempty"`
+
+	// TaskRunSpecs holds a set of runtime specs
+	// +optional
+	TaskRunSpecs []v1beta1.PipelineTaskRunSpec `json:"taskRunSpecs,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
diff --git a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go
index 666876b04c..9670306702 100644
--- a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go
+++ b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go
@@ -604,9 +604,10 @@ func (c *Reconciler) createPipelineRun(ctx context.Context, logger *zap.SugaredL
 		Spec: v1beta1.PipelineRunSpec{
 			Params:             getParameters(run, tls, iteration),
 			Timeout:            tls.Timeout,
-			ServiceAccountName: "",  // TODO: Implement service account name
-			PodTemplate:        nil, // TODO: Implement pod template
+			ServiceAccountName: tls.ServiceAccountName,
+			PodTemplate:        tls.PodTemplate,
 			Workspaces:         tls.Workspaces,
+			TaskRunSpecs:       tls.TaskRunSpecs,
 		}}
 
 	if tls.PipelineRef != nil {


Test patch:
diff --git a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go
index d95813b207..0b67727b41 100644
--- a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go
+++ b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go
@@ -316,6 +316,33 @@ var wsPipelineLoop = &pipelineloopv1alpha1.PipelineLoop{
 	},
 }
 
+var newPipelineLoop = &pipelineloopv1alpha1.PipelineLoop{
+	ObjectMeta: metav1.ObjectMeta{Name: "new-pipelineloop", Namespace: "foo"},
+	Spec: pipelineloopv1alpha1.PipelineLoopSpec{
+		PipelineRef:        &v1beta1.PipelineRef{Name: "a-pipeline"},
+		IterateParam:       "current-item",
+		ServiceAccountName: "default",
+		PodTemplate: &v1beta1.PodTemplate{
+			HostAliases: []corev1.HostAlias{{
+				IP:        "0.0.0.0",
+				Hostnames: []string{"localhost"},
+			}},
+			HostNetwork: true,
+		},
+		TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{{
+			PipelineTaskName:       "test-task",
+			TaskServiceAccountName: "test",
+			TaskPodTemplate: &v1beta1.PodTemplate{
+				HostAliases: []corev1.HostAlias{{
+					IP:        "0.0.0.0",
+					Hostnames: []string{"localhost"},
+				}},
+				HostNetwork: true,
+			},
+		}},
+	},
+}
+
 var nPipeline = &v1beta1.Pipeline{
 	ObjectMeta: metav1.ObjectMeta{Name: "n-pipeline", Namespace: "foo"},
 	Spec: v1beta1.PipelineSpec{
@@ -499,6 +526,34 @@ var runWsPipelineLoop = &v1alpha1.Run{
 	},
 }
 
+var runNewPipelineLoop = &v1alpha1.Run{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "run-new-pipelineloop",
+		Namespace: "foo",
+		Labels: map[string]string{
+			"myTestLabel":                    "myTestLabelValue",
+			"custom.tekton.dev/pipelineLoop": "new-pipelineloop",
+			"tekton.dev/pipeline":            "pr-loop-example",
+			"tekton.dev/pipelineRun":         "pr-loop-example",
+			"tekton.dev/pipelineTask":        "loop-task",
+		},
+		Annotations: map[string]string{
+			"myTestAnnotation": "myTestAnnotationValue",
+		},
+	},
+	Spec: v1alpha1.RunSpec{
+		Params: []v1beta1.Param{{
+			Name:  "current-item",
+			Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"item1", "item2"}},
+		}},
+		Ref: &v1alpha1.TaskRef{
+			APIVersion: pipelineloopv1alpha1.SchemeGroupVersion.String(),
+			Kind:       pipelineloop.PipelineLoopControllerName,
+			Name:       "new-pipelineloop",
+		},
+	},
+}
+
 var runPipelineLoop = &v1alpha1.Run{
 	ObjectMeta: metav1.ObjectMeta{
 		Name:      "run-pipelineloop",
@@ -1196,6 +1251,60 @@ var expectedPipelineRunWithWorkSpace = &v1beta1.PipelineRun{
 	},
 }
 
+var expectedPipelineRunWithPodTemplate = &v1beta1.PipelineRun{
+	ObjectMeta: metav1.ObjectMeta{
+		Name:      "run-new-pipelineloop-00001-9l9zj",
+		Namespace: "foo",
+		OwnerReferences: []metav1.OwnerReference{{
+			APIVersion:         "tekton.dev/v1alpha1",
+			Kind:               "Run",
+			Name:               "run-new-pipelineloop",
+			Controller:         &trueB,
+			BlockOwnerDeletion: &trueB,
+		}},
+		Labels: map[string]string{
+			"custom.tekton.dev/originalPipelineRun":   "pr-loop-example",
+			"custom.tekton.dev/parentPipelineRun":     "pr-loop-example",
+			"custom.tekton.dev/pipelineLoop":          "new-pipelineloop",
+			"tekton.dev/run":                          "run-new-pipelineloop",
+			"custom.tekton.dev/pipelineLoopIteration": "1",
+			"myTestLabel":                             "myTestLabelValue",
+		},
+		Annotations: map[string]string{
+			"myTestAnnotation": "myTestAnnotationValue",
+			"custom.tekton.dev/pipelineLoopCurrentIterationItem": `"item1"`,
+		},
+	},
+	Spec: v1beta1.PipelineRunSpec{
+		PipelineRef: &v1beta1.PipelineRef{
+			Name: "a-pipeline",
+		},
+		Params: []v1beta1.Param{{
+			Name:  "current-item",
+			Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "item1"},
+		}},
+		ServiceAccountName: "default",
+		PodTemplate: &v1beta1.PodTemplate{
+			HostAliases: []corev1.HostAlias{{
+				IP:        "0.0.0.0",
+				Hostnames: []string{"localhost"},
+			}},
+			HostNetwork: true,
+		},
+		TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{{
+			PipelineTaskName:       "test-task",
+			TaskServiceAccountName: "test",
+			TaskPodTemplate: &v1beta1.PodTemplate{
+				HostAliases: []corev1.HostAlias{{
+					IP:        "0.0.0.0",
+					Hostnames: []string{"localhost"},
+				}},
+				HostNetwork: true,
+			},
+		}},
+	},
+}
+
 var expectedPipelineRunWithInlineTaskIteration1 = &v1beta1.PipelineRun{
 	ObjectMeta: metav1.ObjectMeta{
 		Name:      "run-pipelineloop-with-inline-task-00001-9l9zj",
@@ -1511,6 +1620,16 @@ func TestReconcilePipelineLoopRun(t *testing.T) {
 		expectedReason:       pipelineloopv1alpha1.PipelineLoopRunReasonSucceeded,
 		expectedPipelineruns: []*v1beta1.PipelineRun{successfulWithSkipedTasks(expectedConditionPipelineRunIteration1)},
 		expectedEvents:       []string{"Normal Succeeded PipelineRuns completed successfully with the conditions are met"},
+	}, {
+		name:                 "Reconcile a new run with a pipelineloop that contains a PodTemplate, ServiceAccountName, TaskRunSpecs",
+		pipeline:             aPipeline,
+		pipelineloop:         newPipelineLoop,
+		run:                  runNewPipelineLoop,
+		pipelineruns:         []*v1beta1.PipelineRun{},
+		expectedStatus:       corev1.ConditionUnknown,
+		expectedReason:       pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
+		expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithPodTemplate},
+		expectedEvents:       []string{"Normal Started", "Normal Running Iterations completed: 0"},
 	}}
 
 	//testcases = testcases[1:3]