Repository: kubesphere/ks-devops
Pull request #396 (2021-12-03 07:17:33)

[release-3.2] Fix messy sequence of PipelineRuns
This is an automated cherry-pick of #394

ks-ci-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/396#" title="Author self-approved">ks-ci-bot</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **linuxsuren** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @linuxsuren` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubesphere/ks-devops/blob/release-3.2/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["linuxsuren"]} -->
ks-ci-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/396#pullrequestreview-822320288" title="LGTM">JohnNiang</a>*, *<a href="https://github.com/kubesphere/ks-devops/pull/396#" title="Author self-approved">ks-ci-bot</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **linuxsuren** after the PR has been reviewed.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubesphere/ks-devops/blob/release-3.2/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["linuxsuren"]} -->
ks-ci-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubesphere/ks-devops/pull/396#pullrequestreview-822320288" title="LGTM">JohnNiang</a>*, *<a href="https://github.com/kubesphere/ks-devops/pull/396#" title="Author self-approved">ks-ci-bot</a>*, *<a href="https://github.com/kubesphere/ks-devops/pull/396#pullrequestreview-822320465" title="Approved">LinuxSuRen</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubesphere%2Fks-devops).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubesphere/ks-devops/blob/release-3.2/OWNERS)~~ [LinuxSuRen]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/controllers/jenkins/pipelinerun/utils.go b/controllers/jenkins/pipelinerun/utils.go
index e7bfcd1..ccddb09 100644
--- a/controllers/jenkins/pipelinerun/utils.go
+++ b/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) {
diff --git a/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go b/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go
index cc47d90..1cc7235 100644
--- a/pkg/kapis/devops/v1alpha3/pipelinerun/handler.go
+++ b/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{}
 	}
diff --git a/pkg/kapis/devops/v1alpha3/pipelinerun/listhandler.go b/pkg/kapis/devops/v1alpha3/pipelinerun/listhandler.go
new file mode 100644
index 0000000..b85ff15
--- /dev/null
+++ b/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()
+}


Test patch:
diff --git a/pkg/kapis/devops/v1alpha3/pipelinerun/listhandler_test.go b/pkg/kapis/devops/v1alpha3/pipelinerun/listhandler_test.go
new file mode 100644
index 0000000..2e7a87b
--- /dev/null
+++ b/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)
+			}
+		})
+	}
+}