Skip to content

Conversation

guikcd
Copy link
Contributor

@guikcd guikcd commented Aug 28, 2024

Issue #, if available:
Closes: #605, closes: #538

Description of changes:

Add support for tags:

  • service: propagate tags from service during an update
  • run-task: set custom tags

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@guikcd guikcd marked this pull request as draft August 28, 2024 11:13
@s3cube s3cube added the feature-request A feature should be added or improved. label Aug 28, 2024
@guikcd
Copy link
Contributor Author

guikcd commented Aug 29, 2024

I encountered an issue when wanted to propagate tags from task definition for run-task because of #249 (comment). The describe-task-definition can return tags, but it need to be merged to the task definition, which is not easy to do with cli or in amazon-ecs-render-task-definition (see aws-actions/amazon-ecs-render-task-definition#317).

@guikcd guikcd force-pushed the tags branch 2 times, most recently from 09efb45 to ee0f18e Compare August 30, 2024 13:44
@guikcd guikcd marked this pull request as ready for review August 30, 2024 13:46
@guikcd
Copy link
Contributor Author

guikcd commented Aug 30, 2024

@kg-aws ready to review

@guikcd guikcd requested a review from kg-aws August 30, 2024 16:45
Copy link
Contributor

@kg-aws kg-aws left a comment

Choose a reason for hiding this comment

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

LGTM!

expect(core.setFailed).toHaveBeenNthCalledWith(2, 'Could not parse');
});

test('propagate service tags from service and enable ecs managed tags', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @guikcd, thanks for this PR!
I believe we are missing unit tests that checks the run-task-tags functionality. Would it also be possible to add unit tests for the run-task-tags parameter/functionality as part of existing or as a new test.

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've modified the existing test run task with options to verify run-task-tags.

@guikcd guikcd requested a review from amazreech September 3, 2024 15:07
Copy link
Contributor

@amazreech amazreech left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! LGTM!

@amazreech amazreech merged commit 1b137d4 into aws-actions:master Sep 3, 2024
6 checks passed
@guikcd guikcd deleted the tags branch September 5, 2024 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Add tags to task created via the run-task option. Add support for tagging my ECS service
4 participants