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.

Sign up for GitHub

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

Jump to bottom

Add pipecd builtin tags to ECS resources #4140

Merged
merged 24 commits into from
Jan 27, 2023
Merged

Add pipecd builtin tags to ECS resources #4140

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c006e82
Add tags to aws resources.
TonkyH Jan 19, 2023
13fc5bc
Small fix
TonkyH Jan 19, 2023
a973d11
Small fix
TonkyH Jan 19, 2023
ca0a60d
Fix initial slice length.
TonkyH Jan 20, 2023
b2ce287
Add TagResource function
TonkyH Jan 20, 2023
c2a85d1
Fix deploy process
TonkyH Jan 20, 2023
9a9ee5b
Add tags to aws resources.
TonkyH Jan 19, 2023
74c29dc
Small fix
TonkyH Jan 19, 2023
9643e01
Small fix
TonkyH Jan 19, 2023
b072278
Fix initial slice length.
TonkyH Jan 20, 2023
09a423a
Add TagResource function
TonkyH Jan 20, 2023
33831eb
Fix deploy process
TonkyH Jan 20, 2023
1297a75
Merge branch 'ecs-tag' of github.com:TonkyH/pipecd into ecs-tag
TonkyH Jan 20, 2023
c4ae0f5
Delete Unnecessary blank lines
TonkyH Jan 20, 2023
5882609
Change how to add task's tags
TonkyH Jan 23, 2023
14954a7
Delete TagResource method
TonkyH Jan 23, 2023
9b8a215
Fix based on linter
TonkyH Jan 23, 2023
17697c3
Apply review
TonkyH Jan 23, 2023
defc1ba
add builtin-labels
TonkyH Jan 27, 2023
d517e33
Add tagResource func
TonkyH Jan 27, 2023
c7970b4
Merge branch 'master' of github.com:TonkyH/pipecd into ecs-tag
TonkyH Jan 27, 2023
6669ad1
Set default value of PropagateTags
TonkyH Jan 27, 2023
9833e37
Apply review
TonkyH Jan 27, 2023
94c3ba1
Apply review
TonkyH Jan 27, 2023
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
25 changes: 25 additions & 0 deletions pkg/app/piped/executor/ecs/ecs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@ func loadServiceDefinition(in *executor.Input, serviceDefinitionFile string, ds
return types.Service{}, false
}

if serviceDefinition.PropagateTags == "" {
serviceDefinition.PropagateTags = types.PropagateTagsService
}
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I missed this part. I think we should always propagate the tags from the service to its tasks due to tags contains out builtin tags as well. So it's better to set the value to CreateServiceInput.PropagateTags directly in CreateService method. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree.
I think we should always propagate tags from the service to its tasks.
So, set the value directly to CreateServiceInput.PropagateTags in the CreateService method.


serviceDefinition.Tags = append(
serviceDefinition.Tags,
provider.MakeTags(map[string]string{
provider.LabelManagedBy: provider.ManagedByPiped,
provider.LabelPiped: in.PipedConfig.PipedID,
provider.LabelApplication: in.Deployment.ApplicationId,
provider.LabelCommitHash: in.Deployment.CommitHash(),
})...,
)

in.LogPersister.Infof("Successfully loaded the ECS service definition at commit %s", ds.Revision)
return serviceDefinition, true
}
Expand Down Expand Up @@ -147,6 +161,10 @@ func applyServiceDefinition(ctx context.Context, cli provider.Client, serviceDef
if err != nil {
return nil, fmt.Errorf("failed to update ECS service %s: %v", *serviceDefinition.ServiceName, err)
}
if err := cli.TagResource(ctx, *service.ServiceArn, serviceDefinition.Tags); err != nil {
return nil, fmt.Errorf("failed to update tags of ECS service %s: %v", *serviceDefinition.ServiceName, err)
}

} else {
service, err = cli.CreateService(ctx, serviceDefinition)
if err != nil {
Expand All @@ -172,6 +190,12 @@ func runStandaloneTask(
}

in.LogPersister.Infof("Start applying the ECS task definition")
tags := provider.MakeTags(map[string]string{
provider.LabelManagedBy: provider.ManagedByPiped,
provider.LabelPiped: in.PipedConfig.PipedID,
provider.LabelApplication: in.Deployment.ApplicationId,
provider.LabelCommitHash: in.Deployment.CommitHash(),
})
td, err := applyTaskDefinition(ctx, client, taskDefinition)
if err != nil {
in.LogPersister.Errorf("Failed to apply ECS task definition: %v", err)
Expand All @@ -184,6 +208,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
16 changes: 14 additions & 2 deletions pkg/app/piped/platformprovider/ecs/client.go
View file
Open in desktop
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 Expand Up @@ -341,3 +341,15 @@ func (c *client) ModifyListener(ctx context.Context, listenerArn string, routing
_, err := c.elbClient.ModifyListener(ctx, input)
return err
}

func (c *client) TagResource(ctx context.Context, resourceArn string, tags []types.Tag) error {
input := &ecs.TagResourceInput{
ResourceArn: aws.String(resourceArn),
Tags: tags,
}
_, err := c.ecsClient.TagResource(ctx, input)
if err != nil {
return fmt.Errorf("failed to update tag of resource %s: %w", resourceArn, err)
}
return nil
}
20 changes: 19 additions & 1 deletion pkg/app/piped/platformprovider/ecs/ecs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,22 @@ 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 (
LabelManagedBy string = "pipecd-dev-managed-by" // Always be piped.
LabelPiped string = "pipecd-dev-piped" // The id of piped handling this application.
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.

LabelCommitHash string = "pipecd-dev-commit-hash" // Hash value of the deployed commit.
ManagedByPiped string = "piped"
)

// Client is wrapper of ECS client.
type Client interface {
ECS
Expand All @@ -37,11 +46,12 @@ 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
UpdateServicePrimaryTaskSet(ctx context.Context, service types.Service, taskSet types.TaskSet) (*types.TaskSet, error)
TagResource(ctx context.Context, resourceArn string, tags []types.Tag) error
}

type ELB interface {
Expand Down Expand Up @@ -109,3 +119,11 @@ var defaultRegistry = &registry{
func DefaultRegistry() Registry {
return defaultRegistry
}

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