Skip to content

chore: make health checks gooder-er#21

Merged
scottgerring merged 1 commit into
mainfrom
chore/compose-and-healthchecks-fix
Jun 12, 2025
Merged

chore: make health checks gooder-er#21
scottgerring merged 1 commit into
mainfrom
chore/compose-and-healthchecks-fix

Conversation

@scottgerring

@scottgerring scottgerring commented Jun 4, 2025

Copy link
Copy Markdown
Member

Fixes the build and health checks so that we can run images on arm64 and x86-64, and get our docker-compose CI test running!


FROM mcr.microsoft.com/dotnet/aspnet:8.0-noble-chiseled AS runtime
FROM mcr.microsoft.com/dotnet/aspnet:8.0 AS runtime
RUN apt-get update && apt-get install -y curl && rm -rf /var/lib/apt/lists/*

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Quality Violation

package curl should have version pinned (...read more)

When using apt-get install, pin the version to avoid unwanted upgrades and undefined behavior.

View in Datadog  Leave us feedback  Documentation

@scottgerring scottgerring force-pushed the chore/compose-and-healthchecks-fix branch 2 times, most recently from 64cc781 to 03f033f Compare June 4, 2025 17:46
echo "Waiting for all services to be healthy..."
# Wait for services with health checks to be healthy (4 services) and all services to be up
timeout 300 bash -c 'until [ $(docker compose ps --format "{{.Health}}" | grep -c "healthy") -ge 4 ] && [ $(docker compose ps --format "{{.Status}}" | grep -c "Up") -ge 5 ]; do sleep 5; done'
# Wait for services with health checks to be healthy (7 services) and all services to be up

@scottgerring scottgerring Jun 4, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is useful when it struggles starting and I am only moderately ashamed to reveal I let claude code rip on this script.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we drop the curl thing; i'd just make this check that nothing has failed and pause for like 2 minutes before hitting URLs.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah I like that approach. It's not a 'proper' test of the services is it anyway? Just making sure everything starts up correctly in docker compose right?

@scottgerring scottgerring Jun 10, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's not a proper test suite, but it does hit and validate API calls.

i'm thinking coarse-grained "hit these APIs and expect some data back" sort of stuff, where "these APIs" are things that do some integration-y stuff - e.g. hitting DBs, hitting kafka, etc. This serves to validate local dev works, and is a smoke test for deeper issues.

Comment thread docker-compose.yml
Comment thread docker-compose.yml Outdated
KAFKA__SCHEMAREGISTRY: "http://redpanda:8082"
KAFKA__GROUPID: "stickerlandia-user-management"
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8080/api/users/v1/health"]

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We've both done different things for health; I don't have strong opinions either way

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@scottgerring as in, I've put the healthcheck on the sub URL and you've got it on the route?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jeastham1993 Yeah exactly; I don't much mind. On the one hand having it in the URL is consistent, on the other, we'd probably end up wanting to try and hide it from "regular users" ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, unless we put on a /health/service_name endpoint and block that from a load balancer perspective. So it's kind of 'internal only'.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jeastham1993 From a containers POV, it can just be each service exposes /health and the orchestrator can hit that; no need for globally unique. Not sure if this messes with what we've got in store on some of the clouds?

Comment thread user-management/src/Stickerlandia.UserManagement.Api/Dockerfile Outdated
Comment thread user-management/src/Stickerlandia.UserManagement.Api/Dockerfile
@scottgerring scottgerring force-pushed the chore/compose-and-healthchecks-fix branch 2 times, most recently from 6c01215 to 10309e4 Compare June 5, 2025 12:34
@scottgerring scottgerring force-pushed the chore/compose-and-healthchecks-fix branch from 9004478 to f672d28 Compare June 10, 2025 06:25
@jeastham1993 jeastham1993 self-requested a review June 12, 2025 13:40
@scottgerring scottgerring force-pushed the chore/compose-and-healthchecks-fix branch from f672d28 to 0b4ca66 Compare June 12, 2025 13:46
@scottgerring scottgerring merged commit dcf4d56 into main Jun 12, 2025
8 checks passed
@scottgerring scottgerring deleted the chore/compose-and-healthchecks-fix branch June 12, 2025 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants