From b6749f3a7b5498ce4ff81b95144f7e4238a4d141 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 26 Jun 2026 10:34:40 +0000 Subject: [PATCH] Block conflicting MirrorPeer pairs before topology Pair cluster action Extract verifyMirrorPeerExistence and related checks into a shared cluster-peering-validators utility used by both the Create DRPolicy form and the topology Pair cluster flow. The topology context menu now disables targets that would conflict with an existing MirrorPeer or sync DRPolicy, and opening the pair modal is blocked with the same error message shown during DR policy creation. Reuse of an existing identical pair remains allowed. Co-authored-by: Timothy --- .../selected-cluster-validator.tsx | 37 ++----- .../utils/cluster-peering-validators.spec.ts | 99 +++++++++++++++++++ .../utils/cluster-peering-validators.ts | 53 ++++++++++ packages/mco/components/topology/Topology.tsx | 70 ++++++++++++- .../components/ClusterContextMenu.tsx | 4 + .../topology/context/TopologyContext.tsx | 8 ++ .../topology/factory/MCOStyleAppGroup.tsx | 7 +- .../topology/factory/MCOStyleNode.tsx | 5 +- 8 files changed, 246 insertions(+), 37 deletions(-) create mode 100644 packages/mco/components/create-dr-policy/utils/cluster-peering-validators.spec.ts create mode 100644 packages/mco/components/create-dr-policy/utils/cluster-peering-validators.ts diff --git a/packages/mco/components/create-dr-policy/selected-cluster-validator.tsx b/packages/mco/components/create-dr-policy/selected-cluster-validator.tsx index be33070a2..d1aa0950c 100644 --- a/packages/mco/components/create-dr-policy/selected-cluster-validator.tsx +++ b/packages/mco/components/create-dr-policy/selected-cluster-validator.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; import { pluralize } from '@odf/core/components/utils'; -import { ReplicationType, STORAGE_ID_LABEL_KEY } from '@odf/mco/constants'; +import { STORAGE_ID_LABEL_KEY } from '@odf/mco/constants'; import { getDRPolicyResourceObj, useACMSafeFetch } from '@odf/mco/hooks'; import { DRPolicyKind, @@ -10,7 +10,6 @@ import { } from '@odf/mco/types'; import { getLabelsFromSearchResult, - getReplicationType, queryStorageClassesUsingClusterNames, } from '@odf/mco/utils'; import { fireManagedClusterView } from '@odf/mco/utils/managed-cluster-view'; @@ -35,6 +34,10 @@ import { ContentVariants, } from '@patternfly/react-core'; import { TimesIcon } from '@patternfly/react-icons'; +import { + checkSyncPolicyExists, + verifyMirrorPeerExistenceForClusters, +} from './utils/cluster-peering-validators'; import { DRPolicyAction, DRPolicyActionType, @@ -46,39 +49,11 @@ import '../../style.scss'; const PROVISIONER = 'provisioner'; const EXTERNAL_DEPLOYMENT_TYPE = 'external'; -const checkSyncPolicyExists = ( - clusters: string[], - drPolicies: DRPolicyKind[] -): boolean => - drPolicies.some((drPolicy) => { - const { drClusters } = drPolicy.spec; - const isSyncPolicy = getReplicationType(drPolicy) === ReplicationType.SYNC; - return ( - isSyncPolicy && drClusters.every((cluster) => clusters.includes(cluster)) - ); - }); - const checkClientToODFPeering = (clusters: ManagedClusterInfoType[]): boolean => // ODF cluster to client peering is not supported for DR. !!clusters[0]?.odfInfo?.storageClusterInfo?.clientInfo !== !!clusters[1]?.odfInfo?.storageClusterInfo?.clientInfo; -const verifyMirrorPeerExistence = ( - clusters: ManagedClusterInfoType[], - mirrorPeers: MirrorPeerKind[] -): boolean => { - const peerNames: string[] = clusters.map(getName); - const mirrorPeer: MirrorPeerKind = mirrorPeers.find((mirrorPeer) => - mirrorPeer.spec?.items?.some((item) => peerNames.includes(item.clusterName)) - ); - const existingPeerNames = - mirrorPeer?.spec?.items?.map((item) => item.clusterName) ?? []; - // When one of the chosen clusters is already paired with another cluster, return True. - return existingPeerNames.length > 0 - ? existingPeerNames.sort().join(',') !== peerNames.sort().join(',') - : false; -}; - const validateClusterSelection = ( clusters: ManagedClusterInfoType[], drPolicies: DRPolicyKind[], @@ -132,7 +107,7 @@ const validateClusterSelection = ( : checkClientToODFPeering(clusters), invalidPolicyCreation: checkSyncPolicyExists(clusters.map(getName), drPolicies) || - verifyMirrorPeerExistence(clusters, mirrorPeers), + verifyMirrorPeerExistenceForClusters(clusters, mirrorPeers), // Storage class validation uses ODF/Ceph provisioner allowlists. // For third-party storage, replication is externally managed. unSupportedStorageClasses: isThirdPartyPath diff --git a/packages/mco/components/create-dr-policy/utils/cluster-peering-validators.spec.ts b/packages/mco/components/create-dr-policy/utils/cluster-peering-validators.spec.ts new file mode 100644 index 000000000..5e8cb6d61 --- /dev/null +++ b/packages/mco/components/create-dr-policy/utils/cluster-peering-validators.spec.ts @@ -0,0 +1,99 @@ +import { DRPolicyModel } from '@odf/shared'; +import { DRPolicyKind, MirrorPeerKind } from '../../../types'; +import { + checkSyncPolicyExists, + hasConflictingPeering, + verifyMirrorPeerExistence, +} from './cluster-peering-validators'; + +const mirrorPeer = ( + cluster1: string, + cluster2: string +): MirrorPeerKind => ({ + apiVersion: 'multicluster.odf.openshift.io/v1alpha1', + kind: 'MirrorPeer', + metadata: { name: `mirrorpeer-${cluster1}-${cluster2}` }, + spec: { + items: [ + { + clusterName: cluster1, + storageClusterRef: { name: 'ocs-storagecluster' }, + }, + { + clusterName: cluster2, + storageClusterRef: { name: 'ocs-storagecluster' }, + }, + ], + }, +}); + +const syncDRPolicy = (clusters: string[]): DRPolicyKind => ({ + apiVersion: `${DRPolicyModel.apiGroup}/${DRPolicyModel.apiVersion}`, + kind: DRPolicyModel.kind, + metadata: { name: 'sync-policy' }, + spec: { + drClusters: clusters, + schedulingInterval: '0m', + }, +}); + +describe('cluster-peering-validators', () => { + describe('verifyMirrorPeerExistence', () => { + it('returns false when no MirrorPeer references the selected clusters', () => { + expect( + verifyMirrorPeerExistence(['A', 'B'], [mirrorPeer('C', 'D')]) + ).toBe(false); + }); + + it('returns false when the same pair already exists (reuse allowed)', () => { + expect( + verifyMirrorPeerExistence(['A', 'B'], [mirrorPeer('A', 'B')]) + ).toBe(false); + }); + + it('returns true when a selected cluster is paired with a different cluster', () => { + expect( + verifyMirrorPeerExistence(['A', 'B'], [mirrorPeer('A', 'C')]) + ).toBe(true); + }); + }); + + describe('checkSyncPolicyExists', () => { + it('returns true when a sync DRPolicy already covers the selected clusters', () => { + expect( + checkSyncPolicyExists(['east-1', 'east-2'], [ + syncDRPolicy(['east-1', 'east-2']), + ]) + ).toBe(true); + }); + + it('returns false for async policies', () => { + expect( + checkSyncPolicyExists(['east-1', 'west-1'], [ + { + ...syncDRPolicy(['east-1', 'west-1']), + spec: { drClusters: ['east-1', 'west-1'], schedulingInterval: '5m' }, + }, + ]) + ).toBe(false); + }); + }); + + describe('hasConflictingPeering', () => { + it('blocks conflicting mirror peer pairs', () => { + expect( + hasConflictingPeering(['A', 'B'], [mirrorPeer('A', 'C')], []) + ).toBe(true); + }); + + it('allows reuse of the same mirror peer pair', () => { + expect( + hasConflictingPeering(['A', 'B'], [mirrorPeer('A', 'B')], []) + ).toBe(false); + }); + + it('allows creating a new pair when no mirror peer exists', () => { + expect(hasConflictingPeering(['A', 'B'], [], [])).toBe(false); + }); + }); +}); diff --git a/packages/mco/components/create-dr-policy/utils/cluster-peering-validators.ts b/packages/mco/components/create-dr-policy/utils/cluster-peering-validators.ts new file mode 100644 index 000000000..e24e90ce0 --- /dev/null +++ b/packages/mco/components/create-dr-policy/utils/cluster-peering-validators.ts @@ -0,0 +1,53 @@ +import { ReplicationType } from '@odf/mco/constants'; +import { DRPolicyKind, MirrorPeerKind } from '@odf/mco/types'; +import { getReplicationType } from '@odf/mco/utils'; +import { getName } from '@odf/shared'; +import { ManagedClusterInfoType } from './reducer'; + +/** + * Returns true when a selected cluster is already in a MirrorPeer with a different + * cluster than the one being paired. Reuse of the same pair (A+B again) is allowed. + */ +export const verifyMirrorPeerExistence = ( + clusterNames: string[], + mirrorPeers: MirrorPeerKind[] +): boolean => { + const peerNames = clusterNames; + const mirrorPeer = mirrorPeers.find((mp) => + mp.spec?.items?.some((item) => peerNames.includes(item.clusterName)) + ); + const existingPeerNames = + mirrorPeer?.spec?.items?.map((item) => item.clusterName) ?? []; + return existingPeerNames.length > 0 + ? existingPeerNames.sort().join(',') !== peerNames.sort().join(',') + : false; +}; + +export const verifyMirrorPeerExistenceForClusters = ( + clusters: ManagedClusterInfoType[], + mirrorPeers: MirrorPeerKind[] +): boolean => verifyMirrorPeerExistence(clusters.map(getName), mirrorPeers); + +export const checkSyncPolicyExists = ( + clusters: string[], + drPolicies: DRPolicyKind[] +): boolean => + drPolicies.some((drPolicy) => { + const { drClusters } = drPolicy.spec; + const isSyncPolicy = getReplicationType(drPolicy) === ReplicationType.SYNC; + return ( + isSyncPolicy && drClusters.every((cluster) => clusters.includes(cluster)) + ); + }); + +/** + * Block conflicting pairs (e.g. A+C exists, user selects A+B). + * Allow reuse when the same pair already has a MirrorPeer or sync DRPolicy. + */ +export const hasConflictingPeering = ( + clusterNames: string[], + mirrorPeers: MirrorPeerKind[], + drPolicies: DRPolicyKind[] +): boolean => + checkSyncPolicyExists(clusterNames, drPolicies) || + verifyMirrorPeerExistence(clusterNames, mirrorPeers); diff --git a/packages/mco/components/topology/Topology.tsx b/packages/mco/components/topology/Topology.tsx index 283f9c43a..d631fcf1c 100644 --- a/packages/mco/components/topology/Topology.tsx +++ b/packages/mco/components/topology/Topology.tsx @@ -10,8 +10,17 @@ import { useVisualizationSetup, } from '@odf/shared/topology'; import { useCustomTranslation } from '@odf/shared/useCustomTranslationHook'; +import { MirrorPeerModel } from '@odf/shared'; +import { referenceForModel } from '@odf/shared/utils'; import { useK8sWatchResource } from '@openshift-console/dynamic-plugin-sdk'; -import { EmptyState, EmptyStateBody, Title } from '@patternfly/react-core'; +import { + Alert, + AlertActionCloseButton, + AlertVariant, + EmptyState, + EmptyStateBody, + Title, +} from '@patternfly/react-core'; import { TopologyIcon } from '@patternfly/react-icons'; import { GraphElement, @@ -21,11 +30,13 @@ import { import { ODFMCO_OPERATOR } from '../../constants'; import { getManagedClusterResourceObj, + getDRPolicyResourceObj, useProtectedAppsByCluster, useDRPoliciesByClusterPair, useActiveDROperations, } from '../../hooks'; -import { ACMManagedClusterKind } from '../../types'; +import { ACMManagedClusterKind, DRPolicyKind, MirrorPeerKind } from '../../types'; +import { hasConflictingPeering } from '../create-dr-policy/utils/cluster-peering-validators'; import { CreateDRPolicyModal } from '../create-dr-policy/CreateDRPolicyModal'; import { TopologyDataContext } from './context/TopologyContext'; import { mcoTopologyComponentFactory } from './factory/MCOStyleFactory'; @@ -214,6 +225,7 @@ const TopologyEmptyState: React.FC = () => { }; const Topology: React.FC = () => { + const { t } = useCustomTranslation(); const controller = useVisualizationSetup({ componentFactory: mcoTopologyComponentFactory, }); @@ -225,11 +237,22 @@ const Topology: React.FC = () => { const [pairModalClusters, setPairModalClusters] = React.useState( [] ); + const [pairValidationError, setPairValidationError] = React.useState(''); const [managedClusters, loaded, loadError] = useK8sWatchResource< ACMManagedClusterKind[] >(getManagedClusterResourceObj()); + const [mirrorPeers] = useK8sWatchResource({ + kind: referenceForModel(MirrorPeerModel), + isList: true, + namespaced: false, + }); + + const [drPolicies] = useK8sWatchResource( + getDRPolicyResourceObj() + ); + const [clusterAppsMap, appsLoaded, appsLoadError] = useProtectedAppsByCluster(); @@ -246,10 +269,32 @@ const Topology: React.FC = () => { const handleOpenPairModal = React.useCallback( (sourceCluster: string, targetCluster: string) => { - setPairModalClusters([sourceCluster, targetCluster]); + const clusterNames = [sourceCluster, targetCluster]; + if (hasConflictingPeering(clusterNames, mirrorPeers, drPolicies)) { + setPairValidationError( + t( + 'A mirror peer configuration already exists for one or more of the selected clusters, ' + + 'either from an existing or deleted DR policy. To create a new DR policy with these clusters, ' + + 'delete any existing mirror peer configurations associated with them and try again.' + ) + ); + return; + } + setPairValidationError(''); + setPairModalClusters(clusterNames); setIsPairModalOpen(true); }, - [] + [mirrorPeers, drPolicies, t] + ); + + const isClusterPairingBlocked = React.useCallback( + (sourceCluster: string, targetCluster: string) => + hasConflictingPeering( + [sourceCluster, targetCluster], + mirrorPeers, + drPolicies + ), + [mirrorPeers, drPolicies] ); const handleClosePairModal = React.useCallback(() => { @@ -268,6 +313,7 @@ const Topology: React.FC = () => { clusterPairPoliciesMap, clusterPairOperationsMap, onOpenPairModal: handleOpenPairModal, + isClusterPairingBlocked, }; }, [ managedClusters, @@ -278,6 +324,7 @@ const Topology: React.FC = () => { clusterPairPoliciesMap, clusterPairOperationsMap, handleOpenPairModal, + isClusterPairingBlocked, ]); const hasNoClusters = @@ -287,6 +334,21 @@ const Topology: React.FC = () => {
+ {pairValidationError && ( + setPairValidationError('')} + /> + } + > + {pairValidationError} + + )} {hasNoClusters ? ( ) : ( diff --git a/packages/mco/components/topology/components/ClusterContextMenu.tsx b/packages/mco/components/topology/components/ClusterContextMenu.tsx index 435e460f9..09557461b 100644 --- a/packages/mco/components/topology/components/ClusterContextMenu.tsx +++ b/packages/mco/components/topology/components/ClusterContextMenu.tsx @@ -22,6 +22,7 @@ export interface ClusterContextMenuProps { handlers: ClusterContextMenuHandlers; otherClusters: ACMManagedClusterKind[]; rootMenuId?: string; + isPairingBlocked?: (targetClusterName: string) => boolean; } /** @@ -33,6 +34,7 @@ export const ClusterContextMenu: React.FC = ({ handlers, otherClusters, rootMenuId = 'cluster-menu-root', + isPairingBlocked, }) => { const { t } = useCustomTranslation(); const { @@ -95,10 +97,12 @@ export const ClusterContextMenu: React.FC = ({ {otherClusters.length > 0 ? ( otherClusters.map((cluster) => { const clusterName = getName(cluster); + const pairingBlocked = isPairingBlocked?.(clusterName); return ( handlePairWithCluster(clusterName)} > {clusterName} diff --git a/packages/mco/components/topology/context/TopologyContext.tsx b/packages/mco/components/topology/context/TopologyContext.tsx index 149cc1d73..a33966f91 100644 --- a/packages/mco/components/topology/context/TopologyContext.tsx +++ b/packages/mco/components/topology/context/TopologyContext.tsx @@ -20,6 +20,13 @@ type DefaultContext = { * Used by context menu when "Pair cluster" is clicked */ onOpenPairModal?: (sourceCluster: string, targetCluster: string) => void; + /** + * Returns true when the cluster pair cannot be peered (conflicting MirrorPeer or sync DRPolicy). + */ + isClusterPairingBlocked?: ( + sourceCluster: string, + targetCluster: string + ) => boolean; }; const defaultContext: DefaultContext = { @@ -32,6 +39,7 @@ const defaultContext: DefaultContext = { clusterPairPoliciesMap: {}, clusterPairOperationsMap: {}, onOpenPairModal: undefined, + isClusterPairingBlocked: undefined, }; export const TopologyDataContext = diff --git a/packages/mco/components/topology/factory/MCOStyleAppGroup.tsx b/packages/mco/components/topology/factory/MCOStyleAppGroup.tsx index fce43bf17..7fb2b3081 100644 --- a/packages/mco/components/topology/factory/MCOStyleAppGroup.tsx +++ b/packages/mco/components/topology/factory/MCOStyleAppGroup.tsx @@ -22,7 +22,8 @@ const MCOStyleAppGroup: React.FunctionComponent = ({ element, ...rest }) => { - const { clusters, onOpenPairModal } = React.useContext(TopologyDataContext); + const { clusters, onOpenPairModal, isClusterPairingBlocked } = + React.useContext(TopologyDataContext); const data = element.getData(); // Get current cluster name @@ -87,6 +88,10 @@ const MCOStyleAppGroup: React.FunctionComponent = ({ handlers={handlers} otherClusters={otherClusters} rootMenuId="cluster-menu-root" + isPairingBlocked={(targetClusterName) => + isClusterPairingBlocked?.(currentClusterName, targetClusterName) ?? + false + } /> )} diff --git a/packages/mco/components/topology/factory/MCOStyleNode.tsx b/packages/mco/components/topology/factory/MCOStyleNode.tsx index 3206073c5..901fe8a77 100644 --- a/packages/mco/components/topology/factory/MCOStyleNode.tsx +++ b/packages/mco/components/topology/factory/MCOStyleNode.tsx @@ -27,7 +27,7 @@ const MCOStyleNodeComponent: React.FC = ({ element, ...rest }) => { - const { clusterPairOperationsMap, clusters, onOpenPairModal } = + const { clusterPairOperationsMap, clusters, onOpenPairModal, isClusterPairingBlocked } = React.useContext(TopologyDataContext); const data = element.getData(); const detailsLevel = useDetailsLevel(); @@ -100,6 +100,9 @@ const MCOStyleNodeComponent: React.FC = ({ handlers={handlers} otherClusters={otherClusters} rootMenuId="cluster-node-menu-root" + isPairingBlocked={(targetClusterName) => + isClusterPairingBlocked?.(clusterName, targetClusterName) ?? false + } /> );