Layered default layout + preserve manually-repositioned nodes across regeneration #53

Closed
opened 2026-07-10 10:25:14 +00:00 by Damien · 1 comment
Owner

Follow-up to #52. Two fixes for the weathermap node layout, reported after visual review: the auto-generated layout doesn't follow standard network diagram conventions (spine on top, leaf on bottom, etc.), and any manual repositioning done in the Grafana UI gets destroyed on the next script regeneration.

1. Layered default layout, derived from hostname convention

Same standardization principle already used for the site label (#49): hostname encodes device role (*-spine*, *-leaf*, *-border-leaf*, *-access*, core*). No new IPFabric field needed — parse role from hostname, same trust level as the site parsing already in place.

Default Y-tiers (top to bottom): spine → core → border-leaf → leaf → access. X position: grouped/columned by site within each tier. This replaces whatever grid/cluster heuristic is currently used for brand-new nodes only (see point 2 — existing nodes keep their live position regardless of this change).

2. Preserve manually-repositioned nodes across regeneration

Root cause: the script fully regenerates node.position for every node on every run (declarative pattern from #48/#52), which stomps any manual drag-and-drop repositioning done directly in the Grafana UI.

Decision: for position specifically, deviate from #52's "file is the only source of truth" rule. Position is edited live in the Grafana UI by design (drag-and-drop, not JSON editing), so it must be read from live Grafana, not the git-committed manual base file. Everything else from #52 (manual panels sourced from the file, weathermap content otherwise fully regenerated) is unchanged.

New behavior:

  1. Before generating new node positions, fetch the currently provisioned dashboard's weathermap panel via the Grafana API
  2. Build a map of node.id → position from what's live right now
  3. For every node from IPFabric that already existed in that live map, reuse its current position as-is — do not touch it
  4. For nodes that are new (first time seen, not in the live map), compute a position using the new layered layout from point 1
  5. Everything else about the node (statusQuery, statusValueMappings, label) still regenerates fresh as before — only position gets this preserve-if-existing treatment

Tasks

  • Implement hostname-based role parsing (spine/core/border-leaf/leaf/access) for the default layered layout
  • Implement the live-Grafana position fetch + preserve-if-existing logic in the generation step
  • Validate: manually move a node in the Grafana UI, rerun the generator, confirm the moved node keeps its manual position while an untouched node's position is unaffected by the layout change (or confirm it lands sensibly if it was never manually placed)
  • Validate: add/remove a device from the lab topology (or simulate), confirm new nodes get the layered default position and removed nodes don't leave orphaned references
  • Document in this issue: the position source-of-truth exception to #52's file-based rule, and why (UI-edited, not file-edited)
Follow-up to #52. Two fixes for the weathermap node layout, reported after visual review: the auto-generated layout doesn't follow standard network diagram conventions (spine on top, leaf on bottom, etc.), and any manual repositioning done in the Grafana UI gets destroyed on the next script regeneration. ## 1. Layered default layout, derived from hostname convention Same standardization principle already used for the `site` label (#49): hostname encodes device role (`*-spine*`, `*-leaf*`, `*-border-leaf*`, `*-access*`, `core*`). No new IPFabric field needed — parse role from hostname, same trust level as the site parsing already in place. Default Y-tiers (top to bottom): spine → core → border-leaf → leaf → access. X position: grouped/columned by site within each tier. This replaces whatever grid/cluster heuristic is currently used for brand-new nodes only (see point 2 — existing nodes keep their live position regardless of this change). ## 2. Preserve manually-repositioned nodes across regeneration Root cause: the script fully regenerates `node.position` for every node on every run (declarative pattern from #48/#52), which stomps any manual drag-and-drop repositioning done directly in the Grafana UI. Decision: for **position specifically**, deviate from #52's "file is the only source of truth" rule. Position is edited live in the Grafana UI by design (drag-and-drop, not JSON editing), so it must be read from **live Grafana**, not the git-committed manual base file. Everything else from #52 (manual panels sourced from the file, weathermap content otherwise fully regenerated) is unchanged. New behavior: 1. Before generating new node positions, fetch the **currently provisioned** dashboard's weathermap panel via the Grafana API 2. Build a map of `node.id → position` from what's live right now 3. For every node from IPFabric that already existed in that live map, reuse its current position as-is — do not touch it 4. For nodes that are new (first time seen, not in the live map), compute a position using the new layered layout from point 1 5. Everything else about the node (statusQuery, statusValueMappings, label) still regenerates fresh as before — only `position` gets this preserve-if-existing treatment ## Tasks - Implement hostname-based role parsing (spine/core/border-leaf/leaf/access) for the default layered layout - Implement the live-Grafana position fetch + preserve-if-existing logic in the generation step - Validate: manually move a node in the Grafana UI, rerun the generator, confirm the moved node keeps its manual position while an untouched node's position is unaffected by the layout change (or confirm it lands sensibly if it was never manually placed) - Validate: add/remove a device from the lab topology (or simulate), confirm new nodes get the layered default position and removed nodes don't leave orphaned references - Document in this issue: the position source-of-truth exception to #52's file-based rule, and why (UI-edited, not file-edited)
Damien added the telemetry label 2026-07-10 10:25:14 +00:00
Author
Owner

Implemented in fcd7ea2 (branch feat/telemetry).

Role parsing + tiered layout

parse_role(hostname), checked in this order (first match wins, so campus-border-leaf1 matches border-leaf before the more general leaf pattern):

*-spine*        -> spine
core*           -> core
*-border-leaf*  -> border-leaf
*-leaf*         -> leaf
*-access*       -> access

Y-tiers top to bottom: spine(0) → core(300) → border-leaf(600) → leaf(900) → access(1200). X grouped by site within each tier (dc at x-offset 100, core at 1300, campus at 1600, wide enough that the largest group — 8 dc leafs at GRID_COLUMNS=6 — doesn't spill into the next site's band), same grid-wrapping as before within a (role, site) group.

Ran parse_role against all 28 live hostnames — all 28 matched cleanly, no fallback-tier placements needed. The fallback path (log + place below access) is implemented and exercised only in the "no unmatched" sense — i.e. confirmed it doesn't fire spuriously on any real hostname, but there's no live example of an actually-unmatched hostname to show, since the naming convention holds fabric-wide.

Live-position preservation

fetch_live_node_positions() GETs /api/dashboards/uid/<uid>, finds the panel titled "Fabric Weathermap" (the merged title from #52 — same stable identifier), and returns {node_id: position}. 404 → {} (first-ever run / no dashboard yet). Any other HTTP error (e.g. bad token) is raised, not swallowed — silently falling back to defaults on a real auth/config error would risk quietly overwriting every manual position instead of failing loudly.

Confirmed this is the right source vs. #52's file-based rule: position is edited by drag-and-drop directly in the Grafana UI, never in dashboard-base.json — there's no file representation of it to read from. GRAFANA_URL/GRAFANA_TOKEN are no longer provision-only; they're read unconditionally now (optional — skip with a warning if unset, falling back to defaults for everything, rather than hard-requiring Grafana access just to generate a JSON file for inspection).

Merge logic: positions = {host: live_positions.get(host, default) for host, default in default_positions.items()} — iterating the current run's device set (not the live map) means a removed device's stale live-position entry is never looked up again and can't leak into the output; there's structurally no way for it to produce an orphaned position or dangling link reference, since nodes/links are built from live IPFabric data every run regardless.

Validation performed

  • Manual reposition preserved: read the live dashboard, confirmed real positions from a previous provision (e.g. dc-spine1 → [-793, 31], dc-leaf2 → [-1289, 532]) — negative/arbitrary coordinates consistent with actual UI drag-and-drop, not generator output. Reran the generator: both nodes' positions came back byte-identical.
  • New-node default layout: since a real topology change isn't practically simulable in this lab without touching the live IPFabric inventory, simulated the equivalent case directly — stripped dc-leaf1's position from the live-provisioned dashboard via the API (functionally identical to "a node the live map has never seen": no prior position to preserve). Rerun reported 27 node(s) with an existing live position (down from 28), and dc-leaf1 landed at [100, 900] — exactly leaf tier (3 × 300 = 900) × dc site offset (100) × first alphabetically in its (leaf, dc) group (col=0, row=0). All other 27 nodes' positions were unaffected.
  • Idempotency: reran again after provisioning the fix — output byte-identical (all 28 nodes now have live positions, nothing left to default).
  • Removed-device case: not empirically tested (would require actually removing a device from the topology), but structurally impossible to produce an orphan per the reasoning above — nodes/links are rebuilt from live IPFabric data every run, a device absent from that fetch is simply absent from the output entirely.

Provisioned the corrected dashboard to Grafana; dc-leaf1 now has a sensible permanent position instead of the temporarily-stripped state from the test.

Implemented in `fcd7ea2` (branch `feat/telemetry`). ## Role parsing + tiered layout `parse_role(hostname)`, checked in this order (first match wins, so `campus-border-leaf1` matches border-leaf before the more general leaf pattern): ``` *-spine* -> spine core* -> core *-border-leaf* -> border-leaf *-leaf* -> leaf *-access* -> access ``` Y-tiers top to bottom: `spine(0) → core(300) → border-leaf(600) → leaf(900) → access(1200)`. X grouped by site within each tier (`dc` at x-offset 100, `core` at 1300, `campus` at 1600, wide enough that the largest group — 8 dc leafs at `GRID_COLUMNS=6` — doesn't spill into the next site's band), same grid-wrapping as before within a (role, site) group. Ran `parse_role` against all 28 live hostnames — **all 28 matched cleanly**, no fallback-tier placements needed. The fallback path (log + place below `access`) is implemented and exercised only in the "no unmatched" sense — i.e. confirmed it doesn't fire spuriously on any real hostname, but there's no live example of an actually-unmatched hostname to show, since the naming convention holds fabric-wide. ## Live-position preservation `fetch_live_node_positions()` GETs `/api/dashboards/uid/<uid>`, finds the panel titled `"Fabric Weathermap"` (the merged title from #52 — same stable identifier), and returns `{node_id: position}`. 404 → `{}` (first-ever run / no dashboard yet). Any other HTTP error (e.g. bad token) is raised, not swallowed — silently falling back to defaults on a real auth/config error would risk quietly overwriting every manual position instead of failing loudly. Confirmed this is the right source vs. #52's file-based rule: position is edited by drag-and-drop directly in the Grafana UI, never in `dashboard-base.json` — there's no file representation of it to read from. `GRAFANA_URL`/`GRAFANA_TOKEN` are no longer provision-only; they're read unconditionally now (optional — skip with a warning if unset, falling back to defaults for everything, rather than hard-requiring Grafana access just to generate a JSON file for inspection). Merge logic: `positions = {host: live_positions.get(host, default) for host, default in default_positions.items()}` — iterating the *current run's* device set (not the live map) means a removed device's stale live-position entry is never looked up again and can't leak into the output; there's structurally no way for it to produce an orphaned position or dangling link reference, since nodes/links are built from live IPFabric data every run regardless. ## Validation performed - **Manual reposition preserved**: read the live dashboard, confirmed real positions from a previous provision (e.g. `dc-spine1 → [-793, 31]`, `dc-leaf2 → [-1289, 532]`) — negative/arbitrary coordinates consistent with actual UI drag-and-drop, not generator output. Reran the generator: both nodes' positions came back byte-identical. - **New-node default layout**: since a real topology change isn't practically simulable in this lab without touching the live IPFabric inventory, simulated the equivalent case directly — stripped `dc-leaf1`'s `position` from the live-provisioned dashboard via the API (functionally identical to "a node the live map has never seen": no prior position to preserve). Rerun reported `27 node(s) with an existing live position` (down from 28), and `dc-leaf1` landed at `[100, 900]` — exactly `leaf` tier (`3 × 300 = 900`) × `dc` site offset (`100`) × first alphabetically in its (leaf, dc) group (`col=0, row=0`). All other 27 nodes' positions were unaffected. - **Idempotency**: reran again after provisioning the fix — output byte-identical (all 28 nodes now have live positions, nothing left to default). - **Removed-device case**: not empirically tested (would require actually removing a device from the topology), but structurally impossible to produce an orphan per the reasoning above — nodes/links are rebuilt from live IPFabric data every run, a device absent from that fetch is simply absent from the output entirely. Provisioned the corrected dashboard to Grafana; `dc-leaf1` now has a sensible permanent position instead of the temporarily-stripped state from the test.
Damien added reference feat/telemetry 2026-07-10 13:01:13 +00:00
Damien changed reference from feat/telemetry to feat/telemetry 2026-07-10 13:01:58 +00:00
Sign in to join this conversation.