diff --git a/Gopkg.lock b/Gopkg.lock index 5378fcdd226..ad583505db8 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -872,7 +872,6 @@ "k8s.io/apimachinery/pkg/util/rand", "k8s.io/apimachinery/pkg/util/sets/types", "k8s.io/apimachinery/pkg/util/validation", - "k8s.io/apimachinery/pkg/util/validation/field", "k8s.io/apimachinery/pkg/util/wait", "k8s.io/apimachinery/pkg/watch", "k8s.io/client-go/discovery", diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index dad4e51697c..c4ddf8c828a 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/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()} diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go index 32e1198bbce..c89441b6aca 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go +++ b/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 { diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index dbb0bbae42d..bf2fa76e781 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -694,11 +694,14 @@ func (in *PipelineRunStatus) DeepCopyInto(out *PipelineRunStatus) { } if in.TaskRuns != nil { in, out := &in.TaskRuns, &out.TaskRuns - *out = make(map[string]TaskRunStatus, len(*in)) + *out = make(map[string]*PipelineRunTaskRunStatus, len(*in)) for key, val := range *in { - newVal := new(TaskRunStatus) - val.DeepCopyInto(newVal) - (*out)[key] = *newVal + if val == nil { + (*out)[key] = nil + } else { + (*out)[key] = new(PipelineRunTaskRunStatus) + val.DeepCopyInto((*out)[key]) + } } } return @@ -714,6 +717,31 @@ func (in *PipelineRunStatus) DeepCopy() *PipelineRunStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PipelineRunTaskRunStatus) DeepCopyInto(out *PipelineRunTaskRunStatus) { + *out = *in + if in.Status != nil { + in, out := &in.Status, &out.Status + if *in == nil { + *out = nil + } else { + *out = new(TaskRunStatus) + (*in).DeepCopyInto(*out) + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineRunTaskRunStatus. +func (in *PipelineRunTaskRunStatus) DeepCopy() *PipelineRunTaskRunStatus { + if in == nil { + return nil + } + out := new(PipelineRunTaskRunStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { *out = *in diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 9463054feac..71f412f8185 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/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 } } } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 872e84e105c..a3051502c21 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/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, diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go index 27708f61068..d7e76b86bd4 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution.go +++ b/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 diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go index 39a949230c3..8396cce9abd 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinerunresolution_test.go +++ b/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,