Prevent infinite TaskRun creation for long PipelineRun+PipelineTask.N…

…ames

Without this, a base TaskRun name longer than 57 (i.e.,
`maxGeneratedNameLength`) characters will not match the names of any
existing `TaskRun`s, resulting in an infinite number of `TaskRun`s
getting created.

pkg/apis/pipeline/v1alpha1/pipelinerun_types.go

@@ -125,9 +125,18 @@ type PipelineRunStatus struct {
 	// CompletionTime is the time the PipelineRun completed.
 	// +optional
 	CompletionTime *metav1.Time `json:"completionTime,omitempty"`
-	// map of TaskRun Status with the taskRun name as the key
-	//+optional
-	TaskRuns map[string]TaskRunStatus `json:"taskRuns,omitempty"`
+	// map of PipelineRunTaskRunStatus with the taskRun name as the key
+	// +optional
+	TaskRuns map[string]*PipelineRunTaskRunStatus `json:"taskRuns,omitempty"`
+}
+
+// PipelineRunTaskRunStatus contains the name of the PipelineTask for this TaskRun and the TaskRun's Status
+type PipelineRunTaskRunStatus struct {
+	// PipelineTaskName is the name of the PipelineTask
+	PipelineTaskName string `json:"pipelineTaskName"`
+	// Status is the TaskRunStatus for the corresponding TaskRun
+	// +optional
+	Status *TaskRunStatus `json:"status,omitempty"`
 }
 
 var pipelineRunCondSet = duckv1alpha1.NewBatchConditionSet()
