Skip to content

Fix various bugs with displaying VM resources on the topology #6217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fxiang1 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix various bugs with displaying VM resources on the topology #6217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5eafffc
Fix various bugs with displaying VM resources on the topology
fxiang1 May 15, 2026
c8273cd
Update comment
fxiang1 May 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...outes/Applications/ApplicationDetails/ApplicationTopology/model/resourceStatusesAppSet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
277 changes: 277 additions & 0 deletions ...c/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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)
})
})
49 changes: 48 additions & 1 deletion ...nd/src/routes/Applications/ApplicationDetails/ApplicationTopology/model/topologyAppSet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string[]>()

// pre-emptively find controller revisions that are owned by a VirtualMachine
deduplicatedResources.forEach((deployable: Record<string, unknown>) => {
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-<vmUid>-<rev>" 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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}
})

// create nodes for each resource
deduplicatedResources.forEach((deployable: Record<string, unknown>) => {
const typedDeployable = deployable as unknown as ProcessedDeployableResource
Expand All @@ -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 ??
Expand Down Expand Up @@ -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)
Expand Down
Loading