feat!: Add support for creating ECS service and container definition #76

bryantbiggs · 2023-03-12T23:19:41Z

Description

Add support for creating cluster CloudWatch log group
Add support for ECS service connect
Add support for creating task execution from cluster module; this is for scenarios where you wish to create one task execution role shared across tasks within the cluster (there is also support for individual task execution role at the service level)
Add support for creating container definition within HCL; this is consumed by the service sub-module

Motivation and Context

Adds the ability for users to create all ECS resources; now supporting service and container-definition sub-modules
Closes Add support for volumes in task definitions #70

Breaking Changes

No

How Has This Been Tested?

I have updated at least one of the examples/* to demonstrate and validate my change(s)
I have tested and validated these changes using one or more of the provided examples/* projects
I have executed pre-commit run -a on my pull request

⚠️ Note - the changes here have been tested and validated on a number of examples since its used by the ECS Blueprints project. Once these changes are merged, the ECS Blueprints project will be updated to use this module and this copy will be archived/deleted

…efinition` sub-module

…sk definition changes instead

… computed value

…oid diffs

… policy for task execution role

…ask definition, add support for service alarms

Co-authored-by: Bryant Biggs <[email protected]>

…statements` variable

bryantbiggs · 2023-04-04T20:38:28Z

@antonbabenko I made some updates and provided some responses/feedback - please take another look when you have some time and let me know what your thoughts are. I also added a design doc to capture a lot of the information that went into the current proposal. Hopefully that is useful for both the review and future users

antonbabenko

Looks very good. The only big thing is related to the submodule for ECS Cluster, and we are good to go!

docs/DESIGN.md

modules/service/variables.tf

antonbabenko · 2023-04-07T10:45:01Z

modules/service/variables.tf

+variable "memory" {
+  description = "Amount (in MiB) of memory used by the task. If the `requires_compatibilities` is `FARGATE` this field is required"
+  type        = number
+  default     = 2048


Same here. Why not 4096? Trying to understand the logic in the default value :)

variables.tf

README.md

antonbabenko · 2023-04-07T11:07:38Z

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
  - repo: https://github.com/antonbabenko/pre-commit-terraform
-    rev: v1.77.0
+    rev: v1.77.1


Please add this hook after terraform_fmt:

- id: terraform_wrapper_module_for_each

README.md

Co-authored-by: Anton Babenko <[email protected]>

antonbabenko

LGTM! Great work (as usual)!

antonbabenko

I hit approved a bit too early :) Let's make a submodule for the ECS cluster.

bryantbiggs · 2023-04-07T14:14:55Z

@antonbabenko I've moved the container-definition up to the service module. I added the wrapper pre-commit hook as requested but it seems to be fighting with the format hook

bryantbiggs · 2023-04-07T14:32:40Z

I'm not really digging this influence terragrunt is having over the modules. It seems to be more complexity than value

antonbabenko · 2023-04-09T08:45:32Z

Fixed in antonbabenko/pre-commit-terraform#503 , please use v1.77.2 in this pull-request and rerun it. I couldn't push to your fork:

To github.com:terraform-aws-modules/terraform-aws-ecs.git
 ! [remote rejected] clowdhaus/master -> refs/pull/76/head (deny updating a hidden ref)
error: failed to push some refs to 'github.com:terraform-aws-modules/terraform-aws-ecs.git'

antonbabenko · 2023-04-09T08:56:21Z

I'm not really digging this influence terragrunt is having over the modules. It seems to be more complexity than value

Terragrunt is one of the tools, but the overall suggestion I have is to have both options:

composable ECS units (Cluster, Service, Container-Definition) as submodules
root module, which combines calls to Cluster and Service submodules

It will allow users to achieve flexibility as they need regardless of the wrapper tool they use.

I see you merged the container-definition submodule into the service. I think it was good to have it as a submodule, though. This way users may use it completely separately from the rest of the ECS resources we manage in this repository.

…s to root module

bryantbiggs · 2023-04-12T13:42:25Z

@antonbabenko I think I captured all of the changes, please take another look when you get a chance

antonbabenko

Looks very good. It was hard to review in GitHub UI. Minor comments related to docs mostly.

antonbabenko · 2023-04-19T13:11:57Z

README.md


 ```hcl
 module "ecs" {
-  source = "terraform-aws-modules/ecs/aws"
+  source = "terraform-aws-modules/ecs"


Suggested change

source = "terraform-aws-modules/ecs"

source = "terraform-aws-modules/ecs/aws"

This is in many places throughout the docs.

Updated in c6fef3b

antonbabenko · 2023-04-19T13:20:04Z

main.tf

+  task_exec_iam_role_permissions_boundary = var.task_exec_iam_role_permissions_boundary
+  task_exec_iam_role_tags                 = var.task_exec_iam_role_tags
+  task_exec_iam_role_policies             = var.task_exec_iam_role_policies
+  create_task_exec_policy                 = var.create_task_exec_policy


Maybe add a new line before create_... and comments like you do in the service module to separate blocks.

Updated in c6fef3b

antonbabenko · 2023-04-19T13:26:33Z

modules/cluster/README.md

@@ -0,0 +1,214 @@
+# AWS ECS Terraform module


Suggested change

# AWS ECS Terraform module

# AWS ECS Cluster Terraform module

Updated in c6fef3b

antonbabenko · 2023-04-19T13:41:00Z

outputs.tf

+output "services" {
+  description = "Map of services created and their attributes"
+  value       = module.service
+  sensitive   = true


Unfortunately. I really dislike this feature of Terraform... :(

Yes! See below (its now removed)

antonbabenko · 2023-04-19T13:43:54Z

modules/container-definition/outputs.tf

+
+output "container_definition" {
+  description = "Container definition"
+  value       = local.container_definition


Should this output be marked as sensitive? I see that in modules/service/outputs.tf it is marked so, so here it should probably be also marked?

This was my bad - in the examples I was using an SSM parameter data source to pull a container image and the second you use that, it requires outputs to be marked as sensitive. So for now, I removed that and used a static string and will leave this up to users to mark the output as sensitive (if they need to)

rajibahmed · 2023-04-19T20:24:31Z

great work !! I am trying to use this module, really need this to make my billion dollar app work.

antonbabenko

👍👍👍

## [5.0.0](v4.1.3...v5.0.0) (2023-04-21) ### ⚠ BREAKING CHANGES * Add support for creating ECS service and container definition (#76) ### Features * Add support for creating ECS service and container definition ([#76](#76)) ([57244e6](57244e6))

antonbabenko · 2023-04-21T11:33:56Z

This PR is included in version 5.0.0 🎉

github-actions · 2023-05-22T02:13:37Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

bryantbiggs and others added 30 commits January 27, 2023 15:43

feat: Add support for ECS service

d45632c

feat: Add support for ECS task definition

1b0bb21

feat: Add support for ECS service IAM role

bcae52d

feat: Add support for ECS task execution and ECS tasks IAM roles

718353c

feat: Add support for ECS task sets

e6c17dd

feat: Add support for container definition sub-module

1f5f485

feat: Update service module to latest spec and integrate `container-d…

a4f3fcd

…efinition` sub-module

chore: Update examples to use service sub-module

5255af5

feat: Add support for service autoscaling

c59b22a

feat: Setting defaults based on docs and ECS guidance

986e2b2

feat: Add support for ECS service connect

114430e

feat: Add support for timeouts and triggers on ECS service

5750dff

refactor: Switch to default to ignoring scaling and allow ignoring ta…

7cb25cf

…sk definition changes instead

fix: Use lookup() for service connect namespace since this can be a…

e2d3a92

… computed value

fix: Use null for default value on capacity provider strategy to av…

3e3106f

…oid diffs

fix: Set default cluster capacity provider weight to null

eea18bc

fix: Set launch type correctly when not an external deployment

5c56117

feat: Add support for creating shared task execution role plus secret…

1259a58

… policy for task execution role

fix: Add output for shared task execution role

c3614f3

fix: Ensure Sids are unique

6867416

feat: Add support for creating log group for cluster

dd81a01

feat: Add support for creating security group for service/task

3c35e5f

feat: Output the family attribute for task definition

ef28bab

feat: Allow for enable/disabling creation of CloudWatch logging and t…

9ab549e

…ask definition, add support for service alarms

fix: Ensure that the latest task definition is used

36b68eb

fix: Correct logging configuration for container definition

dd756ba

chore: Use SSM for fluentbit image

1a07577

fixed service connect port_name parsing (#2)

c7600d4

Co-authored-by: Bryant Biggs <[email protected]>

feat: Add support for extending IAM policy permissions with generic `…

dd6c56b

…statements` variable

fix: Correct spelling errors and version mis-match

70d4dd5

bryantbiggs and others added 2 commits April 4, 2023 20:06

chore: Add docs

ad37f81

Update DESIGN.md (#3)

d3a4ff3

antonbabenko reviewed Apr 7, 2023

View reviewed changes

antonbabenko changed the title ~~feat: Add support for creating ECS service and container definition~~ feat!: Add support for creating ECS service and container definition Apr 7, 2023

bryantbiggs commented Apr 7, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

Apply suggestions from code review

1db3647

Co-authored-by: Anton Babenko <[email protected]>

antonbabenko approved these changes Apr 7, 2023

View reviewed changes

antonbabenko requested changes Apr 7, 2023

View reviewed changes

antonbabenko mentioned this pull request Apr 9, 2023

fix: Fixed spacing in terraform_wrapper_module_for_each hook antonbabenko/pre-commit-terraform#503

Merged

antonbabenko mentioned this pull request Apr 11, 2023

[BUG] broken docker configuration on amzn2 ECS image terraform-community-modules/tf_aws_ecs#58

Closed

bryantbiggs added 2 commits April 11, 2023 15:42

feat: Add support for generating Terragrunt wrappers

16442e0

refactor: Move root cluster into module; add both cluster and service…

98218e4

…s to root module

bryantbiggs mentioned this pull request Apr 14, 2023

Using an old version of terraform-aws-modules/ecs/aws terraform-aws-modules/terraform-aws-atlantis#297

Closed

antonbabenko reviewed Apr 19, 2023

View reviewed changes

chore: Updates from PR review

c6fef3b

antonbabenko approved these changes Apr 21, 2023

View reviewed changes

bryantbiggs merged commit 57244e6 into terraform-aws-modules:master Apr 21, 2023

bryantbiggs mentioned this pull request Apr 21, 2023

chore: Switch ECS module sources back to main project and off the forked copy; pin Aurora version aws-ia/ecs-blueprints#133

Merged

2 tasks

grahamrhay mentioned this pull request May 18, 2023

InvalidParameterException: You must set logging to 'OVERRIDE' when you supply a log configuration #84

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Add support for creating ECS service and container definition #76

feat!: Add support for creating ECS service and container definition #76

bryantbiggs commented Mar 12, 2023 •

edited

Loading

bryantbiggs commented Apr 4, 2023

antonbabenko left a comment

antonbabenko Apr 7, 2023

antonbabenko Apr 7, 2023

antonbabenko left a comment

antonbabenko left a comment

bryantbiggs commented Apr 7, 2023

bryantbiggs commented Apr 7, 2023

antonbabenko commented Apr 9, 2023

antonbabenko commented Apr 9, 2023

bryantbiggs commented Apr 12, 2023

antonbabenko left a comment

antonbabenko Apr 19, 2023

bryantbiggs Apr 20, 2023

antonbabenko Apr 19, 2023

bryantbiggs Apr 20, 2023

antonbabenko Apr 19, 2023

bryantbiggs Apr 20, 2023

antonbabenko Apr 19, 2023

bryantbiggs Apr 20, 2023

antonbabenko Apr 19, 2023

bryantbiggs Apr 20, 2023

rajibahmed commented Apr 19, 2023

antonbabenko left a comment

antonbabenko commented Apr 21, 2023

github-actions bot commented May 22, 2023

	source = "terraform-aws-modules/ecs"
	source = "terraform-aws-modules/ecs/aws"

	# AWS ECS Terraform module
	# AWS ECS Cluster Terraform module

feat!: Add support for creating ECS service and container definition #76

feat!: Add support for creating ECS service and container definition #76

Conversation

bryantbiggs commented Mar 12, 2023 • edited Loading

Description

Motivation and Context

Breaking Changes

How Has This Been Tested?

bryantbiggs commented Apr 4, 2023

antonbabenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonbabenko left a comment

Choose a reason for hiding this comment

antonbabenko left a comment

Choose a reason for hiding this comment

bryantbiggs commented Apr 7, 2023

bryantbiggs commented Apr 7, 2023

antonbabenko commented Apr 9, 2023

antonbabenko commented Apr 9, 2023

bryantbiggs commented Apr 12, 2023

antonbabenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajibahmed commented Apr 19, 2023

antonbabenko left a comment

Choose a reason for hiding this comment

antonbabenko commented Apr 21, 2023

github-actions bot commented May 22, 2023

bryantbiggs commented Mar 12, 2023 •

edited

Loading