Merge pull request #80747 from zachlite/backport-decommssioned-nodes-…

…list-fix

ui: Use liveness info to populate decommissioned node lists

pkg/ui/workspaces/db-console/src/redux/nodes.ts

@@ -303,6 +303,9 @@ function isNoConnection(
 
 // nodeDisplayNameByIDSelector provides a unique, human-readable display name
 // for each node.
+
+// This function will never be passed decommissioned nodes because
+// #56529 removed a node's status entry once it's decommissioned.
 export const nodeDisplayNameByIDSelector = createSelector(
   nodeStatusesSelector,
   livenessStatusByNodeIDSelector,

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx

@@ -49,6 +49,8 @@ import {
   VersionTooltip,
   StatusTooltip,
 } from "./tooltips";
+import { cockroach } from "src/js/protos";
+import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;
 
 const liveNodesSortSetting = new LocalSetting<AdminUIState, SortSetting>(
   "nodes/live_sort_setting",
@@ -165,13 +167,24 @@ export const getLivenessStatusName = (status: LivenessStatus): string => {
 
 const NodeNameColumn: React.FC<{
   record: NodeStatusRow | DecommissionedNodeStatusRow;
-}> = ({ record }) => {
-  return (
-    <Link className="nodes-table__link" to={`/node/${record.nodeId}`}>
+  shouldLink?: boolean;
+}> = ({ record, shouldLink = true }) => {
+  const columnValue = (
+    <>
       <Text>{record.nodeName}</Text>
       <Text textType={TextTypes.BodyStrong}>{` (n${record.nodeId})`}</Text>
-    </Link>
+    </>
   );
+
+  if (shouldLink) {
+    return (
+      <Link className="nodes-table__link" to={`/node/${record.nodeId}`}>
+        {columnValue}
+      </Link>
+    );
+  }
+
+  return columnValue;
 };
 
 const NodeLocalityColumn: React.FC<{ record: NodeStatusRow }> = ({
@@ -418,14 +431,14 @@ class DecommissionedNodeList extends React.Component<
       key: "nodes",
       title: "decommissioned nodes",
       render: (_text: string, record: DecommissionedNodeStatusRow) => (
-        <NodeNameColumn record={record} />
+        <NodeNameColumn record={record} shouldLink={false} />
       ),
     },
     {
       key: "decommissionedSince",
       title: "decommissioned on",
       render: (_text: string, record: DecommissionedNodeStatusRow) =>
-        record.decommissionedDate.format("LL[ at ]h:mm a"),
+        record.decommissionedDate.format("LL[ at ]h:mm a UTC"),
     },
     {
       key: "status",
@@ -580,11 +593,8 @@ export const liveNodesTableDataSelector = createSelector(
 );
 
 export const decommissionedNodesTableDataSelector = createSelector(
-  partitionedStatuses,
   nodesSummarySelector,
-  (statuses, nodesSummary): DecommissionedNodeStatusRow[] => {
-    const decommissionedStatuses = statuses.decommissioned || [];
-
+  (nodesSummary): DecommissionedNodeStatusRow[] => {
     const getDecommissionedTime = (nodeId: number) => {
       const liveness = nodesSummary.livenessByNodeID[nodeId];
       if (!liveness) {
@@ -594,20 +604,24 @@ export const decommissionedNodesTableDataSelector = createSelector(
       return LongToMoment(deadTime);
     };
 
+    const decommissionedNodes = Object.values(
+      nodesSummary.livenessByNodeID,
+    ).filter(liveness => {
+      return liveness?.membership === MembershipStatus.DECOMMISSIONED;
+    });
+
     // DecommissionedNodeList displays 5 most recent nodes.
-    const data = _.chain(decommissionedStatuses)
-      .orderBy(
-        [(ns: INodeStatus) => getDecommissionedTime(ns.desc.node_id)],
-        ["desc"],
-      )
+    const data = _.chain(decommissionedNodes)
+      .orderBy([liveness => getDecommissionedTime(liveness.node_id)], ["desc"])
       .take(5)
-      .map((ns: INodeStatus, idx: number) => {
+      .map((liveness, idx: number) => {
+        const { node_id } = liveness;
         return {
           key: `${idx}`,
-          nodeId: ns.desc.node_id,
-          nodeName: ns.desc.address.address_field,
-          status: nodesSummary.livenessStatusByNodeID[ns.desc.node_id],
-          decommissionedDate: getDecommissionedTime(ns.desc.node_id),
+          nodeId: node_id,
+          nodeName: `${node_id}`,
+          status: nodesSummary.livenessStatusByNodeID[node_id],
+          decommissionedDate: getDecommissionedTime(node_id),
         };
       })
       .value();

pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/nodesOverview.spec.tsx

@@ -29,6 +29,7 @@ import { livenessByNodeIDSelector, LivenessStatus } from "src/redux/nodes";
 import { SortSetting } from "@cockroachlabs/cluster-ui";
 
 import NodeLivenessStatus = cockroach.kv.kvserver.liveness.livenesspb.NodeLivenessStatus;
+import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;
 
 describe("Nodes Overview page", () => {
   describe("Live <NodeList/> section initial state", () => {
@@ -365,6 +366,7 @@ describe("Nodes Overview page", () => {
               {
                 node_id: 2,
                 expiration: { wall_time: Long.fromNumber(Date.now()) },
+                membership: MembershipStatus.DECOMMISSIONED,
               },
               { node_id: 3 },
               { node_id: 4 },
@@ -373,6 +375,7 @@ describe("Nodes Overview page", () => {
               {
                 node_id: 7,
                 expiration: { wall_time: Long.fromNumber(Date.now()) },
+                membership: MembershipStatus.DECOMMISSIONED,
               },
             ],
             statuses: {
@@ -421,7 +424,6 @@ describe("Nodes Overview page", () => {
       it("returns node records with 'decommissioned' status only", () => {
         const expectedDecommissionedNodeIds = [2, 7];
         const records = decommissionedNodesTableDataSelector.resultFunc(
-          partitionedNodes,
           nodeSummary,
         );
 
@@ -437,12 +439,10 @@ describe("Nodes Overview page", () => {
 
       it("returns correct node name", () => {
         const recordsGroupedByRegion = decommissionedNodesTableDataSelector.resultFunc(
-          partitionedNodes,
           nodeSummary,
         );
         recordsGroupedByRegion.forEach(record => {
-          const expectedName = `127.0.0.${record.nodeId}:50945`;
-          assert.equal(record.nodeName, expectedName);
+          assert.equal(record.nodeName, record.nodeId.toString());
         });
       });
     });

pkg/ui/workspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx

@@ -11,20 +11,25 @@
 import * as React from "react";
 import { Helmet } from "react-helmet";
 import { connect } from "react-redux";
-import { Link, RouteComponentProps, withRouter } from "react-router-dom";
+import { RouteComponentProps, withRouter } from "react-router-dom";
 import { Moment } from "moment";
 import _ from "lodash";
 
 import { AdminUIState } from "src/redux/state";
-import { nodesSummarySelector, partitionedStatuses } from "src/redux/nodes";
+import { nodesSummarySelector } from "src/redux/nodes";
 import { refreshLiveness, refreshNodes } from "src/redux/apiReducers";
-import { INodeStatus } from "src/util/proto";
-import { LongToMoment } from "src/util/convert";
+import { cockroach } from "src/js/protos";
+import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus;
 import { LocalSetting } from "src/redux/localsettings";
 
 import "./decommissionedNodeHistory.styl";
 import { Text } from "src/components";
-import { ColumnsConfig, Table, SortSetting } from "@cockroachlabs/cluster-ui";
+import {
+  ColumnsConfig,
+  Table,
+  SortSetting,
+  util,
+} from "@cockroachlabs/cluster-ui";
 import { createSelector } from "reselect";
 
 const decommissionedNodesSortSetting = new LocalSetting<
@@ -35,7 +40,6 @@ const decommissionedNodesSortSetting = new LocalSetting<
 interface DecommissionedNodeStatusRow {
   key: string;
   nodeId: number;
-  address: string;
   decommissionedDate: Moment;
 }
 
@@ -81,22 +85,12 @@ export class DecommissionedNodeHistory extends React.Component<
       sorter: sortByNodeId,
       render: (_text, record) => <Text>{`n${record.nodeId}`}</Text>,
     },
-    {
-      key: "address",
-      title: "Address",
-      sorter: true,
-      render: (_text, record) => (
-        <Link to={`/node/${record.nodeId}`}>
-          <Text>{record.address}</Text>
-        </Link>
-      ),
-    },
     {
       key: "decommissionedOn",
       title: "Decommissioned On",
       sorter: sortByDecommissioningDate,
       render: (_text, record) => {
-        return record.decommissionedDate.format("LL[ at ]h:mm a");
+        return record.decommissionedDate.format("LL[ at ]h:mm a UTC");
       },
     },
   ];
@@ -131,34 +125,35 @@ export class DecommissionedNodeHistory extends React.Component<
 }
 
 const decommissionedNodesTableData = createSelector(
-  partitionedStatuses,
   nodesSummarySelector,
-  (statuses, nodesSummary): DecommissionedNodeStatusRow[] => {
-    const decommissionedStatuses = statuses.decommissioned || [];
-
+  (nodesSummary): DecommissionedNodeStatusRow[] => {
     const getDecommissionedTime = (nodeId: number) => {
       const liveness = nodesSummary.livenessByNodeID[nodeId];
       if (!liveness) {
         return undefined;
       }
       const deadTime = liveness.expiration.wall_time;
-      return LongToMoment(deadTime);
+      return util.LongToMoment(deadTime);
     };
 
-    const data = _.chain(decommissionedStatuses)
-      .orderBy(
-        [(ns: INodeStatus) => getDecommissionedTime(ns.desc.node_id)],
-        ["desc"],
-      )
-      .map((ns: INodeStatus, idx: number) => {
+    const decommissionedNodes = Object.values(
+      nodesSummary.livenessByNodeID,
+    ).filter(liveness => {
+      return liveness?.membership === MembershipStatus.DECOMMISSIONED;
+    });
+
+    const data = _.chain(decommissionedNodes)
+      .orderBy([liveness => getDecommissionedTime(liveness.node_id)], ["desc"])
+      .map((liveness, idx: number) => {
+        const { node_id } = liveness;
         return {
           key: `${idx}`,
-          nodeId: ns.desc.node_id,
-          address: ns.desc.address.address_field,
-          decommissionedDate: getDecommissionedTime(ns.desc.node_id),
+          nodeId: node_id,
+          decommissionedDate: getDecommissionedTime(node_id),
         };
       })
       .value();
+
     return data;
   },
 );