From 2e5c5323780ebeea9b3914e13a3d02608463f0e9 Mon Sep 17 00:00:00 2001 From: Aviv Turgeman Date: Tue, 8 Oct 2024 15:11:48 +0300 Subject: [PATCH] OCPBUGS-42859: Interface with same name causes unexpected behavior in NNS topology Signed-off-by: Aviv Turgeman --- src/views/states/list/StatesList.tsx | 13 +- src/views/states/topology/Topology.tsx | 141 ++++++++--------- .../components/CustomNode/CustomNode.tsx | 13 +- src/views/states/topology/utils/position.ts | 7 +- src/views/states/topology/utils/utils.ts | 142 +++++++++--------- 5 files changed, 151 insertions(+), 165 deletions(-) diff --git a/src/views/states/list/StatesList.tsx b/src/views/states/list/StatesList.tsx index a92e9288..21a6093c 100644 --- a/src/views/states/list/StatesList.tsx +++ b/src/views/states/list/StatesList.tsx @@ -18,6 +18,7 @@ import { Button, Flex, Icon, Pagination } from '@patternfly/react-core'; import { TopologyIcon } from '@patternfly/react-icons'; import { Table, TableGridBreakpoint, Th, Thead, Tr } from '@patternfly/react-table'; import { V1beta1NodeNetworkState } from '@types'; +import { isEmpty } from '@utils/helpers'; import usePagination from '@utils/hooks/usePagination/usePagination'; import { paginationDefaultValues } from '@utils/hooks/usePagination/utils/constants'; @@ -77,11 +78,13 @@ const StatesList: FC = () => { return ( <> - + {!isEmpty(states) && ( + + )} diff --git a/src/views/states/topology/Topology.tsx b/src/views/states/topology/Topology.tsx index 5bce1748..354aeb81 100644 --- a/src/views/states/topology/Topology.tsx +++ b/src/views/states/topology/Topology.tsx @@ -1,7 +1,7 @@ import React, { FC, useEffect, useMemo, useState } from 'react'; import { NodeNetworkStateModelGroupVersionKind } from '@models'; -import { useK8sWatchResource } from '@openshift-console/dynamic-plugin-sdk'; +import { ListPageBody, useK8sWatchResource } from '@openshift-console/dynamic-plugin-sdk'; import { action, createTopologyControlButtons, @@ -14,6 +14,8 @@ import { VisualizationSurface, } from '@patternfly/react-topology'; import { V1beta1NodeNetworkState } from '@types'; +import AccessDenied from '@utils/components/AccessDenied/AccessDenied'; +import { isEmpty } from '@utils/helpers'; import TopologySidebar from './components/TopologySidebar/TopologySidebar'; import TopologyToolbar from './components/TopologyToolbar/TopologyToolbar'; @@ -39,84 +41,83 @@ const Topology: FC = () => { ); useEffect(() => { - if (loaded && !error) { - const filteredStates = - selectedNodeFilters.length > 0 - ? states.filter((state) => selectedNodeFilters.includes(state.metadata.name)) - : undefined; + if (!loaded || error || isEmpty(states)) return; - const topologyModel = transformDataToTopologyModel(states, filteredStates); + const filteredStates = !isEmpty(selectedNodeFilters) + ? states.filter((state) => selectedNodeFilters.includes(state.metadata.name)) + : undefined; - if (!visualization) { - const newVisualization = new Visualization(); - newVisualization.registerLayoutFactory(layoutFactory); - newVisualization.registerComponentFactory(componentFactory); - newVisualization.addEventListener(SELECTION_EVENT, setSelectedIds); - newVisualization.setFitToScreenOnLayout(true); - newVisualization.fromModel(topologyModel); - restoreNodePositions(newVisualization); + const topologyModel = transformDataToTopologyModel(states, filteredStates); - setVisualization(newVisualization); - } else { - visualization.fromModel(topologyModel); - restoreNodePositions(visualization); - } - } - }, [states, loaded, error, selectedNodeFilters]); - - useEffect(() => { - if (visualization) { - visualization.addEventListener(NODE_POSITIONING_EVENT, () => - saveNodePositions(visualization), + if (!visualization) { + const newVisualization = new Visualization(); + newVisualization.registerLayoutFactory(layoutFactory); + newVisualization.registerComponentFactory(componentFactory); + newVisualization.addEventListener(SELECTION_EVENT, setSelectedIds); + newVisualization.addEventListener(NODE_POSITIONING_EVENT, () => + saveNodePositions(newVisualization), ); - visualization.addEventListener(GRAPH_POSITIONING_EVENT, () => - saveNodePositions(visualization), + newVisualization.addEventListener(GRAPH_POSITIONING_EVENT, () => + saveNodePositions(newVisualization), ); + newVisualization.setFitToScreenOnLayout(true); + newVisualization.fromModel(topologyModel, false); + restoreNodePositions(newVisualization); + setVisualization(newVisualization); + } else { + visualization.fromModel(topologyModel); } - }, [visualization]); + }, [states, loaded, error, selectedNodeFilters]); + + if (error && error?.response?.status === 403) + return ( + + + + ); return ( - - } - viewToolbar={ - - } - controlBar={ - { - visualization.getGraph().scaleBy(4 / 3); - }), - zoomOutCallback: action(() => { - visualization.getGraph().scaleBy(0.75); - }), - fitToScreenCallback: action(() => { - visualization.getGraph().fit(40); - }), - resetViewCallback: action(() => { - visualization.getGraph().reset(); - visualization.getGraph().layout(); - }), - legend: false, - })} - /> - } - > - + + + } + viewToolbar={ + + } + controlBar={ + { + visualization.getGraph().scaleBy(4 / 3); + }), + zoomOutCallback: action(() => { + visualization.getGraph().scaleBy(0.75); + }), + fitToScreenCallback: action(() => { + visualization.getGraph().fit(40); + }), + resetViewCallback: action(() => { + visualization.getGraph().reset(); + visualization.getGraph().layout(); + }), + legend: false, + })} + /> + } + > - - + + ); }; diff --git a/src/views/states/topology/components/CustomNode/CustomNode.tsx b/src/views/states/topology/components/CustomNode/CustomNode.tsx index eab8b2bb..a9a5b4db 100644 --- a/src/views/states/topology/components/CustomNode/CustomNode.tsx +++ b/src/views/states/topology/components/CustomNode/CustomNode.tsx @@ -18,24 +18,15 @@ type CustomNodeProps = { WithDragNodeProps & WithDndDropProps; -const CustomNode: FC = ({ element, onSelect, selected, ...rest }) => { +const CustomNode: FC = ({ element, ...rest }) => { const data = element.getData(); const Icon = data.icon; const { width, height } = element.getBounds(); const xCenter = (width - ICON_SIZE) / 2; const yCenter = (height - ICON_SIZE) / 2; - return ( - + diff --git a/src/views/states/topology/utils/position.ts b/src/views/states/topology/utils/position.ts index 303f7bdc..ba352797 100644 --- a/src/views/states/topology/utils/position.ts +++ b/src/views/states/topology/utils/position.ts @@ -8,16 +8,11 @@ export const saveNodePositions = (visualization: Visualization) => { // Traverse all nodes and their children graph.getNodes().forEach((node) => { + nodePositions[node.getId()] = node.getPosition(); if (node.isGroup()) { - // Save the group node position - nodePositions[node.getId()] = node.getPosition(); - - // Save all child node positions node.getAllNodeChildren().forEach((childNode) => { nodePositions[childNode.getId()] = childNode.getPosition(); }); - } else { - nodePositions[node.getId()] = node.getPosition(); } }); diff --git a/src/views/states/topology/utils/utils.ts b/src/views/states/topology/utils/utils.ts index c9716a1a..020ad725 100644 --- a/src/views/states/topology/utils/utils.ts +++ b/src/views/states/topology/utils/utils.ts @@ -25,7 +25,12 @@ const getStatus = (iface: NodeNetworkConfigurationInterface): NodeStatus => { }; const getIcon = (iface: NodeNetworkConfigurationInterface) => { - if (!isEmpty(iface.bridge)) return BridgeIcon; + if ( + iface.bridge || + iface.type === InterfaceType.OVS_BRIDGE || + iface.type === InterfaceType.LINUX_BRIDGE + ) + return BridgeIcon; if (iface.ethernet || iface.type === InterfaceType.ETHERNET) return EthernetIcon; if (iface.type === InterfaceType.BOND) return LinkIcon; return NetworkIcon; @@ -34,92 +39,83 @@ const getIcon = (iface: NodeNetworkConfigurationInterface) => { const createNodes = ( nnsName: string, interfaces: NodeNetworkConfigurationInterface[], -): NodeModel[] => { - return interfaces.map((iface) => { - const icon = getIcon(iface); - return { - id: `${nnsName}~${iface.name}`, - type: ModelKind.node, - label: iface.name, - width: NODE_DIAMETER, - height: NODE_DIAMETER, - visible: !iface.patch && iface.type !== InterfaceType.LOOPBACK, - shape: NodeShape.circle, - status: getStatus(iface), - data: { - badge: 'I', - icon, - }, - parent: nnsName, - }; - }); -}; +): NodeModel[] => + interfaces.map((iface) => ({ + id: `${nnsName}~${iface.name}~${iface.type}`, + type: ModelKind.node, + label: iface.name, + width: NODE_DIAMETER, + height: NODE_DIAMETER, + visible: !iface.patch && iface.type !== InterfaceType.LOOPBACK, + shape: NodeShape.circle, + status: getStatus(iface), + data: { + icon: getIcon(iface), + type: iface.type, + bridgePorts: iface.bridge?.port, + bondPorts: iface['link-aggregation']?.port, + vlanBaseInterface: iface.vlan?.['base-iface'], + }, + parent: nnsName, + })); -const createEdges = ( - nnsName: string, - interfaces: NodeNetworkConfigurationInterface[], -): EdgeModel[] => { +const createEdges = (childNodes: NodeModel[]): EdgeModel[] => { const edges: EdgeModel[] = []; - const patchConnections: { [key: string]: string } = {}; - interfaces.forEach((iface: NodeNetworkConfigurationInterface) => { - if (iface.patch?.peer) { - patchConnections[iface.name] = iface.patch.peer; - } - }); + childNodes.forEach((sourceNode) => { + // Find bridge connections + if (!isEmpty(sourceNode.data?.bridgePorts)) { + sourceNode.data?.bridgePorts.forEach((port) => { + const targetNode = childNodes.find( + (target) => target.label === port.name && target.id !== sourceNode.id, + ); - interfaces.forEach((iface: NodeNetworkConfigurationInterface) => { - const nodeId = `${nnsName}~${iface.name}`; - - if (iface.bridge?.port) { - iface.bridge.port.forEach((prt) => { - if (patchConnections[prt.name]) { - const peerPatch = patchConnections[prt.name]; - const peerBridge = interfaces.find((intf) => - intf.bridge?.port.some((p) => p.name === peerPatch), - ); - - if (peerBridge) { - const peerBridgeId = `${nnsName}~${peerBridge.name}`; - edges.push({ - id: `${nodeId}~${peerBridgeId}-edge`, - type: ModelKind.edge, - source: nodeId, - target: peerBridgeId, - }); - } - } else if (prt.name && iface.name !== prt.name) { + if (!isEmpty(targetNode)) { edges.push({ - id: `${nodeId}~${prt.name}-edge`, + id: `${sourceNode.id}~${targetNode.id}-edge`, type: ModelKind.edge, - source: nodeId, - target: `${nnsName}~${prt.name}`, + source: sourceNode.id, + target: targetNode.id, }); } }); } - if (iface.vlan?.['base-iface'] && iface.name === iface.vlan?.['base-iface']) { - edges.push({ - id: `${nodeId}~${iface.vlan['base-iface']}-edge`, - type: ModelKind.edge, - source: nodeId, - target: `${nnsName}~${iface.vlan['base-iface']}`, - }); - } + // Find bond connections + if (!isEmpty(sourceNode.data?.vlanBaseInterface)) { + sourceNode.data?.bondPorts.forEach((port) => { + const targetNode = childNodes.find( + (target) => target.label === port && target.id !== sourceNode.id, + ); - if (iface['link-aggregation']?.port) { - iface['link-aggregation'].port.forEach((prt: string) => { - if (iface.name !== prt) { + if (!isEmpty(targetNode)) { edges.push({ - id: `${nodeId}~${prt}-edge`, + id: `${sourceNode.id}~${targetNode.id}-edge`, type: ModelKind.edge, - source: nodeId, - target: `${nnsName}~${prt}`, + source: sourceNode.id, + target: targetNode.id, }); } }); } + + // Find vlan connections + if (!isEmpty(sourceNode.data?.bondPorts)) { + const baseInterface = sourceNode.data?.vlanBaseInterface; + + const targetNode = childNodes.find( + (target) => target.label === baseInterface && target.id !== sourceNode.id, + ); + + if (!isEmpty(targetNode)) { + edges.push({ + id: `${sourceNode.id}~${targetNode.id}-edge`, + type: ModelKind.edge, + source: sourceNode.id, + target: targetNode.id, + }); + } + } }); return edges; @@ -144,7 +140,7 @@ const createGroupNode = (nnsName: string, childNodeIds: string[], visible: boole export const transformDataToTopologyModel = ( data: V1beta1NodeNetworkState[], - filteredData?: V1beta1NodeNetworkState[], // Optional filtered data parameter + filteredData?: V1beta1NodeNetworkState[], ): Model => { const nodes: NodeModel[] = []; const edges: EdgeModel[] = []; @@ -154,7 +150,6 @@ export const transformDataToTopologyModel = ( const childNodes = createNodes(nnsName, nodeState.status.currentState.interfaces); - // Determine visibility based on whether filteredData includes this nodeState const isVisible = filteredData ? filteredData.some((filteredState) => filteredState.metadata.name === nnsName) : true; @@ -164,9 +159,10 @@ export const transformDataToTopologyModel = ( childNodes.map((child) => child.id), isVisible, ); - const nodeEdges = createEdges(nnsName, nodeState.status.currentState.interfaces); - nodes.push(groupNode, ...childNodes); + + const nodeEdges = createEdges(childNodes); + edges.push(...nodeEdges); });