container/ps: add HealthStatus formatter field#6913
container/ps: add HealthStatus formatter field#6913Mohammed-Thaha wants to merge 3 commits intodocker:masterfrom
Conversation
Signed-off-by: Mohammed Thaha <mohammedthahacse@gmail.com>
thaJeztah
left a comment
There was a problem hiding this comment.
Thanks for working on this! I left some comments/suggestions; feedback welcome!
| // HealthCheck returns the container's health status (for example, "healthy","unhealthy", or "starting"). | ||
| // If no healthcheck is configured, an empty | ||
| // string is returned. | ||
| func (c *ContainerContext) HealthCheck() string { |
There was a problem hiding this comment.
I think we should name this HealthStatus, as it's not showing information about the health-check otherwise.
| return string(c.c.Health.Status) | ||
| } | ||
|
|
||
| // Fallback for daemons/API versions that include health only in Status text. |
There was a problem hiding this comment.
It'd be good to reference the API version that introduced the new field to save future visitors of this code to decide whether the fallback is still needed (and perhaps a link to the pull request for future reference);
// Fallback for API versions before v1.52, which include health only in Status text;
// see https://github.com/moby/moby/pull/50281
// see https://github.com/moby/moby/blob/docker-v29.4.3/daemon/container/health.go#L18-L43| switch { | ||
| case strings.HasSuffix(c.c.Status, "(healthy)"): | ||
| return string(container.Healthy) | ||
| case strings.HasSuffix(c.c.Status, "(unhealthy)"): | ||
| return string(container.Unhealthy) | ||
| case strings.HasSuffix(c.c.Status, "(health: starting)"): | ||
| return string(container.Starting) | ||
| } | ||
|
|
||
| return "" |
There was a problem hiding this comment.
Perhaps we should slightly optimise here; also adding an early return; something like;
_, health, ok := strings.Cut(c.c.Status, "(")
if !ok || !strings.HasSuffix(health, ")") {
return ""
}
health = strings.TrimSuffix(health, ")")
health = strings.TrimPrefix(health, "health: ")
switch container.Health(health){
case container.Healthy, container.Unhealthy, container.Starting:
return health
default:
return ""
}Slightly ugly because of the health: prefix but it makes it slightly more explicit that we're matching specific states.
There was a problem hiding this comment.
Used parsedHealth := container.HealthStatus(health) before the switch so the parsed value matches the container.Healthy, container.Unhealthy, and container.Starting typed constants during comparison.
| localVolumes = "LOCAL VOLUMES" | ||
| networksHeader = "NETWORKS" | ||
| platformHeader = "PLATFORM" | ||
| healthCheckHeader = "HEALTHCHECK" |
There was a problem hiding this comment.
Related to my other comment; perhaps HEALTH STATUS
Signed-off-by: Mohammed Thaha <mohammedthahacse@gmail.com>
7c2e8b2 to
ef9f2b7
Compare
- What I did
Added a new
docker ps --formatplaceholder:.HealthStatus, so users can print container health state directly as a dedicated field.- How I did it
HealthStatus()to the container formatter context.c.c.Health.Statuswhen available..Statustext for:(healthy)->healthy(unhealthy)->unhealthy(health: starting)->startingdocs/reference/commandline/container_ls.mdto include.HealthStatusin the format placeholders table.cli/command/formatter/container_test.go.- How to verify it
Start dev shell:
Build the CLI binary:
Run a container that stays in
startingduring the start period:Verify formatter output:
./build/docker-linux-amd64 ps -a --filter name=hc-starting --format "table {{.Names}}\t{{.Status}}\t{{.HealthStatus}}"Expected result:
STATUScontains(health: starting)HEALTHSTATUSshowsstarting- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)