Add pipecd builtin tags to ECS resources #4140

TonkyH · 2023-01-19T10:34:10Z

What this PR does / why we need it:

Add PipeCD builtin tags for ECS resources.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

khanhtc1202 · 2023-01-23T04:04:24Z

pkg/app/piped/executor/ecs/rollback.go

@@ -105,7 +105,7 @@ func rollback(ctx context.Context, in *executor.Input, platformProviderName stri
 	// Re-register TaskDef to get TaskDefArn.
 	// Consider using DescribeServices and get services[0].taskSets[0].taskDefinition (taskDefinition of PRIMARY taskSet)
 	// then store it in metadata store and use for rollback instead.
-	td, err := client.RegisterTaskDefinition(ctx, taskDefinition)
+	td, err := client.RegisterTaskDefinition(ctx, taskDefinition, serviceDefinition.Tags)


I think we only care about the cluster/service/task thus we don't need to tag the taskDefinition, since it isn't a part of the application anyway. wdyt?

~~I can also get TaskDef from Task, and it certainly seems unnecessary...~~
Since TaskDef can also be retrieved from Task, it certainly seems unnecessary to tag the taskDef.
I'll give it some more thought and fix it!🙇

Any besides, we should check whether tags which are added to the service is automatically added to the tasks which are controlled by service or not as well 👀

The --propagateTags parameter can be used to copy tags from a task definition or service to a task in the service🚀

https://docs.aws.amazon.com/cli/latest/reference/ecs/create-service.html#:~:text=Service%20Developer%20Guide%20.-,%2D%2Dpropagate%2Dtags,-(string)

khanhtc1202 · 2023-01-23T06:02:51Z

pkg/app/piped/platformprovider/ecs/ecs.go

 	"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.


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.

khanhtc1202 · 2023-01-27T04:35:56Z

pkg/app/piped/platformprovider/ecs/ecs.go

@@ -109,3 +119,11 @@ var defaultRegistry = &registry{
 func DefaultRegistry() Registry {
 	return defaultRegistry
 }
+
+func CreateTags(keyValue map[string]string) []types.Tag {


Suggested change

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

func MakeTags(tags map[string]string) []types.Tag {

nits

khanhtc1202

Nice work 👍

khanhtc1202 · 2023-01-27T05:07:32Z

pkg/app/piped/executor/ecs/ecs.go

+	if serviceDefinition.PropagateTags == "" {
+		serviceDefinition.PropagateTags = types.PropagateTagsService
+	}


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?

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.

khanhtc1202

Here you go 🙌

* Add tags to aws resources. * Small fix * Small fix * Fix initial slice length. * Add TagResource function * Fix deploy process * Add tags to aws resources. * Small fix * Small fix * Fix initial slice length. * Add TagResource function * Fix deploy process * Delete Unnecessary blank lines * Change how to add task's tags * Delete TagResource method * Fix based on linter * Apply review * add builtin-labels * Add tagResource func * Set default value of PropagateTags * Apply review * Apply review

#4157 #4161 (#4163) * Templating commit message used by Event watcher (#4120) * Added ability to teplate eventWatcher.gitRepos.commitMessage * parse template in the eventWatcher handler side * Parse template in the eventWatcher handler side * Fix html/template to text/template * There was a useless process when retrieving rules, so I omitted it. * Delete html/template * Update pkg/app/piped/eventwatcher/eventwatcher.go Co-authored-by: Khanh Tran <[email protected]> * Update pkg/app/piped/eventwatcher/eventwatcher.go Co-authored-by: Khanh Tran <[email protected]> * Using template variables are only Value and EventName. * Update pkg/app/piped/eventwatcher/eventwatcher.go Co-authored-by: knanao <[email protected]> * Update pkg/app/piped/eventwatcher/eventwatcher.go Co-authored-by: knanao <[email protected]> * Fix syntax error * Update pkg/app/piped/eventwatcher/eventwatcher.go Co-authored-by: Khanh Tran <[email protected]> Co-authored-by: Khanh Tran <[email protected]> Co-authored-by: knanao <[email protected]> * Add environment variable expire web session (#4127) * add field session-ttl to project * edit values * reflect ttl config to session * return values * add field in document * return config * change variable name * modify document and manifest * modify docs * change minites tp hours * regen * Change build tag every time when make run/pipecd (#4130) * add unix time to tail of build tag * modify assigning * change variable name * change type pf assign * Fix docs wrong params name (#4132) * Fix pod config for envoy upstreaming termination error (#4134) * add lifecycle * add preStop to api server's manifest * return makefile * Fix Fix parseComitMsg's comment (#4135) * Add kubectl version to stage log (#4137) * Add kubectl version to stage log * Update pkg/app/piped/executor/kubernetes/kubernetes.go Co-authored-by: knanao <[email protected]> Co-authored-by: knanao <[email protected]> * Add pipecd builtin tags to ECS resources (#4140) * Add tags to aws resources. * Small fix * Small fix * Fix initial slice length. * Add TagResource function * Fix deploy process * Add tags to aws resources. * Small fix * Small fix * Fix initial slice length. * Add TagResource function * Fix deploy process * Delete Unnecessary blank lines * Change how to add task's tags * Delete TagResource method * Fix based on linter * Apply review * add builtin-labels * Add tagResource func * Set default value of PropagateTags * Apply review * Apply review * Add upgrade a release method for helm (#4143) * tool: Replace set-output with GITHUB_OUTPUT (#4155) * Make all changes that not be listed in ignores but placed under appDir considered as touched commit (#4157) * Release v0.41.4 (#4161) * Release v0.41.4 * Update the release note --------- Co-authored-by: funera1 <[email protected]> Co-authored-by: Khanh Tran <[email protected]> Co-authored-by: kevin55156 <[email protected]> Co-authored-by: Tomoki Hori <[email protected]> Co-authored-by: Naoki Kanatani <[email protected]>

Add tags to aws resources.

c006e82

github-actions bot added the area/go label Jan 19, 2023

TonkyH added 11 commits January 19, 2023 19:38

Small fix

13fc5bc

Small fix

a973d11

Fix initial slice length.

ca0a60d

Add TagResource function

b2ce287

Fix deploy process

c2a85d1

Add tags to aws resources.

9a9ee5b

Small fix

74c29dc

Small fix

9643e01

Fix initial slice length.

b072278

Add TagResource function

09a423a

Fix deploy process

33831eb

TonkyH force-pushed the ecs-tag branch from c2a85d1 to 33831eb Compare January 20, 2023 06:50

Merge branch 'ecs-tag' of github.com:TonkyH/pipecd into ecs-tag

1297a75

TonkyH marked this pull request as ready for review January 20, 2023 07:38

TonkyH requested review from nghialv, khanhtc1202 and knanao as code owners January 20, 2023 07:38

TonkyH added 4 commits January 20, 2023 16:52

Delete Unnecessary blank lines

c4ae0f5

Change how to add task's tags

5882609

Delete TagResource method

14954a7

Fix based on linter

9b8a215

khanhtc1202 changed the title ~~Add tags to aws resources.~~ Add pipecd builtin tags to ECS resources Jan 23, 2023

khanhtc1202 reviewed Jan 23, 2023

View reviewed changes

Apply review

17697c3

khanhtc1202 reviewed Jan 23, 2023

View reviewed changes

TonkyH added 3 commits January 27, 2023 10:12

add builtin-labels

defc1ba

Add tagResource func

d517e33

Merge branch 'master' of github.com:TonkyH/pipecd into ecs-tag

c7970b4

khanhtc1202 reviewed Jan 27, 2023

View reviewed changes

TonkyH added 2 commits January 27, 2023 13:45

Set default value of PropagateTags

6669ad1

Apply review

9833e37

khanhtc1202 previously approved these changes Jan 27, 2023

View reviewed changes

khanhtc1202 reviewed Jan 27, 2023

View reviewed changes

Apply review

94c3ba1

TonkyH dismissed khanhtc1202’s stale review via 94c3ba1 January 27, 2023 06:32

khanhtc1202 approved these changes Jan 27, 2023

View reviewed changes

khanhtc1202 merged commit 9f1414c into pipe-cd:master Jan 27, 2023

This was referenced Feb 7, 2023

Release v0.41.4 #4161

Merged

Release v0.42.0 #4162

Closed

knanao mentioned this pull request Feb 7, 2023

Cherry-pick #4120 #4127 #4130 #4132 #4134 #4135 #4137 #4140 #4143 #4155 #4157 #4161 #4163

Merged

github-actions bot mentioned this pull request Feb 27, 2023

Release v0.42.0 #4226

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pipecd builtin tags to ECS resources #4140

Add pipecd builtin tags to ECS resources #4140

TonkyH commented Jan 19, 2023 •

edited by khanhtc1202

Loading

khanhtc1202 Jan 23, 2023

TonkyH Jan 23, 2023 •

edited

Loading

khanhtc1202 Jan 23, 2023

TonkyH Jan 23, 2023 •

edited

Loading

khanhtc1202 Jan 23, 2023

khanhtc1202 Jan 27, 2023

khanhtc1202 left a comment

khanhtc1202 Jan 27, 2023

TonkyH Jan 27, 2023

khanhtc1202 left a comment

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

Add pipecd builtin tags to ECS resources #4140

Add pipecd builtin tags to ECS resources #4140

Conversation

TonkyH commented Jan 19, 2023 • edited by khanhtc1202 Loading

khanhtc1202 Jan 23, 2023

Choose a reason for hiding this comment

TonkyH Jan 23, 2023 • edited Loading

Choose a reason for hiding this comment

khanhtc1202 Jan 23, 2023

Choose a reason for hiding this comment

TonkyH Jan 23, 2023 • edited Loading

Choose a reason for hiding this comment

khanhtc1202 Jan 23, 2023

Choose a reason for hiding this comment

khanhtc1202 Jan 27, 2023

Choose a reason for hiding this comment

khanhtc1202 left a comment

Choose a reason for hiding this comment

khanhtc1202 Jan 27, 2023

Choose a reason for hiding this comment

TonkyH Jan 27, 2023

Choose a reason for hiding this comment

khanhtc1202 left a comment

Choose a reason for hiding this comment

TonkyH commented Jan 19, 2023 •

edited by khanhtc1202

Loading

TonkyH Jan 23, 2023 •

edited

Loading

TonkyH Jan 23, 2023 •

edited

Loading