From 406db4cea02d9741b9736d779b55536b01b9801a Mon Sep 17 00:00:00 2001 From: Shinnosuke Sawada-Dazai Date: Wed, 20 Nov 2024 13:25:15 +0900 Subject: [PATCH] Refactor and implement k8s pipeline builds (#5349) * Refactor pipeline stage definitions to use model.PipelineStage and simplify stage creation Signed-off-by: Shinnosuke Sawada-Dazai * Implement buildPipelineStages function to create pipeline stages with optional auto rollback Signed-off-by: Shinnosuke Sawada-Dazai --------- Signed-off-by: Shinnosuke Sawada-Dazai --- .../plugin/kubernetes/deployment/pipeline.go | 123 +++++++------ .../kubernetes/deployment/pipeline_test.go | 168 +++++++++++++++--- .../plugin/kubernetes/deployment/server.go | 8 +- 3 files changed, 215 insertions(+), 84 deletions(-) diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline.go index 6334644a4f..aa2af4d81b 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline.go +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline.go @@ -16,11 +16,11 @@ package deployment import ( "fmt" - "strings" + "slices" "time" - "github.com/pipe-cd/pipecd/pkg/config" "github.com/pipe-cd/pipecd/pkg/model" + "github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1/deployment" ) type Stage string @@ -71,82 +71,99 @@ const ( PredefinedStageRollback = "K8sRollback" ) -var predefinedStages = map[string]config.PipelineStage{ +var predefinedStages = map[string]*model.PipelineStage{ PredefinedStageK8sSync: { - ID: PredefinedStageK8sSync, - // TODO: we have to change config.PipelineStage.Name to string before releasing pipedv1? - // because we don't want to define stages at piped side. We want to define them at the plugin side. - // Or plugins should use the model.Stage type instead of string or some defined type. - Name: model.Stage(StageK8sSync), - Desc: "Sync by applying all manifests", + Id: PredefinedStageK8sSync, + Name: string(StageK8sSync), + Desc: "Sync by applying all manifests", + Rollback: false, }, PredefinedStageRollback: { - ID: PredefinedStageRollback, - Name: model.Stage(StageK8sRollback), - Desc: "Rollback the deployment", + Id: PredefinedStageRollback, + Name: string(StageK8sRollback), + Desc: "Rollback the deployment", + Rollback: true, }, } // GetPredefinedStage finds and returns the predefined stage for the given id. -func GetPredefinedStage(id string) (config.PipelineStage, bool) { +func GetPredefinedStage(id string) (*model.PipelineStage, bool) { stage, ok := predefinedStages[id] return stage, ok } -// MakeInitialStageMetadata makes the initial metadata for the given state configuration. -func MakeInitialStageMetadata(cfg config.PipelineStage) map[string]string { - switch cfg.Name { - case model.StageWaitApproval: - return map[string]string{ - "Approvers": strings.Join(cfg.WaitApprovalStageOptions.Approvers, ","), - } - default: - return nil +func buildQuickSyncPipeline(autoRollback bool, now time.Time) []*model.PipelineStage { + out := make([]*model.PipelineStage, 0, 2) + + stage, _ := GetPredefinedStage(PredefinedStageK8sSync) + // we copy the predefined stage to avoid modifying the original one. + out = append(out, &model.PipelineStage{ + Id: stage.GetId(), + Name: stage.GetName(), + Desc: stage.GetDesc(), + Rollback: stage.GetRollback(), + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + Metadata: nil, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }, + ) + + if autoRollback { + s, _ := GetPredefinedStage(PredefinedStageRollback) + // we copy the predefined stage to avoid modifying the original one. + out = append(out, &model.PipelineStage{ + Id: s.GetId(), + Name: s.GetName(), + Desc: s.GetDesc(), + Rollback: s.GetRollback(), + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }) } + + return out } -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)) - ) +func buildPipelineStages(stages []*deployment.BuildPipelineSyncStagesRequest_StageConfig, autoRollback bool, now time.Time) []*model.PipelineStage { + out := make([]*model.PipelineStage, 0, len(stages)+1) - for i, s := range stages { - id := s.ID + for _, s := range stages { + id := s.GetId() if id == "" { - id = fmt.Sprintf("kubernetes-stage-%d", i) + id = fmt.Sprintf("stage-%d", s.GetIndex()) } stage := &model.PipelineStage{ - Id: id, - Name: s.Name.String(), - Desc: s.Desc, - Predefined: true, - Visible: true, - Status: model.StageStatus_STAGE_NOT_STARTED_YET, - Metadata: MakeInitialStageMetadata(s), - CreatedAt: now.Unix(), - UpdatedAt: now.Unix(), + Id: id, + Name: s.GetName(), + Desc: s.GetDesc(), + Index: s.GetIndex(), + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), } - if preStageID != "" { - stage.Requires = []string{preStageID} - } - preStageID = id out = append(out, stage) } if autoRollback { + // we set the index of the rollback stage to the minimum index of all stages. + minIndex := slices.MinFunc(stages, func(a, b *deployment.BuildPipelineSyncStagesRequest_StageConfig) int { + return int(a.GetIndex() - b.GetIndex()) + }).GetIndex() + s, _ := GetPredefinedStage(PredefinedStageRollback) + // we copy the predefined stage to avoid modifying the original one. out = append(out, &model.PipelineStage{ - Id: s.ID, - Name: s.Name.String(), - Desc: s.Desc, - Predefined: true, - Visible: false, - Status: model.StageStatus_STAGE_NOT_STARTED_YET, - CreatedAt: now.Unix(), - UpdatedAt: now.Unix(), + Id: s.GetId(), + Name: s.GetName(), + Desc: s.GetDesc(), + Index: minIndex, + Rollback: s.GetRollback(), + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), }) } diff --git a/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline_test.go b/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline_test.go index 26f608ed77..222ee7d6d5 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline_test.go +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/pipeline_test.go @@ -18,8 +18,10 @@ import ( "testing" "time" - "github.com/pipe-cd/pipecd/pkg/model" "github.com/stretchr/testify/assert" + + "github.com/pipe-cd/pipecd/pkg/model" + "github.com/pipe-cd/pipecd/pkg/plugin/api/v1alpha1/deployment" ) func TestBuildQuickSyncPipeline(t *testing.T) { @@ -37,16 +39,15 @@ func TestBuildQuickSyncPipeline(t *testing.T) { autoRollback: false, expected: []*model.PipelineStage{ { - Id: PredefinedStageK8sSync, - Name: StageK8sSync.String(), - Desc: "Sync by applying all manifests", - Index: 0, - Predefined: true, - Visible: true, - Status: model.StageStatus_STAGE_NOT_STARTED_YET, - Metadata: nil, - CreatedAt: now.Unix(), - UpdatedAt: now.Unix(), + Id: PredefinedStageK8sSync, + Name: StageK8sSync.String(), + Desc: "Sync by applying all manifests", + Index: 0, + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + Metadata: nil, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), }, }, }, @@ -55,26 +56,24 @@ func TestBuildQuickSyncPipeline(t *testing.T) { autoRollback: true, expected: []*model.PipelineStage{ { - Id: PredefinedStageK8sSync, - Name: StageK8sSync.String(), - Desc: "Sync by applying all manifests", - Index: 0, - Predefined: true, - Visible: true, - Status: model.StageStatus_STAGE_NOT_STARTED_YET, - Metadata: nil, - CreatedAt: now.Unix(), - UpdatedAt: now.Unix(), + Id: PredefinedStageK8sSync, + Name: StageK8sSync.String(), + Desc: "Sync by applying all manifests", + Index: 0, + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + Metadata: nil, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), }, { - Id: PredefinedStageRollback, - Name: StageK8sRollback.String(), - Desc: "Rollback the deployment", - Predefined: true, - Visible: false, - Status: model.StageStatus_STAGE_NOT_STARTED_YET, - CreatedAt: now.Unix(), - UpdatedAt: now.Unix(), + Id: PredefinedStageRollback, + Name: StageK8sRollback.String(), + Desc: "Rollback the deployment", + Rollback: true, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), }, }, }, @@ -87,3 +86,114 @@ func TestBuildQuickSyncPipeline(t *testing.T) { }) } } + +func TestBuildPipelineStages(t *testing.T) { + t.Parallel() + + now := time.Now() + + tests := []struct { + name string + stages []*deployment.BuildPipelineSyncStagesRequest_StageConfig + autoRollback bool + expected []*model.PipelineStage + }{ + { + name: "without auto rollback", + stages: []*deployment.BuildPipelineSyncStagesRequest_StageConfig{ + { + Id: "stage-1", + Name: "Stage 1", + Desc: "Description 1", + Index: 0, + }, + { + Id: "stage-2", + Name: "Stage 2", + Desc: "Description 2", + Index: 1, + }, + }, + autoRollback: false, + expected: []*model.PipelineStage{ + { + Id: "stage-1", + Name: "Stage 1", + Desc: "Description 1", + Index: 0, + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }, + { + Id: "stage-2", + Name: "Stage 2", + Desc: "Description 2", + Index: 1, + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }, + }, + }, + { + name: "with auto rollback", + stages: []*deployment.BuildPipelineSyncStagesRequest_StageConfig{ + { + Id: "stage-1", + Name: "Stage 1", + Desc: "Description 1", + Index: 0, + }, + { + Id: "stage-2", + Name: "Stage 2", + Desc: "Description 2", + Index: 1, + }, + }, + autoRollback: true, + expected: []*model.PipelineStage{ + { + Id: "stage-1", + Name: "Stage 1", + Desc: "Description 1", + Index: 0, + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }, + { + Id: "stage-2", + Name: "Stage 2", + Desc: "Description 2", + Index: 1, + Rollback: false, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }, + { + Id: PredefinedStageRollback, + Name: StageK8sRollback.String(), + Desc: "Rollback the deployment", + Index: 0, + Rollback: true, + Status: model.StageStatus_STAGE_NOT_STARTED_YET, + CreatedAt: now.Unix(), + UpdatedAt: now.Unix(), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := buildPipelineStages(tt.stages, 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 index 7619e52ec4..28210578b4 100644 --- a/pkg/app/pipedv1/plugin/kubernetes/deployment/server.go +++ b/pkg/app/pipedv1/plugin/kubernetes/deployment/server.go @@ -88,8 +88,12 @@ func (a *DeploymentService) DetermineVersions(ctx context.Context, request *depl } // BuildPipelineSyncStages implements deployment.DeploymentServiceServer. -func (a *DeploymentService) BuildPipelineSyncStages(context.Context, *deployment.BuildPipelineSyncStagesRequest) (*deployment.BuildPipelineSyncStagesResponse, error) { - panic("unimplemented") +func (a *DeploymentService) BuildPipelineSyncStages(ctx context.Context, request *deployment.BuildPipelineSyncStagesRequest) (*deployment.BuildPipelineSyncStagesResponse, error) { + now := time.Now() + stages := buildPipelineStages(request.GetStages(), request.GetRollback(), now) + return &deployment.BuildPipelineSyncStagesResponse{ + Stages: stages, + }, nil } // BuildQuickSyncStages implements deployment.DeploymentServiceServer.