@@ -140,7 +149,7 @@ func (pr *PipelineRunStatus) GetCondition(t duckv1alpha1.ConditionType) *duckv1a
 // InitializeConditions will set all conditions in pipelineRunCondSet to unknown for the PipelineRun
 func (pr *PipelineRunStatus) InitializeConditions() {
 	if pr.TaskRuns == nil {
-		pr.TaskRuns = make(map[string]TaskRunStatus)
+		pr.TaskRuns = make(map[string]*PipelineRunTaskRunStatus)
 	}
 	if pr.StartTime.IsZero() {
 		pr.StartTime = &metav1.Time{time.Now()}

pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go

@@ -80,7 +80,7 @@ func TestInitializeConditions(t *testing.T) {
 		t.Fatalf("PipelineRun StartTime not initialized correctly")
 	}
 
-	p.Status.TaskRuns["fooTask"] = TaskRunStatus{}
+	p.Status.TaskRuns["fooTask"] = &PipelineRunTaskRunStatus{}
 
 	p.Status.InitializeConditions()
 	if len(p.Status.TaskRuns) != 1 {

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go

@@ -375,7 +375,14 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er
 func updateTaskRunsStatus(pr *v1alpha1.PipelineRun, pipelineState []*resources.ResolvedPipelineRunTask) {
 	for _, rprt := range pipelineState {
 		if rprt.TaskRun != nil {
-			pr.Status.TaskRuns[rprt.TaskRun.Name] = rprt.TaskRun.Status
+			prtrs := pr.Status.TaskRuns[rprt.TaskRun.Name]
+			if prtrs == nil {
+				prtrs = &v1alpha1.PipelineRunTaskRunStatus{
+					PipelineTaskName: rprt.PipelineTask.Name,
+				}
+				pr.Status.TaskRuns[rprt.TaskRun.Name] = prtrs
+			}
+			prtrs.Status = &rprt.TaskRun.Status
 		}
 	}
 }

pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go

@@ -408,16 +408,19 @@ func TestUpdateTaskRunsState(t *testing.T) {
 		tb.StepState(tb.StateTerminated(0)),
 	))
 
-	expectedTaskRunsStatus := make(map[string]v1alpha1.TaskRunStatus)
-	expectedTaskRunsStatus["test-pipeline-run-success-unit-test-1"] = v1alpha1.TaskRunStatus{
-		Steps: []v1alpha1.StepState{{
-			ContainerState: corev1.ContainerState{
-				Terminated: &corev1.ContainerStateTerminated{ExitCode: 0},
-			},
-		}},
-		Conditions: []duckv1alpha1.Condition{{
-			Type: duckv1alpha1.ConditionSucceeded,
-		}},
+	expectedTaskRunsStatus := make(map[string]*v1alpha1.PipelineRunTaskRunStatus)
+	expectedTaskRunsStatus["test-pipeline-run-success-unit-test-1"] = &v1alpha1.PipelineRunTaskRunStatus{
+		PipelineTaskName: "unit-test-1",
+		Status: &v1alpha1.TaskRunStatus{
+			Steps: []v1alpha1.StepState{{
+				ContainerState: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{ExitCode: 0},
+				},
+			}},
+			Conditions: []duckv1alpha1.Condition{{
+				Type: duckv1alpha1.ConditionSucceeded,
+			}},
+		},
 	}
 	expectedPipelineRunStatus := v1alpha1.PipelineRunStatus{
 		TaskRuns: expectedTaskRunsStatus,

pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go

@@ -18,7 +18,6 @@ package resources
 
 import (
 	"fmt"
-	"strings"
 	"time"
 
 	"github.com/knative/build-pipeline/pkg/names"
@@ -184,7 +183,7 @@ func ResolvePipelineRun(
 
 		rprt := ResolvedPipelineRunTask{
 			PipelineTask: &pt,
-			TaskRunName:  getTaskRunName(pipelineRun.Status.TaskRuns, pipelineRun.Name, &pt),
+			TaskRunName:  getTaskRunName(pipelineRun.Status.TaskRuns, pt.Name, pipelineRun.Name),
 		}
 
 		// Find the Task that this task in the Pipeline this PipelineTask is using
@@ -239,17 +238,15 @@ func ResolveTaskRuns(getTaskRun GetTaskRun, state PipelineRunState) error {
 	return nil
 }
 
-// getTaskRunName should return a uniquie name for a `TaskRun`.
-func getTaskRunName(taskRunsStatus map[string]v1alpha1.TaskRunStatus, prName string, pt *v1alpha1.PipelineTask) string {
-	base := fmt.Sprintf("%s-%s", prName, pt.Name)
-
-	for k := range taskRunsStatus {
-		if strings.HasPrefix(k, base) {
+// getTaskRunName should return a unique name for a `TaskRun` if one has not already been defined, and the existing one otherwise.
+func getTaskRunName(taskRunsStatus map[string]*v1alpha1.PipelineRunTaskRunStatus, ptName, prName string) string {
+	for k, v := range taskRunsStatus {
+		if v.PipelineTaskName == ptName {
 			return k
 		}
 	}
 
-	return names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(base)
+	return names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("%s-%s", prName, ptName))
 }
 
 // GetPipelineConditionStatus will return the Condition that the PipelineRun prName should be

pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go

@@ -595,7 +595,7 @@ func TestResolvePipelineRun(t *testing.T) {
 	}}
 
 	if d := cmp.Diff(pipelineState, expectedState, cmpopts.IgnoreUnexported(v1alpha1.TaskRunSpec{})); d != "" {
-		t.Fatalf("Expected to get current pipeline state %v, but actual differed: %s", expectedState, d)
+		t.Errorf("Expected to get current pipeline state %v, but actual differed: %s", expectedState, d)
 	}
 }
 
@@ -1035,7 +1035,7 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) {
 
 	p := tb.Pipeline("pipelines", "namespace", tb.PipelineSpec(
 		tb.PipelineDeclaredResource("git-resource", "git"),
-		tb.PipelineTask("mytask1", "task",
+		tb.PipelineTask("mytask-with-a-really-long-name-to-trigger-truncation", "task",
 			tb.PipelineTaskInputResource("input1", "git-resource"),
 		),
 	))
@@ -1053,8 +1053,11 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) {
 			Type: v1alpha1.PipelineResourceTypeGit,
 		},
 	}
-	taskrunStatus := map[string]v1alpha1.TaskRunStatus{}
-	taskrunStatus["pipelinerun-mytask1-9l9zj"] = v1alpha1.TaskRunStatus{}
+	taskrunStatus := map[string]*v1alpha1.PipelineRunTaskRunStatus{}
+	taskrunStatus["pipelinerun-mytask-with-a-really-long-name-to-trigger-tru-9l9zj"] = &v1alpha1.PipelineRunTaskRunStatus{
+		PipelineTaskName: "mytask-with-a-really-long-name-to-trigger-truncation",
+		Status:           &v1alpha1.TaskRunStatus{},
+	}
 
 	pr := v1alpha1.PipelineRun{
 		ObjectMeta: metav1.ObjectMeta{
@@ -1077,7 +1080,7 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) {
 	}
 	expectedState := PipelineRunState{{
 		PipelineTask: &p.Spec.Tasks[0],
-		TaskRunName:  "pipelinerun-mytask1-9l9zj",
+		TaskRunName:  "pipelinerun-mytask-with-a-really-long-name-to-trigger-tru-9l9zj",
 		TaskRun:      nil,
 		ResolvedTaskResources: &resources.ResolvedTaskResources{
 			TaskName: task.Name,