Show server statuses only when they are deployed into the Kubernetes cluster #1603
Conversation
t-eckert
force-pushed
the
te/show-status-contextually
branch
from
ab15cc4
to
f37cbe0
Compare
t-eckert
force-pushed
the
te/show-status-contextually
branch
from
2bafceb
to
c7c87e6
Compare
t-eckert
requested review from
a team,
jmurret and
kschoche
and removed request for
a team
cli/cmd/status/status.go
Outdated
| return "", errors.New("found multiple server stateful sets") | ||
| return err | ||
| } | ||
| if len(clients.Items) != 0 { |
There was a problem hiding this comment.
minor nit. you could make this a shared func for this and below and pass in:
- agentType string (servers vs clients. also used for string replacement in the output)
- desiredAgents int
- readyAgents int
There was a problem hiding this comment.
I appreciate this feedback! I did think about doing that to "DRY" out this function. My thinking with the current way it is written is that one day we will nix the clients check and I don't want an extra function floating around. I think it's nice to just delete the specific lines and not have to change anything else. Once those lines are gone, there won't be any more deduping.
What do you think of that? Am I overthinking it?
Co-authored-by: John Murret <john.murret@hashicorp.com>
cli/cmd/status/status_test.go
Outdated
| "No clients, no agents": {0, 0, 0, 0}, | ||
| "3 clients - 1 healthy, no agents": {3, 1, 0, 0}, | ||
| "3 clients - 3 healthy, no agents": {3, 3, 0, 0}, | ||
| "3 clients - 1 healthy, 3 agents - 1 healthy": {3, 1, 3, 1}, | ||
| "3 clients - 3 healthy, 3 agents - 3 healthy": {3, 3, 3, 3}, | ||
| "No clients, 3 agents - 1 healthy": {0, 0, 3, 1}, | ||
| "No clients, 3 agents - 3 healthy": {0, 0, 3, 3}, |
There was a problem hiding this comment.
I think we're using confusing terminology here, it should be clients and servers since agents means both :)
There was a problem hiding this comment.
Great feedback. I fixed this when removing clients.
t-eckert
changed the title
Previously, if a user had servers deployed outside of a given K8s cluster, the
statuscommand would fail when it attempted to show the health status of servers. We recently removed a similar feature for clients which had the same problem when we moved to the "agentless" architecture.This PR changes the status command to only print the health status of clients and servers if they are expected to be present in the K8s cluster. If the user is running with the agentless architecture or has servers deployed outside of the cluster (e.g. HCP). The
statuscommand will simply not show the health status instead of failing.Changes proposed in this PR:
Screenshots
Agentless status with unhealthy serversAgentless status with healthy servers
0.48.0 unhealthy clients and servers
0.48.0 unhealthy clients and healthy servers
0.48.0 healthy clients a
nd servers
How I've tested this PR:
How I expect reviewers to test this PR:
Checklist: