Remove order for stages in case of quick sync (#5292)

* Remove order for stages in case of quick sync Signed-off-by: khanhtc1202 <[email protected]> * Fix failed test Signed-off-by: khanhtc1202 <[email protected]> --------- Signed-off-by: khanhtc1202 <[email protected]>
pipe-cd · Oct 25, 2024 · 4ebcb7c · 4ebcb7c
1 parent cbbd67d
commit 4ebcb7c
Show file tree

Hide file tree

Showing 8 changed files with 113 additions and 162 deletions.
diff --git a/pkg/app/pipedv1/controller/planner.go b/pkg/app/pipedv1/controller/planner.go
@@ -30,7 +30,7 @@ import (
 	"github.com/pipe-cd/pipecd/pkg/app/pipedv1/controller/controllermetrics"
 	"github.com/pipe-cd/pipecd/pkg/app/pipedv1/metadatastore"
 	"github.com/pipe-cd/pipecd/pkg/app/server/service/pipedservice"
-	"github.com/pipe-cd/pipecd/pkg/configv1"
+	config "github.com/pipe-cd/pipecd/pkg/configv1"
 	"github.com/pipe-cd/pipecd/pkg/model"
 	pluginapi "github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1"
 	"github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1/deployment"
@@ -409,13 +409,11 @@ func (p *planner) buildQuickSyncStages(ctx context.Context, cfg *config.GenericA
 		rollbackStages = []*model.PipelineStage{}
 		rollback       = *cfg.Planner.AutoRollback
 	)
-	// TODO: Consider how to define the order of plugins.
-	for i, plg := range p.plugins {
-		res, err := plg.BuildQuickSyncStages(ctx, &deployment.BuildQuickSyncStagesRequest{StageIndex: int32(i), Rollback: rollback})
+	for _, plg := range p.plugins {
+		res, err := plg.BuildQuickSyncStages(ctx, &deployment.BuildQuickSyncStagesRequest{Rollback: rollback})
 		if err != nil {
 			return nil, fmt.Errorf("failed to build quick sync stage deployment (%w)", err)
 		}
-		// TODO: Ensure responsed stages indexies is valid.
 		for i := range res.Stages {
 			if res.Stages[i].Rollback {
 				rollbackStages = append(rollbackStages, res.Stages[i])
@@ -425,22 +423,6 @@ func (p *planner) buildQuickSyncStages(ctx context.Context, cfg *config.GenericA
 		}
 	}
 
-	// Sort stages by index.
-	sort.Sort(model.PipelineStages(stages))
-	sort.Sort(model.PipelineStages(rollbackStages))
-
-	// In case there is more than one forward stage, build requires for each stage
-	// based on the order of stages.
-	if len(stages) > 1 {
-		preStageID := ""
-		for _, s := range stages {
-			if preStageID != "" {
-				s.Requires = []string{preStageID}
-			}
-			preStageID = s.Id
-		}
-	}
-
 	stages = append(stages, rollbackStages...)
 	if len(stages) == 0 {
 		return nil, fmt.Errorf("unable to build quick sync stages for deployment")

diff --git a/pkg/app/pipedv1/controller/planner_test.go b/pkg/app/pipedv1/controller/planner_test.go
@@ -25,7 +25,7 @@ import (
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
 
-	"github.com/pipe-cd/pipecd/pkg/configv1"
+	config "github.com/pipe-cd/pipecd/pkg/configv1"
 	"github.com/pipe-cd/pipecd/pkg/model"
 	pluginapi "github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1"
 	"github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1/deployment"
@@ -151,29 +151,25 @@ func TestBuildQuickSyncStages(t *testing.T) {
 				&fakePlugin{
 					quickStages: []*model.PipelineStage{
 						{
-							Id:    "plugin-1-stage-1",
-							Index: 0,
+							Id: "plugin-1-stage-1",
 						},
 					},
 					rollbackStages: []*model.PipelineStage{
 						{
 							Id:       "plugin-1-rollback",
-							Index:    0,
 							Rollback: true,
 						},
 					},
 				},
 				&fakePlugin{
 					quickStages: []*model.PipelineStage{
 						{
-							Id:    "plugin-2-stage-1",
-							Index: 1,
+							Id: "plugin-2-stage-1",
 						},
 					},
 					rollbackStages: []*model.PipelineStage{
 						{
 							Id:       "plugin-2-rollback",
-							Index:    1,
 							Rollback: true,
 						},
 					},
@@ -187,22 +183,17 @@ func TestBuildQuickSyncStages(t *testing.T) {
 			wantErr: false,
 			expectedStages: []*model.PipelineStage{
 				{
-					Id:    "plugin-1-stage-1",
-					Index: 0,
+					Id: "plugin-1-stage-1",
 				},
 				{
-					Id:       "plugin-2-stage-1",
-					Index:    1,
-					Requires: []string{"plugin-1-stage-1"},
+					Id: "plugin-2-stage-1",
 				},
 				{
 					Id:       "plugin-1-rollback",
-					Index:    0,
 					Rollback: true,
 				},
 				{
 					Id:       "plugin-2-rollback",
-					Index:    1,
 					Rollback: true,
 				},
 			},
@@ -213,8 +204,7 @@ func TestBuildQuickSyncStages(t *testing.T) {
 				&fakePlugin{
 					quickStages: []*model.PipelineStage{
 						{
-							Id:    "plugin-1-stage-1",
-							Index: 0,
+							Id: "plugin-1-stage-1",
 						},
 					},
 					rollbackStages: []*model.PipelineStage{
@@ -227,8 +217,7 @@ func TestBuildQuickSyncStages(t *testing.T) {
 				&fakePlugin{
 					quickStages: []*model.PipelineStage{
 						{
-							Id:    "plugin-2-stage-1",
-							Index: 1,
+							Id: "plugin-2-stage-1",
 						},
 					},
 					rollbackStages: []*model.PipelineStage{
@@ -247,13 +236,10 @@ func TestBuildQuickSyncStages(t *testing.T) {
 			wantErr: false,
 			expectedStages: []*model.PipelineStage{
 				{
-					Id:    "plugin-1-stage-1",
-					Index: 0,
+					Id: "plugin-1-stage-1",
 				},
 				{
-					Id:       "plugin-2-stage-1",
-					Index:    1,
-					Requires: []string{"plugin-1-stage-1"},
+					Id: "plugin-2-stage-1",
 				},
 			},
 		},
@@ -266,7 +252,7 @@ func TestBuildQuickSyncStages(t *testing.T) {
 			}
 			stages, err := planner.buildQuickSyncStages(context.TODO(), tc.cfg)
 			require.Equal(t, tc.wantErr, err != nil)
-			assert.Equal(t, tc.expectedStages, stages)
+			assert.ElementsMatch(t, tc.expectedStages, stages)
 		})
 	}
 }

diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline.go
@@ -105,24 +105,23 @@ func MakeInitialStageMetadata(cfg config.PipelineStage) map[string]string {
 	}
 }
 
-func buildQuickSyncPipeline(index int32, autoRollback bool, now time.Time) []*model.PipelineStage {
+func buildQuickSyncPipeline(autoRollback bool, now time.Time) []*model.PipelineStage {
 	var (
 		preStageID = ""
 		stage, _   = GetPredefinedStage(PredefinedStageK8sSync)
 		stages     = []config.PipelineStage{stage}
 		out        = make([]*model.PipelineStage, 0, len(stages))
 	)
 
-	for _, s := range stages {
+	for i, s := range stages {
 		id := s.ID
 		if id == "" {
-			id = fmt.Sprintf("kubernetes-stage-%d", index)
+			id = fmt.Sprintf("kubernetes-stage-%d", i)
 		}
 		stage := &model.PipelineStage{
 			Id:         id,
 			Name:       s.Name.String(),
 			Desc:       s.Desc,
-			Index:      int32(index),
 			Predefined: true,
 			Visible:    true,
 			Status:     model.StageStatus_STAGE_NOT_STARTED_YET,

diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline_test.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline_test.go
@@ -29,13 +29,11 @@ func TestBuildQuickSyncPipeline(t *testing.T) {
 
 	tests := []struct {
 		name         string
-		index        int32
 		autoRollback bool
 		expected     []*model.PipelineStage
 	}{
 		{
 			name:         "without auto rollback",
-			index:        0,
 			autoRollback: false,
 			expected: []*model.PipelineStage{
 				{
@@ -54,7 +52,6 @@ func TestBuildQuickSyncPipeline(t *testing.T) {
 		},
 		{
 			name:         "with auto rollback",
-			index:        0,
 			autoRollback: true,
 			expected: []*model.PipelineStage{
 				{
@@ -85,7 +82,7 @@ func TestBuildQuickSyncPipeline(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			actual := buildQuickSyncPipeline(tt.index, tt.autoRollback, now)
+			actual := buildQuickSyncPipeline(tt.autoRollback, now)
 			assert.Equal(t, tt.expected, actual)
 		})
 	}

diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/server.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/server.go
@@ -95,7 +95,7 @@ func (a *DeploymentService) BuildPipelineSyncStages(context.Context, *deployment
 // BuildQuickSyncStages implements deployment.DeploymentServiceServer.
 func (a *DeploymentService) BuildQuickSyncStages(ctx context.Context, request *deployment.BuildQuickSyncStagesRequest) (*deployment.BuildQuickSyncStagesResponse, error) {
 	now := time.Now()
-	stages := buildQuickSyncPipeline(request.GetStageIndex(), request.GetRollback(), now)
+	stages := buildQuickSyncPipeline(request.GetRollback(), now)
 	return &deployment.BuildQuickSyncStagesResponse{
 		Stages: stages,
 	}, nil