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.

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

Add an optional health check to the container definition (#15) #84

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

colincoleman
Copy link
Contributor

This update enables passing a health check object to the container so that ECS can evaluate the task health - very useful if you are not using an ALB / target groups

This update enables passing a health check object to the container so
that ECS can evaluate the task health - very useful if you are not using
an ALB / target groups
@colincoleman colincoleman requested a review from a team as a code owner July 8, 2023 21:07
@rishiraj88
Copy link

Thanks, @colincoleman , for the purpose as well as the code update. It's quite comprehensive and helpful.

The target group name was added for readability but was orignially
excluded because of problems destroying a target group while a listener
was attached. See comment in code. This was removed again in case this
was still an issue and becuase of a length limit of 32 chars for the
name parameter which was causing problems
Comment on lines +166 to 167
"healthCheck" : var.container_health_check
}, local.task_container_secrets, local.repository_credentials)
Copy link
Contributor

@k1rd3rf k1rd3rf Aug 25, 2023

Choose a reason for hiding this comment

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

Following the pattern of some of the other optional variables (line 136-141), you could do something like

Suggested change
"healthCheck" : var.container_health_check
}, local.task_container_secrets, local.repository_credentials)
}, local.task_container_secrets, local.repository_credentials, local.task_container_health_check)
task_container_health_check = var.container_health_check != null ? { "healthCheck" = var.container_health_check } : null

}))
description = "(Optional) Container ulimit settings. This is a list of maps, where each map should contain \"name\", \"hardLimit\" and \"softLimit\""
default = null
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
}

@@ -93,7 +93,6 @@ resource "aws_security_group_rule" "egress_service" {
# LB Target group
# ------------------------------------------------------------------------------
resource "aws_lb_target_group" "task" {
name = "${var.name_prefix}-${var.task_container_port}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing name forces new resource.

colincoleman and others added 2 commits May 8, 2024 15:57
* Bump gopkg.in/yaml.v3 from 3.0.0-20210107192922-496545a6307b to 3.0.0 (telia-oss#88)

Bumps gopkg.in/yaml.v3 from 3.0.0-20210107192922-496545a6307b to 3.0.0.

---
updated-dependencies:
- dependency-name: gopkg.in/yaml.v3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add optional capacity provider strategy (telia-oss#85)

* Add option to set a healthCheck property on the task container definition (telia-oss#91)

* Allow higher versions of null provider (telia-oss#92)

* Allow higher versions of null provider

* Bump Taskfile schema version

* Use Cantara version of container health check

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Feraudet Cyril <[email protected]>
Co-authored-by: Fredrik Oterholt <[email protected]>
Co-authored-by: Pål Sollie <[email protected]>
* Add tags to ECS task definition

This was missing (maybe it wasn't available when this was first written?)

* Add missing tags to ECS Service
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants