thwap/thwap-pagesd

Watch

Fork 0

feat(docker): implement container inspection and health checking #661

Merged

fuzzy merged 1 commit from feat/docker-container-inspect into main

2026-08-14 15:11:57 +00:00

automation commented

2026-08-14 09:35:49 +00:00

Owner

What

Implements container inspection and health checking for standalone mode (#357):

InspectContainer returns the runtime status of a site container (site.ContainerStatus), mapping ErrContainerNotFound to a typed error when the container is gone.
WaitContainerRunning polls until the container is running (and healthy when a healthcheck is configured), failing with ErrContainerTimeout on deadline.
StandaloneDeployer.ContainerStatus implements the optional site.ContainerStatusReporter interface, so GET /api/v1/sites/:name now reports the active deployment's container status as container_status.
Adds standalone.health_check_timeout (default "30s").
Refactors create/remove options to embed a shared ContainerRef.

Why

Closes Phase 2 item #357 and its sub-tasks #354 (verify running before routing), #355 (wait for healthy with timeout), and #356 (report status through the API). These primitives will be wired into the standalone deploy flow in Phase 4.

Testing

Unit tests for inspect (running, exited, not-found)
Unit tests for wait (immediate, transition, timeout, health semantics)
Unit tests for ContainerStatus reporter mapping (production vs preview)
Service tests for Status() container-status wiring (populated, missing, reporter error)
Config tests updated for the new health_check_timeout default
go vet ./..., golangci-lint run, go test -race ./... pass

Breaking Changes

None. ContainerCreateOptions/ContainerRemoveOptions embed ContainerRef; existing field names are promoted and unchanged.

Notes

nginx:alpine images have no healthcheck by default, so "ready" is defined as the running state; when a container reports a healthcheck, running + healthy is required.

Closes #354
Closes #355
Closes #356
Closes #357

## What Implements container inspection and health checking for standalone mode (#357): - `InspectContainer` returns the runtime status of a site container (`site.ContainerStatus`), mapping `ErrContainerNotFound` to a typed error when the container is gone. - `WaitContainerRunning` polls until the container is running (and healthy when a healthcheck is configured), failing with `ErrContainerTimeout` on deadline. - `StandaloneDeployer.ContainerStatus` implements the optional `site.ContainerStatusReporter` interface, so `GET /api/v1/sites/:name` now reports the active deployment's container status as `container_status`. - Adds `standalone.health_check_timeout` (default `"30s"`). - Refactors create/remove options to embed a shared `ContainerRef`. ## Why Closes Phase 2 item #357 and its sub-tasks #354 (verify running before routing), #355 (wait for healthy with timeout), and #356 (report status through the API). These primitives will be wired into the standalone deploy flow in Phase 4. ## Testing - [x] Unit tests for inspect (running, exited, not-found) - [x] Unit tests for wait (immediate, transition, timeout, health semantics) - [x] Unit tests for `ContainerStatus` reporter mapping (production vs preview) - [x] Service tests for `Status()` container-status wiring (populated, missing, reporter error) - [x] Config tests updated for the new `health_check_timeout` default - [x] `go vet ./...`, `golangci-lint run`, `go test -race ./...` pass ## Breaking Changes None. `ContainerCreateOptions`/`ContainerRemoveOptions` embed `ContainerRef`; existing field names are promoted and unchanged. ## Notes nginx:alpine images have no healthcheck by default, so "ready" is defined as the running state; when a container reports a healthcheck, running + healthy is required. Closes #354 Closes #355 Closes #356 Closes #357

automation added 1 commit

2026-08-14 09:35:49 +00:00

feat(docker): implement container inspection and health checking

CI / test (pull_request) Successful in 10m10s

Details

CI / build (pull_request) Successful in 4m44s

Details

CI / docker (pull_request) Successful in 3m17s

Details

CI / lint (pull_request) Successful in 4m28s

Details

CI / build (push) Successful in 10m43s

Details

CI / lint (push) Successful in 11m20s

Details

CI / test (push) Successful in 17m5s

Details

CI / docker (push) Successful in 29m17s

Details

aeff2dc428

Add InspectContainer and WaitContainerRunning primitives to the
StandaloneDeployer, plus a ContainerStatusReporter that surfaces the
active deployment's container status through the site status API.

Closes #354
Closes #355
Closes #356
Closes #357

the.auditor referenced this pull request

2026-08-14 14:20:34 +00:00

Document standalone.health_check_timeout config option #662

the.auditor referenced this pull request

2026-08-14 14:20:35 +00:00

Validate standalone.health_check_timeout as a duration at config load #663

the.auditor referenced this pull request

2026-08-14 14:20:36 +00:00

Add tests for WaitContainerRunning ctx cancellation and non-404 inspect errors #664

the.auditor referenced this pull request

2026-08-14 14:20:39 +00:00

ownerScoped fails open on SiteStatus error #665

the.auditor approved these changes

2026-08-14 14:20:54 +00:00

the.auditor left a comment

Solid PR. Inspection/health-check primitives are well structured, the ContainerRef embed refactor is clean and backward compatible (all in-repo callers updated), error wrapping follows the existing centralized pattern, and test coverage for the new code is strong (health semantics table-driven, not-found mapping, timeout, reporter wiring).

Questions

DOCKER-ROADMAP.md + PR body Closes #354/#355 — #354 ("Verify container is running before routing traffic") is marked done and closed, but routing traffic is not yet implemented (CreateSiteIngress/PublishSite still return "not yet implemented") and WaitContainerRunning is never invoked in a deploy flow. #355 is similar: the primitive and health_check_timeout config exist, but the config value is never read. The PR body itself says these "will be wired into the standalone deploy flow in Phase 4". Consider keeping #354 (and possibly #355) open until the deploy flow actually uses these primitives, and drop them from the Closes list.
internal/site/service.go:410 — reporter errors other than ErrContainerNotFound fail the whole Status() call, so in standalone mode a docker daemon outage turns into 500s on the site status and deployments endpoints. Combined with sitesHandler.ownerScoped failing open on Status() errors, the /deployments endpoint can return data without the owner check while docker is down. Consider degrading gracefully (log the reporter error and leave container_status unset) or failing closed in ownerScoped.

Praise

Good error semantics: ErrContainerNotFound vs ErrContainerTimeout are distinct, wrapped with context, and the not-found case degrades to unset rather than failing the site status.
The ContainerStatus v-prefix heuristic matches the existing isPreviewDeployment convention (service.go:486), keeping production/preview mapping consistent.
WaitContainerRunning handles both ctx cancellation and per-inspect errors, and the poll/ready semantics are documented.

Filed issues

#662 – document standalone.health_check_timeout
#663 – validate health_check_timeout as a duration at config load
#664 – tests for WaitContainerRunning ctx cancellation and non-404 inspect errors
#665 – ownerScoped fails open on SiteStatus error

Solid PR. Inspection/health-check primitives are well structured, the `ContainerRef` embed refactor is clean and backward compatible (all in-repo callers updated), error wrapping follows the existing centralized pattern, and test coverage for the new code is strong (health semantics table-driven, not-found mapping, timeout, reporter wiring). ## Questions 1. `DOCKER-ROADMAP.md` + PR body `Closes #354/#355` — #354 ("Verify container is running before routing traffic") is marked done and closed, but routing traffic is not yet implemented (`CreateSiteIngress`/`PublishSite` still return "not yet implemented") and `WaitContainerRunning` is never invoked in a deploy flow. #355 is similar: the primitive and `health_check_timeout` config exist, but the config value is never read. The PR body itself says these "will be wired into the standalone deploy flow in Phase 4". Consider keeping #354 (and possibly #355) open until the deploy flow actually uses these primitives, and drop them from the `Closes` list. 2. `internal/site/service.go:410` — reporter errors other than `ErrContainerNotFound` fail the whole `Status()` call, so in standalone mode a docker daemon outage turns into 500s on the site status and deployments endpoints. Combined with `sitesHandler.ownerScoped` failing open on `Status()` errors, the `/deployments` endpoint can return data without the owner check while docker is down. Consider degrading gracefully (log the reporter error and leave `container_status` unset) or failing closed in `ownerScoped`. ## Praise - Good error semantics: `ErrContainerNotFound` vs `ErrContainerTimeout` are distinct, wrapped with context, and the not-found case degrades to unset rather than failing the site status. - The `ContainerStatus` `v`-prefix heuristic matches the existing `isPreviewDeployment` convention (`service.go:486`), keeping production/preview mapping consistent. - `WaitContainerRunning` handles both ctx cancellation and per-inspect errors, and the poll/ready semantics are documented. ## Filed issues - #662 – document `standalone.health_check_timeout` - #663 – validate `health_check_timeout` as a duration at config load - #664 – tests for `WaitContainerRunning` ctx cancellation and non-404 inspect errors - #665 – `ownerScoped` fails open on `SiteStatus` error

fuzzy merged commit aeff2dc428 into main

2026-08-14 15:11:57 +00:00

fuzzy deleted branch feat/docker-container-inspect

2026-08-14 15:11:57 +00:00