From 7179469bb52313dae067555f2a7c609f68079e74 Mon Sep 17 00:00:00 2001 From: Zach Lite Date: Mon, 14 Feb 2022 16:19:31 -0500 Subject: [PATCH] ui: Use liveness info to populate decommissioned node lists 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. fix tests fix pb import and add default arg value --- .../workspaces/db-console/src/redux/nodes.ts | 3 ++ .../containers/nodesOverview/index.tsx | 54 ++++++++++++------- .../nodesOverview/nodesOverview.spec.tsx | 8 +-- .../nodeHistory/decommissionedNodeHistory.tsx | 47 +++++++--------- 4 files changed, 60 insertions(+), 52 deletions(-) diff --git a/pkg/ui/workspaces/db-console/src/redux/nodes.ts b/pkg/ui/workspaces/db-console/src/redux/nodes.ts index 0a508bcf8a98..dcc616baf4a5 100644 --- a/pkg/ui/workspaces/db-console/src/redux/nodes.ts +++ b/pkg/ui/workspaces/db-console/src/redux/nodes.ts @@ -306,6 +306,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, diff --git a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx index 9c947bf5957e..57813e1bb70c 100644 --- a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx +++ b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/index.tsx @@ -53,6 +53,8 @@ import { VersionTooltip, StatusTooltip, } from "./tooltips"; +import { cockroach } from "src/js/protos"; +import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus; const liveNodesSortSetting = new LocalSetting( "nodes/live_sort_setting", @@ -169,13 +171,24 @@ export const getLivenessStatusName = (status: LivenessStatus): string => { const NodeNameColumn: React.FC<{ record: NodeStatusRow | DecommissionedNodeStatusRow; -}> = ({ record }) => { - return ( - + shouldLink?: boolean; +}> = ({ record, shouldLink = true }) => { + const columnValue = ( + <> {record.nodeName} {` (n${record.nodeId})`} - + ); + + if (shouldLink) { + return ( + + {columnValue} + + ); + } + + return columnValue; }; const NodeLocalityColumn: React.FC<{ record: NodeStatusRow }> = ({ @@ -422,14 +435,14 @@ class DecommissionedNodeList extends React.Component< key: "nodes", title: "decommissioned nodes", render: (_text: string, record: DecommissionedNodeStatusRow) => ( - + ), }, { 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", @@ -584,11 +597,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) { @@ -598,20 +608,24 @@ export const decommissionedNodesTableDataSelector = createSelector( return util.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(); diff --git a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/nodesOverview.spec.tsx b/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/nodesOverview.spec.tsx index 6b9d222ebe01..1b71fbd7310c 100644 --- a/pkg/ui/workspaces/db-console/src/views/cluster/containers/nodesOverview/nodesOverview.spec.tsx +++ b/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 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()); }); }); }); diff --git a/pkg/ui/workspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx b/pkg/ui/workspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx index b46e68b15557..95c228182381 100644 --- a/pkg/ui/workspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx +++ b/pkg/ui/workspaces/db-console/src/views/reports/containers/nodeHistory/decommissionedNodeHistory.tsx @@ -11,14 +11,15 @@ 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 { cockroach } from "src/js/protos"; +import MembershipStatus = cockroach.kv.kvserver.liveness.livenesspb.MembershipStatus; import { LocalSetting } from "src/redux/localsettings"; import "./decommissionedNodeHistory.styl"; @@ -39,7 +40,6 @@ const decommissionedNodesSortSetting = new LocalSetting< interface DecommissionedNodeStatusRow { key: string; nodeId: number; - address: string; decommissionedDate: Moment; } @@ -85,22 +85,12 @@ export class DecommissionedNodeHistory extends React.Component< sorter: sortByNodeId, render: (_text, record) => {`n${record.nodeId}`}, }, - { - key: "address", - title: "Address", - sorter: true, - render: (_text, record) => ( - - {record.address} - - ), - }, { 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"); }, }, ]; @@ -135,11 +125,8 @@ 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) { @@ -149,20 +136,24 @@ const decommissionedNodesTableData = createSelector( 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; }, );