-
-
Notifications
You must be signed in to change notification settings - Fork 541
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.
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
container-definition module not usable on its own #147
Comments
No, I beg to disagree. The container definition sub-module should render a JSON document that meets the container definition API https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_ContainerDefinition.html The API expects |
I thought about something like this but haven't the possibility to test it right now. However, would it be possible to make the output compatible, without touching the module itself ? This would be the best of both world (unless I miss something else) |
I'd like to know more about whats working on not working. we tend to jump into solutioning which muddies the water a bit. Are you stating that this does not work? variable "subnet_ids" {
type = list(string)
}
locals {
name = "test-ecs-module"
tags = {
Env = "test"
Project = "ecs-module"
}
}
module "cluster" {
source = "terraform-aws-modules/ecs/aws//modules/cluster"
cluster_name = local.name
fargate_capacity_providers = {
FARGATE = {
default_capacity_provider_strategy = {
weight = 100
}
}
}
tags = local.tags
}
module "nginx" {
source = "terraform-aws-modules/ecs/aws//modules/container-definition"
version = "5.7.3"
name = local.name
service = local.name
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
create_cloudwatch_log_group = false
}
module "service" {
source = "terraform-aws-modules/ecs/aws//modules/service"
version = "5.7.3"
name = local.name
cluster_arn = module.cluster.arn
cpu = 256
memory = 512
desired_count = 1
launch_type = "FARGATE"
create_task_exec_iam_role = true
create_tasks_iam_role = true
create_security_group = true
security_group_rules = [
{
description = "Allow egress"
type = "egress"
protocol = "all"
from_port = 0
to_port = 65535
cidr_blocks = ["0.0.0.0/0"]
}
]
subnet_ids = var.subnet_ids
network_mode = "awsvpc"
assign_public_ip = false
container_definitions = {
(local.name) = jsonencode(module.nginx.container_definition)
}
volume = [
{
name : "conf"
}
]
enable_autoscaling = false
ignore_task_definition_changes = false
tags = local.tags
propagate_tags = "TASK_DEFINITION"
} |
Yes, exactly. In the plan, we can see that some fields are missing (all that are composed by multiple words) # module.service.aws_ecs_task_definition.this[0] will be created
+ resource "aws_ecs_task_definition" "this" {
+ arn = (known after apply)
+ arn_without_revision = (known after apply)
+ container_definitions = jsonencode(
[
+ {
+ environment = []
+ essential = true
+ image = "public.ecr.aws/nginx/nginx:1.25.3"
+ interactive = false
+ linuxParameters = {
+ initProcessEnabled = false
}
+ logConfiguration = {
+ logDriver = "awslogs"
+ options = {
+ awslogs-group = "/aws/ecs/test-ecs-module/test-ecs-module"
+ awslogs-region = "eu-west-1"
+ awslogs-stream-prefix = "ecs"
}
}
+ mountPoints = []
+ name = "test-ecs-module"
+ portMappings = []
+ privileged = false
+ pseudoTerminal = false
+ readonlyRootFilesystem = true
+ startTimeout = 30
+ stopTimeout = 120
+ user = "0"
+ volumesFrom = []
},
]
)
+ cpu = "256"
+ execution_role_arn = (known after apply)
+ family = "test-ecs-module"
+ id = (known after apply)
+ memory = "512"
+ network_mode = "awsvpc"
+ requires_compatibilities = [
+ "FARGATE",
]
+ revision = (known after apply)
+ skip_destroy = false
+ tags = {
+ "Env" = "test"
+ "Project" = "ecs-module"
}
+ tags_all = {
+ "Env" = "test"
+ "Project" = "ecs-module"
}
+ task_role_arn = (known after apply)
+ runtime_platform {
+ cpu_architecture = "X86_64"
+ operating_system_family = "LINUX"
}
+ volume {
+ name = "conf"
}
} So we have no errors, but the service may not be functioning normally. |
When I use this: provider "aws" {
region = "us-east-1"
}
locals {
name = "nginx"
}
module "nginx" {
source = "terraform-aws-modules/ecs/aws//modules/container-definition"
version = "~> 5.0"
name = local.name
service = local.name
essential = true
readonly_root_filesystem = false
image = "public.ecr.aws/nginx/nginx:1.25.3"
mount_points = [
{
containerPath = "/conf/"
sourceVolume = "conf"
readOnly = true
}
]
port_mappings = [
{
containerPort = 80
hostPort = 80
protocol = "tcp"
}
]
enable_cloudwatch_logging = true
create_cloudwatch_log_group = false
}
resource "local_file" "out" {
content = jsonencode(module.nginx.container_definition)
filename = "${path.module}/container_definition.json"
} I get a container definition that has all of the expected values based on the inputs: {
"environment": [],
"essential": true,
"image": "public.ecr.aws/nginx/nginx:1.25.3",
"interactive": false,
"linuxParameters": {
"initProcessEnabled": false
},
"logConfiguration": {
"logDriver": "awslogs",
"options": {
"awslogs-group": "",
"awslogs-region": "us-east-1",
"awslogs-stream-prefix": "ecs"
}
},
"mountPoints": [
{
"containerPath": "/conf/",
"readOnly": true,
"sourceVolume": "conf"
}
],
"name": "nginx",
"portMappings": [
{
"containerPort": 80,
"hostPort": 80,
"protocol": "tcp"
}
],
"privileged": false,
"pseudoTerminal": false,
"readonlyRootFilesystem": false,
"startTimeout": 30,
"stopTimeout": 120,
"volumesFrom": []
} |
I do agree and am able to reproduce. mount_points = try(each.value.mount_points, var.container_definition_defaults.mount_points, []) And the same goes for portMappings |
ahhhhhhh ok - now I got ya. thank you! |
You're welcome. |
This issue has been automatically marked as stale because it has been open 30 days |
Yes it appears the output of the container_definiton sub-module container_definition output has a camel casing of container_definiton submodule - service submodule - This means this trying to use the output |
This is also happening with |
A workaround to still be able to create standalone task definitions is modify line 594 of services module
to
This way, when calling the service module, setting I did create a fork with that change here, but as it's not a solution to the container-definition module but only a workaround, I'm not sure this will pass a review if I open a PR. Hope this helps someone! |
Description
This is kinda a reopening of #122.
I also hit the same issues and will try to explain as best as I can.
First thing first, the module is working perfectly fine.
For example (a very simplified example)
This works.
However, in some of our services, we have up to 5 containers.
Each one of them may have its own port mappings, own volumes, own commands and so.
Having that within only one resource is extremely hard to read and maintain.
On our current stack, we split each container definition in its own module, and the service only refers to all container definitions.
This way, one can better identify the resource, its properties and so on.
Using the current module, it would give something like this :
However, this does not work because of the case used in
container-definition
outputs for fields composed by multiple words.Hence, instead of having
portMappings
, it should return the proper property nameport_mappings
I made it work by modifying the local module
from
to
Versions
5.7.3
v1.6.5
provider registry.terraform.io/hashicorp/aws v5.30.0
Reproduction Code [Required]
Provided above.
Steps to reproduce the behavior:
Just run
terraform init && terraform apply -subnet_ids='[<your own list>]'
Expected behavior
It would be better for the container-definition module to be usable on its own.
Actual behavior
We can only use the service module with all container definition inside it.
No split available.
The text was updated successfully, but these errors were encountered: