From ab42d5df649cdd78a89db65def24e41717823c1a Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Wed, 22 Nov 2023 14:39:06 +0900 Subject: [PATCH 01/10] Support QuickSync for ECS Service Discovery pattern Signed-off-by: t-kikuc --- pkg/app/piped/executor/ecs/deploy.go | 11 ++++++++--- pkg/config/application_ecs.go | 7 +++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/app/piped/executor/ecs/deploy.go b/pkg/app/piped/executor/ecs/deploy.go index ca40ecabdc..c11d8da6ec 100644 --- a/pkg/app/piped/executor/ecs/deploy.go +++ b/pkg/app/piped/executor/ecs/deploy.go @@ -17,6 +17,7 @@ package ecs import ( "context" + "github.com/aws/aws-sdk-go-v2/service/ecs/types" "github.com/pipe-cd/pipecd/pkg/app/piped/deploysource" "github.com/pipe-cd/pipecd/pkg/app/piped/executor" "github.com/pipe-cd/pipecd/pkg/config" @@ -97,9 +98,13 @@ func (e *deployExecutor) ensureSync(ctx context.Context) model.StageStatus { return model.StageStatus_STAGE_FAILURE } - primary, _, ok := loadTargetGroups(&e.Input, e.appCfg, e.deploySource) - if !ok { - return model.StageStatus_STAGE_FAILURE + var primary *types.LoadBalancer + // When the service is accessed via ELB, the target group is not used. + if ecsInput.IsAccessedViaELB() { + primary, _, ok = loadTargetGroups(&e.Input, e.appCfg, e.deploySource) + if !ok { + return model.StageStatus_STAGE_FAILURE + } } recreate := e.appCfg.QuickSync.Recreate diff --git a/pkg/config/application_ecs.go b/pkg/config/application_ecs.go index fb0e5e72d3..6bf2d663c1 100644 --- a/pkg/config/application_ecs.go +++ b/pkg/config/application_ecs.go @@ -57,12 +57,19 @@ type ECSDeploymentInput struct { // Run standalone task during deployment. // Default is true. RunStandaloneTask *bool `json:"runStandaloneTask" default:"true"` + // How the ECS service is accessed. + // Default is ELB. + AccessType string `json:"accessType" default:"ELB"` } func (in *ECSDeploymentInput) IsStandaloneTask() bool { return in.ServiceDefinitionFile == "" } +func (in *ECSDeploymentInput) IsAccessedViaELB() bool { + return in.AccessType == "ELB" +} + type ECSVpcConfiguration struct { Subnets []string AssignPublicIP string From b1fd711304c328df8947b85f128ce9e1b894f9ed Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Wed, 22 Nov 2023 15:08:46 +0900 Subject: [PATCH 02/10] Update the test expectation of ELB pattern Signed-off-by: t-kikuc --- pkg/config/application_ecs_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/config/application_ecs_test.go b/pkg/config/application_ecs_test.go index 2b63f733e1..1a79b58de1 100644 --- a/pkg/config/application_ecs_test.go +++ b/pkg/config/application_ecs_test.go @@ -64,6 +64,7 @@ func TestECSApplicationConfig(t *testing.T) { LaunchType: "FARGATE", AutoRollback: newBoolPointer(true), RunStandaloneTask: newBoolPointer(true), + AccessType: "ELB", }, }, expectedError: nil, From 476c3f68f5518a23fb5323cf1cb5f6cea6bcc9a7 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Wed, 22 Nov 2023 15:32:20 +0900 Subject: [PATCH 03/10] Add a test of QuickSync of ECS Service Discovery Signed-off-by: t-kikuc --- pkg/config/application_ecs_test.go | 34 +++++++++++++++++++ .../ecs-app-service-discovery.yaml | 7 ++++ 2 files changed, 41 insertions(+) create mode 100644 pkg/config/testdata/application/ecs-app-service-discovery.yaml diff --git a/pkg/config/application_ecs_test.go b/pkg/config/application_ecs_test.go index 1a79b58de1..b2c942799a 100644 --- a/pkg/config/application_ecs_test.go +++ b/pkg/config/application_ecs_test.go @@ -69,6 +69,40 @@ func TestECSApplicationConfig(t *testing.T) { }, expectedError: nil, }, + { + fileName: "testdata/application/ecs-app-service-discovery.yaml", + expectedKind: KindECSApp, + expectedAPIVersion: "pipecd.dev/v1beta1", + expectedSpec: &ECSApplicationSpec{ + GenericApplicationSpec: GenericApplicationSpec{ + Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + OnCommand: OnCommand{ + Disabled: false, + }, + OnOutOfSync: OnOutOfSync{ + Disabled: newBoolPointer(true), + MinWindow: Duration(5 * time.Minute), + }, + OnChain: OnChain{ + Disabled: newBoolPointer(true), + }, + }, + }, + Input: ECSDeploymentInput{ + ServiceDefinitionFile: "/path/to/servicedef.yaml", + TaskDefinitionFile: "/path/to/taskdef.yaml", + LaunchType: "FARGATE", + AutoRollback: newBoolPointer(true), + RunStandaloneTask: newBoolPointer(true), + AccessType: "SERVICE_DISCOVERY", + }, + }, + expectedError: nil, + }, } for _, tc := range testcases { t.Run(tc.fileName, func(t *testing.T) { diff --git a/pkg/config/testdata/application/ecs-app-service-discovery.yaml b/pkg/config/testdata/application/ecs-app-service-discovery.yaml new file mode 100644 index 0000000000..cc9da20611 --- /dev/null +++ b/pkg/config/testdata/application/ecs-app-service-discovery.yaml @@ -0,0 +1,7 @@ +apiVersion: pipecd.dev/v1beta1 +kind: ECSApp +spec: + input: + serviceDefinitionFile: /path/to/servicedef.yaml + taskDefinitionFile: /path/to/taskdef.yaml + accessType: SERVICE_DISCOVERY \ No newline at end of file From 1a0b60db33c704dee7d95837ccd561fd40c5fb1b Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Wed, 22 Nov 2023 15:36:42 +0900 Subject: [PATCH 04/10] fix comment Signed-off-by: t-kikuc --- pkg/app/piped/executor/ecs/deploy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/app/piped/executor/ecs/deploy.go b/pkg/app/piped/executor/ecs/deploy.go index c11d8da6ec..57c7ce368a 100644 --- a/pkg/app/piped/executor/ecs/deploy.go +++ b/pkg/app/piped/executor/ecs/deploy.go @@ -99,7 +99,7 @@ func (e *deployExecutor) ensureSync(ctx context.Context) model.StageStatus { } var primary *types.LoadBalancer - // When the service is accessed via ELB, the target group is not used. + // When the service is not accessed via ELB, the target group is not used. if ecsInput.IsAccessedViaELB() { primary, _, ok = loadTargetGroups(&e.Input, e.appCfg, e.deploySource) if !ok { From 761b66b33b0c6669bed69e1e8c4fedad40e5b39d Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Wed, 22 Nov 2023 19:14:24 +0900 Subject: [PATCH 05/10] Add a description of Input values Signed-off-by: t-kikuc --- pkg/config/application_ecs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/config/application_ecs.go b/pkg/config/application_ecs.go index 6bf2d663c1..e6bfae8dd9 100644 --- a/pkg/config/application_ecs.go +++ b/pkg/config/application_ecs.go @@ -58,6 +58,9 @@ type ECSDeploymentInput struct { // Default is true. RunStandaloneTask *bool `json:"runStandaloneTask" default:"true"` // How the ECS service is accessed. + // Possible values are: + // - ELB - The service is accessed via ELB and target groups. + // - SERVICE_DISCOVERY - The service is accessed via ECS Service Discovery. // Default is ELB. AccessType string `json:"accessType" default:"ELB"` } From 88b0afeaa95588f13b327fcb88d1fd45ed86b5e8 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 23 Nov 2023 14:02:21 +0900 Subject: [PATCH 06/10] Add consts and validation of accessType Signed-off-by: t-kikuc --- pkg/config/application_ecs.go | 22 +++++++++++- pkg/config/application_ecs_test.go | 35 +++++++++++++++++++ .../ecs-app-invalid-access-type.yaml | 7 ++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 pkg/config/testdata/application/ecs-app-invalid-access-type.yaml diff --git a/pkg/config/application_ecs.go b/pkg/config/application_ecs.go index e6bfae8dd9..152b7fe9f1 100644 --- a/pkg/config/application_ecs.go +++ b/pkg/config/application_ecs.go @@ -16,6 +16,12 @@ package config import ( "encoding/json" + "fmt" +) + +const ( + accessTypeELB string = "ELB" + accessTypeServiveDiscovery string = "SERVICE_DISCOVERY" ) // ECSApplicationSpec represents an application configuration for ECS application. @@ -32,6 +38,11 @@ func (s *ECSApplicationSpec) Validate() error { if err := s.GenericApplicationSpec.Validate(); err != nil { return err } + + if err := s.Input.validateAccessType(); err != nil { + return err + } + return nil } @@ -70,7 +81,7 @@ func (in *ECSDeploymentInput) IsStandaloneTask() bool { } func (in *ECSDeploymentInput) IsAccessedViaELB() bool { - return in.AccessType == "ELB" + return in.AccessType == accessTypeELB } type ECSVpcConfiguration struct { @@ -131,3 +142,12 @@ func (opts ECSTrafficRoutingStageOptions) Percentage() (primary, canary int) { canary = 0 return } + +func (in *ECSDeploymentInput) validateAccessType() error { + switch in.AccessType { + case accessTypeELB, accessTypeServiveDiscovery: + return nil + default: + return fmt.Errorf("invalid accessType: %s", in.AccessType) + } +} diff --git a/pkg/config/application_ecs_test.go b/pkg/config/application_ecs_test.go index b2c942799a..305a4bf437 100644 --- a/pkg/config/application_ecs_test.go +++ b/pkg/config/application_ecs_test.go @@ -16,6 +16,7 @@ package config import ( "encoding/json" + "fmt" "testing" "time" @@ -103,6 +104,40 @@ func TestECSApplicationConfig(t *testing.T) { }, expectedError: nil, }, + { + fileName: "testdata/application/ecs-app-invalid-access-type.yaml", + expectedKind: KindECSApp, + expectedAPIVersion: "pipecd.dev/v1beta1", + expectedSpec: &ECSApplicationSpec{ + GenericApplicationSpec: GenericApplicationSpec{ + Timeout: Duration(6 * time.Hour), + Trigger: Trigger{ + OnCommit: OnCommit{ + Disabled: false, + }, + OnCommand: OnCommand{ + Disabled: false, + }, + OnOutOfSync: OnOutOfSync{ + Disabled: newBoolPointer(true), + MinWindow: Duration(5 * time.Minute), + }, + OnChain: OnChain{ + Disabled: newBoolPointer(true), + }, + }, + }, + Input: ECSDeploymentInput{ + ServiceDefinitionFile: "/path/to/servicedef.yaml", + TaskDefinitionFile: "/path/to/taskdef.yaml", + LaunchType: "FARGATE", + AutoRollback: newBoolPointer(true), + RunStandaloneTask: newBoolPointer(true), + AccessType: "XXX", + }, + }, + expectedError: fmt.Errorf("invalid accessType: XXX"), + }, } for _, tc := range testcases { t.Run(tc.fileName, func(t *testing.T) { diff --git a/pkg/config/testdata/application/ecs-app-invalid-access-type.yaml b/pkg/config/testdata/application/ecs-app-invalid-access-type.yaml new file mode 100644 index 0000000000..4f41eb974e --- /dev/null +++ b/pkg/config/testdata/application/ecs-app-invalid-access-type.yaml @@ -0,0 +1,7 @@ +apiVersion: pipecd.dev/v1beta1 +kind: ECSApp +spec: + input: + serviceDefinitionFile: /path/to/servicedef.yaml + taskDefinitionFile: /path/to/taskdef.yaml + accessType: XXX \ No newline at end of file From 197507e3c404a48e3d521c0ea7cd02b4b55bdb19 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Thu, 23 Nov 2023 22:10:24 +0900 Subject: [PATCH 07/10] Modify consts to public for using in switch-case Signed-off-by: t-kikuc --- pkg/config/application_ecs.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/config/application_ecs.go b/pkg/config/application_ecs.go index 152b7fe9f1..edb642dfb7 100644 --- a/pkg/config/application_ecs.go +++ b/pkg/config/application_ecs.go @@ -20,8 +20,8 @@ import ( ) const ( - accessTypeELB string = "ELB" - accessTypeServiveDiscovery string = "SERVICE_DISCOVERY" + AccessTypeELB string = "ELB" + AccessTypeServiveDiscovery string = "SERVICE_DISCOVERY" ) // ECSApplicationSpec represents an application configuration for ECS application. @@ -81,7 +81,7 @@ func (in *ECSDeploymentInput) IsStandaloneTask() bool { } func (in *ECSDeploymentInput) IsAccessedViaELB() bool { - return in.AccessType == accessTypeELB + return in.AccessType == AccessTypeELB } type ECSVpcConfiguration struct { @@ -145,7 +145,7 @@ func (opts ECSTrafficRoutingStageOptions) Percentage() (primary, canary int) { func (in *ECSDeploymentInput) validateAccessType() error { switch in.AccessType { - case accessTypeELB, accessTypeServiveDiscovery: + case AccessTypeELB, AccessTypeServiveDiscovery: return nil default: return fmt.Errorf("invalid accessType: %s", in.AccessType) From 5e2e6760fc957df5d3cfb7f0e495c92705f7671f Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Sun, 26 Nov 2023 00:28:24 +0900 Subject: [PATCH 08/10] Fix a typo Signed-off-by: t-kikuc --- pkg/config/application_ecs.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/config/application_ecs.go b/pkg/config/application_ecs.go index edb642dfb7..dbfc354e64 100644 --- a/pkg/config/application_ecs.go +++ b/pkg/config/application_ecs.go @@ -21,7 +21,7 @@ import ( const ( AccessTypeELB string = "ELB" - AccessTypeServiveDiscovery string = "SERVICE_DISCOVERY" + AccessTypeServiceDiscovery string = "SERVICE_DISCOVERY" ) // ECSApplicationSpec represents an application configuration for ECS application. @@ -145,7 +145,7 @@ func (opts ECSTrafficRoutingStageOptions) Percentage() (primary, canary int) { func (in *ECSDeploymentInput) validateAccessType() error { switch in.AccessType { - case AccessTypeELB, AccessTypeServiveDiscovery: + case AccessTypeELB, AccessTypeServiceDiscovery: return nil default: return fmt.Errorf("invalid accessType: %s", in.AccessType) From 840f2cea3850d8e0797b3c8dd3f861d3e71b0c92 Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Mon, 27 Nov 2023 15:20:33 +0900 Subject: [PATCH 09/10] Refactor validation func Signed-off-by: t-kikuc --- pkg/config/application_ecs.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/config/application_ecs.go b/pkg/config/application_ecs.go index dbfc354e64..14deab78c6 100644 --- a/pkg/config/application_ecs.go +++ b/pkg/config/application_ecs.go @@ -39,7 +39,7 @@ func (s *ECSApplicationSpec) Validate() error { return err } - if err := s.Input.validateAccessType(); err != nil { + if err := s.Input.validate(); err != nil { return err } @@ -143,11 +143,12 @@ func (opts ECSTrafficRoutingStageOptions) Percentage() (primary, canary int) { return } -func (in *ECSDeploymentInput) validateAccessType() error { +func (in *ECSDeploymentInput) validate() error { switch in.AccessType { case AccessTypeELB, AccessTypeServiceDiscovery: - return nil + break default: return fmt.Errorf("invalid accessType: %s", in.AccessType) } + return nil } From 8159ba7678d4da218bdc78bfb2d142876ac14a8e Mon Sep 17 00:00:00 2001 From: t-kikuc Date: Mon, 27 Nov 2023 15:52:45 +0900 Subject: [PATCH 10/10] Add a blank line in import Signed-off-by: t-kikuc --- pkg/app/piped/executor/ecs/deploy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/app/piped/executor/ecs/deploy.go b/pkg/app/piped/executor/ecs/deploy.go index 57c7ce368a..14f8c517bc 100644 --- a/pkg/app/piped/executor/ecs/deploy.go +++ b/pkg/app/piped/executor/ecs/deploy.go @@ -18,6 +18,7 @@ import ( "context" "github.com/aws/aws-sdk-go-v2/service/ecs/types" + "github.com/pipe-cd/pipecd/pkg/app/piped/deploysource" "github.com/pipe-cd/pipecd/pkg/app/piped/executor" "github.com/pipe-cd/pipecd/pkg/config"