From b6b7bd9a458c70fe683e26f4d24b1703b32e3545 Mon Sep 17 00:00:00 2001 From: KubeSphere CI Bot <47586280+ks-ci-bot@users.noreply.github.com> Date: Fri, 3 Dec 2021 15:20:55 +0800 Subject: [PATCH] [release-3.2] Fix messy sequence of PipelineRuns (#396) * Implement a list handler for PipelineRun Signed-off-by: John Niang * Add unit test for PipelineRun list handler Signed-off-by: John Niang * Add comment on method of Comparator of PipelineRunListHandler Signed-off-by: John Niang Co-authored-by: John Niang --- controllers/jenkins/pipelinerun/utils.go | 1 + .../devops/v1alpha3/pipelinerun/handler.go | 2 +- .../v1alpha3/pipelinerun/listhandler.go | 54 ++++++++++ .../v1alpha3/pipelinerun/listhandler_test.go | 102 ++++++++++++++++++ 4 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 pkg/kapis/devops/v1alpha3/pipelinerun/listhandler.go create mode 100644 pkg/kapis/devops/v1alpha3/pipelinerun/listhandler_test.go diff --git a/controllers/jenkins/pipelinerun/utils.go b/controllers/jenkins/pipelinerun/utils.go index e7bfcd14..ccddb099 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 cc47d908..1cc72352 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 00000000..b85ff155 --- /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() +} 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 00000000..2e7a87bd --- /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) + } + }) + } +}