Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions pkg/app/piped/executor/ecs/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ 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"
Expand Down Expand Up @@ -97,9 +99,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 not 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
Expand Down
31 changes: 31 additions & 0 deletions pkg/config/application_ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ package config

import (
"encoding/json"
"fmt"
)

const (
AccessTypeELB string = "ELB"
AccessTypeServiceDiscovery string = "SERVICE_DISCOVERY"
)

// ECSApplicationSpec represents an application configuration for ECS application.
Expand All @@ -32,6 +38,11 @@ func (s *ECSApplicationSpec) Validate() error {
if err := s.GenericApplicationSpec.Validate(); err != nil {
return err
}

if err := s.Input.validate(); err != nil {
return err
}

return nil
}

Expand All @@ -57,12 +68,22 @@ type ECSDeploymentInput struct {
// Run standalone task during deployment.
// 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"`
}

func (in *ECSDeploymentInput) IsStandaloneTask() bool {
return in.ServiceDefinitionFile == ""
}

func (in *ECSDeploymentInput) IsAccessedViaELB() bool {
return in.AccessType == AccessTypeELB
}

type ECSVpcConfiguration struct {
Subnets []string
AssignPublicIP string
Expand Down Expand Up @@ -121,3 +142,13 @@ func (opts ECSTrafficRoutingStageOptions) Percentage() (primary, canary int) {
canary = 0
return
}

func (in *ECSDeploymentInput) validate() error {
switch in.AccessType {
case AccessTypeELB, AccessTypeServiceDiscovery:
break
default:
return fmt.Errorf("invalid accessType: %s", in.AccessType)
}
return nil
}
70 changes: 70 additions & 0 deletions pkg/config/application_ecs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package config

import (
"encoding/json"
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -64,10 +65,79 @@ func TestECSApplicationConfig(t *testing.T) {
LaunchType: "FARGATE",
AutoRollback: newBoolPointer(true),
RunStandaloneTask: newBoolPointer(true),
AccessType: "ELB",
},
},
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,
},
{
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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: pipecd.dev/v1beta1
kind: ECSApp
spec:
input:
serviceDefinitionFile: /path/to/servicedef.yaml
taskDefinitionFile: /path/to/taskdef.yaml
accessType: XXX
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constant is defined by us or by AWS lib? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"This constant" means 'SERVICE_DISCOVERY'?

If so, it's defined by us, not by AWS.

And other names would be added in the future, like 'APP_MESH'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we should define these values as constant of ECS platform provider and have validate in config package

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I got it. I'll fix it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@khanhtc1202
I did it. Could you check again?
919cc3c