Skip to content

ecs: fix health check guidance for correctness - #20

Merged
philgebhardt merged 1 commit into
mainfrom
hotfix/ecs-example-healthchecks
Jul 30, 2026
Merged

ecs: fix health check guidance for correctness#20
philgebhardt merged 1 commit into
mainfrom
hotfix/ecs-example-healthchecks

Conversation

@philgebhardt

Copy link
Copy Markdown
Contributor

The current ECS quickstart incorrectly declares the failure-flags-sidecar health check as it assumes a shell is available within the container (it isn't).

Updated overal guidance around rollout and troubleshooting to cover safe/stable rollouts and how to think about ff's healthCheck.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the AWS ECS quickstart documentation to make the sidecar container health check configuration correct for the failure-flags-sidecar image (no shell / no PATH) and expands rollout + troubleshooting guidance to reduce startup races and improve operational safety.

Changes:

  • Switches ECS health check from CMD-SHELL to exec-form CMD using an absolute binary path.
  • Adds detailed guidance on dependsOn (HEALTHY vs START) and what the sidecar health check does/does not guarantee.
  • Reworks ingress routing rollout guidance (Option A vs Option B) and expands troubleshooting scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/quickstart-ecs.md Outdated
Comment thread docs/quickstart-ecs.md Outdated
Comment thread docs/quickstart-ecs.md Outdated
Comment thread docs/quickstart-ecs.md Outdated
Comment thread docs/quickstart-ecs.md Outdated
@philgebhardt
philgebhardt force-pushed the hotfix/ecs-example-healthchecks branch 2 times, most recently from 0b806c8 to b077833 Compare July 30, 2026 18:08
The current ECS quickstart incorrectly declares the
failure-flags-sidecar health check as it assumes a
shell is available within the container (it isn't).

Updated overal guidance around rollout and troubleshooting
to cover safe/stable rollouts and how to think about ff's
healthCheck.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

docs/quickstart-ecs.md:41

  • ff-config.yaml sets ingress_proxy_port to :80, but Option B later routes the load balancer to port 5035. As written, someone following Option B will end up with the sidecar listening on 80 while the load balancer targets 5035 (or vice versa). Make the snippet default to :5035 and document both Option A vs B values explicitly.
# this value depends on Option A versus Option B
# in step #5 below.
ingress_proxy_port: :80
ingress_proxied_endpoint: http://localhost:9080

docs/quickstart-ecs.md:112

  • The text describes the health check as failure-flags-sidecar -s, but the task definition example requires an absolute path (/failure-flags-sidecar). This mismatch is easy to copy/paste incorrectly; make the narrative match the exact ECS command form being recommended.
The `healthCheck` configuration uses the sidecar's built-in health check command (`failure-flags-sidecar -s`) to verify the sidecar is running properly. This helps ECS detect when the sidecar is ready and ensures your application container waits for the sidecar to be healthy before starting. Port `5032` is the sidecar's local control port to which the health check connects.

This **must** use the exec form (`"CMD"`) with the binary's absolute path, not `"CMD-SHELL"`. The sidecar image has no shell and sets no `PATH`, so a `CMD-SHELL` command will fail on every single check. Similarly, the full path (`/failure-flags-sidecar`) must be used.

docs/quickstart-ecs.md:188

  • Option A says the sidecar should "take over" the app's original port (e.g. 80), but the task definition snippet only exposes 5032/5034/5035. Without adding the original app port to the sidecar portMappings (and updating the ECS service to register the sidecar container/port with the target group), the load balancer can't actually reach the sidecar on that port.
Change the port your application listens on (e.g. from 80 to 9080), set `ingress_proxy_port` in `ff-config.yaml` to the app’s original port (e.g. `:80`), and set `ingress_proxied_endpoint` to the app’s new port (e.g. `http://localhost:9080`).

**Your load balancer / target group configuration never changes**. Before the rollout it was pointing at your app on that port. After the rollout, it's pointing at the sidecar on that same port. Only the ECS service's container registration changes, which is a routine rolling service update, not an infrastructure change. This also makes rollback safe and symmetric: reverting the task definition to drop the sidecar puts the app straight back on its original port, again with no load balancer changes required.

@philgebhardt
philgebhardt merged commit 45e1c7b into main Jul 30, 2026
1 check passed
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.

2 participants