From 5eafffc7a1dae204af54332e95a81d769a50ec03 Mon Sep 17 00:00:00 2001 From: fxiang1 Date: Fri, 15 May 2026 12:20:13 -0400 Subject: [PATCH 1/2] Fix various bugs with displaying VM resources on the topology Signed-off-by: fxiang1 --- .../model/resourceStatusesAppSet.ts | 2 +- .../model/topologyAppSet.test.ts | 277 ++++++++++++++++++ .../model/topologyAppSet.ts | 49 +++- .../model/topologyUtils.test.ts | 139 +++++++++ .../model/topologyUtils.ts | 58 +++- 5 files changed, 515 insertions(+), 10 deletions(-) create mode 100644 frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.test.ts diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/resourceStatusesAppSet.ts b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/resourceStatusesAppSet.ts index 00160ccd172..12429550ac2 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/resourceStatusesAppSet.ts +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/resourceStatusesAppSet.ts @@ -125,7 +125,7 @@ async function fetchSingleAppResources( const appName = app.metadata?.name if (!appName) return null - const clusterName = getAppTargetCluster(app.spec.destination, clusters) + const clusterName = getAppTargetCluster(app.spec.destination, clusters, appName) if (!clusterName) return null return fetchArgoAppStatusResources(clusterName, appName, namespace) diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts index 3c55bb5022e..ec652a1187f 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts @@ -1394,4 +1394,281 @@ describe('getAppSetTopology', () => { expect(result.links).toBeDefined() expect(toolbarWithActiveApps.setAllApplications).toHaveBeenCalled() }) + + it('should skip VM-owned ControllerRevision resources and create them as children of VirtualMachine', async () => { + const vmUid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + mockSearchClient.query.mockResolvedValue({ + loading: false, + networkStatus: 7, + data: { + searchResult: [ + { + items: [ + { + _uid: 'local-cluster/app-uid-vm', + name: 'vm-appset-local-cluster', + namespace: 'openshift-gitops', + cluster: 'local-cluster', + kind: 'Application', + }, + ], + related: [ + { + kind: 'VirtualMachine', + items: [ + { + _uid: `local-cluster/${vmUid}`, + _relatedUids: ['local-cluster/app-uid-vm'], + name: 'my-vm', + namespace: 'vm-ns', + cluster: 'local-cluster', + kind: 'VirtualMachine', + apiversion: 'v1', + apigroup: 'kubevirt.io', + }, + ], + }, + { + kind: 'ControllerRevision', + items: [ + { + _uid: 'local-cluster/cr-uid-1', + _relatedUids: ['local-cluster/app-uid-vm'], + name: `revision-start-vm-${vmUid}-1`, + namespace: 'vm-ns', + cluster: 'local-cluster', + kind: 'ControllerRevision', + apiversion: 'v1', + apigroup: 'apps', + }, + { + _uid: 'local-cluster/cr-uid-2', + _relatedUids: ['local-cluster/app-uid-vm'], + name: `revision-start-vm-${vmUid}-2`, + namespace: 'vm-ns', + cluster: 'local-cluster', + kind: 'ControllerRevision', + apiversion: 'v1', + apigroup: 'apps', + }, + ], + }, + ], + }, + ], + }, + }) + + const application: ApplicationModel = { + name: 'vm-appset', + namespace: 'openshift-gitops', + app: { + apiVersion: 'argoproj.io/v1alpha1', + kind: 'ApplicationSet', + metadata: { name: 'vm-appset', namespace: 'openshift-gitops' }, + spec: { + generators: [{ list: { elements: [{ cluster: 'local-cluster' }] } }], + template: { spec: { source: { path: 'apps/vm' } } }, + }, + }, + placementDecision: undefined, + isArgoApp: false, + isAppSet: true, + isOCPApp: false, + isFluxApp: false, + isAppSetPullModel: true, + appSetClusters: [{ name: 'local-cluster' }], + appSetApps: [{ metadata: { name: 'vm-appset-local-cluster' }, spec: {} }] as any, + } + + const result: ExtendedTopology = await getAppSetTopology(mockToolbarControl, application, 'local-cluster') + + const vmNode = result.nodes.find((n) => n.type === 'virtualmachine') + expect(vmNode).toBeDefined() + + // VM-owned ControllerRevisions should appear as children of the VM, not as standalone nodes + const crNodes = result.nodes.filter((n) => n.type === 'controllerrevision') + crNodes.forEach((crNode) => { + expect((crNode.specs?.parent as any)?.parentType).toBe('virtualmachine') + }) + expect(crNodes.length).toBe(2) + expect(crNodes.map((n) => n.name).sort()).toEqual([`revision-start-vm-${vmUid}-1`, `revision-start-vm-${vmUid}-2`]) + }) + + it('should skip VirtualMachineInstance resources that have kubevirt.io/vm label', async () => { + mockSearchClient.query.mockResolvedValue({ + loading: false, + networkStatus: 7, + data: { + searchResult: [ + { + items: [ + { + _uid: 'local-cluster/app-uid-vmi', + name: 'vmi-appset-local-cluster', + namespace: 'openshift-gitops', + cluster: 'local-cluster', + kind: 'Application', + }, + ], + related: [ + { + kind: 'VirtualMachine', + items: [ + { + _uid: 'local-cluster/vm-uid-1', + _relatedUids: ['local-cluster/app-uid-vmi'], + name: 'my-vm', + namespace: 'vm-ns', + cluster: 'local-cluster', + kind: 'VirtualMachine', + apiversion: 'v1', + apigroup: 'kubevirt.io', + }, + ], + }, + { + kind: 'VirtualMachineInstance', + items: [ + { + _uid: 'local-cluster/vmi-uid-1', + _relatedUids: ['local-cluster/app-uid-vmi'], + name: 'my-vm', + namespace: 'vm-ns', + cluster: 'local-cluster', + kind: 'VirtualMachineInstance', + apiversion: 'v1', + apigroup: 'kubevirt.io', + label: 'kubevirt.io/vm=my-vm; other-label=value', + }, + ], + }, + ], + }, + ], + }, + }) + + const application: ApplicationModel = { + name: 'vmi-appset', + namespace: 'openshift-gitops', + app: { + apiVersion: 'argoproj.io/v1alpha1', + kind: 'ApplicationSet', + metadata: { name: 'vmi-appset', namespace: 'openshift-gitops' }, + spec: { + generators: [{ list: { elements: [{ cluster: 'local-cluster' }] } }], + template: { spec: { source: { path: 'apps/vm' } } }, + }, + }, + placementDecision: undefined, + isArgoApp: false, + isAppSet: true, + isOCPApp: false, + isFluxApp: false, + isAppSetPullModel: true, + appSetClusters: [{ name: 'local-cluster' }], + appSetApps: [{ metadata: { name: 'vmi-appset-local-cluster' }, spec: {} }] as any, + } + + const result: ExtendedTopology = await getAppSetTopology(mockToolbarControl, application, 'local-cluster') + + // The VM-owned VMI should NOT appear as a standalone top-level node + const standaloneVmiNodes = result.nodes.filter( + (n) => n.type === 'virtualmachineinstance' && (n.specs?.parent as any)?.parentType !== 'virtualmachine' + ) + expect(standaloneVmiNodes).toHaveLength(0) + + // But the VM should exist, and it should have a VMI child created by createVirtualMachineInstance + const vmNode = result.nodes.find((n) => n.type === 'virtualmachine') + expect(vmNode).toBeDefined() + const vmiChildNodes = result.nodes.filter( + (n) => n.type === 'virtualmachineinstance' && (n.specs?.parent as any)?.parentType === 'virtualmachine' + ) + expect(vmiChildNodes).toHaveLength(1) + }) + + it('should keep ControllerRevision resources not owned by a VM as top-level nodes', async () => { + mockSearchClient.query.mockResolvedValue({ + loading: false, + networkStatus: 7, + data: { + searchResult: [ + { + items: [ + { + _uid: 'local-cluster/app-uid-ds', + name: 'ds-appset-local-cluster', + namespace: 'openshift-gitops', + cluster: 'local-cluster', + kind: 'Application', + }, + ], + related: [ + { + kind: 'DaemonSet', + items: [ + { + _uid: 'local-cluster/ds-uid-1', + _relatedUids: ['local-cluster/app-uid-ds'], + name: 'my-daemonset', + namespace: 'ds-ns', + cluster: 'local-cluster', + kind: 'DaemonSet', + apiversion: 'v1', + apigroup: 'apps', + }, + ], + }, + { + kind: 'ControllerRevision', + items: [ + { + _uid: 'local-cluster/cr-uid-normal', + _relatedUids: ['local-cluster/app-uid-ds'], + name: 'my-daemonset-revision-abc123', + namespace: 'ds-ns', + cluster: 'local-cluster', + kind: 'ControllerRevision', + apiversion: 'v1', + apigroup: 'apps', + }, + ], + }, + ], + }, + ], + }, + }) + + const application: ApplicationModel = { + name: 'ds-appset', + namespace: 'openshift-gitops', + app: { + apiVersion: 'argoproj.io/v1alpha1', + kind: 'ApplicationSet', + metadata: { name: 'ds-appset', namespace: 'openshift-gitops' }, + spec: { + generators: [{ list: { elements: [{ cluster: 'local-cluster' }] } }], + template: { spec: { source: { path: 'apps/ds' } } }, + }, + }, + placementDecision: undefined, + isArgoApp: false, + isAppSet: true, + isOCPApp: false, + isFluxApp: false, + isAppSetPullModel: true, + appSetClusters: [{ name: 'local-cluster' }], + appSetApps: [{ metadata: { name: 'ds-appset-local-cluster' }, spec: {} }] as any, + } + + const result: ExtendedTopology = await getAppSetTopology(mockToolbarControl, application, 'local-cluster') + + // Non-VM ControllerRevision should still appear as a regular top-level resource node + const crNodes = result.nodes.filter( + (n) => n.type === 'controllerrevision' && n.name === 'my-daemonset-revision-abc123' + ) + expect(crNodes.length).toBeGreaterThanOrEqual(1) + }) }) diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts index 7fa77ab042c..e68b1f7c0c4 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts @@ -635,6 +635,28 @@ function processResources( // They should become a single node with all target clusters in clustersNames. const deduplicatedResources = deduplicateClusterScopedResources(allResources) + // Map of VM UID -> ControllerRevision name for VM-owned controller revisions + const vmControllerRevisions = new Map() + + // pre-emptively find controller revisions that are owned by a VirtualMachine + deduplicatedResources.forEach((deployable: Record) => { + const typedDeployable = deployable as unknown as ProcessedDeployableResource + const { name: deployableName, kind } = typedDeployable + const type = kind.toLowerCase() + + // ControllerRevision resources owned by a VirtualMachine are already + // represented as child nodes created by createControllerRevisionChild — + // skip them to avoid duplicates. Detected via the + // "revision-start-vm---" naming convention. + if (type === 'controllerrevision') { + const uidMatch = deployableName.match(/.+-vm-([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})-\d+$/) + if (uidMatch) { + vmControllerRevisions.set(uidMatch[1], [...(vmControllerRevisions.get(uidMatch[1]) || []), deployableName]) + return + } + } + }) + // create nodes for each resource deduplicatedResources.forEach((deployable: Record) => { const typedDeployable = deployable as unknown as ProcessedDeployableResource @@ -648,6 +670,24 @@ function processResources( resources: deployableResources, } = typedDeployable const type = kind.toLowerCase() + + // VirtualMachineInstance resources owned by a VirtualMachine (indicated by the + // kubevirt.io/vm label) are already represented as child nodes created by + // createVirtualMachineInstance — skip them to avoid duplicates. + if (type === 'virtualmachineinstance') { + const labelStr: string = (deployable as any).label || '' + if (labelStr.split(';').some((entry: string) => entry.trim().startsWith('kubevirt.io/vm='))) { + return + } + } + + if (type === 'controllerrevision') { + const uidMatch = deployableName.match(/.+-vm-([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})-\d+$/) + if (uidMatch) { + return + } + } + // Use cluster from deployable when present (e.g. concatenated resources from multiple clusters) const deployableCluster = (typedDeployable as any).cluster ?? @@ -708,7 +748,14 @@ function processResources( createReplicaChild(deployableObj, parentClusterNames || [], template, activeTypes, links, nodes) // Create controller revision child nodes (for DaemonSets, StatefulSets) - createControllerRevisionChild(deployableObj, parentClusterNames || [], activeTypes, links, nodes) + createControllerRevisionChild( + deployableObj, + parentClusterNames || [], + activeTypes, + links, + nodes, + vmControllerRevisions + ) // Create data volume child nodes (for KubeVirt) createDataVolumeChild(deployableObj, parentClusterNames || [], activeTypes, links, nodes) diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.test.ts b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.test.ts new file mode 100644 index 00000000000..d94743e34fe --- /dev/null +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.test.ts @@ -0,0 +1,139 @@ +/* Copyright Contributors to the Open Cluster Management project */ + +import { createControllerRevisionChild } from './topologyUtils' +import type { TopologyNode } from '../types' + +const vmNode: TopologyNode = { + name: 'fedora-plum-walrus-98', + namespace: 'feng-vm', + type: 'virtualmachine', + id: 'member--member--deployable--member--clusters----virtualmachine--feng-vm--fedora-plum-walrus-98', + uid: 'member--member--deployable--member--clusters----virtualmachine--feng-vm--fedora-plum-walrus-98', + specs: { + isDesign: false, + raw: { + metadata: { + name: 'fedora-plum-walrus-98', + namespace: 'feng-vm', + }, + group: 'kubevirt.io', + health: { + message: 'Running', + status: 'Healthy', + }, + kind: 'VirtualMachine', + name: 'fedora-plum-walrus-98', + namespace: 'feng-vm', + status: 'OutOfSync', + version: 'v1', + cluster: 'local-cluster', + apiVersion: 'kubevirt.io/v1', + }, + clustersNames: ['local-cluster'], + parent: { + clusterId: 'member--clusters--', + }, + resourceCount: 1, + }, +} + +const expectedGenericChild: TopologyNode = { + id: 'member--member--deployable--member--clusters----virtualmachine--feng-vm--fedora-plum-walrus-98--controllerrevision--fedora-plum-walrus-98', + name: 'fedora-plum-walrus-98', + namespace: 'feng-vm', + specs: { + clustersNames: ['local-cluster'], + isDesign: false, + parent: { + parentId: 'member--member--deployable--member--clusters----virtualmachine--feng-vm--fedora-plum-walrus-98', + parentName: 'fedora-plum-walrus-98', + parentSpecs: undefined, + parentType: 'virtualmachine', + resources: undefined, + }, + replicaCount: 1, + resourceCount: 1, + resources: undefined, + }, + type: 'controllerrevision', + uid: 'member--member--deployable--member--clusters----virtualmachine--feng-vm--fedora-plum-walrus-98--controllerrevision--fedora-plum-walrus-98', +} + +describe('createControllerRevisionChild', () => { + it('creates a generic controllerrevision child for a VirtualMachine without vmControllerRevisions', () => { + expect(createControllerRevisionChild(vmNode, ['local-cluster'], undefined, [], [])).toEqual(expectedGenericChild) + }) + + it('creates named controllerrevision children from vmControllerRevisions map', () => { + const nodeWithUid: TopologyNode = { + ...vmNode, + specs: { + ...vmNode.specs, + raw: { + ...(vmNode.specs as any).raw, + _uid: 'local-cluster/aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', + }, + }, + } + + const vmControllerRevisions = new Map([ + ['aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', ['revision-start-vm-rev-1', 'revision-start-vm-rev-2']], + ]) + + const links: any[] = [] + const nodes: any[] = [] + const result = createControllerRevisionChild( + nodeWithUid, + ['local-cluster'], + undefined, + links, + nodes, + vmControllerRevisions + ) + + expect(nodes).toHaveLength(2) + expect(nodes[0].name).toBe('revision-start-vm-rev-1') + expect(nodes[0].type).toBe('controllerrevision') + expect(nodes[1].name).toBe('revision-start-vm-rev-2') + expect(nodes[1].type).toBe('controllerrevision') + + expect(links).toHaveLength(2) + expect(links[0].from.uid).toBe(nodeWithUid.id) + expect(links[1].from.uid).toBe(nodeWithUid.id) + + expect(result).toEqual(nodes[1]) + }) + + it('falls back to generic controllerrevision when vmControllerRevisions has no match', () => { + const nodeWithUid: TopologyNode = { + ...vmNode, + specs: { + ...vmNode.specs, + raw: { + ...(vmNode.specs as any).raw, + _uid: 'local-cluster/11111111-2222-3333-4444-555555555555', + }, + }, + } + + const vmControllerRevisions = new Map([ + ['aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee', ['some-revision']], + ]) + + const links: any[] = [] + const nodes: any[] = [] + const result = createControllerRevisionChild( + nodeWithUid, + ['local-cluster'], + undefined, + links, + nodes, + vmControllerRevisions + ) + + expect(nodes).toHaveLength(1) + expect(nodes[0].name).toBe('fedora-plum-walrus-98') + expect(nodes[0].type).toBe('controllerrevision') + expect(result).toEqual(nodes[0]) + }) +}) diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.ts b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.ts index 838da8a54a7..d406bc1c08c 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.ts +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyUtils.ts @@ -378,18 +378,39 @@ export const createControllerRevisionChild = ( clustersNames: string[], activeTypes: string[] | undefined, links: TopologyLink[], - nodes: TopologyNode[] + nodes: TopologyNode[], + vmControllerRevisions?: Map ): TopologyNode | undefined => { const parentType = parentNode?.type || '' - if (parentType === 'daemonset' || parentType === 'statefulset' || parentType === 'virtualmachine') { + + if (parentType === 'daemonset' || parentType === 'statefulset') { const pNode = createChildNode(parentNode, clustersNames, 'controllerrevision', activeTypes, links, nodes) + return createChildNode(pNode, clustersNames, 'pod', activeTypes, links, nodes) + } - // Create pod children for non-virtual machine types - if (parentType !== 'virtualmachine') { - return createChildNode(pNode, clustersNames, 'pod', activeTypes, links, nodes) + if (parentType === 'virtualmachine') { + if (vmControllerRevisions) { + const rawUid = (parentNode.specs as any)?.raw?._uid as string | undefined + const k8sUid = rawUid?.includes('/') ? rawUid.split('/').pop() : rawUid + const revisionNames = k8sUid ? vmControllerRevisions.get(k8sUid) : undefined + if (revisionNames && revisionNames.length > 0) { + let lastNode: TopologyNode | undefined + for (const revName of revisionNames) { + lastNode = createChildNode( + { ...parentNode, name: revName }, + clustersNames, + 'controllerrevision', + activeTypes, + links, + nodes + ) + } + return lastNode + } } - return pNode + return createChildNode(parentNode, clustersNames, 'controllerrevision', activeTypes, links, nodes) } + return undefined } @@ -549,17 +570,30 @@ export function areSourcesUniform( * Mirrors the logic in getArgoDestinationCluster (topologyArgo.ts / backend utils.ts): * - destination.name → direct match against cluster names (handles "in-cluster" → hub) * - destination.server → resolves via kubernetes.default.svc, cluster-proxy URL, or raw kube API URL + * + * For pull-model ApplicationSets, hub placeholder Application CRs have + * destination.server set to "https://kubernetes.default.svc" which represents the + * spoke cluster (not the hub). When normal resolution fails, the function falls back + * to extracting the cluster name from the application name. + * + * @param appName - Optional application name used as fallback for cluster resolution. + * ApplicationSet-generated names typically embed the target cluster name. */ export function getAppTargetCluster( destination: { name?: string; server?: string }, - clusters: AppSetClusterInfo[] + clusters: AppSetClusterInfo[], + appName?: string ): string | null { if (destination.name) { const name = destination.name === 'in-cluster' ? 'local-cluster' : destination.name return findClusterByName(clusters, name) } if (destination.server) { - return resolveClusterFromServer(destination.server, clusters) + const resolved = resolveClusterFromServer(destination.server, clusters) + if (resolved) return resolved + } + if (appName && clusters.length > 0) { + return findClusterInAppName(appName, clusters) } return null } @@ -569,6 +603,14 @@ function findClusterByName(clusters: AppSetClusterInfo[], name: string): string return match ? match.name : null } +function findClusterInAppName(appName: string, clusters: AppSetClusterInfo[]): string | null { + const sorted = [...clusters].sort((a, b) => b.name.length - a.name.length) + const match = sorted.find( + (c) => appName === c.name || appName.includes(`-${c.name}`) || appName.includes(`${c.name}-`) + ) + return match ? match.name : null +} + const PROXY_URL_PATTERN = /\.svc\.cluster\.local:\d+\/(.+)$/ function resolveClusterFromServer(server: string, clusters: AppSetClusterInfo[]): string | null { From c8273cd7ebd32d954cd02c18a810111ee1d1704b Mon Sep 17 00:00:00 2001 From: fxiang1 Date: Fri, 15 May 2026 13:44:01 -0400 Subject: [PATCH 2/2] Update comment Signed-off-by: fxiang1 --- .../ApplicationTopology/model/topologyAppSet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts index e68b1f7c0c4..2716c162a49 100644 --- a/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts +++ b/frontend/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts @@ -647,7 +647,7 @@ function processResources( // ControllerRevision resources owned by a VirtualMachine are already // represented as child nodes created by createControllerRevisionChild — // skip them to avoid duplicates. Detected via the - // "revision-start-vm---" naming convention. + // "revision-start-vm--" naming convention. if (type === 'controllerrevision') { const uidMatch = deployableName.match(/.+-vm-([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})-\d+$/) if (uidMatch) {