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

feat: Add support for cluster and container definition custom CloudWatch log group names #160

Merged
merged 2 commits into from
Feb 12, 2024
Merged

feat: Add support for cluster and container definition custom CloudWatch log group names #160

merged 2 commits into from
Feb 12, 2024

Conversation

ivan-sukhomlyn
Copy link
Contributor

Description

The possibility to define a custom AWS CloudWatch LogGroup name instead of a pre-defined pattern at the module.

Motivation and Context

Previously, it was impossible to define the custom name of CW LogGroup while creating a group with the modules.

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

ECS Cluster Log Group

  • config
module "ecs_cluster" {
  source = "../../../../../../open-source/terraform-aws-ecs/modules/cluster"
  #  source  = "terraform-aws-modules/ecs/aws//modules//cluster"
  #  version = "~> 5.7"
  # ...
  cloudwatch_log_group_name = "/aws/ecs/custom-cluster-test"
  • terraform plan
  # module.ecs_cluster.aws_cloudwatch_log_group.this[0] must be replaced
-/+ resource "aws_cloudwatch_log_group" "this" {
      ~ arn               = "arn:aws:logs:xxx:xxx:log-group:/aws/ecs/xxx" -> (known after apply)
...
      ~ name              = "/aws/ecs/xxx" -> "/aws/ecs/custom-cluster-test" # forces replacement
...
    }
  • config
module "ecs_cluster" {
  # ...
  # cloudwatch_log_group_name = "/aws/ecs/custom-cluster-test"
  • terraform plan
No changes. Your infrastructure matches the configuration.

ECS Containers' Log Group

  • config
module "ecs_container_def_custom" {
  source = "../../../../../../open-source/terraform-aws-ecs/modules/container-definition"
  #  source  = "terraform-aws-modules/ecs/aws//modules//container-definition"
  #  version = "~> 5.7"
  # ...
  enable_cloudwatch_logging   = true
  create_cloudwatch_log_group = true
  cloudwatch_log_group_name = "/aws/ecs/test-custom-name"
  • terraform plan
Terraform will perform the following actions:

  # module.ecs_container_def_custom.aws_cloudwatch_log_group.this[0] will be created
  + resource "aws_cloudwatch_log_group" "this" {
      + arn               = (known after apply)
      + id                = (known after apply)
      + log_group_class   = (known after apply)
      + name              = "/aws/ecs/test-custom-name"
      ...
    }
  • I have executed pre-commit run -a on my pull request

@bryantbiggs
Copy link
Member

Why is this change necessary or warranted?

@ivan-sukhomlyn
Copy link
Contributor Author

ivan-sukhomlyn commented Jan 30, 2024 •
edited
Loading

@bryantbiggs For example, there's a custom convention for AWS CW LogGroups at some projects, which is different from the current pattern /aws/ecs/${var.service}/${var.name}.

In that case, it will be nice to have the possibility to define a custom name for a log group instead of creating it outside of the module and passing it through vars.

Also, the same functionality is already present for IAM roles via separate vars like tasks_iam_role_name, as for example.

bryantbiggs
bryantbiggs approved these changes Feb 12, 2024
View reviewed changes
Copy link
Member

@bryantbiggs bryantbiggs left a comment

Choose a reason for hiding this comment

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

I can't say I agree with it - seems to add more complexity than its worth but we'll go with it

@bryantbiggs bryantbiggs changed the title feat: Custom CW LogGroup name for ECS cluster and service containers feat: Add support for cluster and container definition custom CloudWatch log group names Feb 12, 2024
@bryantbiggs bryantbiggs merged commit 9a8c7d3 into terraform-aws-modules:master Feb 12, 2024
12 checks passed
antonbabenko pushed a commit that referenced this pull request Feb 12, 2024
@semantic-release-bot
chore(release): version 5.9.0 [skip ci]
8b97783 
## [5.9.0](v5.8.1...v5.9.0) (2024-02-12)

### Features

* Add support for cluster and container definition custom CloudWatch log group names ([#160](#160)) ([9a8c7d3](9a8c7d3))
@antonbabenko
Copy link
Member

This PR is included in version 5.9.0 🎉

@ivan-sukhomlyn
Copy link
Contributor Author

@bryantbiggs thank you! 🙂
It will allow the creation of a CW LogGroup inside the module instead of managing it outside in case of the custom log group name requirement.

@ivan-sukhomlyn ivan-sukhomlyn deleted the feat/custom_log_group_name branch February 12, 2024 16:05
@ivan-sukhomlyn ivan-sukhomlyn mentioned this pull request Feb 16, 2024
Merged
3 tasks
ivan-sukhomlyn added a commit to ivan-sukhomlyn/terraform-aws-ecs that referenced this pull request Feb 20, 2024
@ivan-sukhomlyn
feat: Add support for cluster and container definition custom CloudWa…
2e6ab83 
…tch log group names (terraform-aws-modules#160)

* feat: Custom CW Log Group name for cluster with cloudwatch_log_group_name var

* feat: Custom CW Log Group name for ECS service containers
ivan-sukhomlyn pushed a commit to ivan-sukhomlyn/terraform-aws-ecs that referenced this pull request Feb 20, 2024
@semantic-release-bot @ivan-sukhomlyn
chore(release): version 5.9.0 [skip ci]
cb106f4 
## [5.9.0](terraform-aws-modules/terraform-aws-ecs@v5.8.1...v5.9.0) (2024-02-12)

### Features

* Add support for cluster and container definition custom CloudWatch log group names ([terraform-aws-modules#160](terraform-aws-modules#160)) ([9a8c7d3](terraform-aws-modules@9a8c7d3))
@github-actions GitHub Actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bryantbiggs bryantbiggs bryantbiggs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ivan-sukhomlyn @bryantbiggs @antonbabenko