ecs: fix health check guidance for correctness - #20
Merged
Conversation
There was a problem hiding this comment.
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-SHELLto exec-formCMDusing an absolute binary path. - Adds detailed guidance on
dependsOn(HEALTHYvsSTART) 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.
philgebhardt
force-pushed
the
hotfix/ecs-example-healthchecks
branch
2 times, most recently
from
July 30, 2026 18:08
0b806c8 to
b077833
Compare
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.
philgebhardt
force-pushed
the
hotfix/ecs-example-healthchecks
branch
from
July 30, 2026 18:09
b077833 to
71ded1f
Compare
There was a problem hiding this comment.
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.yamlsetsingress_proxy_portto:80, but Option B later routes the load balancer to port5035. 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:5035and 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.