Skip to content

Commit 518d618

Browse files
authored
fix(topology): stop duplicated hypervisor CT chips (#1379)
- Slice aliasing in the #1374 absorption rewrote the shared backing array; the last distnode (dist-pve-default-citadel-02) reached the overview twice and the frontend drew that host's CT grid twice. Absorption now builds a fresh slice and exposes TopoSemantics.AbsorbedDists; live.go filters with a fresh slice. - PVE inventory keys resolving to the same host can no longer emit the distnode twice. - Hypervisor guests never pin by their own saved position; they always follow the host grid. Closes #1378
1 parent 8547b32 commit 518d618

7 files changed

Lines changed: 81 additions & 4 deletions

File tree

‎app/src/components/topology/model.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,9 +1225,12 @@ export function buildTopologyModel({ routers, devices, wan, wireguard, distribut
12251225
// posición real del chip tras el override.
12261226
if (seedPositions) {
12271227
for (const c of chips) {
1228-
// Los CTs de un host hipervisores con semilla NO se pinnean: siguen la
1229-
// posición del host (el grid ya se re-derivó alrededor de él arriba).
1230-
if (c.isCt && hypervisorHosts.has(c.hubId) && seedPositions[c.hubId]) continue
1228+
// Los CTs de un host hipervisor NO se pinnean nunca por su propia
1229+
// semilla: siguen la posición del host (el grid se deriva de él).
1230+
// Con la condición antigua (solo si el host tenía semilla), un CT con
1231+
// posición guardada y un host sin fijar se pintaba dos veces: el grid
1232+
// bajo el host y el pin de su semilla vieja (#1378).
1233+
if (c.isCt && hypervisorHosts.has(c.hubId)) continue
12311234
const p = seedPositions[c.id]
12321235
if (p) {
12331236
c.x = p.x

‎server-go/internal/adapters/live.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3425,6 +3425,22 @@ func (l *Live) buildOverview(ctx context.Context) (*Overview, error) {
34253425
usteerAvailable := l.usteerAvailableCached()
34263426
dawnDetected := dawnDeprecatedFromPolled(polled)
34273427
topo := BuildTopoSemantics(routerList, devices, wgStats, distNodes, wan.Gateway, fdbEvidence, lldpEvidence, overrideUplinks) // SPEC-65 D65-3 + #1042/#1051/#1279/#1321
3428+
// #1374: retirar de la lista que ve el frontend los distnodes absorbidos
3429+
// como sombra de un switch gestionado. Slice NUEVO: filtrar in situ
3430+
// reescribe el backing array compartido y duplica el último (#1378).
3431+
if len(topo.AbsorbedDists) > 0 {
3432+
absorbed := map[string]bool{}
3433+
for _, id := range topo.AbsorbedDists {
3434+
absorbed[id] = true
3435+
}
3436+
filtered := make([]DistributionNode, 0, len(distNodes))
3437+
for _, dn := range distNodes {
3438+
if !absorbed[dn.ID] {
3439+
filtered = append(filtered, dn)
3440+
}
3441+
}
3442+
distNodes = filtered
3443+
}
34283444
fillUplinkSpeeds(polled, distNodes, topo.Links)
34293445
return &Overview{
34303446
Health: computeHealth(routerList, adguard),

‎server-go/internal/adapters/proxmox.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,10 @@ func applyPVEInfra(devices []Device, dists []DistributionNode, inv *pveInventory
451451
HostDeviceID: id, Name: n.Node, MacCount: ctCountByHost[id],
452452
Source: "proxmox", Instance: n.Instance,
453453
})
454+
// #1378: dos claves de inventario pueden resolver al mismo host (IP
455+
// repetida o IP+nombre); sin marcarlo aquí el distnode salía duplicado
456+
// y el frontend dibujaba los CTs dos veces.
457+
existingHost[id] = true
454458
}
455459
// #1053: retirar los nodos inferred absorbidos por un host PVE.
456460
kept := dists[:0]

‎server-go/internal/adapters/proxmox_test.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,3 +627,36 @@ func TestPVENoAbsorbeInferredConDesconocidos(t *testing.T) {
627627
t.Fatalf("host AttachTo=%q (want sin cambios)", devices[0].AttachTo)
628628
}
629629
}
630+
631+
// #1378: dos claves de inventario que resuelven al mismo host (p. ej. dos
632+
// entradas nodeIPs con la misma IP, o IP + nombre) no deben emitir el
633+
// distnode hypervisor dos veces: el frontend dibujaba los CTs duplicados.
634+
func TestPVEHypervisorDistNodesNoDuplicaHostRepetido(t *testing.T) {
635+
inv := &pveInventory{
636+
ctByMAC: map[string]pveVM{
637+
"BC:24:11:A4:9E:BB": {Name: "webs", Node: "citadel-02", Type: "lxc", Instance: "default"},
638+
},
639+
nodes: map[string]pveNode{
640+
"default|citadel-02": {Instance: "default", Node: "citadel-02"},
641+
"default|pve2": {Instance: "default", Node: "pve2"},
642+
},
643+
nodeIPs: map[string][]string{
644+
"default|citadel-02": {"192.168.1.101"},
645+
"default|pve2": {"192.168.1.101"}, // misma IP: mismo host físico
646+
},
647+
}
648+
devices := []Device{
649+
{ID: "e8-ff-1e-dd-c7-ed", MAC: "E8:FF:1E:DD:C7:ED", Name: "beelink-nas", IP: "192.168.1.101", RouterID: "switch16", Port: "lan8"},
650+
{ID: "bc-24-11-a4-9e-bb", MAC: "BC:24:11:A4:9E:BB", Name: "webs"},
651+
}
652+
dists := applyPVEInfra(devices, nil, inv)
653+
count := 0
654+
for _, dn := range dists {
655+
if dn.Kind == "hypervisor" && dn.HostDeviceID == "e8-ff-1e-dd-c7-ed" {
656+
count++
657+
}
658+
}
659+
if count != 1 {
660+
t.Fatalf("distnodes hypervisor para el mismo host: %d (want 1): %+v", count, dists)
661+
}
662+
}

‎server-go/internal/adapters/topo_semantics.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,10 @@ func BuildTopoSemantics(routers []Router, devices []Device, wg WireGuardStats, d
466466
}
467467
if len(switchUnit) > 0 {
468468
absorbed := map[string]string{} // distnode -> switch que lo absorbe
469-
kept := dists[:0]
469+
// kept es un slice NUEVO: filtrar in situ (dists[:0]) reescribe el
470+
// backing array del caller, que conserva su len original y acaba con
471+
// el último elemento duplicado (#1378).
472+
kept := make([]DistributionNode, 0, len(dists))
470473
for _, dn := range dists {
471474
hub := ""
472475
if dn.Kind == "inferred" {
@@ -485,6 +488,11 @@ func BuildTopoSemantics(routers []Router, devices []Device, wg WireGuardStats, d
485488
}
486489
if len(absorbed) > 0 {
487490
dists = kept
491+
sem.AbsorbedDists = make([]string, 0, len(absorbed))
492+
for id := range absorbed {
493+
sem.AbsorbedDists = append(sem.AbsorbedDists, id)
494+
}
495+
sort.Strings(sem.AbsorbedDists)
488496
for i := range online {
489497
// Re-anclar al switch SOLO los clientes atribuidos a él: los
490498
// atribuidos a otro router (p. ej. equipos de un switch NO

‎server-go/internal/adapters/topo_semantics_test.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -762,4 +762,12 @@ func TestTopoSemanticsAbsorbeDistnodeSombra(t *testing.T) {
762762
if fromOf["pc"] != "gw" {
763763
t.Fatalf("pc cuelga de %q, want gw (cliente del switch tonto, no del gestionado)", fromOf["pc"])
764764
}
765+
// #1378: la absorción no debe corromper el slice del caller (aliasing):
766+
// dists conserva su len y contenido; los absorbidos se exponen aparte.
767+
if len(dists) != 1 || dists[0].ID != "dist-gw-lan1" {
768+
t.Fatalf("dists del caller corrupto: %+v", dists)
769+
}
770+
if len(sem.AbsorbedDists) != 1 || sem.AbsorbedDists[0] != "dist-gw-lan1" {
771+
t.Fatalf("AbsorbedDists: %+v", sem.AbsorbedDists)
772+
}
765773
}

‎server-go/internal/adapters/types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,11 @@ type TopoSemantics struct {
414414
// El mapa lo dibuja bajo el nodo Internet en vez de como cliente LAN.
415415
// Vacío si no hay coincidencia.
416416
WanPeer string `json:"wanPeer,omitempty"`
417+
// AbsorbedDists (#1374): ids de distnodes inferred absorbidos como sombra
418+
// de un switch gestionado. El caller filtra su lista con un slice NUEVO:
419+
// reescribir el backing array compartido duplicaba el último elemento
420+
// (#1378).
421+
AbsorbedDists []string `json:"absorbedDists,omitempty"`
417422
}
418423

419424
// TopoLink es un enlace semántico del mapa (sin geometría).

0 commit comments

Comments
 (0)