Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pipecd builtin tags to ECS resources #4140

Merged
merged 24 commits into from
Jan 27, 2023
Merged
Show file tree
Hide file tree
Changes from 18 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
7 changes: 7 additions & 0 deletions pkg/app/piped/executor/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ func loadServiceDefinition(in *executor.Input, serviceDefinitionFile string, ds
return types.Service{}, false
}

serviceDefinition.Tags = append(
serviceDefinition.Tags,
provider.CreateTags(map[string]string{provider.LabelApplication: in.Deployment.ApplicationId})...,
)

in.LogPersister.Infof("Successfully loaded the ECS service definition at commit %s", ds.Revision)
return serviceDefinition, true
}
Expand Down Expand Up @@ -172,6 +177,7 @@ func runStandaloneTask(
}

in.LogPersister.Infof("Start applying the ECS task definition")
tags := provider.CreateTags(map[string]string{provider.LabelApplication: in.Application.Id})
td, err := applyTaskDefinition(ctx, client, taskDefinition)
if err != nil {
in.LogPersister.Errorf("Failed to apply ECS task definition: %v", err)
Expand All @@ -184,6 +190,7 @@ func runStandaloneTask(
ecsInput.ClusterArn,
ecsInput.LaunchType,
&ecsInput.AwsVpcConfiguration,
tags,
)
if err != nil {
in.LogPersister.Errorf("Failed to run ECS task: %v", err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/app/piped/platformprovider/ecs/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ func (c *client) CreateService(ctx context.Context, service types.Service) (*typ
ServiceRegistries: service.ServiceRegistries,
Tags: service.Tags,
}

output, err := c.ecsClient.CreateService(ctx, input)
if err != nil {
return nil, fmt.Errorf("failed to create ECS service %s: %w", *service.ServiceName, err)
Expand Down Expand Up @@ -151,7 +150,7 @@ func (c *client) RegisterTaskDefinition(ctx context.Context, taskDefinition type
return output.TaskDefinition, nil
}

func (c *client) RunTask(ctx context.Context, taskDefinition types.TaskDefinition, clusterArn string, launchType string, awsVpcConfiguration *appconfig.ECSVpcConfiguration) error {
func (c *client) RunTask(ctx context.Context, taskDefinition types.TaskDefinition, clusterArn string, launchType string, awsVpcConfiguration *appconfig.ECSVpcConfiguration, tags []types.Tag) error {
if taskDefinition.TaskDefinitionArn == nil {
return fmt.Errorf("failed to run task of task family %s: no task definition provided", *taskDefinition.Family)
}
Expand All @@ -160,6 +159,7 @@ func (c *client) RunTask(ctx context.Context, taskDefinition types.TaskDefinitio
TaskDefinition: taskDefinition.Family,
Cluster: aws.String(clusterArn),
LaunchType: types.LaunchType(launchType),
Tags: tags,
}

if len(awsVpcConfiguration.Subnets) > 0 {
Expand Down
15 changes: 14 additions & 1 deletion pkg/app/piped/platformprovider/ecs/ecs.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,18 @@ import (
"path/filepath"
"sync"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/ecs/types"
"go.uber.org/zap"
"golang.org/x/sync/singleflight"

"github.com/pipe-cd/pipecd/pkg/config"
)

const (
LabelApplication string = "pipecd-dev-application" // The application this resource belongs to.
Copy link
Member

Choose a reason for hiding this comment

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

Could you add more builtin tags for ECS resources. For instant, in case of CloudRun, we have as below
https://github.com/pipe-cd/pipecd/blob/master/pkg/app/piped/platformprovider/cloudrun/cloudrun.go#L86-L93

I think we should also have: TagManagedBy (value will be piped in all cases), LabelPiped, LabelCommitHash as the PipeCD builtin tags for ECS resources.

)

// Client is wrapper of ECS client.
type Client interface {
ECS
Expand All @@ -37,7 +42,7 @@ type ECS interface {
CreateService(ctx context.Context, service types.Service) (*types.Service, error)
UpdateService(ctx context.Context, service types.Service) (*types.Service, error)
RegisterTaskDefinition(ctx context.Context, taskDefinition types.TaskDefinition) (*types.TaskDefinition, error)
RunTask(ctx context.Context, taskDefinition types.TaskDefinition, clusterArn string, launchType string, awsVpcConfiguration *config.ECSVpcConfiguration) error
RunTask(ctx context.Context, taskDefinition types.TaskDefinition, clusterArn string, launchType string, awsVpcConfiguration *config.ECSVpcConfiguration, tags []types.Tag) error
GetPrimaryTaskSet(ctx context.Context, service types.Service) (*types.TaskSet, error)
CreateTaskSet(ctx context.Context, service types.Service, taskDefinition types.TaskDefinition, targetGroup *types.LoadBalancer, scale int) (*types.TaskSet, error)
DeleteTaskSet(ctx context.Context, service types.Service, taskSetArn string) error
Expand Down Expand Up @@ -109,3 +114,11 @@ var defaultRegistry = &registry{
func DefaultRegistry() Registry {
return defaultRegistry
}

func CreateTags(keyValue map[string]string) []types.Tag {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
func CreateTags(keyValue map[string]string) []types.Tag {
func MakeTags(tags map[string]string) []types.Tag {

nits

tags := make([]types.Tag, 0, len(keyValue))
for key, value := range keyValue {
tags = append(tags, types.Tag{Key: aws.String(key), Value: aws.String(value)})
}
return tags
}