Conversation
…, Management, Data networks) Co-authored-by: rebelinux <1002783+rebelinux@users.noreply.github.com>
Co-authored-by: rebelinux <1002783+rebelinux@users.noreply.github.com>
…able and adjusting label positions for management and data network subgraphs
Co-authored-by: rebelinux <1002783+rebelinux@users.noreply.github.com>
…, adjusting label positions, and increasing edge thickness for better visibility
There was a problem hiding this comment.
Pull request overview
This PR enhances the ONTAP diagram generation by migrating edge rendering to the Add-DiaNodeEdge wrapper (Diagrammer.Core 0.2.39+) and expanding the cluster diagram’s “Data Network” section to include per-broadcast-domain topology details.
Changes:
- Replaced multiple raw PSGraph
Edgecalls withAdd-DiaNodeEdgeacross several diagram scripts. - Enhanced cluster diagram to render each non-Cluster broadcast domain as its own subgraph with member ports.
- Bumped the required
Diagrammer.Coreminimum version to0.2.39.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Src/Private/Get-AbrOntapClusterDiagram.ps1 | Adds infrastructure switches + per-broadcast-domain Data Network detail; migrates HA edges to Add-DiaNodeEdge. |
| Src/Private/Get-AbrOntapNodeNetworkDiagram.ps1 | Migrates cluster/port/HA edges to Add-DiaNodeEdge. |
| Src/Private/Get-AbrOntapVserverDiagram.ps1 | Migrates some edges to Add-DiaNodeEdge (Volumes/LIFs). |
| Src/Private/Get-AbrOntapClusterReplicationDiagram.ps1 | Migrates replication edges to Add-DiaNodeEdge. |
| AsBuiltReport.NetApp.ONTAP.psd1 | Updates required module version for Diagrammer.Core to support the new edge wrapper. |
Comments suppressed due to low confidence (3)
Src/Private/Get-AbrOntapClusterDiagram.ps1:76
Get-NcNetInterface -Controller $Arrayis called twice per node (for node_mgmt and intercluster), which can be expensive on large clusters and increases API load. Fetch interfaces once outside the node loop (e.g., cache the result in a variable) and then filter that cached collection per node/role.
$ClusterHa = try { Get-NcClusterHa -Node $Node.Node -Controller $Array } catch { Write-PScriboMessage -IsWarning $_.Exception.Message }
$NodeMgmtAddress = Get-NcNetInterface -Controller $Array | Where-Object { $_.Role -eq 'node_mgmt' -and $_.HomeNode -eq $Node.Node } | Select-Object -ExpandProperty Address
$NodeInterClusterAddress = Get-NcNetInterface -Controller $Array | Where-Object { $_.Role -eq 'intercluster' -and $_.HomeNode -eq $Node.Node } | Select-Object -ExpandProperty Address
Src/Private/Get-AbrOntapClusterDiagram.ps1:184
- The PR description says each broadcast-domain label should include IPSpace (e.g.,
BroadcastDomain | IPSpace: ... | MTU: ...), but the label currently only includes MTU. Either update the label to include$BDomain.Ipspaceor adjust the PR description so they match.
-TableBorder 1 `
-Label "$($BDomain.BroadcastDomain) | MTU: $($BDomain.Mtu)" `
-LabelPos 'top' `
Src/Private/Get-AbrOntapVserverDiagram.ps1:201
- Edge-cmdlet migration looks incomplete in this file: there is still a raw
Edgecall in the Aggregates section (Edge -To "$($VserverNodeName)Aggrs" -From $VserverNodeName ...). If the intent is to replace all PSGraphEdgeusages withAdd-DiaNodeEdge(as stated in the PR description), migrate the remaining call(s) as well to avoid mixed APIs and keep the Diagrammer.Core minimum-version bump justified.
if ($VolSubGraphObj) {
Node "$($VserverNodeName)Vols" @{Label = $VolSubGraphObj; shape = 'plain'; fillColor = 'transparent'; fontsize = 14 }
Add-DiaNodeEdge -From $VserverNodeName -To "$($VserverNodeName)Vols" -EdgeColor $Edgecolor -Arrowhead 'box' -Arrowtail 'box' -EdgeLength 2 -GraphvizAttributes @{style = 'filled'}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Add-DiaHtmlSubGraph -Name "$($Port.NodeName)ClusterPorts" -TableArray $ClusterPortObj -ImagesObj $Images -IconDebug $IconDebug -TableBorder 1 -IconType 'Ontap_Network_Port' -Label 'Cluster Network Ports' -LabelPos top -TableStyle 'rounded,dashed' -TableBorderColor '#71797E' -FontName 'Segoe Ui Bold' -ColumnSize $ClusterPortObjColumnSize -NodeObject | ||
|
|
||
| Edge -From 'ClusterNetwork' -To "$($Port.NodeName)ClusterPorts" @{color = $Edgecolor; fontcolor = $Fontcolor; fontsize = 12; style = 'dashed'; penwidth = 1; arrowhead = 'box'; arrowtail = 'box' } | ||
| Add-DiaNodeEdge -From 'ClusterNetwork' -To "$($Port.NodeName)ClusterPorts" -EdgeColor $Edgecolor -EdgeStyle 'dashed' -EdgeThickness 1 -Arrowhead 'box' -Arrowtail 'box' -EdgeLabelFontColor $Fontcolor -EdgeLabelFontSize 12 | ||
|
|
||
| Edge -From "$($Port.NodeName)ClusterPorts" -To $Node.NodeName @{minlen = 1; color = $Edgecolor; fontcolor = $Fontcolor; fontsize = 12; style = 'dashed'; penwidth = 1; arrowhead = 'box'; arrowtail = 'box' } | ||
| Add-DiaNodeEdge -From "$($Port.NodeName)ClusterPorts" -To $Node.NodeName -EdgeColor $Edgecolor -EdgeStyle 'dashed' -EdgeThickness 1 -Arrowhead 'box' -Arrowtail 'box' -EdgeLabelFontColor $Fontcolor -EdgeLabelFontSize 12 -EdgeLength 1 |
There was a problem hiding this comment.
$Port.NodeName is referenced when naming/connecting the per-node ClusterPorts subgraph and edges, but $Port comes from the prior foreach ($Port in ...) loop. If a node has zero cluster ports, $Port will be $null or a stale value from a previous node, causing incorrect node names/edges (or runtime errors). Use $Node.NodeName (or an explicitly constructed per-node name variable) instead of relying on $Port outside its loop, and only create the subgraph/edges when there are cluster ports for that node.
Two enhancements to the ONTAP cluster diagram:
Edge cmdlet migration
Replaces all raw
Edge -From … @{…}PSGraph calls across four diagram files with theAdd-DiaNodeEdgewrapper introduced in Diagrammer.Core 0.2.39. Module minimum version bumped accordingly.color/style/penwidth(int)-EdgeColor/-EdgeStyle/-EdgeThicknesspenwidth(float) /style='filled'-GraphvizAttributes @{penwidth=1.5}/@{style='filled'}label/fontcolor/fontsize-EdgeLabel/-EdgeLabelFontColor/-EdgeLabelFontSizeminlen/tailport/headport-EdgeLength/-TailPort/-HeadPortFloat
penwidthandstyle='filled'pass through-GraphvizAttributessince they fall outsideEdgeThickness [int]and theEdgeStyleValidateSet respectively.Files changed:
Get-AbrOntapClusterDiagram.ps1,Get-AbrOntapNodeNetworkDiagram.ps1,Get-AbrOntapVserverDiagram.ps1,Get-AbrOntapClusterReplicationDiagram.ps1Per-broadcast-domain Data Network detail
The cluster diagram previously showed a single generic
Data Networkswitch with no topology context. Now, each non-Cluster broadcast domain is rendered as its own subgraph node below the switch, listing its port members.Domains with no ports render a "No Ports Assigned" placeholder. The section is isolated in its own
try/catchso failures don't affect the rest of the diagram.Original prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.