Update v1alpha1 status types to line fields

Splits up the status types into two structs: * XXXStatus * XXXStatusFields This first inlines both knative's duck status type and the XXXStatusFields. The second stores all the fields that were originially in XXXStatus. This is advantageous as downstream users can now embed a the XXXStatusFields if their CRDs manage a Tekton object. This pattern is demonstrated in Knative (e.g., Serving). Moving to this pattern does not change the JSON/YAML structure in anyway, however it does change how status objects have to be instantiated in Go. fixes tektoncd#1590
poy · Nov 27, 2019 · 6303dd0 · 6303dd0
1 parent 5ef2ec0
commit 6303dd0
Show file tree

Hide file tree

Showing 13 changed files with 274 additions and 136 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/condition_types.go b/pkg/apis/pipeline/v1alpha1/condition_types.go
@@ -44,6 +44,14 @@ type Condition struct {
 type ConditionCheckStatus struct {
 	duckv1beta1.Status `json:",inline"`
 
+	// ConditionCheckStatusFields inlines the status fields.
+	ConditionCheckStatusFields `json:",inline"`
+}
+
+// ConditionCheckStatusFields holds the fields of ConfigurationCheck's status.
+// This is defined separately and inlined so that other types can readily
+// consume these fields via duck typing.
+type ConditionCheckStatusFields struct {
 	// PodName is the name of the pod responsible for executing this condition check.
 	PodName string `json:"podName"`
 

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
@@ -104,6 +104,14 @@ type PipelineRef struct {
 type PipelineRunStatus struct {
 	duckv1beta1.Status `json:",inline"`
 
+	// PipelineRunStatusFields inlines the status fields.
+	PipelineRunStatusFields `json:",inline"`
+}
+
+// PipelineRunStatusFields holds the fields of PipelineRunStatus' status.
+// This is defined separately and inlined so that other types can readily
+// consume these fields via duck typing.
+type PipelineRunStatusFields struct {
 	// StartTime is the time the PipelineRun is actually started.
 	// +optional
 	StartTime *metav1.Time `json:"startTime,omitempty"`

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go
@@ -150,13 +150,17 @@ func TestPipelineRunHasStarted(t *testing.T) {
 	}, {
 		name: "prWithStartTime",
 		prStatus: v1alpha1.PipelineRunStatus{
-			StartTime: &metav1.Time{Time: time.Now()},
+			PipelineRunStatusFields: v1alpha1.PipelineRunStatusFields{
+				StartTime: &metav1.Time{Time: time.Now()},
+			},
 		},
 		expectedValue: true,
 	}, {
 		name: "prWithZeroStartTime",
 		prStatus: v1alpha1.PipelineRunStatus{
-			StartTime: &metav1.Time{},
+			PipelineRunStatusFields: v1alpha1.PipelineRunStatusFields{
+				StartTime: &metav1.Time{},
+			},
 		},
 		expectedValue: false,
 	}}

diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go
@@ -97,6 +97,14 @@ var taskRunCondSet = apis.NewBatchConditionSet()
 type TaskRunStatus struct {
 	duckv1beta1.Status `json:",inline"`
 
+	// TaskRunStatusFields inlines the status fields.
+	TaskRunStatusFields `json:",inline"`
+}
+
+// TaskRunStatusFields holds the fields of TaskRun's status.  This is defined
+// separately and inlined so that other types can readily consume these fields
+// via duck typing.
+type TaskRunStatusFields struct {
 	// PodName is the name of the pod responsible for executing this task's steps.
 	PodName string `json:"podName"`
 

diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_types_test.go
@@ -132,13 +132,17 @@ func TestTaskRunHasStarted(t *testing.T) {
 	}, {
 		name: "trWithStartTime",
 		trStatus: v1alpha1.TaskRunStatus{
-			StartTime: &metav1.Time{Time: time.Now()},
+			TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+				StartTime: &metav1.Time{Time: time.Now()},
+			},
 		},
 		expectedValue: true,
 	}, {
 		name: "trWithZeroStartTime",
 		trStatus: v1alpha1.TaskRunStatus{
-			StartTime: &metav1.Time{},
+			TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+				StartTime: &metav1.Time{},
+			},
 		},
 		expectedValue: false,
 	}}

diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -466,18 +466,21 @@ func TestUpdateTaskRunsState(t *testing.T) {
 	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},
-				},
-			}},
+			TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+				Steps: []v1alpha1.StepState{{
+					ContainerState: corev1.ContainerState{
+						Terminated: &corev1.ContainerStateTerminated{ExitCode: 0},
+					},
+				}}},
 			Status: duckv1beta1.Status{
 				Conditions: []apis.Condition{{Type: apis.ConditionSucceeded}},
 			},
 		},
 	}
 	expectedPipelineRunStatus := v1alpha1.PipelineRunStatus{
-		TaskRuns: expectedTaskRunsStatus,
+		PipelineRunStatusFields: v1alpha1.PipelineRunStatusFields{
+			TaskRuns: expectedTaskRunsStatus,
+		},
 	}
 
 	state := []*resources.ResolvedPipelineRunTask{{
@@ -531,8 +534,10 @@ func TestUpdateTaskRunStateWithConditionChecks(t *testing.T) {
 	successConditionCheckStatus := &v1alpha1.PipelineRunConditionCheckStatus{
 		ConditionName: successCondition.Name,
 		Status: &v1alpha1.ConditionCheckStatus{
-			Check: corev1.ContainerState{
-				Terminated: &corev1.ContainerStateTerminated{ExitCode: 0},
+			ConditionCheckStatusFields: v1alpha1.ConditionCheckStatusFields{
+				Check: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{ExitCode: 0},
+				},
 			},
 			Status: duckv1beta1.Status{
 				Conditions: []apis.Condition{{Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue}},
@@ -542,8 +547,10 @@ func TestUpdateTaskRunStateWithConditionChecks(t *testing.T) {
 	failingConditionCheckStatus := &v1alpha1.PipelineRunConditionCheckStatus{
 		ConditionName: failingCondition.Name,
 		Status: &v1alpha1.ConditionCheckStatus{
-			Check: corev1.ContainerState{
-				Terminated: &corev1.ContainerStateTerminated{ExitCode: 127},
+			ConditionCheckStatusFields: v1alpha1.ConditionCheckStatusFields{
+				Check: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{ExitCode: 127},
+				},
 			},
 			Status: duckv1beta1.Status{
 				Conditions: []apis.Condition{{Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse}},

diff --git a/pkg/reconciler/pipelinerun/resources/conditionresolution.go b/pkg/reconciler/pipelinerun/resources/conditionresolution.go
@@ -162,11 +162,13 @@ func (rcc *ResolvedConditionCheck) NewConditionCheckStatus() *v1alpha1.Condition
 	}
 
 	return &v1alpha1.ConditionCheckStatus{
-		Status:         trs.Status,
-		PodName:        trs.PodName,
-		StartTime:      trs.StartTime,
-		CompletionTime: trs.CompletionTime,
-		Check:          checkStep,
+		Status: trs.Status,
+		ConditionCheckStatusFields: v1alpha1.ConditionCheckStatusFields{
+			PodName:        trs.PodName,
+			StartTime:      trs.StartTime,
+			CompletionTime: trs.CompletionTime,
+			Check:          checkStep,
+		},
 	}
 }
 

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
@@ -1341,7 +1341,9 @@ func TestResolvePipelineRun_withExistingTaskRuns(t *testing.T) {
 			Name: "pipelinerun",
 		},
 		Status: v1alpha1.PipelineRunStatus{
-			TaskRuns: taskrunStatus,
+			PipelineRunStatusFields: v1alpha1.PipelineRunStatusFields{
+				TaskRuns: taskrunStatus,
+			},
 		},
 	}
 
@@ -1555,8 +1557,10 @@ func TestResolveConditionCheck_UseExistingConditionCheckName(t *testing.T) {
 			Name: "pipelinerun",
 		},
 		Status: v1alpha1.PipelineRunStatus{
-			Status:   duckv1beta1.Status{},
-			TaskRuns: trStatus,
+			Status: duckv1beta1.Status{},
+			PipelineRunStatusFields: v1alpha1.PipelineRunStatusFields{
+				TaskRuns: trStatus,
+			},
 		},
 	}
 

diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -1548,7 +1548,9 @@ func TestReconcilePodUpdateStatus(t *testing.T) {
 		t.Fatalf("MakePod: %v", err)
 	}
 	taskRun.Status = v1alpha1.TaskRunStatus{
-		PodName: pod.Name,
+		TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+			PodName: pod.Name,
+		},
 	}
 	d := test.Data{
 		TaskRuns: []*v1alpha1.TaskRun{taskRun},
@@ -2167,7 +2169,9 @@ func TestTryGetPod(t *testing.T) {
 	}, {
 		desc: "non-existent-pod",
 		trs: v1alpha1.TaskRunStatus{
-			PodName: "no-longer-exist",
+			TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+				PodName: "no-longer-exist",
+			},
 		},
 		gp: func(name string, opts metav1.GetOptions) (*corev1.Pod, error) {
 			return nil, kerrors.NewNotFound(schema.GroupResource{}, name)
@@ -2177,7 +2181,9 @@ func TestTryGetPod(t *testing.T) {
 	}, {
 		desc: "existing-pod",
 		trs: v1alpha1.TaskRunStatus{
-			PodName: "exists",
+			TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+				PodName: "exists",
+			},
 		},
 		gp: func(name string, opts metav1.GetOptions) (*corev1.Pod, error) {
 			return &corev1.Pod{}, nil
@@ -2187,7 +2193,9 @@ func TestTryGetPod(t *testing.T) {
 	}, {
 		desc: "pod-fetch-error",
 		trs: v1alpha1.TaskRunStatus{
-			PodName: "something-went-wrong",
+			TaskRunStatusFields: v1alpha1.TaskRunStatusFields{
+				PodName: "something-went-wrong",
+			},
 		},
 		gp: func(name string, opts metav1.GetOptions) (*corev1.Pod, error) {
 			return nil, err