Trying to get AWS to create log groups fails with Log driver awslogs option 'awslogs-group' should not be null or empty.

barnabyibis commented 5 months ago

Description

After successfully deploying an ECS cluster, I wanted to prevent TF creating the log groups so they weren't deleted after each image upgrade.

Noting the description of the container-definition module,

variable "create_cloudwatch_log_group" {
  description = "Determines whether a log group is created by this module. If not, AWS will automatically create one if logging is enabled"
  type        = bool
  default     = true
}

I set this to 'false'.

I got an error Error: creating ECS Task Definition (): ClientException: Log driver awslogs option 'awslogs-group' should not be null or empty. I believe this occurs due to the try clause resolving as an empty string in

  log_configuration = merge(
    { for k, v in {
      logDriver = "awslogs",
      options = {
        awslogs-region        = data.aws_region.current.name,
        awslogs-group         = try(aws_cloudwatch_log_group.this[0].name, ""),
        awslogs-stream-prefix = "ecs"
      },
    } : k => v if var.enable_cloudwatch_logging },

I imagine the solution would be to use the local value log_group_name = try(coalesce(var.cloudwatch_log_group_name, "/aws/ecs/${var.service}/${var.name}"), "")

as in awslogs-group = try(aws_cloudwatch_log_group.this[0].name, local.log_group_name),

Versions

Module version [Required]: 5.11.1
Terraform version: Terraform v1.8.1 on windows_amd64
Provider version(s):
provider registry.terraform.io/hashicorp/archive v2.4.2
provider registry.terraform.io/hashicorp/aws v5.46.0
provider registry.terraform.io/hashicorp/local v2.5.1

Reproduction Code [Required]

# Container definition(s)
  container_definition_defaults = {
      readonly_root_filesystem = false
      enable_cloudwatch_logging              = true 
      create_cloudwatch_log_group            = false # Avoids deletion during upgrade : "Determines whether a log group is created by this module. If not, AWS will automatically create one if logging is enabled"
      cloudwatch_log_group_retention_in_days = 5
      dns_search_domains = ["${each.value.subnets[0].env}.${var.dns_domain}"] # Maps to ECS container definition option dnsSearchDomains
  }
  container_definitions = {for container_name, container in each.value.container_details :
     container_name => {
      image = one([for img in data.aws_ecr_image.component_images : 
                    "${split("@",img.image_uri)[0]}:${container.image_tag}" if strcontains(img.repository_name, each.value.image_repo) && img.image_tag == container.image_tag
                  ])
      port_mappings = [
        {
          name          = container_name
          containerPort = container.container_port
          protocol      = "tcp"
        }
      ]
      memory_reservation = parseint(format("%.0f",container.mem_res_mult * data.aws_ec2_instance_type.defined[each.value.instance_type].memory_size),10)
      environment = local.environment_variables
      cloudwatch_log_group_name = "/aws/ecs/${var.cluster_name}/${each.key}"
    }
  }

Steps to reproduce the behavior:

No N/A See intro ## Expected behavior Set enable_cloudwatch_logging = true create_cloudwatch_log_group = false and AWS creates the log for you (which won't be deleted when the Task is destroyed during an upgrade) ## Actual behavior 'Apply' fails with quoted error ### Terminal Output Screenshot(s)

Additional context

barnabyibis commented 5 months ago

After making my change, I noticed 2 things:

The task def version on the service was locked to the previous version, even though it was marked Invalid.
I could manually request a forced redeploy, but the ECS Container agent reports an error failed to initialize logging driver: failed to create Cloudwatch log stream: ResourceNotFoundException: Manually creating these logs allowed the Task to start successfully, which suggests the comment about AWS automatically creating the logs is either incorrect, or I'm assigning an inappropriate set of parameters to get this to happen

bryantbiggs commented 5 months ago

so they weren't deleted after each image upgrade.

What is an "image upgrade", and why is it deleting your logs?

barnabyibis commented 5 months ago

As we produce test builds of our product - as Docker images uploaded to the Amazon ECR, hosted by ECS - we need to update the Task Definition with a new version number. As TerraForm creates the logs based on the TD, it deleted this log group and creates a new one with the newer release, which means we lose history and continuity when the old logs are deleted. I was trying to indicate that the module should provide a log name to the TD but not create the log entity via TerraForm explicit declaration, so that TF wouldn't track it's existence and delete it each time the TD was changed. When I did this, I got the error from AWS about a blank log name, due to the line of code I highlighted. When I made my proposed correction, I got passed the AWS error, but found AWS didn't automatically create the log defined in the TD, causing the ECS agent to throw an error when starting the Task regarding this log group not existing. So I created the required log group as a separate declaration in TerraForm, which in theory will persistent when a newer image causes a replacement of the TD. Thanks, Barnaby

Get Outlook for Androidhttps://aka.ms/AAb9ysg

From: Bryant Biggs @.> Sent: Wednesday, June 19, 2024 11:47:54 PM To: terraform-aws-modules/terraform-aws-ecs @.> Cc: Barnaby Arnott @.>; Author @.> Subject: Re: [terraform-aws-modules/terraform-aws-ecs] Trying to get AWS to create log groups fails with Log driver awslogs option 'awslogs-group' should not be null or empty. (Issue #206)

Caution: This email has come from an external source. Please take care when clicking links or opening attachments. Please consider safe cyber security practices.

so they weren't deleted after each image upgrade.

What is an "image upgrade", and why is it deleting your logs?

— Reply to this email directly, view it on GitHubhttps://github.com/terraform-aws-modules/terraform-aws-ecs/issues/206#issuecomment-2178496260, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BHXU3JG3QJXH3ISL25FMKMDZIFVWVAVCNFSM6AAAAABJRKLJV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZYGQ4TMMRWGA. You are receiving this because you authored the thread.Message ID: @.***>

barnabyibis commented 4 months ago

A further flow-on issue: after setting

create_cloudwatch_log_group = false

in my calling module, every Apply suggests an update to resource "aws_ecs_cluster" "this"

      - configuration {
          - execute_command_configuration {
              - logging    = "DEFAULT" -> null

This is because I am not passing any cluster_configuration declaration, so there is no logic for supplying defaults, yet AWS must be supplying a value of logging = "DEFAULT", which Terraform is continually reverting to null

  dynamic "configuration" {
    for_each = !var.create_cloudwatch_log_group && length(var.cluster_configuration) > 0 ? [var.cluster_configuration] : []

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 3 months ago

This issue was automatically closed because of stale in 10 days

github-actions[bot] commented 2 months ago

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

terraform-aws-modules / terraform-aws-ecs