[release-3.2] Fix messy sequence of PipelineRuns (#396)

* Implement a list handler for PipelineRun

Signed-off-by: John Niang <[email protected]>

* Add unit test for PipelineRun list handler

Signed-off-by: John Niang <[email protected]>

* Add comment on method of Comparator of PipelineRunListHandler

Signed-off-by: John Niang <[email protected]>

Co-authored-by: John Niang <[email protected]>

controllers/jenkins/pipelinerun/utils.go

@@ -94,6 +94,7 @@ func (pbApplier pipelineBuildApplier) apply(prStatus *v1alpha3.PipelineRunStatus
 	}
 	prStatus.AddCondition(&condition)
 	prStatus.UpdateTime = &v1.Time{Time: time.Now()}
+	prStatus.StartTime = &v1.Time{Time: pbApplier.StartTime.Time}
 }
 
 func (pbApplier pipelineBuildApplier) whenPipelineRunFinished(condition *v1alpha3.Condition, prStatus *v1alpha3.PipelineRunStatus) {

pkg/kapis/devops/v1alpha3/pipelinerun/handler.go

@@ -71,7 +71,7 @@ func (h *apiHandler) listPipelineRuns(request *restful.Request, response *restfu
 		return
 	}
 
-	var listHandler resourcesV1alpha3.ListHandler
+	var listHandler resourcesV1alpha3.ListHandler = listHandler{}
 	if backward {
 		listHandler = backwardListHandler{}
 	}

pkg/kapis/devops/v1alpha3/pipelinerun/listhandler.go

@@ -0,0 +1,54 @@
+package pipelinerun
+
+import (
+	"strings"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
+	"kubesphere.io/devops/pkg/apiserver/query"
+	resourcesV1alpha3 "kubesphere.io/devops/pkg/models/resources/v1alpha3"
+)
+
+// listHandler is default implementation for PipelineRun.
+type listHandler struct {
+}
+
+// Make sure backwardListHandler implement ListHandler interface.
+var _ resourcesV1alpha3.ListHandler = listHandler{}
+
+// Comparator compares times first, which is from start time and creation time(only when start time is nil or zero).
+// If times are equal, we will compare the unique name at last to
+// ensure that the order result is stable forever.
+func (b listHandler) Comparator() resourcesV1alpha3.CompareFunc {
+	return func(left, right runtime.Object, f query.Field) bool {
+		leftPipelineRun, ok := left.(*v1alpha3.PipelineRun)
+		if !ok {
+			return false
+		}
+		rightPipelineRun, ok := right.(*v1alpha3.PipelineRun)
+		if !ok {
+			return false
+		}
+		// Compare start time and creation time(if missing former)
+		leftTime := leftPipelineRun.Status.StartTime
+		if leftTime.IsZero() {
+			leftTime = &leftPipelineRun.CreationTimestamp
+		}
+		rightTime := rightPipelineRun.Status.StartTime
+		if rightTime.IsZero() {
+			rightTime = &rightPipelineRun.CreationTimestamp
+		}
+		if !leftTime.Equal(rightTime) {
+			return leftTime.After(rightTime.Time)
+		}
+		return strings.Compare(leftPipelineRun.Name, rightPipelineRun.Name) < 0
+	}
+}
+
+func (b listHandler) Filter() resourcesV1alpha3.FilterFunc {
+	return resourcesV1alpha3.DefaultFilter()
+}
+
+func (b listHandler) Transformer() resourcesV1alpha3.TransformFunc {
+	return resourcesV1alpha3.NoTransformFunc()
+}

pkg/kapis/devops/v1alpha3/pipelinerun/listhandler_test.go

@@ -0,0 +1,102 @@
+package pipelinerun
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"kubesphere.io/devops/pkg/api/devops/v1alpha3"
+)
+
+func Test_listHandler_Comparator(t *testing.T) {
+	now := v1.Now()
+	tomorrow := v1.Time{Time: now.Add(1 * time.Hour)}
+	createPipelineRun := func(name string, creationTime v1.Time, startTime *v1.Time) *v1alpha3.PipelineRun {
+		return &v1alpha3.PipelineRun{
+			ObjectMeta: v1.ObjectMeta{
+				Name:              name,
+				CreationTimestamp: creationTime,
+			},
+			Status: v1alpha3.PipelineRunStatus{
+				StartTime: startTime,
+			},
+		}
+	}
+	type args struct {
+		left  *v1alpha3.PipelineRun
+		right *v1alpha3.PipelineRun
+	}
+	tests := []struct {
+		name string
+		args args
+		// expect whether we need to exchange left and right while sorting.
+		// false: left position should swap with right.
+		// true: left and right should keep their position.
+		shouldNotSwap bool
+	}{{
+		name: "Compare with start time and first and left is earlier than right",
+		args: args{
+			left:  createPipelineRun("b", now, &now),
+			right: createPipelineRun("a", now, &tomorrow),
+		},
+		shouldNotSwap: false,
+	}, {
+		name: "Compare with start time and first and left is later than right",
+		args: args{
+			left:  createPipelineRun("b", now, &tomorrow),
+			right: createPipelineRun("a", now, &now),
+		},
+		shouldNotSwap: true,
+	}, {
+		name: "Compare with start time and start times are equal",
+		args: args{
+			left:  createPipelineRun("b", now, &now),
+			right: createPipelineRun("a", now, &now),
+		},
+		shouldNotSwap: false,
+	}, {
+		name: "Return to compare with creation time while start time is nil",
+		args: args{
+			left:  createPipelineRun("b", now, nil),
+			right: createPipelineRun("a", now, &tomorrow),
+		},
+		shouldNotSwap: false,
+	}, {
+		name: "Return to compare with creation time while one of start time is nil",
+		args: args{
+			left:  createPipelineRun("b", now, nil),
+			right: createPipelineRun("a", now, &tomorrow),
+		},
+		shouldNotSwap: false,
+	}, {
+		name: "Return to compare with name while one of start time is nil and star time is equal to creation time",
+		args: args{
+			left:  createPipelineRun("a", now, nil),
+			right: createPipelineRun("b", now, &now),
+		},
+		shouldNotSwap: true,
+	}, {
+		name: "Return to compare with name while start times are equal",
+		args: args{
+			left:  createPipelineRun("a", now, &now),
+			right: createPipelineRun("b", now, &now),
+		},
+		shouldNotSwap: true,
+	}, {
+		name: "Return to compare with name while start times are nil and creation times are equal",
+		args: args{
+			left:  createPipelineRun("b", now, nil),
+			right: createPipelineRun("a", now, nil),
+		},
+		shouldNotSwap: false,
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			h := listHandler{}
+			if got := h.Comparator()(tt.args.left, tt.args.right, ""); !reflect.DeepEqual(got, tt.shouldNotSwap) {
+				t.Errorf("pipelineRunListHandler.Comparator() = %v, want %v", got, tt.shouldNotSwap)
+			}
+		})
+	}
+}