Skip to content

Add health check to container list - #201

Merged
jesseduffield merged 3 commits into
jesseduffield:masterfrom
stirante:health-check
Sep 23, 2020
Merged

Add health check to container list#201
jesseduffield merged 3 commits into
jesseduffield:masterfrom
stirante:health-check

Conversation

@stirante

Copy link
Copy Markdown
Contributor

If container has health check configured, it will display with container status e.g. "running (healthy)".

Also I added a utility method for adding more than one colour attribute to a string.

@jesseduffield

jesseduffield commented Jan 29, 2020

Copy link
Copy Markdown
Owner

Thanks for making this :)

I'm not a big fan of using background colours unless it's absolutely necessary because given the number of terminal colour configurations it can often be hard to see the text clearly (as was the case when testing this myself). I propose a different approach:
When the container is running, show 'running' in green, and then colour the health status in whatever colour is appropriate for that status (healthy -> green, starting -> yellow, unhealthy -> red).

image

This way it's clear that the container is running because it's green (if it were black/grey like in your approach, it makes it hard to tell it's running), and it's clear that it's in an unhealthy state.

tinkering locally, this patch worked well for me:

diff --git a/pkg/commands/container.go b/pkg/commands/container.go
index c126ed2..cc896ae 100644
--- a/pkg/commands/container.go
+++ b/pkg/commands/container.go
@@ -258,16 +258,26 @@ func (c *Container) GetDisplayStrings(isFocused bool) []string {
 func (c *Container) GetDisplayStatus() string {
 	state := c.Container.State
 	if c.Container.State == "exited" {
-		state += " (" + strconv.Itoa(c.Details.State.ExitCode) + ")"
+		return utils.ColoredString(state+" ("+strconv.Itoa(c.Details.State.ExitCode)+")", c.GetColor())
 	}
-	if c.Container.State == "running" && c.Details.State.Health.Status != "" {
-		state += " (" + c.Details.State.Health.Status + ")"
+
+	return utils.ColoredString(state, c.GetColor()) + c.healthStatusString()
+}
+
+func (c *Container) healthStatusString() string {
+	healthStatusColorMap := map[string]color.Attribute{
+		"healthy":   color.FgGreen,
+		"unhealthy": color.FgRed,
+		"starting":  color.FgYellow,
 	}
-	if c.Container.State == "running" && c.Details.State.Health.Status == "unhealthy" {
-		return utils.MultiColoredString(state, c.GetColor(), color.BgRed)
+	if c.Container.State != "running" {
+		return ""
 	}
-
-	return utils.ColoredString(state, c.GetColor())
+	healthStatus := c.Details.State.Health.Status
+	if healthStatusColor, ok := healthStatusColorMap[healthStatus]; ok {
+		return utils.ColoredString(" ("+healthStatus+")", healthStatusColor)
+	}
+	return ""
 }
 
 // GetDisplayCPUPerc colors the cpu percentage based on how extreme it is
@@ -312,12 +322,6 @@ func (c *Container) GetColor() color.Attribute {
 	case "created":
 		return color.FgCyan
 	case "running":
-		if c.Details.State.Health.Status == "starting" {
-			return color.FgYellow
-		}
-		if c.Details.State.Health.Status == "unhealthy" {
-			return color.FgBlack
-		}
 		return color.FgGreen
 	case "paused":
 		return color.FgYellow

what are your thoughts?

@stirante

Copy link
Copy Markdown
Contributor Author

I added the patch you wrote. I actually just didn't know how to do string partially colored in different way. Creating console UI as well as writing anything in go is a first time for me :)

@jesseduffield

Copy link
Copy Markdown
Owner

@stirante sorry for the delay! I'm gonna merge this and then add an extra thing on top so that exit codes are aligned with health statuses (they looked a bit messy just being appended to the original string and not being in their own column). Thanks for making this PR and thanks for your patience :)

@jesseduffield
jesseduffield merged commit 386334b into jesseduffield:master Sep 23, 2020
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.

2 participants