ui: Use liveness info to populate decommissioned node lists

[zachlite]

Previously, the decommissioned node lists considered node status entries
to determine decommissioning and decommissioned status. This changed in #56529,
resulting in empty lists. Now, the node's liveness entry is considered
and these lists are correctly populated.

Release note (bug fix): The list of recently decommissioned nodes
and the historical list of decommissioned nodes correctly display
decommissioned nodes.

[cockroach-teamcity]

This change is 

[zachlite]

Addresses #61812 and cockroachdb/docs#9968

[zachlite]

With these changes, here's what decommissioning via cockroach node looks like:

1. Done moving replicas
   

2. Immediately
   

3. After 5 minutes
   

4. Full History
   

[koorosh]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @erikgrinaker, @Santamaura, @tbg, and @zachlite)

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx, line 56 at r1 (raw file):

  StatusTooltip,
} from "./tooltips";
import { cockroach } from "oss/src/js/protos";

nit. change import path to avoid oss part (it is added with auto-imports).
Should be: from "src/js/protos"

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx, line 174 at r1 (raw file):

const NodeNameColumn: React.FC<{
  record: NodeStatusRow | DecommissionedNodeStatusRow;
  shouldLink: boolean;

Can it be defined with default value (shouldLink = true), so it shouldn't be defined explicitly in other places?

---

pkg/ui/workspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx, line 21 at r1 (raw file):

import { nodesSummarySelector } from "src/redux/nodes";
import { refreshLiveness, refreshNodes } from "src/redux/apiReducers";
import { cockroach } from "oss/src/js/protos";

nit. the same as above, remove oss part from import.

[tbg]

Haven't reviewed any of the code (since it's all in the UI), but I did go through the PR message and the new behavior looks good. For my education, where does the behaviour to display the node as "unavailable" in the decommissioned section come from? I think if we're being idealistic about the UX then once a node is decommissioned, it shouldn't matter whether it has been offline for five minutes or not.

Thanks!

[erikgrinaker]

What Tobi said, LGTM!

Also, in general I'd encourage always joining kv_node_status data against kv_node_liveness before use. The liveness data is the source of truth for node existence. Using a status entry when a liveness entry does not exist, or ignoring a liveness entry when the status entry does not exist, both yield incorrect and surprising results -- we've had several cases where failing to do this has caused pretty severe downstream problems.

[zachlite]

Thanks for the review, all.
@tbg, the unavailable status comes from the enum cockroach.kv.kvserver.liveness.livenesspb.NodeLivenessStatus
@erikgrinaker, ack. I'm treating liveness data as the source of truth.

[koorosh]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Santamaura and @zachlite)

a discussion (no related file):

---

[zachlite]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)