container/ps: add HealthStatus formatter field#6913
Conversation
thaJeztah
left a comment
There was a problem hiding this comment.
Thanks for working on this! I left some comments/suggestions; feedback welcome!
| 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.
7c2e8b2 to
ef9f2b7
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Two minor linting issues; We should also squash the commits (I may have a look later to make sure it doesn't miss the release) |
ef9f2b7 to
21d432b
Compare
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
21d432b to
b665814
Compare
thaJeztah
left a comment
There was a problem hiding this comment.
did a quick rebase and squash, and added a commit to re-format the markdown table
LGTM
Signed-off-by: Mohammed Thaha <mohammedthahacse@gmail.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
b665814 to
8d8d405
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)