Skip to content

[codex] Use Podman ps status for health#99

Draft
techulus-agent wants to merge 1 commit into
mainfrom
codex/podman6-health-status
Draft

[codex] Use Podman ps status for health#99
techulus-agent wants to merge 1 commit into
mainfrom
codex/podman6-health-status

Conversation

@techulus-agent

@techulus-agent techulus-agent commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

What changed

  • Parse health status from Podman ps JSON Status field.
  • Report parsed status for running containers before falling back to podman inspect.
  • Preserve previous semantics for stopped containers by reporting healthStatus none.
  • Keep missing Status backward-compatible by falling back to inspect, while treating an explicit empty Status as no health check.
  • Add parser tests for real Status output, missing status fallback, empty status normalization, unknown status fallback, and case/whitespace normalization.

Why

Podman v6 source shows ListContainerBatch populates the list response Status from HealthCheckStatus. Using that value avoids extra per-container inspect calls when Podman includes Status, while preserving older or unexpected output through the existing inspect fallback path.

Review follow-up

The original PR parsed a nonexistent Health field. Review feedback correctly flagged that as inert. This update switches the implementation and tests to the actual Status field from Podman list output.

Validation

  • GOCACHE=/private/tmp/techulus-cloud-go-build GOMODCACHE=/private/tmp/techulus-cloud-go-mod go test ./...

@techulus-agent

Copy link
Copy Markdown
Collaborator Author

Review

I read both branches locally (origin/main vs origin/codex/podman6-health-status). The code itself is clean, defensive, and well-tested — go test ./internal/container/ and go vet both pass, and the fallback path guarantees no regression versus current behavior. But I think the central premise of the PR is unfounded, and as written the change is effectively a no-op that never delivers its stated benefit.

Blocking concern: podman ps --format json does not emit a Health field

The change hinges on podman ps -a --format json (in container.List) returning a top-level Health key that the new Health json.RawMessage \json:"Health"`` tag can bind to. It doesn't.

podman ps --format json serializes Podman's ListContainer struct. The current struct (pkg/domain/entities/types/container_ps.go on main) has no Health field — the health-related fields are only:

  • State string — e.g. "running"
  • Status string — documented in-source as "a human-readable approximation of a duration" (e.g. "Up 2 minutes"); it does not contain the health word for JSON output.

Health status (healthy/unhealthy/starting) lives under .State.Health.Status, which is only reachable via podman inspect — exactly what the existing GetHealthStatus already does. Podman only surfaces health in ps via the {{.Status}} table column and the --filter health= filter, neither of which is a JSON field.

Consequences:

  1. pc.Health will always be empty → normalizeHealthStatus always returns ""reporting.go always falls through to container.GetHealthStatus(c.ID). So the promised win ("avoids extra per-container inspect calls on newer Podman versions") is never realized; every running container still gets an inspect call, same as before.
  2. The new runtime_test.go cases feed hand-crafted JSON that includes a "Health" key. That input shape is never produced by real podman ps, so the tests pass while validating a code path that won't fire in production — false confidence.

This matches the open question you flagged in the PR description ("verify the exact Health JSON shape on a live Podman 6 host"). Based on the Podman source, I don't believe that shape exists in the ps JSON on any version, including 6.

If the goal is genuinely to avoid the per-container inspect, the health signal would have to be parsed out of the Status string (which the health column derives from) rather than a nonexistent Health field — or the health of many containers batched via a single podman ps --format '{{.Names}} {{.Status}}' / podman healthcheck call. Worth confirming on a live Podman 6 host before merging, since if the field is absent this PR adds surface area without the optimization.

Minor

  • reporting.go: the trailing if healthStatus == "" { healthStatus = "none" } is dead in practice — GetHealthStatus never returns "" (it maps empty/<no value> to "none"), and the c.State == "running" branch already re-checks emptiness. Harmless, but it's belt-and-suspenders.
  • normalizeHealthString only whitelists healthy/unhealthy/starting/none and maps anything else to "" (→ inspect fallback), whereas the inspect path (GetHealthStatus) returns whatever podman prints verbatim. So the two paths normalize differently. Fine given the fallback, just noting the asymmetry.

Verdict

The code is safe to merge in the sense that it can't regress anything. But I'd hold it until the Health-in-ps-json assumption is confirmed on a real Podman 6 host — from the source it looks like the field never appears, which would make the whole change (plus its tests) inert.

@techulus-agent techulus-agent force-pushed the codex/podman6-health-status branch from 030a2ab to 962b12d Compare July 3, 2026 06:08
@techulus-agent techulus-agent changed the title [codex] Use Podman list health status [codex] Use Podman ps status for health Jul 3, 2026
@techulus-agent

Copy link
Copy Markdown
Collaborator Author

Addressed the review comment: verified Podman v6 source has no Health field in ps JSON, then amended the PR to parse the real Status field populated from HealthCheckStatus. Also updated tests to match real Status output and kept missing Status as an inspect fallback.

@arjunkomath arjunkomath marked this pull request as ready for review July 3, 2026 08:36
@techulus-agent techulus-agent force-pushed the codex/podman6-health-status branch from 962b12d to 6e4cd5f Compare July 3, 2026 13:30
@arjunkomath arjunkomath marked this pull request as draft July 7, 2026 07:22
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.

1 participant