From cff45039a41b6f7667c6fd38cfc94cfb263cf8a6 Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:08:35 +0100 Subject: [PATCH 1/8] netpick: admit DNS names as dialable node addresses A node reachable only over an encrypted overlay may have no address a peer can assume. A Tailscale node's MagicDNS name outlives every literal it holds, and its IPv4 literal is a 100.64/10 CGNAT address that netpick already scores below a LAN address -- correctly, as a ranking. The exclusion was elsewhere: Candidates, RankRemote and IPsFromTXT all gated entries on net.ParseIP, so a node whose only address was a name produced an empty candidate list and Primary answered "". Every consumer had grown its own ad-hoc fallback for that, and they did not agree. Admit names through one shared test (Hostname / dialable) and score them between the public and private classes: a name re-resolves, so it survives the renumbering that strands a literal, but a LAN peer's own literal is still the better answer when both exist. The per-caller fallbacks that existed only to recover a .local name are now a second reading of the same source, so they collapse to the one case they still answer -- the node published no address at all and only its discovery host name is left. CGNAT, IPv6 ULA and virtual-adapter demotion stay rankings. Nothing here promises reachability; a consumer that must connect still confirms by connecting. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- services/lmstudio-proxy/overlay_test.go | 90 +++++++++++++ services/lmstudio-proxy/proxy.go | 17 ++- .../nvpair-cluster-manager/mdns_browser.go | 10 +- services/ollama-proxy/overlay_test.go | 90 +++++++++++++ services/ollama-proxy/proxy.go | 17 ++- services/shared/netpick/netpick.go | 120 +++++++++++++++--- services/shared/netpick/netpick_test.go | 95 +++++++++++++- 7 files changed, 399 insertions(+), 40 deletions(-) create mode 100644 services/lmstudio-proxy/overlay_test.go create mode 100644 services/ollama-proxy/overlay_test.go diff --git a/services/lmstudio-proxy/overlay_test.go b/services/lmstudio-proxy/overlay_test.go new file mode 100644 index 00000000..d1a0cae5 --- /dev/null +++ b/services/lmstudio-proxy/overlay_test.go @@ -0,0 +1,90 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Routing to a node reachable only over an encrypted overlay such as a Tailscale +// tailnet. Such a node is never discovered — a tailnet carries no multicast — so +// it arrives as a manually added address, and that address is a 100.64/10 CGNAT +// literal, an IPv6 ULA, or a MagicDNS name. netpick demotes all three relative to +// a LAN literal; none of them may be excluded, or the node has nowhere to be +// dialed. + +package main + +import ( + "reflect" + "testing" +) + +func TestNodeCandidates_OverlayOnlyNodeIsRoutable(t *testing.T) { + tests := []struct { + name string + node Node + want []string + }{ + { + name: "cgnat only", + node: Node{Addresses: []string{"100.101.102.103"}, Port: 1234}, + want: []string{"100.101.102.103:1234"}, + }, + { + name: "ipv6 ula only", + node: Node{Addresses: []string{"fd7a:115c:a1e0::1701:b2c3"}, Port: 1234}, + want: []string{"[fd7a:115c:a1e0::1701:b2c3]:1234"}, + }, + { + name: "magicdns name only", + node: Node{Addresses: []string{"gpu-box.tail1234.ts.net"}, Port: 1234}, + want: []string{"gpu-box.tail1234.ts.net:1234"}, + }, + { + name: "magicdns name carried as the node's canonical ip= TXT", + node: Node{ + TXT: []string{"ip=gpu-box.tail1234.ts.net"}, + Addresses: []string{"gpu-box.tail1234.ts.net"}, + Port: 1234, + }, + want: []string{"gpu-box.tail1234.ts.net:1234"}, + }, + { + name: "lan literal still preferred when the node has both", + node: Node{Addresses: []string{"100.101.102.103", "192.0.2.10"}, Port: 1234}, + want: []string{"192.0.2.10:1234", "100.101.102.103:1234"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := nodeCandidates(tc.node); !reflect.DeepEqual(got, tc.want) { + t.Errorf("nodeCandidates = %v, want %v", got, tc.want) + } + }) + } +} + +// A manual entry that names this host's own tailnet address must resolve to +// loopback rather than being dialed back around the tunnel to ourselves. The +// local-address set comes from netmon, which enumerates every interface without +// filtering, so an overlay adapter's address is in it exactly like a LAN one — +// this pins that, because a set built from the *publishing* picker instead would +// omit an unproven overlay address and let the entry loop. +func TestIsLocalAddress_CountsThisHostsOverlayAddresses(t *testing.T) { + const ourTailnetIP = "100.64.7.7" + + localAddrsMu.Lock() + prev := localAddrs + localAddrs = map[string]bool{"192.0.2.5": true, ourTailnetIP: true} + localAddrsMu.Unlock() + t.Cleanup(func() { + localAddrsMu.Lock() + localAddrs = prev + localAddrsMu.Unlock() + }) + + if !isLocalAddress(ourTailnetIP) { + t.Fatalf("isLocalAddress(%q) = false, want true", ourTailnetIP) + } + got := nodeCandidates(Node{Addresses: []string{ourTailnetIP}, Port: 1234}) + want := []string{"127.0.0.1:1234"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("nodeCandidates for our own tailnet address = %v, want %v", got, want) + } +} diff --git a/services/lmstudio-proxy/proxy.go b/services/lmstudio-proxy/proxy.go index 6e619e77..42091307 100644 --- a/services/lmstudio-proxy/proxy.go +++ b/services/lmstudio-proxy/proxy.go @@ -1602,15 +1602,14 @@ func nodeCandidates(n Node) []string { port := strconv.Itoa(n.Port) sorted := netpick.Candidates(n.TXT, n.Addresses) if len(sorted) == 0 { - // A non-IP entry (a .local hostname) that netpick cannot parse. - hosts := n.Addresses - if len(hosts) == 0 { - if n.Host == "" { - return nil - } - hosts = []string{n.Host} - } - sorted = append([]string(nil), hosts...) + // The node published no dialable address of its own. Its discovery host + // name is a different source, not a re-reading of the same one — netpick + // admits IP literals and DNS names alike, so an address list holding a + // MagicDNS or .local name has already been ranked above. + if n.Host == "" { + return nil + } + sorted = []string{n.Host} } seen := make(map[string]bool, len(sorted)) diff --git a/services/nvpair-cluster-manager/mdns_browser.go b/services/nvpair-cluster-manager/mdns_browser.go index 2950d719..a81ed178 100644 --- a/services/nvpair-cluster-manager/mdns_browser.go +++ b/services/nvpair-cluster-manager/mdns_browser.go @@ -103,12 +103,18 @@ func (b *Browser) Resolve(idOrUUID string) (hosts []string, port int, ok bool) { } // pickHosts returns the node's dialable addresses in ranked order, falling back -// to whatever it advertised when none of them rank. +// to its discovery host name when it published no address at all. netpick ranks +// IP literals and DNS names alike, so a node reachable only by name is already +// covered by the first branch; the host name is a separate source, not a second +// reading of the same one. func pickHosts(n discovery.Node) []string { if h := netpick.Candidates(n.TXT, n.Addresses); len(h) > 0 { return h } - return n.Addresses + if n.Host != "" { + return []string{n.Host} + } + return nil } // seed primes a known peer into the resolver map without a live event (tests). diff --git a/services/ollama-proxy/overlay_test.go b/services/ollama-proxy/overlay_test.go new file mode 100644 index 00000000..dc4d4ac6 --- /dev/null +++ b/services/ollama-proxy/overlay_test.go @@ -0,0 +1,90 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Routing to a node reachable only over an encrypted overlay such as a Tailscale +// tailnet. Such a node is never discovered — a tailnet carries no multicast — so +// it arrives as a manually added address, and that address is a 100.64/10 CGNAT +// literal, an IPv6 ULA, or a MagicDNS name. netpick demotes all three relative to +// a LAN literal; none of them may be excluded, or the node has nowhere to be +// dialed. + +package main + +import ( + "reflect" + "testing" +) + +func TestNodeCandidates_OverlayOnlyNodeIsRoutable(t *testing.T) { + tests := []struct { + name string + node Node + want []string + }{ + { + name: "cgnat only", + node: Node{Addresses: []string{"100.101.102.103"}, Port: 11434}, + want: []string{"100.101.102.103:11434"}, + }, + { + name: "ipv6 ula only", + node: Node{Addresses: []string{"fd7a:115c:a1e0::1701:b2c3"}, Port: 11434}, + want: []string{"[fd7a:115c:a1e0::1701:b2c3]:11434"}, + }, + { + name: "magicdns name only", + node: Node{Addresses: []string{"gpu-box.tail1234.ts.net"}, Port: 11434}, + want: []string{"gpu-box.tail1234.ts.net:11434"}, + }, + { + name: "magicdns name carried as the node's canonical ip= TXT", + node: Node{ + TXT: []string{"ip=gpu-box.tail1234.ts.net"}, + Addresses: []string{"gpu-box.tail1234.ts.net"}, + Port: 11434, + }, + want: []string{"gpu-box.tail1234.ts.net:11434"}, + }, + { + name: "lan literal still preferred when the node has both", + node: Node{Addresses: []string{"100.101.102.103", "192.0.2.10"}, Port: 11434}, + want: []string{"192.0.2.10:11434", "100.101.102.103:11434"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := nodeCandidates(tc.node); !reflect.DeepEqual(got, tc.want) { + t.Errorf("nodeCandidates = %v, want %v", got, tc.want) + } + }) + } +} + +// A manual entry that names this host's own tailnet address must resolve to +// loopback rather than being dialed back around the tunnel to ourselves. The +// local-address set comes from netmon, which enumerates every interface without +// filtering, so an overlay adapter's address is in it exactly like a LAN one — +// this pins that, because a set built from the *publishing* picker instead would +// omit an unproven overlay address and let the entry loop. +func TestIsLocalAddress_CountsThisHostsOverlayAddresses(t *testing.T) { + const ourTailnetIP = "100.64.7.7" + + localAddrsMu.Lock() + prev := localAddrs + localAddrs = map[string]bool{"192.0.2.5": true, ourTailnetIP: true} + localAddrsMu.Unlock() + t.Cleanup(func() { + localAddrsMu.Lock() + localAddrs = prev + localAddrsMu.Unlock() + }) + + if !isLocalAddress(ourTailnetIP) { + t.Fatalf("isLocalAddress(%q) = false, want true", ourTailnetIP) + } + got := nodeCandidates(Node{Addresses: []string{ourTailnetIP}, Port: 11434}) + want := []string{"127.0.0.1:11434"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("nodeCandidates for our own tailnet address = %v, want %v", got, want) + } +} diff --git a/services/ollama-proxy/proxy.go b/services/ollama-proxy/proxy.go index ad4ac7a2..76b0f134 100644 --- a/services/ollama-proxy/proxy.go +++ b/services/ollama-proxy/proxy.go @@ -1859,15 +1859,14 @@ func nodeCandidates(n Node) []string { port := strconv.Itoa(n.Port) sorted := netpick.Candidates(n.TXT, n.Addresses) if len(sorted) == 0 { - // A non-IP entry (a .local hostname) that netpick cannot parse. - hosts := n.Addresses - if len(hosts) == 0 { - if n.Host == "" { - return nil - } - hosts = []string{n.Host} - } - sorted = append([]string(nil), hosts...) + // The node published no dialable address of its own. Its discovery host + // name is a different source, not a re-reading of the same one — netpick + // admits IP literals and DNS names alike, so an address list holding a + // MagicDNS or .local name has already been ranked above. + if n.Host == "" { + return nil + } + sorted = []string{n.Host} } seen := make(map[string]bool, len(sorted)) diff --git a/services/shared/netpick/netpick.go b/services/shared/netpick/netpick.go index 641a289d..f29fd878 100644 --- a/services/shared/netpick/netpick.go +++ b/services/shared/netpick/netpick.go @@ -2,9 +2,15 @@ // SPDX-License-Identifier: Apache-2.0 // Package netpick is the single source of truth for choosing which of a node's -// IPv4 addresses to publish and which to dial, so every NVPAIR service agrees on -// the same answer instead of reducing a multi-homed host's address set with its -// own ad-hoc rule. +// addresses to publish and which to dial, so every NVPAIR service agrees on the +// same answer instead of reducing a multi-homed host's address set with its own +// ad-hoc rule. +// +// "Address" here means anything a dialer can use: an IP literal or a DNS name. +// The local picker publishes IPv4 literals, because that is what a node can +// observe about itself; the remote side additionally accepts names, because a +// node reachable only over an encrypted overlay may have no address a peer can +// assume — a Tailscale node's MagicDNS name outlives every literal it holds. // // Selection is evidence-driven, not inferred from what a subnet number is // assumed to mean. A private range says nothing about whether the fleet lives on @@ -54,9 +60,83 @@ const ( scoreIPv6Global = 20 scoreIPv6ULA = 30 scorePublic = 40 // routable, but rarely how LAN peers reach each other - scorePrivate = 100 + // scoreHostname rates a DNS name. A name is not an address class at all: it + // carries no prefix to judge, and it re-resolves on every dial, so it + // survives a renumbering that strands every literal in the list. It sits + // above the public and IPv6 classes and below the private blocks, which is + // where a LAN peer's own literal still deserves to win. + scoreHostname = 60 + scorePrivate = 100 +) + +// maxHostnameLen and maxLabelLen are the DNS name limits (RFC 1035 2.3.4). +const ( + maxHostnameLen = 253 + maxLabelLen = 63 ) +// Hostname reports whether addr is a DNS name a dialer can hand to a resolver: +// one or more dot-separated labels of letters, digits, and interior hyphens, +// with an optional trailing root dot. +// +// It exists because an overlay network can give a node a name and no address a +// peer may assume. A Tailscale node's MagicDNS name (host.tailnet.ts.net) is the +// only identifier that stays correct across a re-authentication, and an mDNS peer +// can likewise be reachable only as host.local. Neither parses as an IP, so a +// candidate list that admits addresses alone would report such a node as having +// nowhere to be dialed. +// +// It deliberately rejects a "host:port" string: a colon is not a legal name +// character, and every consumer appends its own service port, so accepting one +// would produce "host:port:port". +func Hostname(addr string) bool { + addr = strings.TrimSpace(addr) + if addr == "" || len(addr) > maxHostnameLen || net.ParseIP(addr) != nil { + return false + } + // One trailing root dot is legal and canonical; anything else empty is not. + addr = strings.TrimSuffix(addr, ".") + if addr == "" { + return false + } + for _, label := range strings.Split(addr, ".") { + if len(label) == 0 || len(label) > maxLabelLen { + return false + } + for i := 0; i < len(label); i++ { + c := label[i] + switch { + case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9': + case c == '-' && i > 0 && i < len(label)-1: + default: + return false + } + } + } + return true +} + +// dialable reports whether addr is something a consumer can hand to a dialer at +// all — an IP literal or a DNS name. It is the single admission test for every +// candidate list here, so "which addresses does this node have" has one answer +// rather than one per caller. +func dialable(addr string) bool { + addr = strings.TrimSpace(addr) + return net.ParseIP(addr) != nil || Hostname(addr) +} + +// scoreAddr rates any dialable entry, name or literal. A name has no address +// class to judge, so it takes scoreHostname; everything else falls to scoreIP. +func scoreAddr(addr string) int { + if ip := net.ParseIP(strings.TrimSpace(addr)); ip != nil { + return scoreIP(ip) + } + if Hostname(addr) { + return scoreHostname + } + return scoreUnparseable +} + // dockerDefaultBridge reports whether ip is in 172.17/16, Docker's default bridge // subnet. It is the one private address that is not a host's own: every Docker // host has 172.17.0.1, so it identifies no machine and must never be published as @@ -105,10 +185,10 @@ func scoreIP(ip net.IP) int { return scoreIPv6Global } -// RankRemote returns the parseable addresses sorted best-first for dialing a -// remote peer. The sort is stable and ties break on the address string, so a -// multi-homed peer's chosen address does not flap between equally-ranked -// candidates from scan to scan. +// RankRemote returns the dialable entries — IP literals and DNS names alike — +// sorted best-first for dialing a remote peer. The sort is stable and ties break +// on the address string, so a multi-homed peer's chosen address does not flap +// between equally-ranked candidates from scan to scan. // // With the RFC1918 blocks scored equally, most of a peer's addresses now tie and // resolve by string order. That is intentional: this ranking is a last resort for @@ -121,8 +201,8 @@ func RankRemote(addrs []string) []string { } ranked := make([]scored, 0, len(addrs)) for _, a := range addrs { - if ip := net.ParseIP(strings.TrimSpace(a)); ip != nil { - ranked = append(ranked, scored{addr: a, score: scoreIP(ip)}) + if dialable(a) { + ranked = append(ranked, scored{addr: a, score: scoreAddr(a)}) } } sort.SliceStable(ranked, func(i, j int) bool { @@ -150,9 +230,9 @@ func IPFromTXT(txt []string) string { } // IPsFromTXT returns the ranked candidate list a node published in "ips=", in -// the node's own order, dropping entries that are not valid IPs. Empty when the -// key is absent — a node that publishes only "ip=" is not distinguishable here -// from one with a single address, and both are handled the same way. +// the node's own order, dropping entries nothing could dial. Empty when the key +// is absent — a node that publishes only "ip=" is not distinguishable here from +// one with a single address, and both are handled the same way. func IPsFromTXT(txt []string) []string { for _, kv := range txt { v, ok := strings.CutPrefix(kv, noderec.KeyIPs+"=") @@ -162,7 +242,7 @@ func IPsFromTXT(txt []string) []string { var out []string for _, part := range strings.Split(v, noderec.IPsSeparator) { part = strings.TrimSpace(part) - if part != "" && net.ParseIP(part) != nil { + if dialable(part) { out = append(out, part) } } @@ -185,16 +265,24 @@ func IPsFromTXT(txt []string) []string { // they are a fallback for a node whose published list is stale or truncated, not // a competing opinion, so they never displace a ranked entry. // +// An entry is kept when a dialer could use it: an IP literal, or a DNS name (see +// Hostname). Admitting names is what lets a node reachable only through an +// encrypted overlay — a Tailscale peer known by its MagicDNS name, or an mDNS +// peer known only as host.local — appear here at all rather than as a node with +// nowhere to be dialed. +// // The result is capped at noderec.MaxAdvertisedIPs. Nothing here comes from a // source this process controls — the TXT keys and the resolved address set both // arrive from an unauthenticated mDNS record — and every entry costs a dialer one -// connect timeout, so the bound has to hold on the reading side too. +// connect timeout, so the bound has to hold on the reading side too. A name is no +// more trusted than a literal from the same record: both are claims a consumer +// confirms by connecting, and a name resolves under the host's own resolver. func Candidates(txt []string, addrs []string) []string { var out []string seen := make(map[string]bool) add := func(a string) { a = strings.TrimSpace(a) - if a == "" || seen[a] || len(out) >= noderec.MaxAdvertisedIPs || net.ParseIP(a) == nil { + if a == "" || seen[a] || len(out) >= noderec.MaxAdvertisedIPs || !dialable(a) { return } seen[a] = true diff --git a/services/shared/netpick/netpick_test.go b/services/shared/netpick/netpick_test.go index 23380385..721682e1 100644 --- a/services/shared/netpick/netpick_test.go +++ b/services/shared/netpick/netpick_test.go @@ -54,7 +54,10 @@ func TestRankRemote_PrivateBeatsPublic(t *testing.T) { } func TestRankRemote_DropsUnparseableAndStableTie(t *testing.T) { - got := RankRemote([]string{"not-an-ip", "192.168.0.9", "192.168.0.3", ""}) + // "not an ip" holds a space and "gpu:11434" a colon; neither is a legal DNS + // name, so neither survives. A bare "not-an-ip" WOULD survive — it is a valid + // single-label name — which is the point of the hostname admission. + got := RankRemote([]string{"not an ip", "gpu:11434", "192.168.0.9", "192.168.0.3", ""}) want := []string{"192.168.0.3", "192.168.0.9"} // equal score -> string order, junk dropped if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { t.Fatalf("RankRemote = %v, want %v", got, want) @@ -70,7 +73,7 @@ func TestPrimary_TXTWins(t *testing.T) { } func TestPrimary_InvalidTXTFallsBackToRanked(t *testing.T) { - if got := Primary([]string{"ip=garbage", "other=x"}, []string{"10.0.0.1", "192.168.1.5"}); got != "10.0.0.1" { + if got := Primary([]string{"ip=not a name", "other=x"}, []string{"10.0.0.1", "192.168.1.5"}); got != "10.0.0.1" { t.Fatalf("Primary fallback = %q, want the top-ranked advertised address", got) } } @@ -79,7 +82,7 @@ func TestPrimary_Empty(t *testing.T) { if got := Primary(nil, nil); got != "" { t.Fatalf("Primary(nil,nil) = %q, want empty", got) } - if got := Primary(nil, []string{"junk"}); got != "" { + if got := Primary(nil, []string{"not a name"}); got != "" { t.Fatalf("Primary with only junk = %q, want empty", got) } } @@ -94,7 +97,7 @@ func TestIPFromTXT(t *testing.T) { } func TestIPsFromTXT(t *testing.T) { - got := IPsFromTXT([]string{"uuid=abc", "ips=10.172.54.70,192.168.240.2, 192.168.240.6 ,junk"}) + got := IPsFromTXT([]string{"uuid=abc", "ips=10.172.54.70,192.168.240.2, 192.168.240.6 ,not a name"}) want := []string{"10.172.54.70", "192.168.240.2", "192.168.240.6"} if len(got) != len(want) { t.Fatalf("IPsFromTXT = %v, want %v (junk dropped, whitespace trimmed)", got, want) @@ -520,3 +523,87 @@ func TestRouteSourceIP_Smoke(t *testing.T) { t.Fatalf("routeSourceIP = %q, want empty or a non-loopback IPv4", got) } } + +// --- Overlay networks: a peer reachable only over an encrypted overlay --- +// +// A Tailscale tailnet carries no multicast, so such a peer is never discovered; +// it is added by address. Its addresses are a 100.64/10 CGNAT literal, an IPv6 +// ULA out of fd7a:115c:a1e0::/48, or a MagicDNS name. Each of those is demoted +// by scoreIP relative to a LAN literal — deliberately, because a LAN peer's own +// literal is the better answer when both exist — but a node whose ONLY address +// is one of them must still be dialable. Demotion, never exclusion. + +func TestCandidates_CGNATOnlyNodeIsDialable(t *testing.T) { + got := Candidates([]string{"ip=100.101.102.103"}, []string{"100.101.102.103"}) + if len(got) != 1 || got[0] != "100.101.102.103" { + t.Fatalf("Candidates for a CGNAT-only node = %v, want [100.101.102.103]", got) + } + if Primary([]string{"ip=100.101.102.103"}, nil) != "100.101.102.103" { + t.Fatal("Primary must answer a CGNAT-only node's single address") + } +} + +func TestCandidates_IPv6ULAOnlyNodeIsDialable(t *testing.T) { + const ula = "fd7a:115c:a1e0::1701:b2c3" // Tailscale's IPv6 ULA prefix + got := Candidates([]string{"ip=" + ula}, []string{ula}) + if len(got) != 1 || got[0] != ula { + t.Fatalf("Candidates for a ULA-only node = %v, want [%s]", got, ula) + } +} + +func TestCandidates_HostnameOnlyNodeIsDialable(t *testing.T) { + const magicDNS = "gpu-box.tail1234.ts.net" + got := Candidates(nil, []string{magicDNS}) + if len(got) != 1 || got[0] != magicDNS { + t.Fatalf("Candidates for a MagicDNS-only node = %v, want [%s]", got, magicDNS) + } + if Primary([]string{"ip=" + magicDNS}, nil) != magicDNS { + t.Fatal("Primary must answer a node whose canonical address is a DNS name") + } +} + +func TestRankRemote_LANLiteralStillOutranksOverlayAddresses(t *testing.T) { + got := RankRemote([]string{"gpu-box.tail1234.ts.net", "100.101.102.103", "192.168.1.10"}) + if len(got) != 3 || got[0] != "192.168.1.10" { + t.Fatalf("RankRemote = %v, want the LAN literal first", got) + } + // A name outranks a CGNAT literal: it re-resolves, so it survives the + // renumbering that strands the literal. + if got[1] != "gpu-box.tail1234.ts.net" || got[2] != "100.101.102.103" { + t.Fatalf("RankRemote = %v, want the name ahead of the CGNAT literal", got) + } +} + +func TestHostname(t *testing.T) { + valid := []string{ + "gpu-box.tail1234.ts.net", + "gpu-box", + "node.local", + "node.local.", // one trailing root dot is canonical + "a1-b2.example.com", + } + for _, h := range valid { + if !Hostname(h) { + t.Errorf("Hostname(%q) = false, want true", h) + } + } + invalid := []string{ + "", + "192.168.1.1", // an IP literal is not a name + "fd7a:115c:a1e0::1", // nor is an IPv6 literal + "gpu-box.tail1234.ts.net:14318", // a colon: ports are appended by the caller + "gpu box", // space + "-lead.example", // leading hyphen in a label + "trail-.example", // trailing hyphen in a label + "a..b", // empty label + "under_score.example", // underscore + ".", // root alone + strings.Repeat("a", 64), // label over 63 bytes + strings.Repeat("a.", 200), // name over 253 bytes + } + for _, h := range invalid { + if Hostname(h) { + t.Errorf("Hostname(%q) = true, want false", h) + } + } +} From ef3259e052b0b43230d74c865da80f2cfa07630e Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:12:39 +0100 Subject: [PATCH 2/8] node-info: report this node's service map on /v1/node-info A node's {service: port} set existed in exactly one place a peer could read it: the ni=/ol=/lm=/em=/ec=/... keys on its mDNS record. Multicast does not cross a routed or overlay network, so a peer on a Tailscale tailnet can learn that a node exists -- someone typed its address -- and nothing about what it runs. It cannot tell a PAIR node from a bare Ollama box, and it has no way to find the node's engine manager or its promoted proxy ports. node-info is the one inter-node surface deliberately kept plain, which makes it the one place such a peer can ask. Carry the set there. The broker owns the set (it assigns and re-assigns those ports) and already holds it as the registration cache it replays to the scanner. It now projects that same cache into a nodeinfo:set-services push, on node-info spawn and on every register/unregister, so the HTTP answer and the mDNS record are one derivation rather than two. The set is always sent whole: a service that stopped is expressed by its key being absent, exactly as an unregister is on the record. Absent from the response entirely means the parent has not pushed yet -- not "this node runs nothing", which a peer would act on. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- .../nvpair-node-info/cluster_identity_test.go | 2 +- services/nvpair-node-info/main.go | 78 +++++++++++- services/nvpair-node-info/services_test.go | 119 ++++++++++++++++++ services/nvpair-node-info/stats_test.go | 3 +- services/nvpair-ui-broker/broker.go | 32 +++++ services/nvpair-ui-broker/discovery.go | 29 +++++ services/nvpair-ui-broker/nodeinfo.go | 11 ++ .../nodeinfo_services_test.go | 106 ++++++++++++++++ services/shared/noderec/noderec.go | 25 ++++ 9 files changed, 399 insertions(+), 6 deletions(-) create mode 100644 services/nvpair-node-info/services_test.go create mode 100644 services/nvpair-ui-broker/nodeinfo_services_test.go diff --git a/services/nvpair-node-info/cluster_identity_test.go b/services/nvpair-node-info/cluster_identity_test.go index 1cbc141c..fb4b8785 100644 --- a/services/nvpair-node-info/cluster_identity_test.go +++ b/services/nvpair-node-info/cluster_identity_test.go @@ -97,7 +97,7 @@ func TestBuildResponseClusterUUIDWireStates(t *testing.T) { raw := func(clusterUUID *string) map[string]any { t.Helper() var out map[string]any - if err := json.Unmarshal(buildResponse(nil, nil, 0, statsSnapshot{}, "host", clusterUUID), &out); err != nil { + if err := json.Unmarshal(buildResponse(nil, nil, 0, statsSnapshot{}, "host", clusterUUID, nil), &out); err != nil { t.Fatalf("decode: %v", err) } return out diff --git a/services/nvpair-node-info/main.go b/services/nvpair-node-info/main.go index 0b1f34c0..b4305221 100644 --- a/services/nvpair-node-info/main.go +++ b/services/nvpair-node-info/main.go @@ -99,6 +99,18 @@ type NodeInfoResponse struct { // evidence would have a peer clear a correct annotation and offer an invite // its target will reject. ClusterUUID *string `json:"clusterUuid,omitempty"` + // Services is this node's {service key: port} set — node-info, both proxies, + // engine manager, engine control, errors, workloads, cluster manager — the + // same set the node-scanner carries on this host's mDNS record. + // + // It is here because that record is the only other place it exists, and + // multicast does not cross a routed or overlay network. A peer that reached + // this node by a typed address (a Tailscale MagicDNS name, say) can read this + // and learn that the node is a PAIR node and where each of its services + // listens, which is everything a discovered peer knows. Absent means the + // parent has not pushed the set yet, or that this is not a PAIR node at all — + // a bare inference host answers no /v1/node-info. + Services map[noderec.ServiceKey]int `json:"services,omitempty"` } // clusterIdentity is the cluster principal this node reports, kept current by @@ -147,6 +159,61 @@ func handleClusterIdentity(msg applog.StdinMessage, identity *clusterIdentity) { slog.Info("cluster identity updated", "clustered", params.ClusterUUID != "") } +// serviceMap is this node's {service key: port} set, kept current by the parent +// broker over stdin (noderec.MethodSetServices). node-info cannot derive it: the +// ports belong to sibling processes the broker owns and re-assigns. Guarded +// because the stdin reader and every HTTP handler touch it. +type serviceMap struct { + mu sync.RWMutex + services map[noderec.ServiceKey]int +} + +// set replaces the whole set. The broker always sends it complete, so a service +// that stopped is expressed by its key being absent — merging would leave a +// departed service advertised forever. +func (s *serviceMap) set(services map[noderec.ServiceKey]int) { + next := make(map[noderec.ServiceKey]int, len(services)) + for k, port := range services { + if k != "" && port > 0 { + next[k] = port + } + } + s.mu.Lock() + defer s.mu.Unlock() + s.services = next +} + +// get returns a copy, or nil when nothing has been pushed. A copy because the +// value is marshaled outside the lock on every request. +func (s *serviceMap) get() map[noderec.ServiceKey]int { + s.mu.RLock() + defer s.mu.RUnlock() + if len(s.services) == 0 { + return nil + } + out := make(map[noderec.ServiceKey]int, len(s.services)) + for k, v := range s.services { + out[k] = v + } + return out +} + +// handleSetServices applies a MethodSetServices notification. Like the cluster +// identity push, a malformed payload is dropped rather than latching a wrong +// set: the broker re-pushes on every change, so the next one corrects us. +func handleSetServices(msg applog.StdinMessage, services *serviceMap) { + if msg.Method != noderec.MethodSetServices { + return + } + var params noderec.ServicesParams + if err := json.Unmarshal(msg.Params, ¶ms); err != nil { + slog.Warn("ignoring malformed service map push", "err", err) + return + } + services.set(params.Services) + slog.Info("service map updated", "services", len(params.Services)) +} + // detectGPUs lives in gpu_windows.go (DXGI), gpu_linux.go (nvidia-smi with a // ghw fallback), gpu_darwin.go (IORegistry), and gpu_other.go (ghw fallback). // detectCPU lives in cpu_detect.go; memory detection is split so macOS can use @@ -169,11 +236,11 @@ func handleClusterIdentity(msg applog.StdinMessage, identity *clusterIdentity) { // cpuStatic is nil when static CPU introspection failed; memTotal is zero when // physical-memory introspection failed. Both conditions omit their respective // top-level object from the JSON entirely. -func buildResponse(gpus []GPUInfo, cpuStatic *CPUInfo, memTotal uint64, snap statsSnapshot, hostUUID string, clusterUUID *string) []byte { - return buildResponseAt(gpus, cpuStatic, memTotal, snap, hostUUID, clusterUUID, time.Now()) +func buildResponse(gpus []GPUInfo, cpuStatic *CPUInfo, memTotal uint64, snap statsSnapshot, hostUUID string, clusterUUID *string, services map[noderec.ServiceKey]int) []byte { + return buildResponseAt(gpus, cpuStatic, memTotal, snap, hostUUID, clusterUUID, services, time.Now()) } -func buildResponseAt(gpus []GPUInfo, cpuStatic *CPUInfo, memTotal uint64, snap statsSnapshot, hostUUID string, clusterUUID *string, now time.Time) []byte { +func buildResponseAt(gpus []GPUInfo, cpuStatic *CPUInfo, memTotal uint64, snap statsSnapshot, hostUUID string, clusterUUID *string, services map[noderec.ServiceKey]int, now time.Time) []byte { outGPUs := mergeGPUInventory(gpus, snap.GPUInventory) for i := range outGPUs { gpu := &outGPUs[i] @@ -195,6 +262,7 @@ func buildResponseAt(gpus []GPUInfo, cpuStatic *CPUInfo, memTotal uint64, snap s MSSince: msSince, HostUUID: hostUUID, ClusterUUID: clusterUUID, + Services: services, } if cpuStatic != nil { cpu := *cpuStatic @@ -392,6 +460,7 @@ func main() { // push the answer is genuinely unknown and the field is omitted. The two // sources are mutually exclusive by construction. identity := &clusterIdentity{} + services := &serviceMap{} clusterPrincipal := func() *string { if !clusterGated { uuid, told := identity.get() @@ -409,7 +478,7 @@ func main() { mux := http.NewServeMux() mux.HandleFunc("/v1/node-info", nodeInfoHandler(mesh, func() []byte { - return buildResponse(gpus, cpu, memTotal, collector.Snapshot(), hostUUID, clusterPrincipal()) + return buildResponse(gpus, cpu, memTotal, collector.Snapshot(), hostUUID, clusterPrincipal(), services.get()) })) // Listener layout (set up below depending on flags). Exactly one of the two @@ -549,6 +618,7 @@ func main() { go applog.StdinRPC(notifier, func(msg applog.StdinMessage) { handleClusterIdentity(msg, identity) + handleSetServices(msg, services) }, func() { log.Print("stdin closed, shutting down") cancel() diff --git a/services/nvpair-node-info/services_test.go b/services/nvpair-node-info/services_test.go new file mode 100644 index 00000000..3e35f068 --- /dev/null +++ b/services/nvpair-node-info/services_test.go @@ -0,0 +1,119 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// The {service: port} set reported on /v1/node-info. It is what lets a peer that +// reached this node by a typed address — one on a Tailscale tailnet, where no +// mDNS record ever arrives — learn that this is a PAIR node and where each of its +// services listens. + +package main + +import ( + "encoding/json" + "testing" + + "nvpair-shared/applog" + "nvpair-shared/noderec" +) + +func servicesPush(t *testing.T, services map[noderec.ServiceKey]int) applog.StdinMessage { + t.Helper() + params, err := json.Marshal(noderec.ServicesParams{Services: services}) + if err != nil { + t.Fatalf("marshal services params: %v", err) + } + return applog.StdinMessage{Method: noderec.MethodSetServices, Params: params} +} + +func TestServiceMap_AbsentUntilPushed(t *testing.T) { + services := &serviceMap{} + if got := services.get(); got != nil { + t.Fatalf("service map before any push = %v, want nil", got) + } + + var out map[string]any + if err := json.Unmarshal(buildResponse(nil, nil, 0, statsSnapshot{}, "host", nil, services.get()), &out); err != nil { + t.Fatalf("decode response: %v", err) + } + if _, present := out["services"]; present { + // A node that has not been told its own ports must not claim it has none: + // a peer would read that as "not a PAIR node" and fall back to probing + // raw engine ports that answer 403. + t.Fatal("services must be absent from the response until the parent pushes the set") + } +} + +func TestServiceMap_ReportedAfterPush(t *testing.T) { + services := &serviceMap{} + handleSetServices(servicesPush(t, map[noderec.ServiceKey]int{ + noderec.ServiceNodeInfo: 14318, + noderec.ServiceOllama: 11434, + noderec.ServiceEngineManager: 14322, + noderec.ServiceEngineControl: 14323, + }), services) + + var typed NodeInfoResponse + if err := json.Unmarshal(buildResponse(nil, nil, 0, statsSnapshot{}, "host", nil, services.get()), &typed); err != nil { + t.Fatalf("decode response: %v", err) + } + if got := typed.Services[noderec.ServiceOllama]; got != 11434 { + t.Fatalf("ol port = %d, want 11434", got) + } + if got := typed.Services[noderec.ServiceEngineControl]; got != 14323 { + t.Fatalf("ec port = %d, want 14323", got) + } + if len(typed.Services) != 4 { + t.Fatalf("services = %v, want 4 entries", typed.Services) + } +} + +// The set is replaced, never merged: a service that stopped is expressed by its +// key being absent, exactly as an unregister is on the discovery record. Merging +// would leave a departed service advertised to peers forever. +func TestServiceMap_PushReplacesRatherThanMerges(t *testing.T) { + services := &serviceMap{} + handleSetServices(servicesPush(t, map[noderec.ServiceKey]int{ + noderec.ServiceOllama: 11434, + noderec.ServiceLMStudio: 1234, + }), services) + handleSetServices(servicesPush(t, map[noderec.ServiceKey]int{ + noderec.ServiceOllama: 11434, + }), services) + + got := services.get() + if _, present := got[noderec.ServiceLMStudio]; present { + t.Fatalf("services = %v, want the departed lm key gone", got) + } +} + +func TestServiceMap_DropsUnusableEntriesAndMalformedPushes(t *testing.T) { + services := &serviceMap{} + handleSetServices(servicesPush(t, map[noderec.ServiceKey]int{ + noderec.ServiceOllama: 11434, + "": 9000, // no key + noderec.ServiceErrors: 0, // no port + }), services) + if got := services.get(); len(got) != 1 || got[noderec.ServiceOllama] != 11434 { + t.Fatalf("services = %v, want only the ol entry", got) + } + + // A malformed payload is dropped rather than latching: the broker re-pushes + // on every change, so the next one corrects us. + handleSetServices(applog.StdinMessage{ + Method: noderec.MethodSetServices, + Params: json.RawMessage(`{"services":"not a map"}`), + }, services) + if got := services.get(); len(got) != 1 { + t.Fatalf("services after a malformed push = %v, want the previous set kept", got) + } +} + +func TestServiceMap_IgnoresOtherMethods(t *testing.T) { + services := &serviceMap{} + msg := servicesPush(t, map[noderec.ServiceKey]int{noderec.ServiceOllama: 11434}) + msg.Method = noderec.MethodSetClusterIdentity + handleSetServices(msg, services) + if got := services.get(); got != nil { + t.Fatalf("services = %v, want nil for an unrelated method", got) + } +} diff --git a/services/nvpair-node-info/stats_test.go b/services/nvpair-node-info/stats_test.go index ad785017..05cf1e80 100644 --- a/services/nvpair-node-info/stats_test.go +++ b/services/nvpair-node-info/stats_test.go @@ -215,6 +215,7 @@ func TestBuildResponseTelemetryFreshness(t *testing.T) { }, "", nil, + nil, now, ) var typed NodeInfoResponse @@ -247,7 +248,7 @@ func TestBuildResponseTelemetryFreshness(t *testing.T) { // with no "cpu" key at all, not `"cpu":null`. func buildResponseDecode(t *testing.T, static []GPUInfo, cpu *CPUInfo, memTotal uint64, snap statsSnapshot) (NodeInfoResponse, map[string]any) { t.Helper() - body := buildResponse(static, cpu, memTotal, snap, "", nil) + body := buildResponse(static, cpu, memTotal, snap, "", nil, nil) var typed NodeInfoResponse if err := json.Unmarshal(body, &typed); err != nil { t.Fatalf("typed decode: %v", err) diff --git a/services/nvpair-ui-broker/broker.go b/services/nvpair-ui-broker/broker.go index 0d189578..6354a128 100644 --- a/services/nvpair-ui-broker/broker.go +++ b/services/nvpair-ui-broker/broker.go @@ -400,6 +400,7 @@ func (b *Broker) registerService(p noderec.RegisterParams) { if sc := b.getScanner(); sc != nil { go sc.pushRegister(p) } + go b.pushServicesToNodeInfo() } // unregisterService removes a local service from the cache and the daemon. @@ -410,6 +411,32 @@ func (b *Broker) unregisterService(svc noderec.ServiceKey) { if sc := b.getScanner(); sc != nil { go sc.pushUnregister(svc) } + go b.pushServicesToNodeInfo() +} + +// localServices projects the registration cache onto the {service: port} map +// node-info reports. One derivation, so what a peer reads over HTTP and what it +// would have read off this host's mDNS record are the same set. +func (b *Broker) localServices() map[noderec.ServiceKey]int { + regs := b.regCache.Snapshot() + services := make(map[noderec.ServiceKey]int, len(regs)) + for _, p := range regs { + services[p.Service] = p.Port + } + return services +} + +// pushServicesToNodeInfo sends node-info this node's current service map. +// Best-effort on a node-info that isn't running (never spawned, or mid-restart): +// the next spawn pushes again. +func (b *Broker) pushServicesToNodeInfo() { + np := b.getNodeInfo() + if np == nil { + return + } + if err := np.SetServices(b.localServices()); err != nil { + slog.Warn("failed to push service map to node-info", "err", err) + } } // get*/set* are the workersMu-guarded accessors for the supervised worker @@ -626,6 +653,11 @@ func (b *Broker) spawnNodeInfo() (supervisedHandle, error) { // /v1/node-info but holds no cluster dir to read it from, so this push is the // only source. It runs on every spawn, which also covers a supervised restart. b.pushClusterIdentityToNodeInfo() + // And the service map, for the same reason: node-info is the only surface a + // peer that never saw this host's mDNS record can ask which services it runs. + // Pushed here as well as from registerService so a restart re-seeds the set + // the workers registered before node-info came back. + b.pushServicesToNodeInfo() // Register node-info's service so the daemon advertises ni= on _nvpair-node. // node-info binds the fixed :14318 (force_ports is inert), so the broker // knows its port. Idempotent across restarts. diff --git a/services/nvpair-ui-broker/discovery.go b/services/nvpair-ui-broker/discovery.go index 8f216533..c0334927 100644 --- a/services/nvpair-ui-broker/discovery.go +++ b/services/nvpair-ui-broker/discovery.go @@ -672,3 +672,32 @@ func writeClusterIdentityFrame(mu *sync.Mutex, w io.Writer, clusterUUID string) _, err = w.Write(data) return err } + +// writeServicesFrame marshals a newline-delimited nodeinfo:set-services +// notification and writes it to a child's stdin under mu. The set is always sent +// whole: a service that stopped is expressed by its key being absent, exactly as +// an unregister is on the discovery record. +func writeServicesFrame(mu *sync.Mutex, w io.Writer, services map[noderec.ServiceKey]int) error { + if services == nil { + services = map[noderec.ServiceKey]int{} + } + frame := struct { + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + Params noderec.ServicesParams `json:"params"` + }{ + JSONRPC: "2.0", + Method: noderec.MethodSetServices, + Params: noderec.ServicesParams{Services: services}, + } + data, err := json.Marshal(frame) + if err != nil { + return err + } + data = append(data, '\n') + + mu.Lock() + defer mu.Unlock() + _, err = w.Write(data) + return err +} diff --git a/services/nvpair-ui-broker/nodeinfo.go b/services/nvpair-ui-broker/nodeinfo.go index af890547..4092ed29 100644 --- a/services/nvpair-ui-broker/nodeinfo.go +++ b/services/nvpair-ui-broker/nodeinfo.go @@ -13,6 +13,8 @@ import ( "os" "os/exec" "sync" + + "nvpair-shared/noderec" ) // maxNodeInfoLine caps one stdout frame from node-info. Its frames are a handful @@ -58,6 +60,15 @@ func (n *nodeInfoProcess) SetClusterIdentity(clusterUUID string) error { return writeClusterIdentityFrame(&n.stdinMu, n.stdin, clusterUUID) } +// SetServices tells node-info this node's whole {service: port} set, so +// /v1/node-info can report it. The set otherwise exists only on this host's mDNS +// record, and multicast does not cross a routed or overlay network — a peer that +// reached this node across a Tailscale tailnet has node-info and nothing else to +// ask. Sent on spawn and on every registration change. +func (n *nodeInfoProcess) SetServices(services map[noderec.ServiceKey]int) error { + return writeServicesFrame(&n.stdinMu, n.stdin, services) +} + // Done implements supervisedHandle: the returned channel closes once the // node-info process has exited (cmd.Wait returned). func (n *nodeInfoProcess) Done() <-chan struct{} { return n.done } diff --git a/services/nvpair-ui-broker/nodeinfo_services_test.go b/services/nvpair-ui-broker/nodeinfo_services_test.go new file mode 100644 index 00000000..4696bf27 --- /dev/null +++ b/services/nvpair-ui-broker/nodeinfo_services_test.go @@ -0,0 +1,106 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "encoding/json" + "strings" + "sync" + "testing" + + "nvpair-shared/noderec" + "nvpair-ui-broker/relay" +) + +// TestWriteServicesFrame pins the wire form of the service-map push node-info +// decodes. It is the only way a peer that never received this host's mDNS record +// — anything across a routed or overlay network — learns which services this node +// runs, so a renamed method or a missing newline silently reduces such a peer to +// probing raw engine ports that answer 403. +func TestWriteServicesFrame(t *testing.T) { + var buf bytes.Buffer + var mu sync.Mutex + services := map[noderec.ServiceKey]int{ + noderec.ServiceNodeInfo: 14318, + noderec.ServiceOllama: 11434, + } + if err := writeServicesFrame(&mu, &buf, services); err != nil { + t.Fatalf("write: %v", err) + } + + line := buf.String() + if !strings.HasSuffix(line, "\n") { + t.Error("frame is not newline-terminated; node-info reads line-delimited frames") + } + if strings.Count(line, "\n") != 1 { + t.Errorf("frame contains %d newlines, want exactly one", strings.Count(line, "\n")) + } + + var frame struct { + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + ID json.RawMessage `json:"id"` + Params noderec.ServicesParams `json:"params"` + } + if err := json.Unmarshal([]byte(line), &frame); err != nil { + t.Fatalf("decode frame %q: %v", line, err) + } + if frame.JSONRPC != "2.0" { + t.Errorf("jsonrpc = %q, want 2.0", frame.JSONRPC) + } + if frame.Method != noderec.MethodSetServices { + t.Errorf("method = %q, want %q", frame.Method, noderec.MethodSetServices) + } + // A notification, not a request: node-info's stdout is drained to io.Discard, + // so an id-bearing frame would strand a reply. + if len(frame.ID) != 0 { + t.Errorf("frame carries an id (%s); the push must be a notification", frame.ID) + } + if frame.Params.Services[noderec.ServiceOllama] != 11434 { + t.Errorf("services = %v, want ol=11434", frame.Params.Services) + } +} + +// An empty set is a real value: it is how "every service went away" is expressed, +// and merging on the far side would leave a departed service advertised forever. +func TestWriteServicesFrame_EmptySetIsStillSent(t *testing.T) { + var buf bytes.Buffer + var mu sync.Mutex + if err := writeServicesFrame(&mu, &buf, nil); err != nil { + t.Fatalf("write: %v", err) + } + if !strings.Contains(buf.String(), `"services":{}`) { + t.Errorf("frame did not carry an empty services object: %s", buf.String()) + } +} + +// The map node-info reports and the ports the scanner advertises come from one +// cache, so a peer reading /v1/node-info and a peer reading the mDNS record agree. +func TestLocalServices_MirrorsTheRegistrationCache(t *testing.T) { + b := &Broker{regCache: relay.NewRegistrationCache()} + b.regCache.Register(noderec.RegisterParams{Service: noderec.ServiceNodeInfo, Port: 14318}) + b.regCache.Register(noderec.RegisterParams{Service: noderec.ServiceOllama, Port: 11434}) + b.regCache.Register(noderec.RegisterParams{Service: noderec.ServiceEngineControl, Port: 14323}) + + got := b.localServices() + want := map[noderec.ServiceKey]int{ + noderec.ServiceNodeInfo: 14318, + noderec.ServiceOllama: 11434, + noderec.ServiceEngineControl: 14323, + } + if len(got) != len(want) { + t.Fatalf("localServices = %v, want %v", got, want) + } + for k, v := range want { + if got[k] != v { + t.Errorf("localServices[%s] = %d, want %d", k, got[k], v) + } + } + + b.regCache.Unregister(noderec.ServiceOllama) + if _, present := b.localServices()[noderec.ServiceOllama]; present { + t.Error("an unregistered service must leave the map a peer reads") + } +} diff --git a/services/shared/noderec/noderec.go b/services/shared/noderec/noderec.go index 112e7fe3..41b12eda 100644 --- a/services/shared/noderec/noderec.go +++ b/services/shared/noderec/noderec.go @@ -326,6 +326,23 @@ const ( // record keeps its last observed value indefinitely. MethodSetClusterIdentity = "nodeinfo:set-cluster-identity" + // MethodSetServices tells nvpair-node-info which services this node runs and + // on which ports — the same {key: port} set the broker registers with + // nvpair-node-scanner — so it can report them on /v1/node-info. + // + // It exists because that set is otherwise published only on this host's mDNS + // record, and multicast does not cross a routed or overlay network. A peer on + // a Tailscale tailnet never sees the record, so without this it can learn + // that a node exists (it was typed in) but not that the node is a PAIR node, + // nor where its engine-manager, proxies or cluster manager listen. node-info + // is the one inter-node surface kept plain, which makes it the one place such + // a peer can ask. + // + // The broker owns the set (it is the process that assigns and re-assigns + // those ports) and re-pushes it on every change, so node-info reports one + // live value rather than deriving a second one. + MethodSetServices = "nodeinfo:set-services" + // NotifyObservedAddresses is nvpair-node-info -> broker: the local addresses // peers have actually reached this node on, learned from its own accepted // connections. @@ -398,6 +415,14 @@ type ClusterIdentityParams struct { ClusterUUID string `json:"clusterUuid"` } +// ServicesParams carries this node's whole {service: port} set for +// MethodSetServices. The set is always sent complete rather than as a delta: a +// service that stopped is expressed by its key being absent, which is the same +// thing an unregister means on the discovery record. +type ServicesParams struct { + Services map[ServiceKey]int `json:"services"` +} + // ObservedAddressesParams carries the local addresses remote peers have reached // this node on, for NotifyObservedAddresses and MethodSetObservedAddresses. The // set is always complete: a receiver replaces what it holds, so an address a peer From 430d9ac68e4d69631dca601a79cdbaf8b7d4ebaf Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:22:50 +0100 Subject: [PATCH 3/8] manual nodes: a PAIR peer added by address behaves like a discovered one Adding a PAIR node by address did not work, and could not have. The prober asked its 11434 and 1234 in plaintext, but on a PAIR node those ports carry the proxy facades, which refuse plaintext from anything but loopback -- so every probe answered 403 and the peer read as having no engines. If it had been bridged anyway, the proxies would have dialed it plain (a manual candidate carries no cluster principal, so HasPin is false and routing falls to the manual arm) into the same refusal. Manual nodes only ever worked for the bare Ollama / LM Studio box they were built for. That box still exists and is unchanged. What is new is the second kind of manual node: a peer that is a PAIR node and is simply on the far side of a network that carries no multicast, such as a Tailscale tailnet. It is not a special case, it is an ordinary peer with no discovery between here and there, so it is made into one. - The prober asks node-info FIRST, and its answer decides everything else. A service map identifies a PAIR node, and such a node is never probed on its engine ports again. - A PAIR node's models come from its engine manager over pinned mTLS, the same fetch the scanner makes for a discovered peer. No pin, no models: before pairing the node still appears with its hardware. - The broker synthesizes the directory record the scanner would have produced and pushes it into the discovery relay, so both proxies, the scheduler, engine-manager's remote operations, the workload relay and the errors peer sync all see a pinned peer. It carries the peer's cluster principal, which is what gets it dialed over mTLS instead of 403'd. - It refuses to synthesize a record for this host itself (by identity, so the overlay name for this machine is caught too) or for a node the scanner already owns, and it withdraws only records it put there. - The raw-engine bridge is withdrawn for a PAIR node rather than left alongside, so there is one route to a node and not two. node/add also gains per-service port overrides, and now rejects a "host:port" address with the reason instead of accepting an entry that can never be reached: service ports are appended to that value. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- services/nvpair-manual-nodes/manager.go | 371 +++++++++++++++--- services/nvpair-manual-nodes/manager_test.go | 4 +- services/nvpair-manual-nodes/pairnode_test.go | 268 +++++++++++++ services/nvpair-ui-broker/broker.go | 19 +- services/nvpair-ui-broker/discovery.go | 13 + .../nvpair-ui-broker/manual_pairnode_test.go | 196 +++++++++ services/nvpair-ui-broker/manualnodes.go | 163 +++++++- 7 files changed, 980 insertions(+), 54 deletions(-) create mode 100644 services/nvpair-manual-nodes/pairnode_test.go create mode 100644 services/nvpair-ui-broker/manual_pairnode_test.go diff --git a/services/nvpair-manual-nodes/manager.go b/services/nvpair-manual-nodes/manager.go index 55a4040a..ecedad7a 100644 --- a/services/nvpair-manual-nodes/manager.go +++ b/services/nvpair-manual-nodes/manager.go @@ -13,12 +13,15 @@ import ( "net" "net/http" "strconv" + "strings" "sync" "time" "nvpair-shared/applog" "nvpair-shared/clustertrust" "nvpair-shared/errors" + "nvpair-shared/netpick" + "nvpair-shared/noderec" ) // Version is stamped at build time via -ldflags "-X main.Version=...". @@ -43,6 +46,18 @@ const ( probeFailThreshold = 3 ) +// Default service ports for a manual node. A manual node is remote, so its ports +// are assumed rather than resolved (the engine manager governs only the local +// engine) — an entry's "ports" object overrides any of them for a host that runs +// something somewhere else. +const ( + defaultOllamaPort = 11434 + defaultLMStudioPort = 1234 + defaultVLLMPort = 8000 + defaultNodeInfoPort = 14318 + defaultClusterPort = 14321 +) + type GPUInfo struct { Name string `json:"name"` VramBytes uint64 `json:"vram_bytes,omitempty"` @@ -78,6 +93,26 @@ type NodeInfoResponse struct { // machine if it's also discovered over mDNS. Empty when the remote // predates this field or isn't a NVPAIR node-info server. HostUUID string `json:"hostUuid,omitempty"` + // ClusterUUID is the remote's cluster principal. Tri-state on the wire and + // therefore a pointer here: absent means the remote does not know its own + // membership, present-and-empty means it belongs to no cluster, and a value is + // that principal. Reading absent as unclustered would have this node clear a + // correct annotation and dial a clustered peer's mTLS surfaces in plaintext. + ClusterUUID *string `json:"clusterUuid,omitempty"` + // Services is the remote's {service key: port} set. Its presence is what + // identifies the remote as a PAIR node: a bare Ollama or LM Studio host serves + // no /v1/node-info at all, and a PAIR node too old to report the set is + // treated as a bare host, which is what it can be reached as. + Services map[noderec.ServiceKey]int `json:"services,omitempty"` +} + +// peerModels is the inventory read from a paired PAIR node's engine-manager. It +// is the same body the node-scanner fetches for a discovered peer, so a manual +// peer's models reach the fleet through the same shape. +type peerModels struct { + Models []string `json:"models"` + ModelsByEngine map[string][]string `json:"modelsByEngine"` + LoadedByEngine map[string][]string `json:"loadedByEngine"` } // ManualEntry is the user-supplied identity of a manually added @@ -93,6 +128,80 @@ type ManualEntry struct { Name string `json:"name"` TLSPort int `json:"tls_port,omitempty"` MTLS bool `json:"mtls,omitempty"` + // Ports overrides the assumed port of any single service on this host. It + // exists because the defaults are assumptions about a machine this process + // cannot introspect: a peer may run its engine on a second port, a whole PAIR + // node may be reachable on a forwarded range, and two nodes may share one + // loopback in a test. A zero or absent field keeps that service's default, so + // an entry overrides only what it means to. + Ports *ManualPorts `json:"ports,omitempty"` +} + +// ManualPorts is the per-service port override set for one manual entry. Field +// names follow the service names manual-nodes already uses rather than the +// compact mDNS TXT keys, because this is what an operator types. +// +// VLLM is carried and persisted but not probed here yet; it is in the set so an +// entry written today keeps its meaning when the vLLM leg lands. +type ManualPorts struct { + NodeInfo int `json:"node_info,omitempty"` + Cluster int `json:"cluster,omitempty"` + Ollama int `json:"ollama,omitempty"` + LMStudio int `json:"lmstudio,omitempty"` + VLLM int `json:"vllm,omitempty"` +} + +// resolved returns this entry's ports with every unset field filled from the +// default for that service, so probe code reads one value per service and never +// repeats the defaulting rule. +func (e ManualEntry) resolved() ManualPorts { + p := ManualPorts{} + if e.Ports != nil { + p = *e.Ports + } + p.NodeInfo = portOr(p.NodeInfo, defaultNodeInfoPort) + p.Cluster = portOr(p.Cluster, defaultClusterPort) + p.Ollama = portOr(p.Ollama, defaultOllamaPort) + p.LMStudio = portOr(p.LMStudio, defaultLMStudioPort) + p.VLLM = portOr(p.VLLM, defaultVLLMPort) + return p +} + +func portOr(override, fallback int) int { + if override > 0 { + return override + } + return fallback +} + +// validateManualAddress rejects what cannot work as a manual address, with the +// reason rather than a silent permanently-down entry. +// +// A "host:port" string is the common mistake: every probe appends its own +// service port to this value, so such an entry dials host:port:port and reads +// down forever. Per-service ports belong in "ports". Bare IPv6 literals are +// accepted in both plain and bracketed form; net.JoinHostPort re-brackets on the +// way out. +func validateManualAddress(addr string) (string, error) { + addr = strings.TrimSpace(addr) + if addr == "" { + return "", fmt.Errorf("address is required") + } + if unbracketed, ok := strings.CutPrefix(addr, "["); ok { + if inner, closed := strings.CutSuffix(unbracketed, "]"); closed { + addr = inner + } + } + if net.ParseIP(addr) != nil { + return addr, nil + } + if host, port, err := net.SplitHostPort(addr); err == nil && host != "" && port != "" { + return "", fmt.Errorf("address %q carries a port; give the host on its own (%q) and put per-service ports in \"ports\" — a service port is appended to this value, so a host:port entry can never be reached", addr, host) + } + if !netpick.Hostname(addr) { + return "", fmt.Errorf("address %q is neither an IP address nor a host name", addr) + } + return addr, nil } // ManualNodeStatus mirrors a manual entry plus the latest probe @@ -128,6 +237,29 @@ type ManualNodeStatus struct { // manual node carries the same permanent identity the rest of the system // keys on. Empty when node-info didn't report one. HostUUID string `json:"hostUuid,omitempty"` + // Ports echoes the entry's port overrides so a caller can render what it + // configured without holding its own copy. + Ports *ManualPorts `json:"ports,omitempty"` + // PairNode is true when node-info answered AND reported a service map — the + // remote is a PAIR node, not a bare inference host. It is the switch between + // the two kinds of manual node: a PAIR node is folded into the directory as a + // peer and reached through its proxies over cluster mTLS, and a bare host is + // bridged into the local proxies by its raw engine ports. The engine fields + // below stay zero for a PAIR node, because its 11434 / 1234 are proxy facades + // that refuse plaintext from anything but loopback. + PairNode bool `json:"pair_node"` + // ClusterUUID is the remote's cluster principal, carried through with its + // three states intact (see NodeInfoResponse.ClusterUUID). A consumer keys the + // peer's certificate pin on it. + ClusterUUID *string `json:"cluster_uuid,omitempty"` + // Services is the remote's {service key: port} set, verbatim from node-info. + Services map[noderec.ServiceKey]int `json:"services,omitempty"` + // Models / ModelsByEngine / LoadedByEngine are a paired PAIR node's inventory, + // read from its engine manager over cluster mTLS. Empty until this node holds + // a pin for the peer: model names are not served to a stranger. + Models []string `json:"models,omitempty"` + ModelsByEngine map[string][]string `json:"models_by_engine,omitempty"` + LoadedByEngine map[string][]string `json:"loaded_by_engine,omitempty"` } type ReadyParams struct { @@ -250,50 +382,34 @@ func (m *Manager) probeAll(ctx context.Context) { func (m *Manager) probeNode(entry ManualEntry) { addr := entry.Address id := nodeID(entry) + ports := entry.resolved() - ollamaUp, ollamaModels := m.probeOllama(addr, 11434) - lmStudioUp, lmStudioModels := m.probeLMStudio(addr, lmStudioPort) - - // Pick scheme + port + client based on the entry's TLS hint. - // The operator decides which scheme this manual node uses; we - // don't probe both. TLSPort > 0 means HTTPS on that port via - // the TLS client (which carries the operator's client cert, - // if configured). Otherwise it's plain HTTP on the historical - // 14318. - scheme := "http" - nodeInfoPort := 14318 - probeClient := m.client - if entry.TLSPort > 0 { - scheme = "https" - nodeInfoPort = entry.TLSPort - probeClient = m.tlsClient - // Clustered: a TLS manual node is a cluster peer whose node-info is - // pin-gated mTLS with no plaintext listener. Dial it with our cluster - // leaf, accepting any currently-pinned server cert (a manual node has no - // cluster-uuid= TXT to key a specific pin on). Refresh first so a cluster - // joined, or a peer paired, after startup is seen; falls back to the BYO - // tlsClient while unclustered. - m.mesh.Refresh() - if cfg, ok := m.mesh.ClientTLSConfigAny(); ok { - probeClient = &http.Client{Timeout: probeTimeout, Transport: &http.Transport{TLSClientConfig: cfg, DisableKeepAlives: true}} - } - } - nodeInfoUp, info := m.probeNodeInfo(probeClient, scheme, addr, nodeInfoPort) + // node-info is asked FIRST, because its answer decides what kind of node this + // is and therefore what else may be probed at all. + // + // A PAIR node's 11434 and 1234 are its proxy facades, not its engines: the + // engines bind loopback and the facades refuse plaintext from anything but + // loopback. Probing them would 403 every cycle and report a healthy peer as + // having no engines, so a node that identifies itself as PAIR is never probed + // there. Its engines are read from its engine manager instead, and it is + // routed to through those same facades over cluster mTLS. + nodeInfoUp, info := m.probeNodeInfo(entry, ports) + pairNode := nodeInfoUp && len(info.Services) > 0 newStatus := ManualNodeStatus{ ID: id, Name: entry.Name, Address: addr, - OllamaUp: ollamaUp, - OllamaPort: 11434, - OllamaModels: ollamaModels, - LMStudioUp: lmStudioUp, - LMStudioPort: lmStudioPort, - LMStudioModels: lmStudioModels, + OllamaPort: ports.Ollama, + LMStudioPort: ports.LMStudio, NodeInfoUp: nodeInfoUp, - NodeInfoPort: nodeInfoPort, + NodeInfoPort: m.nodeInfoProbePort(entry, ports), TLSEnabled: entry.TLSPort > 0, MTLSRequired: entry.TLSPort > 0 && entry.MTLS, + Ports: entry.Ports, + PairNode: pairNode, + ClusterUUID: info.ClusterUUID, + Services: info.Services, GPUs: info.GPUs, CPU: info.CPU, Memory: info.Memory, @@ -302,6 +418,20 @@ func (m *Manager) probeNode(entry ManualEntry) { HostUUID: info.HostUUID, } + if pairNode { + models := m.fetchPeerModels(addr, info) + newStatus.Models = models.Models + newStatus.ModelsByEngine = models.ModelsByEngine + newStatus.LoadedByEngine = models.LoadedByEngine + } else { + // A bare inference host: the engines themselves are on these ports, in + // plaintext, which is what manual nodes were originally for. + for _, leg := range m.engineLegs(ports) { + up, models := leg.probe(addr, leg.port) + leg.apply(&newStatus, up, models) + } + } + reachable := newStatus.OllamaUp || newStatus.LMStudioUp || newStatus.NodeInfoUp m.mu.Lock() @@ -333,6 +463,12 @@ func (m *Manager) probeNode(entry ManualEntry) { prev.LMStudioUp != newStatus.LMStudioUp || prev.NodeInfoUp != newStatus.NodeInfoUp || prev.HostUUID != newStatus.HostUUID || + prev.PairNode != newStatus.PairNode || + !clusterUUIDEqual(prev.ClusterUUID, newStatus.ClusterUUID) || + !servicesEqual(prev.Services, newStatus.Services) || + !sliceEqual(prev.Models, newStatus.Models) || + !byEngineEqual(prev.ModelsByEngine, newStatus.ModelsByEngine) || + !byEngineEqual(prev.LoadedByEngine, newStatus.LoadedByEngine) || !sliceEqual(prev.OllamaModels, newStatus.OllamaModels) || !sliceEqual(prev.LMStudioModels, newStatus.LMStudioModels) || !gpusEqual(prev.GPUs, newStatus.GPUs) || @@ -397,11 +533,36 @@ func probeFailedID(nodeID string) string { return "manual-nodes:probe-failed:" + nodeID } -// lmStudioPort is LM Studio's default OpenAI-API server port, probed the same -// way Ollama is hardcoded to 11434. A manual node is remote, so (like Ollama) -// we assume the engine's default port rather than resolving it via the engine -// manager (which only governs the local engine). -const lmStudioPort = 1234 +// engineLeg is one plain-HTTP inference-engine probe against a bare host. The +// legs are a table rather than a sequence of calls so adding an engine is one +// entry: its port, how it is probed, and where its result lands on the status. +type engineLeg struct { + name string + port int + probe func(addr string, port int) (bool, []string) + apply func(s *ManualNodeStatus, up bool, models []string) +} + +func (m *Manager) engineLegs(ports ManualPorts) []engineLeg { + return []engineLeg{ + { + name: "ollama", + port: ports.Ollama, + probe: m.probeOllama, + apply: func(s *ManualNodeStatus, up bool, models []string) { + s.OllamaUp, s.OllamaModels = up, models + }, + }, + { + name: "lmstudio", + port: ports.LMStudio, + probe: m.probeLMStudio, + apply: func(s *ManualNodeStatus, up bool, models []string) { + s.LMStudioUp, s.LMStudioModels = up, models + }, + }, + } +} // probeLMStudio checks LM Studio's OpenAI-compatible server on addr:port. A // single GET /v1/models doubles as the liveness check and the model list (the @@ -496,7 +657,83 @@ func (m *Manager) fetchOllamaModels(addr string, port int) []string { return names } -func (m *Manager) probeNodeInfo(client *http.Client, scheme, addr string, port int) (bool, NodeInfoResponse) { +// nodeInfoProbePort is the port node-info is actually reached on: the entry's +// tls_port when one is set (it names an HTTPS listener, so it also names the +// port), otherwise the resolved plain node-info port. +func (m *Manager) nodeInfoProbePort(entry ManualEntry, ports ManualPorts) int { + if entry.TLSPort > 0 { + return entry.TLSPort + } + return ports.NodeInfo +} + +// nodeInfoClient picks the scheme, port and transport for the node-info probe. +// The operator decides which scheme a manual node uses; we do not probe both. +// tls_port > 0 means HTTPS on that port via the TLS client (carrying the +// operator's client cert, if configured), or over cluster mTLS while this node is +// a cluster member — a clustered peer running node-info standalone serves its +// inventory only to pinned peers. Otherwise it is plain HTTP on the resolved +// node-info port, which is what a peer under the broker serves: node-info is the +// one inter-node surface deliberately left readable by any peer. +func (m *Manager) nodeInfoClient(entry ManualEntry, ports ManualPorts) (*http.Client, string, int) { + if entry.TLSPort == 0 { + return m.client, "http", ports.NodeInfo + } + // Refresh first so a cluster joined, or a peer paired, after startup is seen; + // falls back to the BYO tlsClient while unclustered. + m.mesh.Refresh() + if cfg, ok := m.mesh.ClientTLSConfigAny(); ok { + return &http.Client{Timeout: probeTimeout, Transport: &http.Transport{TLSClientConfig: cfg, DisableKeepAlives: true}}, "https", entry.TLSPort + } + return m.tlsClient, "https", entry.TLSPort +} + +// fetchPeerModels reads a paired PAIR node's model inventory from its engine +// manager, the same body the node-scanner fetches for a discovered peer. +// +// It is pinned mTLS or nothing: engine-manager's LAN surface serves plaintext +// only over loopback, and a peer we hold no pin for is a stranger. An empty +// result is therefore the correct answer before pairing, not a failure — the node +// still appears with its hardware, and its models arrive once it is paired. +func (m *Manager) fetchPeerModels(addr string, info NodeInfoResponse) peerModels { + port, ok := info.Services[noderec.ServiceEngineManager] + if !ok || port <= 0 { + return peerModels{} + } + if info.ClusterUUID == nil || *info.ClusterUUID == "" { + slog.Debug("manual peer model fetch skipped: peer reports no cluster principal", "addr", addr) + return peerModels{} + } + m.mesh.Refresh() + cfg, ok := m.mesh.ClientTLSConfig(*info.ClusterUUID) + if !ok { + slog.Debug("manual peer model fetch skipped: no pin held for peer", "addr", addr) + return peerModels{} + } + client := &http.Client{Timeout: probeTimeout, Transport: &http.Transport{TLSClientConfig: cfg, DisableKeepAlives: true}} + url := "https://" + net.JoinHostPort(addr, strconv.Itoa(port)) + "/v1/models" + resp, err := client.Get(url) + if err != nil { + slog.Debug("manual peer model fetch failed", "addr", addr, "port", port, "err", err) + return peerModels{} + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + slog.Debug("manual peer model fetch non-OK", "addr", addr, "port", port, "status", resp.StatusCode) + return peerModels{} + } + var models peerModels + if err := json.NewDecoder(resp.Body).Decode(&models); err != nil { + slog.Debug("manual peer model fetch decode failed", "addr", addr, "port", port, "err", err) + return peerModels{} + } + slog.Debug("manual peer models fetched", "addr", addr, "port", port, "models", len(models.Models)) + return models +} + +func (m *Manager) probeNodeInfo(entry ManualEntry, ports ManualPorts) (bool, NodeInfoResponse) { + client, scheme, port := m.nodeInfoClient(entry, ports) + addr := entry.Address url := scheme + "://" + net.JoinHostPort(addr, strconv.Itoa(port)) + "/v1/node-info" start := time.Now() resp, err := client.Get(url) @@ -531,18 +768,17 @@ func (m *Manager) probeNodeInfo(client *http.Client, scheme, addr string, port i func (m *Manager) addNode(entry ManualEntry) ManualNodeStatus { id := nodeID(entry) - nodeInfoPort := 14318 - if entry.TLSPort > 0 { - nodeInfoPort = entry.TLSPort - } + ports := entry.resolved() status := ManualNodeStatus{ ID: id, Name: entry.Name, Address: entry.Address, - OllamaPort: 11434, - NodeInfoPort: nodeInfoPort, + OllamaPort: ports.Ollama, + LMStudioPort: ports.LMStudio, + NodeInfoPort: m.nodeInfoProbePort(entry, ports), TLSEnabled: entry.TLSPort > 0, MTLSRequired: entry.TLSPort > 0 && entry.MTLS, + Ports: entry.Ports, } m.mu.Lock() @@ -648,10 +884,12 @@ func (m *Manager) handleMessage(msg *Message) { m.codec.RespondError(msg.ID, -32602, "invalid params: expected {\"address\": \"...\"}") return } - if entry.Address == "" { - m.codec.RespondError(msg.ID, -32602, "address is required") + address, err := validateManualAddress(entry.Address) + if err != nil { + m.codec.RespondError(msg.ID, -32602, err.Error()) return } + entry.Address = address status := m.addNode(entry) if err := m.codec.Respond(msg.ID, status); err != nil { log.Printf("failed to respond to node/add: %v", err) @@ -701,6 +939,41 @@ func nodeID(entry ManualEntry) string { return "manual:" + entry.Address } +// clusterUUIDEqual compares the tri-state principal by value, so a peer that +// went from "unknown" to "unclustered" reports as changed rather than as the same +// empty string. +func clusterUUIDEqual(a, b *string) bool { + if a == nil || b == nil { + return a == b + } + return *a == *b +} + +func servicesEqual(a, b map[noderec.ServiceKey]int) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + if b[k] != v { + return false + } + } + return true +} + +func byEngineEqual(a, b map[string][]string) bool { + if len(a) != len(b) { + return false + } + for k, v := range a { + other, ok := b[k] + if !ok || !sliceEqual(v, other) { + return false + } + } + return true +} + func sliceEqual(a, b []string) bool { if len(a) != len(b) { return false diff --git a/services/nvpair-manual-nodes/manager_test.go b/services/nvpair-manual-nodes/manager_test.go index e52d9d5d..74fd509d 100644 --- a/services/nvpair-manual-nodes/manager_test.go +++ b/services/nvpair-manual-nodes/manager_test.go @@ -140,7 +140,7 @@ func TestProbeLMStudioReportsModels(t *testing.T) { m, _, rt := newTestManager() configureHealthyLMStudio(rt, "node.local", []string{"qwen2.5-7b", "llama-3.1-8b"}) - up, models := m.probeLMStudio("node.local", lmStudioPort) + up, models := m.probeLMStudio("node.local", defaultLMStudioPort) if !up { t.Fatal("expected lmstudio up") } @@ -148,7 +148,7 @@ func TestProbeLMStudioReportsModels(t *testing.T) { t.Fatalf("models = %#v", models) } - downUp, downModels := m.probeLMStudio("absent.local", lmStudioPort) + downUp, downModels := m.probeLMStudio("absent.local", defaultLMStudioPort) if downUp || downModels != nil { t.Fatalf("expected absent lmstudio down, got up=%v models=%#v", downUp, downModels) } diff --git a/services/nvpair-manual-nodes/pairnode_test.go b/services/nvpair-manual-nodes/pairnode_test.go new file mode 100644 index 00000000..d4dff7c4 --- /dev/null +++ b/services/nvpair-manual-nodes/pairnode_test.go @@ -0,0 +1,268 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Two kinds of manual node, told apart by one probe. +// +// A bare Ollama / LM Studio box is what manual nodes were originally for: its +// engines answer plain HTTP on their own ports and it serves no /v1/node-info. +// A PAIR node is the other kind, and it is the one an operator adds when the only +// route between two machines is an overlay network such as a Tailscale tailnet, +// where multicast never arrives and nothing is ever discovered. Its 11434 and +// 1234 carry proxy facades that refuse plaintext from anything but loopback, so +// probing them reports a healthy peer as having no engines. + +package main + +import ( + "encoding/json" + "net" + "net/http" + "strconv" + "strings" + "sync/atomic" + "testing" + + "nvpair-shared/noderec" +) + +func strptr(s string) *string { return &s } + +// probeSync registers an entry and probes it on this goroutine, so a test reads a +// settled status instead of racing addNode's background probe. +func probeSync(m *Manager, entry ManualEntry) { + m.mu.Lock() + m.nodes[nodeID(entry)] = &trackedNode{entry: entry} + m.mu.Unlock() + m.probeNode(entry) +} + +func statusFor(t *testing.T, m *Manager, id string) ManualNodeStatus { + t.Helper() + m.mu.RLock() + defer m.mu.RUnlock() + tn, ok := m.nodes[id] + if !ok { + t.Fatalf("no tracked node %q", id) + } + return tn.status +} + +// configurePairNode answers /v1/node-info on addr:port as a PAIR node: with a +// service map, which is what identifies it. +func configurePairNode(rt *fakeRoundTripper, addr string, port int, info NodeInfoResponse) { + host := net.JoinHostPort(addr, strconv.Itoa(port)) + rt.set(http.MethodGet, host, "/v1/node-info", func(*http.Request) (*http.Response, error) { + data, _ := json.Marshal(info) + return httpJSON(http.StatusOK, string(data)) + }) +} + +func pairNodeInfo() NodeInfoResponse { + return NodeInfoResponse{ + GPUs: []GPUInfo{{Name: "NVIDIA GeForce RTX 4090", UtilizationPercent: 12}}, + TelemetryValid: true, + HostUUID: "peer-host-uuid", + ClusterUUID: strptr("peer-cluster-uuid"), + Services: map[noderec.ServiceKey]int{ + noderec.ServiceNodeInfo: 14318, + noderec.ServiceOllama: 11434, + noderec.ServiceEngineManager: 14322, + noderec.ServiceEngineControl: 14323, + noderec.ServiceCluster: 14321, + }, + } +} + +// The regression gate: a PAIR node is never probed on its engine ports. A +// plaintext connection there is a guaranteed 403 from the peer's proxy facade, +// and treating that as "engine down" is what made a manually added PAIR peer read +// as having nothing to route to. +func TestProbeNode_PairNodeIsNotProbedOnItsEnginePorts(t *testing.T) { + m, _, rt := newTestManager() + + var engineProbes atomic.Int32 + for _, port := range []string{"11434", "1234"} { + host := net.JoinHostPort("gpu-box.tail1234.ts.net", port) + for _, path := range []string{"/", "/api/tags", "/v1/models"} { + rt.set(http.MethodGet, host, path, func(*http.Request) (*http.Response, error) { + engineProbes.Add(1) + return httpJSON(http.StatusForbidden, `{"error":"loopback-only"}`) + }) + } + } + configurePairNode(rt, "gpu-box.tail1234.ts.net", 14318, pairNodeInfo()) + + entry := ManualEntry{Address: "gpu-box.tail1234.ts.net"} + probeSync(m, entry) + + status := statusFor(t, m, nodeID(entry)) + if !status.PairNode { + t.Fatal("a node-info answer carrying a service map must mark the node as a PAIR node") + } + if n := engineProbes.Load(); n != 0 { + t.Fatalf("%d plaintext engine probes were made against a PAIR node, want 0", n) + } + if status.OllamaUp || status.LMStudioUp { + t.Fatalf("a PAIR node must report no raw engines (ollama_up=%v lmstudio_up=%v)", status.OllamaUp, status.LMStudioUp) + } + if status.Services[noderec.ServiceEngineManager] != 14322 { + t.Fatalf("services = %v, want the peer's em port carried through", status.Services) + } + if status.ClusterUUID == nil || *status.ClusterUUID != "peer-cluster-uuid" { + t.Fatalf("cluster_uuid = %v, want the peer's principal", status.ClusterUUID) + } + if status.HostUUID != "peer-host-uuid" { + t.Fatalf("hostUuid = %q, want the peer's identity", status.HostUUID) + } +} + +// A node-info answer with no service map is a host too old to report one, or one +// that is not a PAIR node at all. Either way the only thing it can be reached as +// is a bare inference host, so the plain engine probes still run. +func TestProbeNode_NodeInfoWithoutServicesStaysABareHost(t *testing.T) { + m, _, rt := newTestManager() + configureHealthyNode(rt, "10.0.0.9", []string{"llama3.2:latest"}, NodeInfoResponse{ + HostUUID: "bare-host", + TelemetryValid: true, + }) + + entry := ManualEntry{Address: "10.0.0.9"} + probeSync(m, entry) + + status := statusFor(t, m, nodeID(entry)) + if status.PairNode { + t.Fatal("a node-info answer without a service map must not read as a PAIR node") + } + if !status.OllamaUp || len(status.OllamaModels) != 1 { + t.Fatalf("bare host = ollama_up:%v models:%v, want the engine probed", status.OllamaUp, status.OllamaModels) + } +} + +// A paired peer's model inventory is pinned mTLS or nothing. Without a pin the +// node still appears with its hardware; its models arrive once it is paired. +func TestFetchPeerModels_RequiresAPin(t *testing.T) { + m, _, _ := newTestManager() + + if got := m.fetchPeerModels("gpu-box.tail1234.ts.net", pairNodeInfo()); len(got.Models) != 0 { + t.Fatalf("models = %v, want none while this node holds no pin for the peer", got.Models) + } + // An unclustered peer serves its engine manager over loopback only, so there + // is nothing to ask for either. + info := pairNodeInfo() + info.ClusterUUID = strptr("") + if got := m.fetchPeerModels("gpu-box.tail1234.ts.net", info); len(got.Models) != 0 { + t.Fatalf("models = %v, want none for an unclustered peer", got.Models) + } +} + +func TestValidateManualAddress(t *testing.T) { + for _, tc := range []struct { + in string + want string + }{ + {"10.0.0.9", "10.0.0.9"}, + {"gpu-box.tail1234.ts.net", "gpu-box.tail1234.ts.net"}, + {"gpu-box", "gpu-box"}, + {"100.101.102.103", "100.101.102.103"}, + {"fd7a:115c:a1e0::1", "fd7a:115c:a1e0::1"}, + {"[fd7a:115c:a1e0::1]", "fd7a:115c:a1e0::1"}, + {" 10.0.0.9 ", "10.0.0.9"}, + } { + got, err := validateManualAddress(tc.in) + if err != nil { + t.Errorf("validateManualAddress(%q) = error %v, want %q", tc.in, err, tc.want) + continue + } + if got != tc.want { + t.Errorf("validateManualAddress(%q) = %q, want %q", tc.in, got, tc.want) + } + } + + // A host:port entry used to be accepted and then read permanently down, + // because every probe appends its own port to the value. + for _, bad := range []string{"gpu-box.tail1234.ts.net:14318", "10.0.0.9:11434", "[fd7a::1]:14318"} { + got, err := validateManualAddress(bad) + if err == nil { + t.Errorf("validateManualAddress(%q) = %q, want a rejection", bad, got) + continue + } + if !strings.Contains(err.Error(), "ports") { + t.Errorf("rejection of %q = %q, want it to point at the ports object", bad, err) + } + } + + for _, bad := range []string{"", " ", "gpu box", "under_score.example"} { + if _, err := validateManualAddress(bad); err == nil { + t.Errorf("validateManualAddress(%q) succeeded, want a rejection", bad) + } + } +} + +func TestManualPorts_OverrideEveryServiceAndDefaultTheRest(t *testing.T) { + full := ManualEntry{Ports: &ManualPorts{ + NodeInfo: 24318, Cluster: 24321, Ollama: 21434, LMStudio: 2234, VLLM: 8001, + }}.resolved() + want := ManualPorts{NodeInfo: 24318, Cluster: 24321, Ollama: 21434, LMStudio: 2234, VLLM: 8001} + if full != want { + t.Fatalf("resolved = %+v, want %+v", full, want) + } + + partial := ManualEntry{Ports: &ManualPorts{Ollama: 21434}}.resolved() + if partial.Ollama != 21434 { + t.Errorf("ollama = %d, want the override", partial.Ollama) + } + if partial.NodeInfo != defaultNodeInfoPort || partial.LMStudio != defaultLMStudioPort || + partial.Cluster != defaultClusterPort || partial.VLLM != defaultVLLMPort { + t.Errorf("resolved = %+v, want every unset field defaulted", partial) + } + + if none := (ManualEntry{}).resolved(); none.NodeInfo != defaultNodeInfoPort || none.Ollama != defaultOllamaPort { + t.Errorf("resolved with no overrides = %+v, want the defaults", none) + } +} + +// Two PAIR nodes can share one loopback when each is addressed by its own ports. +// That is what makes a manual entry usable for a forwarded range, and what lets a +// cross-process test stand up a peer without a second machine. +func TestProbeNode_PortOverridesAddressTheRightService(t *testing.T) { + m, _, rt := newTestManager() + configurePairNode(rt, "127.0.0.1", 24318, pairNodeInfo()) + + entry := ManualEntry{Address: "127.0.0.1", Name: "peer-b", Ports: &ManualPorts{NodeInfo: 24318}} + probeSync(m, entry) + + status := statusFor(t, m, "peer-b") + if !status.NodeInfoUp { + t.Fatal("node-info on the overridden port must be probed") + } + if status.NodeInfoPort != 24318 { + t.Fatalf("node_info_port = %d, want the override echoed", status.NodeInfoPort) + } + if status.Ports == nil || status.Ports.NodeInfo != 24318 { + t.Fatalf("ports = %+v, want the entry's overrides echoed back", status.Ports) + } +} + +// The engine legs are a table so an engine is one entry rather than a new pair of +// hardcoded calls. A bare host with a relocated engine is reached at its port. +func TestEngineLegs_FollowThePortOverrides(t *testing.T) { + m, _, rt := newTestManager() + configureHealthyLMStudio(rt, "10.0.0.9", []string{"qwen2.5-7b-instruct"}) + rt.set(http.MethodGet, net.JoinHostPort("10.0.0.9", "21434"), "/", func(*http.Request) (*http.Response, error) { + return httpJSON(http.StatusOK, `{}`) + }) + rt.set(http.MethodGet, net.JoinHostPort("10.0.0.9", "21434"), "/api/tags", func(*http.Request) (*http.Response, error) { + return httpJSON(http.StatusOK, `{"models":[{"name":"llama3.2:latest"}]}`) + }) + + entry := ManualEntry{Address: "10.0.0.9", Name: "relocated", Ports: &ManualPorts{Ollama: 21434}} + probeSync(m, entry) + + status := statusFor(t, m, "relocated") + if !status.OllamaUp || status.OllamaPort != 21434 { + t.Fatalf("ollama = up:%v port:%d, want the relocated engine found", status.OllamaUp, status.OllamaPort) + } + if !status.LMStudioUp || status.LMStudioPort != defaultLMStudioPort { + t.Fatalf("lmstudio = up:%v port:%d, want the default port still used", status.LMStudioUp, status.LMStudioPort) + } +} diff --git a/services/nvpair-ui-broker/broker.go b/services/nvpair-ui-broker/broker.go index 6354a128..bb8392b3 100644 --- a/services/nvpair-ui-broker/broker.go +++ b/services/nvpair-ui-broker/broker.go @@ -305,6 +305,12 @@ type Broker struct { manualMu sync.Mutex manualNodeKeys map[string]string manualNodeStatuses map[string]manualNodeStatusEntry + // manualRelayKeys are the directory keys this broker synthesized for manual + // PAIR nodes. The relay directory is keyed by hostUuid and the scanner writes + // the same keys, so a withdrawal has to know what it put there: removing a + // record the daemon owns would evict a live discovered peer from every + // consumer until its next browse event. + manualRelayKeys map[string]bool // schedMu guards each engine's cached priority and generation. Per-engine // delivery locks serialize asynchronous node/set-priority calls; a stale @@ -382,6 +388,7 @@ func NewBroker(codec *Codec, paths workerPaths) *Broker { relayDir: relay.NewDirectory(), regCache: relay.NewRegistrationCache(), manualNodeKeys: make(map[string]string), + manualRelayKeys: make(map[string]bool), manualNodeStatuses: make(map[string]manualNodeStatusEntry), workloads: workloadstore.New(), ollamaPortReady: make(chan struct{}), @@ -1295,7 +1302,7 @@ func (b *Broker) forwardManualNodesNotification(method string, params json.RawMe // the alias's key changed (node-info revealed its real UUID) the // old key is reprojected from a surviving alias or released. func (b *Broker) upsertManualNode(s manualNodeStatus) { - en := manualToEnriched(s) + en := b.manualEnriched(s) key := en.storeKey() receivedAt := time.Now() @@ -1307,6 +1314,9 @@ func (b *Broker) upsertManualNode(s manualNodeStatus) { b.store.Upsert(en, sourceManual) b.ingestTelemetryAt(sourceManual, manualNodeTelemetry(s, key), receivedAt) + // A PAIR node joins the discovery relay as if it had been found over mDNS, so + // every consumer treats it as the pinned peer it is. + b.applyManualDirectory(s, key) // Bridge a reachable manual node into each engine's proxy (ollama-proxy / // lmstudio-proxy) so inference can route to it; an unreachable engine is // pulled back out. No-op for a proxy the broker doesn't supervise. @@ -1350,13 +1360,15 @@ func (b *Broker) reprojectOrRelease(key string) { survivor, ok := b.survivingAliasLocked(key) b.manualMu.Unlock() if ok { - b.store.Upsert(manualToEnriched(survivor.status), sourceManual) + b.store.Upsert(b.manualEnriched(survivor.status), sourceManual) b.bridgeManualNode(survivor.status, key) + b.applyManualDirectory(survivor.status, key) b.ingestTelemetryAt(sourceManual, manualNodeTelemetry(survivor.status, key), survivor.receivedAt) return } b.store.Remove(key, sourceManual) b.removeManualNodeFromProxies(key) + b.releaseManualDirectory(key) b.removeTelemetry(sourceManual, key) } @@ -1403,6 +1415,9 @@ func (b *Broker) clearManualNodesState() { // doesn't keep a stale manual target the crashed prober can no // longer vouch for. Clients re-add manual nodes after the restart. b.removeManualNodeFromProxies(key) + // And out of the discovery relay, for the same reason: a synthesized PAIR + // peer is only as good as the prober that keeps vouching for it. + b.releaseManualDirectory(key) } } diff --git a/services/nvpair-ui-broker/discovery.go b/services/nvpair-ui-broker/discovery.go index c0334927..4f1ca124 100644 --- a/services/nvpair-ui-broker/discovery.go +++ b/services/nvpair-ui-broker/discovery.go @@ -273,6 +273,19 @@ func (s *discoveryStore) Remove(key string, source nodeSource) bool { return gone } +// hasSource reports whether the given source currently claims key. It answers +// "does the daemon already own this record?" for the manual-node synthesis, +// which must not write a directory entry the scanner is authoritative for. +func (s *discoveryStore) hasSource(key string, source nodeSource) bool { + if key == "" { + return false + } + s.mu.RLock() + defer s.mu.RUnlock() + sn, ok := s.nodes[key] + return ok && sn.hasSource(source) +} + // Snapshot returns the narrow wire-format view used by // discovery:get-nodes and discovery:nodes-changed, sorted by id for // stable rendering. The rich EnrichedNode payload stays in the store diff --git a/services/nvpair-ui-broker/manual_pairnode_test.go b/services/nvpair-ui-broker/manual_pairnode_test.go new file mode 100644 index 00000000..796b1d86 --- /dev/null +++ b/services/nvpair-ui-broker/manual_pairnode_test.go @@ -0,0 +1,196 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Folding a manually added PAIR node into the discovery relay. +// +// A peer reachable only across an overlay network such as a Tailscale tailnet is +// never discovered — no multicast crosses it — but it is not a different kind of +// node. Once its node-info reports a service map, the broker synthesizes the +// directory record the scanner would have produced, so both inference proxies, +// the scheduler, engine-manager's remote operations, the workload relay and the +// errors peer sync all see a pinned peer rather than a special case. + +package main + +import ( + "testing" + + "nvpair-shared/noderec" + "nvpair-ui-broker/relay" +) + +func pairStatus() manualNodeStatus { + principal := "peer-cluster-uuid" + return manualNodeStatus{ + ID: "gpu-box.tail1234.ts.net", + Address: "gpu-box.tail1234.ts.net", + NodeInfoPort: 14318, + HostUUID: "peer-host-uuid", + PairNode: true, + ClusterUUID: &principal, + Services: map[noderec.ServiceKey]int{ + noderec.ServiceNodeInfo: 14318, + noderec.ServiceOllama: 11434, + noderec.ServiceLMStudio: 1234, + noderec.ServiceEngineManager: 14322, + noderec.ServiceEngineControl: 14323, + noderec.ServiceCluster: 14321, + }, + Models: []string{"llama3.2:latest"}, + ModelsByEngine: map[string][]string{"ollama": {"llama3.2:latest"}}, + TelemetryValid: true, + } +} + +func newPairPeerTestBroker(nodeID string) *Broker { + return &Broker{ + nodeID: nodeID, + store: newDiscoveryStore(), + relayDir: relay.NewDirectory(), + manualRelayKeys: make(map[string]bool), + } +} + +func TestManualDirectoryNode_CarriesEverythingAPeerNeedsToBeRouted(t *testing.T) { + b := newPairPeerTestBroker("this-host") + s := pairStatus() + + node, ok := b.manualDirectoryNode(s, s.HostUUID) + if !ok { + t.Fatal("a PAIR node with a service map must enter the directory") + } + if node.HostUUID != "peer-host-uuid" { + t.Errorf("hostUuid = %q, want the peer's identity", node.HostUUID) + } + // The typed address is this node's canonical one. It is a MagicDNS name here, + // which is the case that used to be dropped: a name is dialable, and it + // re-resolves, so it outlives every literal the peer holds. + if node.IP != "gpu-box.tail1234.ts.net" { + t.Errorf("ip = %q, want the address the operator typed", node.IP) + } + if got := node.CandidateIPs(); len(got) != 1 || got[0] != "gpu-box.tail1234.ts.net" { + t.Errorf("candidate addresses = %v, want the typed address", got) + } + if got := node.AddressTXT(); len(got) != 1 || got[0] != "ip=gpu-box.tail1234.ts.net" { + t.Errorf("address TXT = %v, want ip=
", got) + } + // The cluster principal is what a consumer pins the peer's certificate on. + // Without it every mTLS surface is dialed in plaintext and answers 403. + if node.ClusterUUID != "peer-cluster-uuid" || !node.Clustered() { + t.Errorf("clusterUuid = %q, want the peer's principal", node.ClusterUUID) + } + for svc, want := range map[noderec.ServiceKey]int{ + noderec.ServiceOllama: 11434, + noderec.ServiceLMStudio: 1234, + noderec.ServiceEngineManager: 14322, + noderec.ServiceEngineControl: 14323, + } { + got, ok := node.Services[svc] + if !ok || got.Port != want { + t.Errorf("service %s = %d (present:%v), want %d", svc, got.Port, ok, want) + } + } + if len(node.Models) != 1 || node.Models[0] != "llama3.2:latest" { + t.Errorf("models = %v, want the peer's inventory", node.Models) + } +} + +// A bare Ollama / LM Studio box has no proxy and no engine manager, so it is not +// a peer. It stays on the raw-engine bridge, which is what manual nodes were for. +func TestManualDirectoryNode_BareHostStaysOutOfTheDirectory(t *testing.T) { + b := newPairPeerTestBroker("this-host") + s := manualNodeStatus{ + ID: "10.0.0.9", Address: "10.0.0.9", HostUUID: "bare-host", + OllamaUp: true, OllamaPort: 11434, OllamaModels: []string{"llama3.2:latest"}, + } + if _, ok := b.manualDirectoryNode(s, s.HostUUID); ok { + t.Fatal("a bare inference host must not be synthesized as a directory peer") + } +} + +// A manual entry naming this machine must never become a routing target. The +// test is identity, not address: the same host reached by its overlay name is +// still us, and an address comparison would miss it. +func TestManualDirectoryNode_RefusesThisHost(t *testing.T) { + b := newPairPeerTestBroker("peer-host-uuid") + if _, ok := b.manualDirectoryNode(pairStatus(), "peer-host-uuid"); ok { + t.Fatal("a manual entry naming this host must not enter the directory") + } +} + +// When the daemon already holds the node, its record wins: it carries the peer's +// full ranked address list and its liveness probes, which this synthesis cannot. +func TestManualDirectoryNode_YieldsToTheScanner(t *testing.T) { + b := newPairPeerTestBroker("this-host") + b.store.Upsert(EnrichedNode{ID: "gpu-box", HostUUID: "peer-host-uuid"}, sourceScanner) + if _, ok := b.manualDirectoryNode(pairStatus(), "peer-host-uuid"); ok { + t.Fatal("the scanner's record is authoritative; the manual synthesis must stand down") + } +} + +func TestApplyManualDirectory_AddsAndWithdraws(t *testing.T) { + b := newPairPeerTestBroker("this-host") + s := pairStatus() + + b.applyManualDirectory(s, s.HostUUID) + if got := b.relayDir.Snapshot(noderec.ServiceOllama); len(got) != 1 || got[0].HostUUID != "peer-host-uuid" { + t.Fatalf("relay directory = %v, want the synthesized peer", got) + } + + // It went down, or turned out not to be a PAIR node: withdraw it, or the + // proxies keep a routing target nothing vouches for. + down := s + down.PairNode = false + b.applyManualDirectory(down, s.HostUUID) + if got := b.relayDir.Snapshot(""); len(got) != 0 { + t.Fatalf("relay directory = %v, want the withdrawn peer gone", got) + } +} + +// The relay directory is keyed by hostUuid and the scanner writes the same keys. +// A withdrawal must remove only what this broker synthesized, or a live +// discovered peer disappears from every consumer until its next browse event. +func TestReleaseManualDirectory_NeverWithdrawsADiscoveredPeer(t *testing.T) { + b := newPairPeerTestBroker("this-host") + b.relayDir.Apply(noderec.NotifyNodeDiscovered, noderec.DirectoryNode{ + HostUUID: "discovered-peer", + Name: "gpu-box", + IP: "192.168.1.10", + Services: map[noderec.ServiceKey]noderec.ServiceStatus{noderec.ServiceOllama: {Port: 11434}}, + }) + + b.releaseManualDirectory("discovered-peer") + if got := b.relayDir.Snapshot(""); len(got) != 1 { + t.Fatalf("relay directory = %v, want the discovered peer untouched", got) + } +} + +// A machine the scanner took over mid-life must not be withdrawn either: the +// manual claim goes away, the daemon's record stays. +func TestReleaseManualDirectory_YieldsWhenTheScannerTookOver(t *testing.T) { + b := newPairPeerTestBroker("this-host") + s := pairStatus() + b.applyManualDirectory(s, s.HostUUID) + b.store.Upsert(EnrichedNode{ID: "gpu-box", HostUUID: s.HostUUID}, sourceScanner) + + b.releaseManualDirectory(s.HostUUID) + if got := b.relayDir.Snapshot(""); len(got) != 1 { + t.Fatalf("relay directory = %v, want the record kept for the scanner", got) + } +} + +func TestManualToEnriched_PairNodeCarriesItsInventoryAndMembership(t *testing.T) { + en := manualToEnriched(pairStatus()) + if !en.Clustered { + t.Error("a peer reporting a cluster principal must project as clustered") + } + if len(en.Models) != 1 || en.Models[0] != "llama3.2:latest" { + t.Errorf("models = %v, want the peer's inventory", en.Models) + } + if len(en.Addresses) != 1 || en.Addresses[0] != "gpu-box.tail1234.ts.net" { + t.Errorf("addresses = %v, want the typed address", en.Addresses) + } + if len(en.TXT) != 1 || en.TXT[0] != "ip=gpu-box.tail1234.ts.net" { + t.Errorf("txt = %v, want ip=
so netpick ranks it", en.TXT) + } +} diff --git a/services/nvpair-ui-broker/manualnodes.go b/services/nvpair-ui-broker/manualnodes.go index 47fe0ff1..6e740e55 100644 --- a/services/nvpair-ui-broker/manualnodes.go +++ b/services/nvpair-ui-broker/manualnodes.go @@ -9,6 +9,7 @@ import ( "log/slog" "time" + "nvpair-shared/clustertrust" "nvpair-shared/noderec" ) @@ -39,6 +40,39 @@ type manualNodeStatus struct { // identity as mDNS-discovered nodes (and dedup with itself when the same // machine is also discovered). Empty until the node-info probe succeeds. HostUUID string `json:"hostUuid,omitempty"` + // PairNode is true when the prober's node-info leg answered AND reported a + // service map: the remote is a PAIR node, not a bare inference host. It is the + // switch between the two ways a manual node reaches inference. A bare host is + // bridged into the local proxies by its raw engine ports; a PAIR node is + // folded into the discovery relay instead and reached through its own proxies + // over cluster mTLS, because its 11434 / 1234 are proxy facades that refuse + // plaintext from anything but loopback. + PairNode bool `json:"pair_node"` + // ClusterUUID is the remote's cluster principal, tri-state (see the prober's + // NodeInfoResponse): absent means it does not know, present-and-empty means it + // belongs to no cluster. It is the key a consumer pins the peer's certificate + // on, so getting it wrong is the difference between mTLS and a 403. + ClusterUUID *string `json:"cluster_uuid,omitempty"` + // Services is the remote's {service key: port} set, read from its node-info. + // It is everything this host would otherwise have read off the peer's mDNS + // record, which never arrives across a routed or overlay network. + Services map[noderec.ServiceKey]int `json:"services,omitempty"` + // Models / ModelsByEngine / LoadedByEngine are a paired PAIR node's inventory, + // read from its engine manager over cluster mTLS. Empty until this node holds + // a pin for the peer. + Models []string `json:"models,omitempty"` + ModelsByEngine map[string][]string `json:"models_by_engine,omitempty"` + LoadedByEngine map[string][]string `json:"loaded_by_engine,omitempty"` +} + +// clusterUUID flattens the tri-state principal for the consumers that only need +// a value. Unknown and unclustered both answer "" — which is correct for every +// use here, since both mean "we hold no principal to pin on". +func (s manualNodeStatus) clusterUUID() string { + if s.ClusterUUID == nil { + return "" + } + return *s.ClusterUUID } type manualNodeStatusEntry struct { @@ -87,15 +121,132 @@ func manualToEnriched(s manualNodeStatus) EnrichedNode { GPUs: s.GPUs, CPU: s.CPU, Memory: s.Memory, - Models: mergeModels(s.OllamaModels, s.LMStudioModels), + Clustered: s.clusterUUID() != "", + Models: mergeModels(s.Models, s.OllamaModels, s.LMStudioModels), ModelsByEngine: manualModelsByEngine(s), + LoadedByEngine: s.LoadedByEngine, } if s.Address != "" { en.Addresses = []string{s.Address} + en.TXT = []string{noderec.KeyIP + "=" + s.Address} } return en } +// manualEnriched is manualToEnriched plus the one field it cannot derive from +// the status alone: whether this node pins the peer's certificate. That answer +// lives in the trust store, and it is read live rather than cached, because it +// changes the moment a pairing completes or a member is removed. +func (b *Broker) manualEnriched(s manualNodeStatus) EnrichedNode { + en := manualToEnriched(s) + en.Trusted = b.holdsPinFor(s.clusterUUID()) + return en +} + +// manualDirectoryNode synthesizes the directory record a manual PAIR node would +// have had if it had been discovered, so every consumer of the discovery relay — +// both inference proxies, the scheduler's inventory, engine-manager's remote +// operations, the workload relay and the errors peer sync — treats it exactly +// like a pinned peer found over mDNS. That is the whole point: a peer reachable +// only across an overlay network is not a second kind of node, it is the same +// node with no multicast between here and there. +// +// ok is false when the node must NOT enter the directory: +// - it is a bare inference host (no service map): it has no proxy or engine +// manager to be a peer with, and is bridged into the local proxies instead. +// - it is this host (its hostUuid is ours): a manual entry naming ourselves +// must never become a routing target, or the proxy would forward to its own +// ingress. Identity is the exact test; an address comparison would miss the +// overlay name for this same machine. +// - the scanner already holds the node: the daemon's record is authoritative +// and carries evidence this synthesis cannot (its full ranked address list, +// its liveness probes), and two writers on one key would fight. +func (b *Broker) manualDirectoryNode(s manualNodeStatus, key string) (noderec.DirectoryNode, bool) { + if !s.PairNode || len(s.Services) == 0 || key == "" || s.Address == "" { + return noderec.DirectoryNode{}, false + } + if key == b.nodeID { + slog.Debug("manual node names this host; not folding it into the directory", "id", s.ID) + return noderec.DirectoryNode{}, false + } + if b.store.hasSource(key, sourceScanner) { + return noderec.DirectoryNode{}, false + } + services := make(map[noderec.ServiceKey]noderec.ServiceStatus, len(s.Services)) + for svc, port := range s.Services { + if svc != "" && port > 0 { + services[svc] = noderec.ServiceStatus{Port: port} + } + } + if len(services) == 0 { + return noderec.DirectoryNode{}, false + } + clusterUUID := s.clusterUUID() + return noderec.DirectoryNode{ + HostUUID: key, + Name: s.ID, + // The address the operator typed is this node's canonical one: it is the + // only route we know works, and unlike a discovered peer there is no + // published ranking to defer to. + IP: s.Address, + ClusterUUID: clusterUUID, + Trusted: b.holdsPinFor(clusterUUID), + Services: services, + GPUs: s.GPUs, + CPU: s.CPU, + Memory: s.Memory, + Models: s.Models, + ModelsByEngine: s.ModelsByEngine, + LoadedByEngine: s.LoadedByEngine, + LastSeen: time.Now().Unix(), + }, true +} + +// holdsPinFor reports whether this node pins the given cluster principal's +// certificate — the same question the scanner answers for a browsed peer, asked +// against live membership rather than a cached annotation. +func (b *Broker) holdsPinFor(clusterUUID string) bool { + if clusterUUID == "" || b.clusterDir == "" { + return false + } + mesh := clustertrust.Open(b.clusterDir) + mesh.Refresh() + return mesh.HasPin(clusterUUID) +} + +// applyManualDirectory folds a manual PAIR node into the discovery relay, or +// withdraws it when it stopped qualifying (it went down, it turned out to be a +// bare host, or the scanner took the record over). +func (b *Broker) applyManualDirectory(s manualNodeStatus, key string) { + node, ok := b.manualDirectoryNode(s, key) + if !ok { + b.releaseManualDirectory(key) + return + } + b.manualMu.Lock() + b.manualRelayKeys[key] = true + b.manualMu.Unlock() + b.relayDir.Apply(noderec.NotifyNodeUpdated, node) +} + +// releaseManualDirectory withdraws a record this broker synthesized. It withdraws +// only what it put there: the relay directory is keyed by hostUuid and the +// scanner writes the same keys, so removing one the daemon owns would evict a +// live discovered peer from every consumer until its next browse event. +func (b *Broker) releaseManualDirectory(key string) { + if key == "" { + return + } + b.manualMu.Lock() + synthesized := b.manualRelayKeys[key] + delete(b.manualRelayKeys, key) + b.manualMu.Unlock() + if !synthesized || b.store.hasSource(key, sourceScanner) { + return + } + b.relayDir.Apply(noderec.NotifyNodeRemoved, noderec.DirectoryNode{HostUUID: key}) +} + // manualModelsByEngine builds the per-engine attribution for a manual node from // the per-engine lists the prober already collected, keyed by the same // engine-manager engine names discovered nodes use ("ollama", "lmstudio") so the @@ -160,6 +311,16 @@ type proxyManualNode struct { // daemon to carry — so without this explicit add the proxies can't route // inference to them even though both workers are broker-owned. func (b *Broker) bridgeManualNode(s manualNodeStatus, key string) { + if s.PairNode { + // A PAIR node reaches the proxies through the discovery relay instead, + // which is what gets it dialed over cluster mTLS against its pinned + // certificate. Bridging it here as well would add a second candidate for + // the same node that the proxy dials in plaintext — straight into the + // peer's loopback-only refusal — so the raw-engine bridge is withdrawn + // rather than left alongside. + b.removeManualNodeFromProxies(key) + return + } b.bridgeToProxy(b.getProxy(), "ollama", s, key, s.OllamaUp, s.OllamaPort, s.OllamaModels) b.bridgeToProxy(b.getLMStudioProxy(), "lmstudio", s, key, s.LMStudioUp, s.LMStudioPort, s.LMStudioModels) } From 79ef58ce9d5cdd4b4c5556f186bf659aee442fab Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:34:03 +0100 Subject: [PATCH 4/8] tests: cross-process gate for a PAIR peer added by address Node A is a real broker with its real workers; node B is a real cluster-manager and a real ollama-proxy in front of a fake engine, plus the two HTTP surfaces a peer exposes. A pairs with B over the real PIN exchange addressed by nothing but an address and a port -- no nodeId, because A never discovered B and never will -- and is then told about it with one node/add. It asserts the three things "behaves like a discovered peer" has to mean: B appears in A's directory as a trusted, clustered peer with the model list A read from B's engine manager over cluster mTLS; A's proxy routes an inference request to B over cluster mTLS and B's engine serves it; and A opens zero plaintext connections to B's engine ports, which is the regression this change exists to fix. Two real brokers cannot share one loopback -- every broker-owned port is a compiled-in constant -- so B is a stub peer on ephemeral ports, reached through node/add's new per-service port overrides. Everything the assertions turn on is real: the identities, the pins, the pairing, the proxy, and the transport choice. Also adds the PairNode hysteresis the test implies. Recomputing it per probe meant one missed node-info answer -- three seconds, routine across an overlay network -- probed the peer's proxy facades in plaintext, blanked its service map and withdrew it from every consumer for a cycle. It now holds across a failure episode on the same counter discovery uses, and reverts past it. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- services/nvpair-manual-nodes/manager.go | 47 +- services/nvpair-manual-nodes/pairnode_test.go | 65 +++ services/nvpair-ui-broker/broker.go | 3 +- services/nvpair-ui-broker/discovery.go | 5 +- services/nvpair-ui-broker/main.go | 2 +- services/tests/broker_test.go | 3 + services/tests/manual_pair_peer_test.go | 453 ++++++++++++++++++ 7 files changed, 573 insertions(+), 5 deletions(-) create mode 100644 services/tests/manual_pair_peer_test.go diff --git a/services/nvpair-manual-nodes/manager.go b/services/nvpair-manual-nodes/manager.go index ecedad7a..b363e89d 100644 --- a/services/nvpair-manual-nodes/manager.go +++ b/services/nvpair-manual-nodes/manager.go @@ -379,6 +379,18 @@ func (m *Manager) probeAll(ctx context.Context) { } } +// lastProbe returns a node's previous status and consecutive-failure count, and +// whether it is still tracked at all (it can be removed mid-probe). +func (m *Manager) lastProbe(id string) (ManualNodeStatus, int, bool) { + m.mu.RLock() + defer m.mu.RUnlock() + tn, ok := m.nodes[id] + if !ok { + return ManualNodeStatus{}, 0, false + } + return tn.status, tn.consecutiveFails, true +} + func (m *Manager) probeNode(entry ManualEntry) { addr := entry.Address id := nodeID(entry) @@ -394,7 +406,27 @@ func (m *Manager) probeNode(entry ManualEntry) { // there. Its engines are read from its engine manager instead, and it is // routed to through those same facades over cluster mTLS. nodeInfoUp, info := m.probeNodeInfo(entry, ports) + + // What this node was on the previous cycle, read before anything else runs: + // the sticky decision below has to be made before the engine legs, not after. + last, lastFails, tracked := m.lastProbe(id) + if !tracked { + return + } + + // A single missed node-info answer must not turn a peer back into a stranger. + // Three seconds is a routine gap across an overlay network — a relayed path, a + // wake from sleep, a peer under load — and without hysteresis that gap would + // probe the peer's proxy facades in plaintext, blank its service map and + // withdraw it from every consumer, only to restore it 10 seconds later. + // Discovery tolerates three consecutive misses before evicting an mDNS node; + // this is the same tolerance, bounded by the same counter, so a node that + // genuinely stops being a PAIR node still reverts within one failure episode. pairNode := nodeInfoUp && len(info.Services) > 0 + sticky := !pairNode && last.PairNode && lastFails < probeFailThreshold + if sticky { + pairNode = true + } newStatus := ManualNodeStatus{ ID: id, @@ -418,7 +450,20 @@ func (m *Manager) probeNode(entry ManualEntry) { HostUUID: info.HostUUID, } - if pairNode { + if sticky { + // Carry the peer's identity forward across the gap, exactly as HostUUID is + // carried below: what a consumer holds must not oscillate because one + // probe timed out. NodeInfoUp stays false, so the node still reads as + // unreachable and the failure counter still runs. + newStatus.ClusterUUID = last.ClusterUUID + newStatus.Services = last.Services + newStatus.Models = last.Models + newStatus.ModelsByEngine = last.ModelsByEngine + newStatus.LoadedByEngine = last.LoadedByEngine + newStatus.GPUs = last.GPUs + newStatus.CPU = last.CPU + newStatus.Memory = last.Memory + } else if pairNode { models := m.fetchPeerModels(addr, info) newStatus.Models = models.Models newStatus.ModelsByEngine = models.ModelsByEngine diff --git a/services/nvpair-manual-nodes/pairnode_test.go b/services/nvpair-manual-nodes/pairnode_test.go index d4dff7c4..d3762cdf 100644 --- a/services/nvpair-manual-nodes/pairnode_test.go +++ b/services/nvpair-manual-nodes/pairnode_test.go @@ -15,6 +15,7 @@ package main import ( "encoding/json" + "errors" "net" "net/http" "strconv" @@ -27,6 +28,9 @@ import ( func strptr(s string) *string { return &s } +// errFakeTimeout stands in for a probe that got no answer. +var errFakeTimeout = errors.New("simulated probe timeout") + // probeSync registers an entry and probes it on this goroutine, so a test reads a // settled status instead of racing addNode's background probe. func probeSync(m *Manager, entry ManualEntry) { @@ -116,6 +120,67 @@ func TestProbeNode_PairNodeIsNotProbedOnItsEnginePorts(t *testing.T) { } } +// One missed node-info answer must not turn a peer back into a stranger. Three +// seconds is a routine gap across an overlay network, and without hysteresis that +// gap would probe the peer's proxy facades in plaintext, blank its service map, +// and withdraw it from every consumer for a cycle. Discovery tolerates three +// consecutive misses before evicting an mDNS node; this tolerates the same. +func TestProbeNode_PairNodeSurvivesAMissedProbe(t *testing.T) { + m, _, rt := newTestManager() + + var engineProbes atomic.Int32 + for _, port := range []string{"11434", "1234"} { + host := net.JoinHostPort("gpu-box.tail1234.ts.net", port) + for _, path := range []string{"/", "/api/tags", "/v1/models"} { + rt.set(http.MethodGet, host, path, func(*http.Request) (*http.Response, error) { + engineProbes.Add(1) + return httpJSON(http.StatusForbidden, `{"error":"loopback-only"}`) + }) + } + } + + answering := true + rt.set(http.MethodGet, net.JoinHostPort("gpu-box.tail1234.ts.net", "14318"), "/v1/node-info", + func(*http.Request) (*http.Response, error) { + if !answering { + return nil, errFakeTimeout + } + data, _ := json.Marshal(pairNodeInfo()) + return httpJSON(http.StatusOK, string(data)) + }) + + entry := ManualEntry{Address: "gpu-box.tail1234.ts.net"} + probeSync(m, entry) + + answering = false + for i := 0; i < probeFailThreshold; i++ { + m.probeNode(entry) + status := statusFor(t, m, nodeID(entry)) + if !status.PairNode { + t.Fatalf("probe %d: a missed node-info answer must not un-pair a peer", i+1) + } + if status.NodeInfoUp { + t.Fatalf("probe %d: the node must still read as unreachable", i+1) + } + if status.Services[noderec.ServiceEngineManager] != 14322 { + t.Fatalf("probe %d: services = %v, want the peer's map carried across the gap", i+1, status.Services) + } + if status.ClusterUUID == nil || *status.ClusterUUID != "peer-cluster-uuid" { + t.Fatalf("probe %d: the peer's principal must survive the gap", i+1) + } + } + if n := engineProbes.Load(); n != 0 { + t.Fatalf("%d plaintext engine probes during the gap, want 0", n) + } + + // Past the tolerance it does revert: a node that genuinely stopped being a + // PAIR node must not be remembered as one forever. + m.probeNode(entry) + if statusFor(t, m, nodeID(entry)).PairNode { + t.Fatal("past the failure threshold the node must revert to a bare host") + } +} + // A node-info answer with no service map is a host too old to report one, or one // that is not a PAIR node at all. Either way the only thing it can be reached as // is a bare inference host, so the plain engine probes still run. diff --git a/services/nvpair-ui-broker/broker.go b/services/nvpair-ui-broker/broker.go index bb8392b3..7e6d5429 100644 --- a/services/nvpair-ui-broker/broker.go +++ b/services/nvpair-ui-broker/broker.go @@ -75,7 +75,8 @@ type AvailableNode struct { // is the key clients should dedup/track nodes by; id/name stay the hostname // for display. Two machines sharing a hostname are distinct by HostUUID. It // is the same value the cluster surface exposes as nodeUuid. - // Omitted for manual nodes, which carry no UUID and are keyed by id. + // A manually added node carries one too: its real UUID once its node-info + // reports one, and its manual id until then, so it is never keyless. HostUUID string `json:"hostUuid,omitempty"` IPAddress string `json:"ipAddress"` // IPAddresses is every address this node published, in its own ranked order diff --git a/services/nvpair-ui-broker/discovery.go b/services/nvpair-ui-broker/discovery.go index 4f1ca124..23048c85 100644 --- a/services/nvpair-ui-broker/discovery.go +++ b/services/nvpair-ui-broker/discovery.go @@ -39,8 +39,9 @@ type EnrichedNode struct { // It's the discovery-store key, so a PC rename — which changes the hostname // (ID) but not the UUID — updates the existing entry in place instead of // leaving a ghost under the old name. It stays off the wire; the - // client-facing id/name remain the hostname. Empty for manual nodes, which - // fall back to keying by their own ID. + // client-facing id/name remain the hostname. A manual node carries one too: + // its real UUID once its node-info reports one, and its manual id until then + // (see manualToEnriched), so the store key is never empty. HostUUID string `json:"-"` Host string `json:"host"` Port int `json:"port"` diff --git a/services/nvpair-ui-broker/main.go b/services/nvpair-ui-broker/main.go index ced0d783..741b63b7 100644 --- a/services/nvpair-ui-broker/main.go +++ b/services/nvpair-ui-broker/main.go @@ -33,7 +33,7 @@ func main() { settingsPath := flag.String("settings-path", "", "path to nvpair-node-settings binary (default: ./nvpair-node-settings in the current working directory)") clusterMgrPath := flag.String("cluster-manager-path", "", "path to nvpair-cluster-manager binary (default: ./nvpair-cluster-manager in the current working directory)") schedulerPath := flag.String("scheduler-path", "", "path to nvpair-job-scheduler binary (default: ./nvpair-job-scheduler in the current working directory)") - clusterDirFlag := flag.String("cluster-dir", "", "cluster config dir (node.crt/node.key + trusted/) the broker passes to its mDNS workers (nvpair-errors, nvpair-workload-manager, nvpair-node-info, nvpair-node-scanner, nvpair-manual-nodes) to enable cluster-scoped inter-node mTLS; defaults to the per-user Nvidia Corporation/Personal AI Router cluster/ dir, where nvpair-cluster-manager mints them") + clusterDirFlag := flag.String("cluster-dir", "", "cluster config dir (node.crt/node.key + trusted/) the broker passes to its inter-node workers (nvpair-errors, nvpair-workload-manager, nvpair-node-scanner, nvpair-manual-nodes, nvpair-engine-manager, both proxies) to enable cluster-scoped inter-node mTLS. nvpair-node-info is deliberately NOT among them: its inventory is the one inter-node surface kept plain so any peer can read it; defaults to the per-user Nvidia Corporation/Personal AI Router cluster/ dir, where nvpair-cluster-manager mints them") showVersion := flag.Bool("version", false, "print version and exit") resolveLevel := applog.RegisterFlag(nil, slog.LevelInfo) flag.Parse() diff --git a/services/tests/broker_test.go b/services/tests/broker_test.go index 22944f5b..235b9015 100644 --- a/services/tests/broker_test.go +++ b/services/tests/broker_test.go @@ -41,9 +41,12 @@ import ( type availableNode struct { ID string `json:"id"` Name string `json:"name"` + HostUUID string `json:"hostUuid"` IPAddress string `json:"ipAddress"` Port int `json:"port"` LastSeen int64 `json:"lastSeen"` + Trusted bool `json:"trusted"` + Clustered bool `json:"clustered"` Models []string `json:"models,omitempty"` ModelsByEngine map[string][]string `json:"modelsByEngine,omitempty"` LoadedByEngine map[string][]string `json:"loadedByEngine,omitempty"` diff --git a/services/tests/manual_pair_peer_test.go b/services/tests/manual_pair_peer_test.go new file mode 100644 index 00000000..2a73bba2 --- /dev/null +++ b/services/tests/manual_pair_peer_test.go @@ -0,0 +1,453 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Cross-process gate for a PAIR peer that can only be reached by a typed +// address — the shape of every peer on a Tailscale tailnet, where no multicast +// crosses and nothing is ever discovered. +// +// Node A is a real broker with its real workers. Node B is a real ollama-proxy +// serving cluster mTLS in front of a fake engine, plus the two HTTP surfaces a +// peer exposes: a plain node-info reporting its identity and service map, and a +// pin-gated engine manager serving its model list. The two nodes cross-pin, as +// they would after pairing. +// +// A is then told about B with nothing but `node/add`, and must end up treating it +// exactly as if it had discovered it: +// +// - B appears in A's directory with its identity, hardware and models, the +// models having been read over cluster mTLS from B's engine manager; +// - A's proxy routes an inference request to B over cluster mTLS and B's +// engine serves it; +// - and A never opens a plaintext connection to B's engine ports. That is the +// regression gate: on a PAIR node those ports are proxy facades that refuse +// plaintext, so probing them reported a healthy peer as having no engines. +// +// Two real brokers cannot share one loopback — every broker-owned port is a +// compiled-in constant — so B is a stub peer on ephemeral ports, addressed +// through node/add's per-service port overrides. Everything the assertions turn +// on (the mTLS identities, the pins, the proxy, the transport choice) is real. + +package tests + +import ( + "bytes" + "crypto/tls" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "os" + "os/exec" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "nvpair-shared/clustertrust" + "nvpair-shared/jsonrpc" +) + +// connectionTrap is a listener that accepts and immediately closes, counting +// every connection. It stands in for a port A must never touch. +type connectionTrap struct { + port int + count atomic.Int32 +} + +func startConnectionTrap(t *testing.T) *connectionTrap { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("trap listen: %v", err) + } + trap := &connectionTrap{port: ln.Addr().(*net.TCPAddr).Port} + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + trap.count.Add(1) + _ = conn.Close() + } + }() + t.Cleanup(func() { _ = ln.Close() }) + return trap +} + +// startPeerNodeInfo serves B's /v1/node-info in plaintext, the way a PAIR node +// under a broker does. Its service map is what identifies B as a PAIR node and +// tells A where B's proxy and engine manager listen. +func startPeerNodeInfo(t *testing.T, hostUUID, clusterUUID string, services map[string]int) int { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("node-info listen: %v", err) + } + port := ln.Addr().(*net.TCPAddr).Port + services["ni"] = port + mux := http.NewServeMux() + mux.HandleFunc("/v1/node-info", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "GPUs": []map[string]any{{"name": "NVIDIA GeForce RTX 4090", "utilization_percent": 11}}, + "telemetryValid": true, + "msSince": 120, + "hostUuid": hostUUID, + "clusterUuid": clusterUUID, + "services": services, + }) + }) + srv := &http.Server{Handler: mux} + go func() { _ = srv.Serve(ln) }() + t.Cleanup(func() { _ = srv.Close() }) + return port +} + +// startPeerEngineManager serves B's /v1/models over cluster mTLS, refusing any +// caller B does not pin — the same gate the real engine manager applies. +func startPeerEngineManager(t *testing.T, mesh *clustertrust.Mesh, models []string) int { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("engine-manager listen: %v", err) + } + port := ln.Addr().(*net.TCPAddr).Port + mux := http.NewServeMux() + mux.HandleFunc("/v1/models", func(w http.ResponseWriter, r *http.Request) { + if _, ok := mesh.VerifyClientPin(r); !ok { + http.Error(w, "forbidden: not a pinned cluster peer", http.StatusForbidden) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "models": models, + "modelsByEngine": map[string][]string{"ollama": models}, + "loadedByEngine": map[string][]string{"ollama": {}}, + }) + }) + cfg := mesh.ServerTLSConfig() + if cfg == nil { + t.Fatal("the stub peer must be able to serve cluster mTLS") + } + srv := &http.Server{Handler: mux, TLSConfig: cfg} + go func() { _ = srv.Serve(tls.NewListener(ln, cfg)) }() + t.Cleanup(func() { _ = srv.Close() }) + return port +} + +// brokerProc drives a real broker over stdio. +type brokerProc struct { + t *testing.T + cmd *exec.Cmd + stdin io.WriteCloser + msgs <-chan jsonrpc.Message + buf []jsonrpc.Message + nextID int +} + +func startBrokerWithClusterDir(t *testing.T, clusterDir string) *brokerProc { + t.Helper() + cfg := t.TempDir() + cmd := exec.Command(brokerBin, + "--cluster-dir", clusterDir, + "--scanner-path", scannerBin, + "--node-info-path", nodeInfoBin, + "--proxy-path", proxyBin, + "--lmstudio-proxy-path", lmstudioProxyBin, + "--workload-manager-path", workloadMgrBin, + "--errors-path", errorsBin, + "--engine-manager-path", engineMgrBin, + "--manual-nodes-path", manualNodesBin, + "--settings-path", nodeSettingsBin, + "--cluster-manager-path", clusterMgrBin, + "--scheduler-path", schedulerBin, + ) + cmd.Env = append(os.Environ(), + "HOME="+cfg, "XDG_CONFIG_HOME="+cfg, "APPDATA="+cfg, "LOCALAPPDATA="+cfg, + ) + cmd.Stderr = os.Stderr + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatalf("broker stdin pipe: %v", err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatalf("broker stdout pipe: %v", err) + } + if err := cmd.Start(); err != nil { + t.Fatalf("start broker: %v", err) + } + b := &brokerProc{t: t, cmd: cmd, stdin: stdin, msgs: startMsgReader(stdout), nextID: 1} + t.Cleanup(func() { + _ = stdin.Close() + done := make(chan struct{}) + go func() { _ = cmd.Wait(); close(done) }() + select { + case <-done: + case <-time.After(8 * time.Second): + _ = cmd.Process.Kill() + <-done + } + }) + return b +} + +func (b *brokerProc) call(method string, params any) jsonrpc.Message { + b.t.Helper() + id := b.nextID + b.nextID++ + req := map[string]any{"jsonrpc": "2.0", "id": id, "method": method} + if params != nil { + req["params"] = params + } + raw, _ := json.Marshal(req) + raw = append(raw, '\n') + if _, err := b.stdin.Write(raw); err != nil { + b.t.Fatalf("write %s: %v", method, err) + } + resp := b.pump(func(m jsonrpc.Message) bool { return m.Method == "" && idEquals(m.ID, id) }, 20*time.Second) + if resp.Error != nil { + b.t.Fatalf("%s returned error %d: %s", method, resp.Error.Code, resp.Error.Message) + } + return resp +} + +func (b *brokerProc) pump(want func(jsonrpc.Message) bool, timeout time.Duration) jsonrpc.Message { + b.t.Helper() + for i, m := range b.buf { + if want(m) { + b.buf = append(b.buf[:i], b.buf[i+1:]...) + return m + } + } + timer := time.NewTimer(timeout) + defer timer.Stop() + for { + select { + case m, ok := <-b.msgs: + if !ok { + b.t.Fatal("broker stdout closed unexpectedly") + } + if want(m) { + return m + } + b.buf = append(b.buf, m) + case <-timer.C: + b.t.Fatal("timed out waiting on a broker message") + } + } +} + +func (b *brokerProc) nodes() []availableNode { + b.t.Helper() + var result struct { + Nodes []availableNode `json:"nodes"` + } + if err := json.Unmarshal(b.call("discovery:get-nodes", nil).Result, &result); err != nil { + b.t.Fatalf("decode discovery:get-nodes: %v", err) + } + return result.Nodes +} + +// awaitNode polls the directory until a node matching want appears. +func (b *brokerProc) awaitNode(hostUUID string, want func(availableNode) bool, why string) availableNode { + b.t.Helper() + deadline := time.Now().Add(45 * time.Second) + var last []availableNode + for time.Now().Before(deadline) { + last = b.nodes() + for _, n := range last { + if n.HostUUID == hostUUID && want(n) { + return n + } + } + time.Sleep(300 * time.Millisecond) + } + b.t.Fatalf("node %s never %s; directory = %+v", hostUUID, why, last) + return availableNode{} +} + +func TestManualPairPeerBehavesLikeADiscoveredPeer(t *testing.T) { + baseA, baseB := t.TempDir(), t.TempDir() + dirA := filepath.Join(baseA, "cluster") + dirB := filepath.Join(baseB, "cluster") + + // B runs a real cluster-manager so the pairing below is the real EAP-NOOB + // exchange, and the pins both sides end up holding are the ones it mints. + portB := freePort(t) + cmB := startCM(t, baseB, portB) + t.Cleanup(cmB.stop) + + // A is a real broker with its own real cluster-manager. + brokerA := startBrokerWithClusterDir(t, dirA) + brokerA.call("discovery:subscribe", nil) + brokerA.call("proxy:subscribe", nil) + + // The listeners need a moment to bind before pairing. + time.Sleep(time.Second) + + // Pair A to B by address and port alone — no nodeId, because A has not + // discovered B and never will. This is the invite an operator sends after + // typing a peer's overlay address into Add node. + pairFromBroker(t, brokerA, cmB, portB) + + bInfo := decodeResult[cmNodeID](t, cmB.call("cluster:get-node-id", nil)) + if bInfo.NodeUUID == "" { + t.Fatal("the peer must report a node uuid after pairing") + } + uuidB := bInfo.NodeUUID + + meshB := clustertrust.Open(dirB) + meshB.Refresh() + if !meshB.Clustered() { + t.Fatal("the peer must read as a cluster member after pairing") + } + + // B: a real ollama-proxy over cluster mTLS in front of a fake engine. + engineHost, enginePort, generates := startFakeOllama(t) + proxyB := startProxyProc(t, dirB, freePort(t)) + t.Cleanup(proxyB.stop) + proxyB.setLocalBackend("ollama", engineHost, enginePort, true) + + // B's two HTTP surfaces, and two traps standing where a bare host's engines + // would be. On a PAIR node those ports carry proxy facades; A must not touch + // them. + emPort := startPeerEngineManager(t, meshB, []string{"m:latest"}) + ollamaTrap := startConnectionTrap(t) + lmStudioTrap := startConnectionTrap(t) + niPort := startPeerNodeInfo(t, uuidB, uuidB, map[string]int{ + "ol": proxyB.port, + "em": emPort, + "cl": portB, + }) + + // Everything A is told about B. No discovery, no mDNS: one address and the + // ports its services sit on. + brokerA.call("node/add", map[string]any{ + "address": "127.0.0.1", + "name": "peer-b", + "ports": map[string]any{ + "node_info": niPort, + "ollama": ollamaTrap.port, + "lmstudio": lmStudioTrap.port, + }, + }) + + t.Run("B joins A's directory as a trusted peer with its models", func(t *testing.T) { + node := brokerA.awaitNode(uuidB, func(n availableNode) bool { + return len(n.Models) > 0 + }, "appeared with a model list") + if !node.Trusted { + t.Errorf("node.trusted = false, want true: A holds B's pin") + } + if !node.Clustered { + t.Errorf("node.clustered = false, want true: B reported a cluster principal") + } + if node.Models[0] != "m:latest" { + t.Errorf("models = %v, want B's inventory read over cluster mTLS", node.Models) + } + if node.IPAddress != "127.0.0.1" { + t.Errorf("ipAddress = %q, want the address the operator typed", node.IPAddress) + } + }) + + t.Run("A routes inference to B over cluster mTLS", func(t *testing.T) { + port := awaitRoutablePeerAtProxy(t, brokerA, uuidB) + before := atomic.LoadInt32(generates) + resp := postInference(t, fmt.Sprintf("http://127.0.0.1:%d/api/generate", port), + []byte(`{"model":"m:latest","prompt":"hi","stream":false}`)) + defer resp.Body.Close() + payload, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusOK { + t.Fatalf("A->B inference status = %d, body = %s", resp.StatusCode, payload) + } + if !bytes.Contains(payload, []byte("hello from the backend")) { + t.Fatalf("response did not come from B's engine: %s", payload) + } + if got := atomic.LoadInt32(generates); got != before+1 { + t.Fatalf("B engine generate count = %d, want %d", got, before+1) + } + }) + + // The regression gate. A PAIR node's engine ports are proxy facades that + // refuse plaintext from anything but loopback; probing them reports a healthy + // peer as having no engines, and bridging them gives the proxy a second route + // to the same node that can only ever 403. + t.Run("A never opens a plaintext connection to B's engine ports", func(t *testing.T) { + if n := ollamaTrap.count.Load(); n != 0 { + t.Errorf("%d plaintext connections to B's ollama port, want 0", n) + } + if n := lmStudioTrap.count.Load(); n != 0 { + t.Errorf("%d plaintext connections to B's lmstudio port, want 0", n) + } + }) +} + +// pairFromBroker drives a PIN pairing from the broker's own cluster-manager to a +// standalone peer, addressed by nothing but an address and a port. It is the +// invite an operator sends after typing a peer's overlay address into Add node: +// no nodeId, because the peer was never discovered and never will be. +func pairFromBroker(t *testing.T, b *brokerProc, joiner *cmProc, joinerPort int) { + t.Helper() + var invite struct { + InviteID string `json:"inviteId"` + State string `json:"state"` + Pin string `json:"pin"` + } + resp := b.call("cluster:invite-node", map[string]any{ + "address": "127.0.0.1", + "port": joinerPort, + }) + if err := json.Unmarshal(resp.Result, &invite); err != nil { + t.Fatalf("decode invite: %v", err) + } + if invite.State != "pending" || len(invite.Pin) != 6 { + t.Fatalf("invite-node = %+v, want pending with a six-digit pin", invite) + } + joiner.waitNotify("cluster:invite-received") + var accepted struct { + State string `json:"state"` + } + if err := json.Unmarshal(joiner.call("cluster:respond-to-invite", map[string]any{ + "inviteId": invite.InviteID, "accept": true, "pin": invite.Pin, + }).Result, &accepted); err != nil { + t.Fatalf("decode respond-to-invite: %v", err) + } + if accepted.State != "paired" { + t.Fatalf("respond-to-invite state = %q, want paired", accepted.State) + } +} + +// awaitRoutablePeerAtProxy waits until the broker's ollama-proxy lists the peer +// as a routing target and returns the proxy's listen port. +func awaitRoutablePeerAtProxy(t *testing.T, b *brokerProc, hostUUID string) int { + t.Helper() + deadline := time.Now().Add(45 * time.Second) + var lastNodes json.RawMessage + for time.Now().Before(deadline) { + var status struct { + Port int `json:"port"` + } + if err := json.Unmarshal(b.call("proxy:get-status", nil).Result, &status); err == nil && status.Port > 0 { + var listed struct { + Nodes []struct { + ID string `json:"id"` + } `json:"nodes"` + } + resp := b.call("proxy:nodes/list", nil) + lastNodes = resp.Result + if json.Unmarshal(resp.Result, &listed) == nil { + for _, n := range listed.Nodes { + if n.ID == hostUUID { + return status.Port + } + } + } + } + time.Sleep(300 * time.Millisecond) + } + t.Fatalf("the proxy never listed %s as a routing target; nodes = %s", hostUUID, lastNodes) + return 0 +} From 6914681f348b4bdc457241c737d1d254355a24ef Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:40:08 +0100 Subject: [PATCH 5/8] desktop: Add node creates the node, then pairs with it Add node only ever sent an invite. On a LAN that was enough, because discovery brought the peer in anyway. On a network that carries no multicast it is not: nothing writes manual-nodes.json, no record ever arrives, and a peer could pair successfully and stay invisible forever. The dialog now records the address as a manual node and tells the broker about it before inviting. That order is the useful one: the node appears with its hardware as soon as it answers, which is the only feedback an operator gets that a hand-typed address is right, and a pairing that fails leaves something on screen to retry against instead of nothing. The field is an address or a host name, with the guidance a VPN user needs -- use the MagicDNS name, it re-resolves -- and a Service ports disclosure for a node whose services do not sit on the defaults. The overrides are persisted with the entry, because the replay after a restart is the only thing that re-creates it: without them a node on non-default ports comes back unreachable. The two spellings of those ports (camelCase here, snake_case on the wire) meet in one projection, narrowed rather than cast. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- .../electron/service-bridge/empty-handlers.ts | 38 +++++- .../service-bridge/manual-nodes-store.ts | 83 +++++++++++- .../service-bridge/modular-supervisor.ts | 10 +- desktop/src/shared/types/manual-node.ts | 22 ++++ desktop/src/shared/types/ws-channels.ts | 13 +- desktop/src/ui/api/pair-api.ts | 6 +- desktop/src/ui/components/AddNodeModal.tsx | 105 ++++++++++++++- desktop/src/ui/hooks/useInvitePairing.ts | 13 +- .../modular/cluster-pairing-timeout.test.ts | 5 + .../tests/modular/manual-node-ports.test.ts | 120 ++++++++++++++++++ 10 files changed, 394 insertions(+), 21 deletions(-) create mode 100644 desktop/src/shared/types/manual-node.ts create mode 100644 desktop/tests/modular/manual-node-ports.test.ts diff --git a/desktop/src/electron/service-bridge/empty-handlers.ts b/desktop/src/electron/service-bridge/empty-handlers.ts index dc5c2dd8..a828a7f8 100644 --- a/desktop/src/electron/service-bridge/empty-handlers.ts +++ b/desktop/src/electron/service-bridge/empty-handlers.ts @@ -11,6 +11,7 @@ import { MODULAR_ENGINE_LIFECYCLE_CALL_TIMEOUT_MS } from '@/shared/constants/modular-runtime' import getErrorString from '@/shared/utils/get-error-string' +import { createStructuredLogger } from '@/shared/utils/log' import { getEngineHubModels } from '@/electron/model-hub' import { getModularSupervisor } from './modular-supervisor' import { @@ -23,7 +24,12 @@ import { import type { ProxyEngine } from './modular-state' import type { JsonObject, JsonValue } from './json-rpc-subprocess' import { emptyInvite, parseClusterNodes, parseInvite, parseNodeIdentity } from './cluster-json' -import { removeManualNodeEntry, resolveManualNodeKey } from './manual-nodes-store' +import { + addManualNodeEntry, + manualPortsToWire, + removeManualNodeEntry, + resolveManualNodeKey +} from './manual-nodes-store' type BridgeHandler = ( payload?: WsInvokeRequest @@ -34,6 +40,8 @@ type BridgeHandlerMap = { } // The broker allows pairing exchanges up to 30 seconds; keep the UI bridge outside that deadline. +const log = createStructuredLogger('service-bridge') + const CLUSTER_PAIRING_CALL_TIMEOUT_MS = 35_000 function wait(ms: number): Promise { @@ -798,6 +806,32 @@ async function handleClusterInviteNode( ): Promise { if (!payload) return emptyInvite() + // Add the address as a manual node BEFORE inviting it. + // + // On a network that carries no multicast — a Tailscale tailnet, a routed + // subnet — nothing is ever discovered, so a peer that is only paired stays + // invisible: no record arrives for it, ever. Adding it first also gives the + // operator the one piece of feedback that matters when an address was typed + // by hand: the node appears with its hardware as soon as it answers, whether + // or not the pairing that follows succeeds. + const supervisor = getModularSupervisor() + if (supervisor.hasProcess('broker')) { + const entry = addManualNodeEntry(payload.ipAddress, payload.ports) + const params: JsonObject = { address: entry.address, name: entry.name } + const wirePorts = manualPortsToWire(entry.ports) + if (wirePorts) params.ports = wirePorts + try { + await supervisor.callProcess('broker', 'node/add', params) + } catch (err) { + // Non-fatal: the entry is persisted and replayed on the next start, + // and the invite below is what the operator asked for. + log.warn({ + sublevel: 'manual-nodes', + message: `Failed to add manual node ${entry.address}: ${getErrorString(err)}` + }) + } + } + // cluster-manager auto-founds a solo cluster on the first invite while // unclustered (under inviteMu, with invite-created provenance). Do not // pre-call cluster:create here: parallel Invites used to race concurrent @@ -813,7 +847,7 @@ async function handleClusterInviteNode( 'cluster:invite-node', { address: payload.ipAddress, - port: MODULAR_CLUSTER_MANAGER_PORT + port: payload.ports?.cluster ?? MODULAR_CLUSTER_MANAGER_PORT }, CLUSTER_PAIRING_CALL_TIMEOUT_MS ) diff --git a/desktop/src/electron/service-bridge/manual-nodes-store.ts b/desktop/src/electron/service-bridge/manual-nodes-store.ts index d79ccea9..dc8896bd 100644 --- a/desktop/src/electron/service-bridge/manual-nodes-store.ts +++ b/desktop/src/electron/service-bridge/manual-nodes-store.ts @@ -4,12 +4,19 @@ import fs from 'fs' import path from 'path' import { getPaths } from '@/electron/globals' +import type { ManualServicePorts } from '@/shared/types/manual-node' import type { JsonObject, JsonValue } from './json-rpc-subprocess' interface ManualNodeEntry { id: string address: string name: string + /** + * Per-service port overrides, persisted because the replay after a restart + * is the only thing that re-creates the entry: without them a node reachable + * on non-default ports comes back unreachable. + */ + ports?: ManualServicePorts } function configFilePath(): string { @@ -25,6 +32,57 @@ function stringValue(value: JsonValue | undefined): string { return typeof value === 'string' ? value : '' } +/** A usable TCP port, or undefined for anything else. Narrowing, never a cast. */ +function portValue(value: JsonValue | undefined): number | undefined { + if (typeof value !== 'number' || !Number.isInteger(value)) return undefined + if (value < 1 || value > 65535) return undefined + return value +} + +function portsValue(value: JsonValue | undefined): ManualServicePorts | undefined { + const obj = objectValue(value) + if (!obj) return undefined + const ports: ManualServicePorts = { + nodeInfo: portValue(obj.nodeInfo), + cluster: portValue(obj.cluster), + ollama: portValue(obj.ollama), + lmstudio: portValue(obj.lmstudio), + vllm: portValue(obj.vllm) + } + return definedPorts(ports) +} + +/** + * Drops every unset field, and the object itself when nothing is set. An empty + * overrides object is not the same thing as none: it would persist and replay as + * a value the operator never chose. + */ +function definedPorts(ports: ManualServicePorts): ManualServicePorts | undefined { + const kept: ManualServicePorts = {} + if (ports.nodeInfo !== undefined) kept.nodeInfo = ports.nodeInfo + if (ports.cluster !== undefined) kept.cluster = ports.cluster + if (ports.ollama !== undefined) kept.ollama = ports.ollama + if (ports.lmstudio !== undefined) kept.lmstudio = ports.lmstudio + if (ports.vllm !== undefined) kept.vllm = ports.vllm + return Object.keys(kept).length > 0 ? kept : undefined +} + +/** + * Projects the overrides onto the snake_case field names `node/add` reads + * (`node_info`, `cluster`, `ollama`, `lmstudio`, `vllm`). The two spellings meet + * here and nowhere else. + */ +export function manualPortsToWire(ports: ManualServicePorts | undefined): JsonObject | undefined { + if (!ports) return undefined + const wire: JsonObject = {} + if (ports.nodeInfo !== undefined) wire.node_info = ports.nodeInfo + if (ports.cluster !== undefined) wire.cluster = ports.cluster + if (ports.ollama !== undefined) wire.ollama = ports.ollama + if (ports.lmstudio !== undefined) wire.lmstudio = ports.lmstudio + if (ports.vllm !== undefined) wire.vllm = ports.vllm + return Object.keys(wire).length > 0 ? wire : undefined +} + function entryValue(value: JsonValue | undefined): ManualNodeEntry | null { const obj = objectValue(value) if (!obj) return null @@ -34,7 +92,30 @@ function entryValue(value: JsonValue | undefined): ManualNodeEntry | null { const name = stringValue(obj.name) || address const id = stringValue(obj.id) || name - return { id, address, name } + const ports = portsValue(obj.ports) + return ports ? { id, address, name, ports } : { id, address, name } +} + +/** + * Record a manually added node so it survives a restart, and hand back the entry + * the broker should be told about. + * + * `nvpair-manual-nodes` keeps no durable state of its own — the list belongs to + * the application — and the backend keys the node by `name`, which is the + * address here so {@link resolveManualNodeKey} and the `node/remove` relay agree + * on it. Re-adding the same address replaces its entry rather than duplicating + * it, so changing a node's ports is just adding it again. + */ +export function addManualNodeEntry(address: string, ports?: ManualServicePorts): ManualNodeEntry { + const trimmed = address.trim() + const kept = ports ? definedPorts(ports) : undefined + const entry: ManualNodeEntry = kept + ? { id: trimmed, address: trimmed, name: trimmed, ports: kept } + : { id: trimmed, address: trimmed, name: trimmed } + const entries = listManualNodeEntries().filter(existing => existing.address !== trimmed) + entries.push(entry) + saveManualNodeEntries(entries) + return entry } export function listManualNodeEntries(): ManualNodeEntry[] { diff --git a/desktop/src/electron/service-bridge/modular-supervisor.ts b/desktop/src/electron/service-bridge/modular-supervisor.ts index 943d0f84..78478d94 100644 --- a/desktop/src/electron/service-bridge/modular-supervisor.ts +++ b/desktop/src/electron/service-bridge/modular-supervisor.ts @@ -38,7 +38,7 @@ import { isModularLogLevel, type ModularLogLevel } from '@/shared/constants/modular-runtime' -import { listManualNodeEntries } from './manual-nodes-store' +import { listManualNodeEntries, manualPortsToWire } from './manual-nodes-store' import { MODULAR_RUNTIME_BINARIES, modularBinaryFileName @@ -851,10 +851,10 @@ class ModularSupervisor { const entries = listManualNodeEntries() for (const entry of entries) { try { - await this.callProcess('broker', 'node/add', { - address: entry.address, - name: entry.name - }) + const params: JsonObject = { address: entry.address, name: entry.name } + const wirePorts = manualPortsToWire(entry.ports) + if (wirePorts) params.ports = wirePorts + await this.callProcess('broker', 'node/add', params) } catch (err) { log.warn({ sublevel: 'manual-nodes', diff --git a/desktop/src/shared/types/manual-node.ts b/desktop/src/shared/types/manual-node.ts new file mode 100644 index 00000000..87efbe92 --- /dev/null +++ b/desktop/src/shared/types/manual-node.ts @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Per-service port overrides for a manually added node. + * + * The defaults `nvpair-manual-nodes` assumes describe a machine it cannot + * introspect: a peer may run an engine on a second port, a whole node may be + * reachable only through a forwarded range, and a test may put two nodes on one + * loopback. An unset field keeps that service's default, so an entry overrides + * only what the operator meant to. + * + * `vllm` is carried and persisted but not probed yet; it is here so an entry + * written today keeps its meaning when that engine lands. + */ +export interface ManualServicePorts { + nodeInfo?: number + cluster?: number + ollama?: number + lmstudio?: number + vllm?: number +} diff --git a/desktop/src/shared/types/ws-channels.ts b/desktop/src/shared/types/ws-channels.ts index 07398eab..841a96e5 100644 --- a/desktop/src/shared/types/ws-channels.ts +++ b/desktop/src/shared/types/ws-channels.ts @@ -23,6 +23,7 @@ * - `export` is reserved for the handful of symbols consumed by the * service bridge generics — nothing else. */ +import type { ManualServicePorts } from '@/shared/types/manual-node' import type { EngineCommandPayload, EngineHubSearchResponse, @@ -75,7 +76,17 @@ export interface WsInvokeChannelMap { // Cluster — PIN-pairing handshake (nvpair-cluster-manager) 'cluster:get-initial': { request: void; response: ClusterInitialSnapshot } - 'cluster:invite-node': { request: { ipAddress: string }; response: Invite } + // Add a node by address and start pairing with it, in that order. The + // address is added as a manual node first so it appears with its hardware + // as soon as it answers -- on a network without multicast that is the only + // feedback the operator gets about whether the address is right, and it is + // also what keeps a successfully paired peer visible afterwards, since no + // discovery record will ever arrive for it. `ports` overrides the assumed + // port of any single service on that host. + 'cluster:invite-node': { + request: { ipAddress: string; ports?: ManualServicePorts } + response: Invite + } 'cluster:invite-status': { request: { inviteId: string }; response: Invite } 'cluster:respond-to-invite': { request: { inviteId: string; accept: boolean; pin?: string } diff --git a/desktop/src/ui/api/pair-api.ts b/desktop/src/ui/api/pair-api.ts index a0a34155..980e7c30 100644 --- a/desktop/src/ui/api/pair-api.ts +++ b/desktop/src/ui/api/pair-api.ts @@ -9,6 +9,7 @@ import type { ClusterNode, Invite } from '@/shared/types/cluster' +import type { ManualServicePorts } from '@/shared/types/manual-node' import type { NodeItem } from '@/shared/types/nodes' import type { ServiceError } from '@/shared/types/errors' import type { NodeItemMetrics } from '@/shared/types/metrics' @@ -51,7 +52,7 @@ export interface IClusterApi { /** Fetch cluster bootstrap state: identity, settings, and membership. */ getInitial(): Promise /** Start PIN pairing with a remote node; the returned invite carries the PIN to display. */ - inviteNode(ipAddress: string): Promise + inviteNode(ipAddress: string, ports?: ManualServicePorts): Promise /** Poll the state of an outbound pairing session. */ inviteStatus(inviteId: string): Promise /** Respond to an inbound invite: accept with the PIN from the inviter, or decline. */ @@ -152,7 +153,8 @@ export function createPairApi(transport: ServiceTransport): IPairApi { }, cluster: { getInitial: () => transport.invoke('cluster:get-initial'), - inviteNode: ipAddress => transport.invoke('cluster:invite-node', { ipAddress }), + inviteNode: (ipAddress, ports) => + transport.invoke('cluster:invite-node', { ipAddress, ports }), inviteStatus: inviteId => transport.invoke('cluster:invite-status', { inviteId }), respondToInvite: (inviteId, accept, pin) => transport.invoke('cluster:respond-to-invite', { inviteId, accept, pin }), diff --git a/desktop/src/ui/components/AddNodeModal.tsx b/desktop/src/ui/components/AddNodeModal.tsx index d54abde9..dc0f6ba5 100644 --- a/desktop/src/ui/components/AddNodeModal.tsx +++ b/desktop/src/ui/components/AddNodeModal.tsx @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { useCallback, useState } from 'react' +import { useCallback, useMemo, useState } from 'react' import { Button, Divider, @@ -19,6 +19,42 @@ import { InvitePairingPanel } from './InvitePairingPanel' import { useBlurOnOpen } from '@/ui/hooks/useBlurOnOpen' import { useInvitePairing } from '@/ui/hooks/useInvitePairing' import { useInvitablePeers } from '@/ui/hooks/useInvitablePeers' +import type { ManualServicePorts } from '@/shared/types/manual-node' + +/** + * The advanced port fields, in the order they are shown. A node on its default + * ports needs none of them; a node behind a forwarded range, or one running an + * engine somewhere else, needs exactly the one it moved. + */ +const PORT_FIELDS = [ + { key: 'nodeInfo', label: 'Node info', placeholder: '14318' }, + { key: 'cluster', label: 'Pairing', placeholder: '14321' }, + { key: 'ollama', label: 'Ollama', placeholder: '11434' }, + { key: 'lmstudio', label: 'LM Studio', placeholder: '1234' } +] as const + +type PortField = (typeof PORT_FIELDS)[number]['key'] + +type PortDrafts = Partial> + +/** + * Reads the typed ports, dropping anything that is not a usable TCP port so a + * half-typed field never travels as an override. Returns undefined when nothing + * was set: an empty overrides object is not the same as none. + */ +function draftPorts(drafts: PortDrafts): ManualServicePorts | undefined { + const ports: ManualServicePorts = {} + let any = false + for (const field of PORT_FIELDS) { + const raw = drafts[field.key]?.trim() + if (!raw) continue + const port = Number(raw) + if (!Number.isInteger(port) || port < 1 || port > 65535) continue + ports[field.key] = port + any = true + } + return any ? ports : undefined +} interface AddNodeModalProps { open: boolean @@ -28,12 +64,18 @@ interface AddNodeModalProps { export function AddNodeModal({ open, onOpenChange }: AddNodeModalProps) { useBlurOnOpen(open) const [manualIp, setManualIp] = useState('') + const [showPorts, setShowPorts] = useState(false) + const [portDrafts, setPortDrafts] = useState({}) const pairing = useInvitePairing() const nodesThatCanBeAdded = useInvitablePeers() + const ports = useMemo(() => draftPorts(portDrafts), [portDrafts]) + const handleOpenChange = useCallback( (next: boolean) => { setManualIp('') + setShowPorts(false) + setPortDrafts({}) pairing.reset() onOpenChange(next) }, @@ -41,10 +83,14 @@ export function AddNodeModal({ open, onOpenChange }: AddNodeModalProps) { ) const handleManualInvite = useCallback(() => { - const ip = manualIp.trim() - if (!ip) return - void pairing.start(ip) - }, [manualIp, pairing]) + const address = manualIp.trim() + if (!address) return + void pairing.start(address, ports) + }, [manualIp, ports, pairing]) + + const setPortDraft = useCallback((field: PortField, value: string) => { + setPortDrafts(previous => ({ ...previous, [field]: value })) + }, []) const showPairing = pairing.invite !== null || pairing.error !== null const inviteInFlight = pairing.submitting || pairing.invite?.state === 'pending' @@ -73,7 +119,7 @@ export function AddNodeModal({ open, onOpenChange }: AddNodeModalProps) { ) : ( <> - + + + On a VPN or overlay network such as Tailscale, use the host name + — for example gpu-box.tail1234.ts.net. A name is re-resolved on + every check, so the node keeps working after it changes address. + + + + + {showPorts && ( + + + Leave a field empty to use this service's + default port. Enter the address on its own above — + ports belong here, not after a colon. + + + {PORT_FIELDS.map(field => ( + + + setPortDraft(field.key, value) + } + placeholder={field.placeholder} + inputMode="numeric" + disabled={inviteInFlight} + /> + + ))} + + + )} + {nodesThatCanBeAdded.length > 0 && ( diff --git a/desktop/src/ui/hooks/useInvitePairing.ts b/desktop/src/ui/hooks/useInvitePairing.ts index d7b0e6bb..cb562261 100644 --- a/desktop/src/ui/hooks/useInvitePairing.ts +++ b/desktop/src/ui/hooks/useInvitePairing.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import type { Invite } from '@/shared/types/cluster' +import type { ManualServicePorts } from '@/shared/types/manual-node' import { MODULAR_INVITE_STATUS_POLL_INTERVAL_MS } from '@/shared/constants/modular-runtime' import getErrorString from '@/shared/utils/get-error-string' import { formatClusterInviteError } from '@/ui/utils/cluster-invite-error' @@ -13,8 +14,12 @@ interface InvitePairing { /** True while the initial `cluster:invite-node` request is in flight. */ submitting: boolean error: string | null - /** Begin PIN pairing with a node, then poll its status until it resolves. */ - start: (ipAddress: string) => Promise + /** + * Add a node by address and begin PIN pairing with it, then poll its status + * until it resolves. `ports` overrides the assumed port of any single service + * on that host. + */ + start: (address: string, ports?: ManualServicePorts) => Promise /** * Cancel a still-pending outbound invite: tell the backend to tear down the * pairing session (invalidating the PIN so a remote user can no longer @@ -73,12 +78,12 @@ export function useInvitePairing(): InvitePairing { }, [reset, stopPolling]) const start = useCallback( - async (ipAddress: string) => { + async (address: string, ports?: ManualServicePorts) => { setSubmitting(true) setError(null) stopPolling() try { - const result = await window.pairApi.cluster.inviteNode(ipAddress) + const result = await window.pairApi.cluster.inviteNode(address, ports) setInvite(result) if (result.state === 'pending' && result.inviteId) { inviteIdRef.current = result.inviteId diff --git a/desktop/tests/modular/cluster-pairing-timeout.test.ts b/desktop/tests/modular/cluster-pairing-timeout.test.ts index 4811d208..eac88fc0 100644 --- a/desktop/tests/modular/cluster-pairing-timeout.test.ts +++ b/desktop/tests/modular/cluster-pairing-timeout.test.ts @@ -16,6 +16,11 @@ const mocks = vi.hoisted(() => ({ }, supervisor: { callProcess: vi.fn(), + // An outbound invite now adds the address as a manual node first, so it + // stays visible on a network that discovers nothing. These tests are + // about the pairing RPC timeout, so the broker reads as absent and that + // step is skipped. + hasProcess: vi.fn(() => false), markAutoCreatedSoloForInvite: vi.fn() } })) diff --git a/desktop/tests/modular/manual-node-ports.test.ts b/desktop/tests/modular/manual-node-ports.test.ts new file mode 100644 index 00000000..5fe04912 --- /dev/null +++ b/desktop/tests/modular/manual-node-ports.test.ts @@ -0,0 +1,120 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from 'fs' +import os from 'os' +import path from 'path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +let userDataDir = '' + +vi.mock('electron', () => ({ BrowserWindow: { getAllWindows: () => [] } })) +vi.mock('@/electron/globals', () => ({ + getPaths: () => ({ getUserData: () => userDataDir }) +})) + +import { + addManualNodeEntry, + listManualNodeEntries, + manualPortsToWire, + removeManualNodeEntry, + resolveManualNodeKey +} from '@/electron/service-bridge/manual-nodes-store' + +// The durable manual-node list is what makes a peer on a network without +// multicast visible at all: nothing is ever discovered there, so the entry the +// application persists and replays is the node's only route into the directory. +// Its port overrides have to survive that round trip, or a node reachable on +// non-default ports comes back unreachable after a restart. +describe('manual node entries', () => { + beforeEach(() => { + userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'pair-manual-nodes-')) + }) + + afterEach(() => { + fs.rmSync(userDataDir, { recursive: true, force: true }) + }) + + it('persists an address with no overrides and keys it by the address', () => { + const entry = addManualNodeEntry('gpu-box.tail1234.ts.net') + expect(entry).toEqual({ + id: 'gpu-box.tail1234.ts.net', + address: 'gpu-box.tail1234.ts.net', + name: 'gpu-box.tail1234.ts.net' + }) + expect(listManualNodeEntries()).toEqual([entry]) + }) + + it('round-trips port overrides through the persisted list', () => { + addManualNodeEntry('gpu-box.tail1234.ts.net', { nodeInfo: 24318, ollama: 21434 }) + const [entry] = listManualNodeEntries() + expect(entry.ports).toEqual({ nodeInfo: 24318, ollama: 21434 }) + }) + + it('drops ports that are not usable, and the object when none are', () => { + addManualNodeEntry('a.example', { nodeInfo: 0, ollama: 70000, cluster: 14321 }) + expect(listManualNodeEntries()[0].ports).toEqual({ cluster: 14321 }) + + addManualNodeEntry('b.example', { nodeInfo: -1 }) + const b = listManualNodeEntries().find(entry => entry.address === 'b.example') + expect(b?.ports).toBeUndefined() + }) + + it('replaces rather than duplicates when the same address is added again', () => { + addManualNodeEntry('gpu-box.tail1234.ts.net', { ollama: 21434 }) + addManualNodeEntry('gpu-box.tail1234.ts.net', { ollama: 21435 }) + const entries = listManualNodeEntries() + expect(entries).toHaveLength(1) + expect(entries[0].ports).toEqual({ ollama: 21435 }) + }) + + it('resolves a host-name entry from the node addresses the backend reports', () => { + addManualNodeEntry('gpu-box.tail1234.ts.net') + // A node added by name is reported by that name, since a name is a + // dialable address like any other. + expect(resolveManualNodeKey(['gpu-box.tail1234.ts.net'])).toBe('gpu-box.tail1234.ts.net') + expect(resolveManualNodeKey(['192.0.2.10'])).toBeNull() + }) + + it('forgets an entry on removal', () => { + addManualNodeEntry('gpu-box.tail1234.ts.net') + removeManualNodeEntry('gpu-box.tail1234.ts.net') + expect(listManualNodeEntries()).toEqual([]) + }) + + it('ignores a persisted ports value that is not an object', () => { + const file = path.join(userDataDir, 'configs', 'manual-nodes.json') + fs.mkdirSync(path.dirname(file), { recursive: true }) + fs.writeFileSync(file, JSON.stringify([{ address: 'a.example', ports: 'nonsense' }])) + expect(listManualNodeEntries()[0].ports).toBeUndefined() + }) +}) + +// The application spells the overrides in camelCase and the service reads them +// in snake_case. The two spellings meet in one projection, so a rename cannot +// silently drop a field on the way to the backend. +describe('manualPortsToWire', () => { + it('projects every field onto the names node/add reads', () => { + expect( + manualPortsToWire({ + nodeInfo: 24318, + cluster: 24321, + ollama: 21434, + lmstudio: 2234, + vllm: 8001 + }) + ).toEqual({ + node_info: 24318, + cluster: 24321, + ollama: 21434, + lmstudio: 2234, + vllm: 8001 + }) + }) + + it('omits unset fields, and the object itself when nothing is set', () => { + expect(manualPortsToWire({ ollama: 21434 })).toEqual({ ollama: 21434 }) + expect(manualPortsToWire({})).toBeUndefined() + expect(manualPortsToWire(undefined)).toBeUndefined() + }) +}) From b1235fc7fd2c456fa24aa0212a61421854636b6b Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:44:15 +0100 Subject: [PATCH 6/8] docs: running PAIR across a Tailscale tailnet, and version bumps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A new page for the case the rest of the documentation did not cover: two machines that can reach each other but have no local link between them. What a tailnet changes (no discovery, CGNAT addresses, MagicDNS names), the add-by-name → pair → verify walkthrough, a Tailscale ACL grant that opens exactly the eight ports PAIR needs between tagged nodes and says what each is for, and the four failures that are specific to this setup -- node with no models, 403 from a peer, a name that will not resolve, and a node that was found and went quiet. Linked from the README reading order, from the empty-discovery section of Troubleshooting (where an empty list is the expected result, not a fault), and from the Manual Nodes section of Architecture, which now describes the two kinds of manual node rather than one. The README and SECURITY.md said prompts remain on the local network. They now say the network you route them over -- your local network, or an encrypted overlay you configured -- which is what was always true and is now reachable. SECURITY.md's local-network boundary section says what an overlay changes about it: the encryption and the pinning are the same, but everyone admitted to the overlay is on that boundary. Versions: MINOR for node-info, manual-nodes, ui-broker and both proxies -- each gained behavior visible over IPC or HTTP, and a node reachable only by name is newly routable. PATCH for scanner, errors, workload-manager and cluster-manager, which only pick up the netpick change. Product and installer MINOR. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- README.md | 19 ++- SECURITY.md | 20 ++- docs/architecture.mdx | 33 +++- docs/remote-networks.mdx | 210 +++++++++++++++++++++++++ docs/troubleshooting.mdx | 6 + services/nvpair-manual-nodes/README.md | 29 +++- services/nvpair-node-info/README.md | 7 +- services/nvpair-ui-broker/README.md | 16 +- services/versions.json | 22 +-- 9 files changed, 322 insertions(+), 40 deletions(-) create mode 100644 docs/remote-networks.mdx diff --git a/README.md b/README.md index 0f0a7242..9b0b45d2 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,12 @@ requests can be routed to eligible nodes according to engine availability, model availability, and current workload. PAIR is useful for concurrent local workloads such as multi-agent applications. -Prompts and responses are intended to remain on the local network when every -configured client, model source, engine, and node is local. +Prompts and responses are intended to stay on the network you route them over — +your local network, or an encrypted overlay such as a Tailscale tailnet that you +configured yourself — when every configured client, model source, engine, and +node is one of yours. Refer to +[Running PAIR across a Tailscale tailnet](docs/remote-networks.mdx) for nodes +that are not on the same local link. > PAIR routes each independent request to one node. It does **not** pool GPU > memory, combine GPUs into a larger logical GPU, shard one model across @@ -224,19 +228,22 @@ Each entry assumes the ones before it. 4. **[Terminal interface](docs/terminal-interface.mdx)** — the same tasks from a terminal, for a machine with no desktop environment. Skip it if every machine you run has a desktop. -5. **[Troubleshooting](docs/troubleshooting.mdx)** — worth skimming once before +5. **[Remote networks](docs/remote-networks.mdx)** — read this if your machines + are joined by a VPN or overlay network rather than a local link: discovery + does not cross one, so you add the peer by name instead. +6. **[Troubleshooting](docs/troubleshooting.mdx)** — worth skimming once before you need it, so you know where the diagnostics live. Alongside it, **[Known issues](docs/known-issues.mdx)** lists the significant limitations we are already aware of, and **[Collecting and sanitizing logs](docs/log-collection.mdx)** covers preparing a log you can share. -6. **[Architecture](docs/architecture.mdx)** — the process model, how a request is +7. **[Architecture](docs/architecture.mdx)** — the process model, how a request is routed, and where the trust boundaries are. Read this before changing anything, or if you want to know why PAIR behaves the way it does. -7. **[Building and running](docs/building.mdx)** — prerequisites, building from +8. **[Building and running](docs/building.mdx)** — prerequisites, building from source, running the services without the desktop application, and writing your own client against the JSON-RPC API. -8. **[Developer guide](docs/developing.mdx)** — read this before contributing: +9. **[Developer guide](docs/developing.mdx)** — read this before contributing: where the code lives, how a change travels through the layers, and the conventions the project enforces. diff --git a/SECURITY.md b/SECURITY.md index 957f9da4..8b3639fe 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -48,7 +48,10 @@ PAIR is a LAN-first, multi-process application: JSON-RPC 2.0 over the broker's standard input and output. - The broker supervises Go workers and relays their control-plane methods and notifications. -- Discovery and selected metadata endpoints operate on the local network. +- Discovery and selected metadata endpoints operate on the local network, or on + an encrypted overlay network the operator configured, such as a Tailscale + tailnet. Discovery itself is multicast and does not cross one; a node on the + far side is added by address. - Ollama-compatible and OpenAI-compatible local HTTP proxies carry inference traffic and may route a request to another paired node. - The cluster manager uses a six-digit PIN to bootstrap trust. Cluster-scoped @@ -76,7 +79,7 @@ open relay for inference to anything that can route to it. Run an application on a node and use that node's local endpoint. Exposing an engine to the network directly is outside PAIR and is the operator's decision and risk. -### Local Network Is a Trust-Relevant Boundary +### The Network You Route Over Is a Trust-Relevant Boundary PAIR discovers nodes and exposes service metadata on the LAN. Some discovery enrichment and node-information traffic can use plain HTTP. Treat an untrusted @@ -84,9 +87,18 @@ Wi-Fi, shared office network, compromised router, and hostile local process as potentially adversarial. Network segmentation and host firewall rules remain the operator's responsibility. +Nodes joined by an encrypted overlay network such as a Tailscale tailnet, rather +than by a local link, are the same boundary reached a different way. The overlay +carries its own encryption and its own admission policy, and PAIR's inter-node +traffic is certificate-pinned mutual TLS in either case; what changes is that +everyone admitted to the overlay is on this boundary, so who may join it is the +operator's decision. Refer to +[Running PAIR across a Tailscale tailnet](docs/remote-networks.mdx). + “Local-first” describes the intended topology. It does not prove that no data -leaves the machine or LAN. Inference engines, model catalogs, update systems, -applications, and user configuration may contact external services. +leaves the machine, the LAN, or an overlay you configured. Inference engines, +model catalogs, update systems, applications, and user configuration may contact +external services. ### Pairing PIN Is a Bootstrap Convenience diff --git a/docs/architecture.mdx b/docs/architecture.mdx index f7e7ffed..f8c94fac 100644 --- a/docs/architecture.mdx +++ b/docs/architecture.mdx @@ -646,11 +646,34 @@ its address. ### Manual Nodes -Some networks block or filter multicast, so discovery is not the only path in. -`nvpair-manual-nodes` takes an address you enter directly and probes it on a -fixed interval, and a manual node that answers is folded into the same directory -as a discovered one. It is initially keyed by the address you typed, and re-keyed -to the peer's real UUID as soon as that node reports it. +Some networks block or filter multicast, and some never carry it at all — a +Tailscale tailnet, a WireGuard tunnel, a routed link — so discovery is not the +only path in. `nvpair-manual-nodes` takes an address you enter directly, a name +or a literal, and probes it on a fixed interval. It is initially keyed by the +address you typed, and re-keyed to the peer's real UUID as soon as that node +reports it. + +What it finds decides how the node is used, and there are two answers: + +- **A bare inference host.** Ollama or LM Studio on a machine that does not run + PAIR. Its engines answer plain HTTP on their own ports, and it is bridged into + this node's proxies as a routing target. This is what manual nodes were + originally for. +- **Another PAIR node.** It answers on its node-info port with its identity, its + cluster principal, and the set of services it runs — the same set it would have + carried on an mDNS record that never arrives here. From that, this node + synthesizes the directory record discovery would have produced, and the peer is + a peer: inventory over mutual TLS from its engine manager, inference routed to + its proxy over mutual TLS, telemetry into the scheduler. Nothing downstream + knows it was typed rather than discovered. + +The distinction matters because a PAIR node's engine ports are proxy front doors +that refuse plaintext from anything but their own loopback. Probing them would +report a healthy peer as having no engines, so a node that identifies itself as a +PAIR node is never asked there. + +Refer to [Running PAIR across a Tailscale tailnet](remote-networks.mdx) for the +operator's walkthrough. ## Trust Boundaries diff --git a/docs/remote-networks.mdx b/docs/remote-networks.mdx new file mode 100644 index 00000000..29a31ba8 --- /dev/null +++ b/docs/remote-networks.mdx @@ -0,0 +1,210 @@ +{/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 +*/} + +# Running PAIR across a Tailscale tailnet + +PAIR finds peers by multicast DNS, which never leaves a local link. Two machines +joined only by a Tailscale tailnet, a WireGuard tunnel, or a routed link will not +discover each other no matter how long you wait. Nothing is wrong with either +one: the announcement has nowhere to go. + +They can still be a cluster. You add the peer by address instead of waiting for +it to appear, and everything after that — pairing, inventory, telemetry, and +routing inference — works exactly as it does on a LAN, over the same +certificate-pinned mutual TLS. PAIR's trust is in the certificates, not in the +route the packets took. + +This page uses Tailscale because it is the common case. The same steps apply to +any network where the two machines can open TCP connections to each other but +multicast does not cross. + +## What changes on a tailnet + +**No discovery.** Nothing appears under **Add node** on its own. This is the only +part of PAIR that a tailnet actually breaks, and adding the peer by address is +the whole fix. + +**Addresses look unusual.** A Tailscale node's IPv4 address is in `100.64.0.0/10` +— carrier-grade NAT space, not a private LAN range — and its IPv6 address is a +unique local address under `fd7a:115c:a1e0::/48`. Both are ordinary addresses to +PAIR. When a node has a LAN address as well, PAIR prefers the LAN one, because a +peer on the same LAN reaches it faster; when the tailnet address is all a node +has, it is used without hesitation. + +**Use the MagicDNS name.** Tailscale gives every node a name like +`gpu-box.tail1234.ts.net`. Prefer it over the numeric address. PAIR re-resolves a +name on every check, so a node that is re-authenticated, re-created, or simply +handed a different address keeps working; a node added by IP address is stranded +the moment its address changes and has to be re-added by hand. + +**Latency is higher, and sometimes much higher.** A tailnet connection is usually +direct, but it falls back to a relay when both machines are behind restrictive +NATs. Inference still works; the first token just takes longer to arrive. + +## Add a node by host name, pair it, and check it + +Do this on one machine. Both must have PAIR running, and both must be on the same +tailnet. + +1. **Confirm the tailnet reaches the peer.** On the machine you are adding + *from*: + + ```bash + tailscale status # the peer should be listed and online + tailscale ping gpu-box # a direct or relayed path, either is fine + ``` + + On Windows, the command is `"C:\Program Files\Tailscale\tailscale.exe" status`. + On macOS, the CLI lives inside the app bundle at + `/Applications/Tailscale.app/Contents/MacOS/Tailscale`. + +2. **Open Add node** and type the peer's MagicDNS name — `gpu-box.tail1234.ts.net` + — into **Address or host name**. Enter the name on its own. Do not append a + port: PAIR adds each service's own port to what you type, so `host:14318` + becomes unreachable, and PAIR will tell you so rather than accepting it. + + If any of the peer's services are not on their default ports, open **Service + ports** and fill in only the ones that moved. Everything you leave blank keeps + its default. + +3. **Select Invite.** PAIR adds the node first and then starts pairing, so the + node appears in your node list within about ten seconds whether or not the + pairing succeeds. That is the fastest way to find out whether the address was + right. + +4. **Read the PIN to the other machine.** Someone there accepts the invitation + and types the six digits. This is the same PIN pairing as on a LAN. + +5. **Check the result.** The peer's card should show its GPU, CPU, and memory + within a probe cycle, and its models shortly after the pairing completes. + Models arrive only once you are paired: a peer serves its model list to + certificates it has pinned and to nobody else. + +6. **Route something to it.** Send a request to your local endpoint and confirm + the job is attributed to the remote node in the jobs view. + +Repeat from the other machine only if you want it to be able to *start* pairings +too; membership itself is mutual after one successful pairing. + +## Opening the right ports in a Tailscale ACL + +Tailscale's default policy allows everything between your own nodes, in which +case nothing here is required. If you have narrowed it, this grant opens exactly +what PAIR needs between machines tagged `tag:pair`, and nothing else: + +```json +{ + "tagOwners": { + "tag:pair": ["autogroup:admin"] + }, + "acls": [ + { + "action": "accept", + "src": ["tag:pair"], + "dst": ["tag:pair:1234,11434,14318,14319,14320,14321,14322,14323"] + } + ] +} +``` + +What each port is for: + +| Port | Service | Why a peer needs it | +| --- | --- | --- | +| `14318` | Node info | Hardware inventory, node identity, and the list of services this node runs. It is how a node added by address is recognized as a PAIR node at all. | +| `14321` | Cluster manager | PIN pairing, then the mutually authenticated membership channel. | +| `14322` | Engine manager | The peer's model inventory. Mutual TLS; a peer you have not paired with gets nothing. | +| `14323` | Engine control | Remote engine and model operations. Mutual TLS, cluster members only. | +| `11434` | Ollama-compatible proxy | Where inference is routed. Mutual TLS from a peer; plaintext is refused from anything but the machine's own loopback. | +| `1234` | OpenAI-compatible proxy | The same, for the OpenAI-compatible surface. | +| `14319` | Error sync | Cross-node error reporting. Mutual TLS. | +| `14320` | Workload manager | Job and telemetry exchange. Mutual TLS. | + +Two things this table is worth reading carefully for. First, `11434` and `1234` +on a PAIR node are **proxy front doors, not the engines themselves**. The engines +listen on loopback only and are never exposed. A peer connects to the front door +with its pinned certificate and the proxy passes the request to the local engine. +Second, `14318` is the one surface deliberately served in plaintext, so that a +machine which has not paired yet can still be seen. It carries hardware +inventory, a node identity, and port numbers — no prompts, no responses, no keys. + +Do not port-forward any of these on a public interface. The point of running PAIR +over a tailnet is that these ports are reachable only from machines you have +admitted to it. + +## Troubleshooting + +### The node appears but has no models + +Expected until the pairing completes. A peer serves its model list only over +mutual TLS to a certificate it has pinned. Check that the node card shows the +peer as paired; if the pairing was declined or expired, start it again from +**Add node** using the same address. + +If it is paired and models are still missing, the peer's engine-manager port +(`14322`) is most likely blocked. Confirm with: + +```bash +tailscale ping gpu-box +``` + +and check the ACL grant above. + +### 403 from the peer + +A `403` means the connection reached the peer and the peer declined to serve it. +Two causes, and they are worth telling apart: + +- **Not paired, or no longer paired.** The peer serves its inventory, engine + control, workloads, and inference only to certificates it pins. Removing a node + from a cluster takes effect on the very next request, so a node that worked + yesterday and 403s today may simply have been removed. Pair again. +- **Something dialed a proxy port in plaintext.** `11434` and `1234` accept + plaintext only from the machine's own loopback. If you are testing by hand with + `curl http://gpu-box.tail1234.ts.net:11434/`, a `403` is the correct answer and + not a fault. Route inference through your *local* endpoint and let PAIR make the + connection. + +### The host name does not resolve + +MagicDNS has to be enabled for the tailnet, and the machine has to be using +Tailscale's resolver. + +- **Everywhere:** confirm MagicDNS is on in the Tailscale admin console under + **DNS**, and that `tailscale status` shows the peer's full name. +- **Windows:** MagicDNS names sometimes fail to resolve when another VPN client + or a corporate DNS policy has claimed the resolver. Test with + `Resolve-DnsName gpu-box.tail1234.ts.net`. If it fails while `tailscale ping + gpu-box` succeeds, restart the Tailscale service, and if that does not help, + add the node by its `100.x.y.z` address instead. It works; it just will not + survive the node changing address. +- **Linux:** a host using `systemd-resolved` needs Tailscale's resolver + registered. `tailscale status` reporting the peer while + `getent hosts gpu-box.tail1234.ts.net` returns nothing points at that. + +### The node was found, then went unreachable + +PAIR checks a manual node every ten seconds and reports it after three +consecutive failures, so about thirty seconds of silence. A relayed tailnet path +can be slow enough to miss a check without the node being down; PAIR tolerates +that and keeps the peer in the routing pool across a short gap. + +A node added by IP address that never comes back has probably changed address. +Re-add it by its MagicDNS name so it can recover on its own next time. + +### Both machines are on a LAN *and* a tailnet + +Nothing to do. They will discover each other over the LAN, and PAIR prefers the +LAN address because it is the faster path. Adding the tailnet name as well is +harmless — the discovered record wins and the manual entry stands down. + +## Reading next + +- [Getting started](getting-started.mdx) — the LAN version of the same walkthrough +- [Architecture](architecture.mdx#manual-nodes) — how a node added by address is + folded into the same directory as a discovered one +- [Troubleshooting](troubleshooting.mdx) — everything that is not specific to a + remote network +- [SECURITY.md](../SECURITY.md) — what PAIR does and does not defend against diff --git a/docs/troubleshooting.mdx b/docs/troubleshooting.mdx index e4d8822c..a1949b4e 100644 --- a/docs/troubleshooting.mdx +++ b/docs/troubleshooting.mdx @@ -46,6 +46,12 @@ PAIR discovers peers on the local network. If **Add node** or - Allow PAIR through host firewalls on each system. - Retry with the peer's IP address in **Add node**. +**If the systems are joined by a VPN or overlay network rather than a local +link, discovery will never find them.** Multicast does not cross a tailnet, a +WireGuard tunnel, or a routed link, so an empty list is the expected result +rather than a fault. Add the peer by name instead: +[Running PAIR across a Tailscale tailnet](remote-networks.mdx). + ## Pairing Fails or Stalls A PIN belongs to one invitation attempt. A mistyped PIN, a canceled or expired diff --git a/services/nvpair-manual-nodes/README.md b/services/nvpair-manual-nodes/README.md index 5fb648a6..10381cd3 100644 --- a/services/nvpair-manual-nodes/README.md +++ b/services/nvpair-manual-nodes/README.md @@ -5,7 +5,14 @@ SPDX-License-Identifier: Apache-2.0 # nvpair-manual-nodes -A Go service for managing manually configured nodes on networks where mDNS discovery is unavailable. Accepts node addresses via JSON-RPC, probes each for Ollama, LM Studio, and node-info, and emits status events. +A Go service for managing manually configured nodes on networks where mDNS discovery is unavailable — a filtered LAN, or an overlay network such as a Tailscale tailnet, which carries no multicast at all. Accepts node addresses via JSON-RPC, probes each one, and emits status events. + +There are two kinds of manual node, and one probe tells them apart. **node-info is asked first**, and its answer decides everything else: + +- **A bare inference host** — Ollama or LM Studio on a machine that does not run PAIR. It serves no `/v1/node-info`, so its engines are probed on their own ports in plain HTTP and a supervising broker bridges it into the local proxies as a routing target. This is what manual nodes were originally for. +- **A PAIR node** — it answers `/v1/node-info` *with a `services` map*. It is then reported with `pair_node: true`, its cluster principal, its service map, and its model inventory read from its engine manager over cluster mTLS. A supervising broker folds it into the discovery directory as if it had been found over mDNS. + +A PAIR node is **never** probed on its engine ports. On such a node `:11434` and `:1234` are the proxy facades, which refuse plaintext from anything but loopback, so a probe there is a guaranteed `403` that would report a healthy peer as having no engines. ## Communication @@ -52,6 +59,7 @@ Emitted when a manually added node has been probed and its initial status determ "lmstudio_models":["qwen2.5-7b-instruct"], "node_info_up":true, "node_info_port":14318, + "pair_node":false, "gpus":[{"name":"NVIDIA GeForce RTX 3080","utilization_percent":37}], "telemetryValid":true, "msSince":120, @@ -91,11 +99,14 @@ A hostname is preferred over an IP literal: probe clients disable keep-alives sp | Param | Required | Description | |---|---|---| -| `address` | Yes | IP address or hostname of the node, with no port | +| `address` | Yes | IP address or host name of the node, with no port. A `host:port` string is **rejected** with an actionable error: every probe appends its own service port, so such an entry could never be reached | | `name` | No | Friendly name (used as node ID; defaults to `manual:
`) | -| `tls_port` | No | Probe node-info over HTTPS on this port instead of plain HTTP on `14318`. Echoed back as `tls_enabled` | +| `ports` | No | Per-service port overrides: `{node_info, cluster, ollama, lmstudio, vllm}`. An unset field keeps that service's default. Persisted and echoed back as `ports`. `vllm` is carried but not probed yet | +| `tls_port` | No | Probe node-info over HTTPS on this port instead of plain HTTP. Takes precedence over `ports.node_info`, since it names an HTTPS listener and therefore names its port. Echoed back as `tls_enabled` | | `mtls` | No | Stored and echoed back as `mtls_required`. The probe transport itself is chosen by `tls_port` and live cluster membership, so this field records intent rather than driving it | +An address may be a MagicDNS name (`gpu-box.tail1234.ts.net`), a `.local` name, an IPv4 literal, or an IPv6 literal in plain or bracketed form. + Response: the initial node status object. ### `node/remove` @@ -132,15 +143,17 @@ Changes the log level at runtime. Accepted as either a request (answered with `{ ## Probing -Each manual node is probed every 10 seconds, with a 3-second timeout per leg, for: +Each manual node is probed every 10 seconds, with a 3-second timeout per leg. **node-info is asked first**, because its answer decides which other legs may run at all: -- **Ollama** on port 11434: health check (`GET /`) and model list (`GET /api/tags`) -- **LM Studio** on port 1234: `GET /v1/models`, which doubles as the liveness check and the model list -- **Node Info** on port 14318, or `tls_port` over HTTPS: hardware inventory and identity (`GET /v1/node-info`) +- **Node Info** on port 14318 (or `ports.node_info`, or `tls_port` over HTTPS): hardware inventory, identity, cluster principal, and service map (`GET /v1/node-info`) +- Then, **only for a bare host** (node-info reported no service map): + - **Ollama** on port 11434 (or `ports.ollama`): health check (`GET /`) and model list (`GET /api/tags`) + - **LM Studio** on port 1234 (or `ports.lmstudio`): `GET /v1/models`, which doubles as the liveness check and the model list +- Or, **only for a PAIR node**, its model inventory from its engine manager (`GET /v1/models` on the `em` port from the service map) over cluster mTLS, pinned to the peer's cluster principal. No pin, no models: a peer does not serve its inventory to a stranger, so the node appears with its hardware and gains its models once paired. A node can have any combination of these, or none if the target is unreachable. Status changes trigger `node/updated` events. Because change detection compares CPU, memory, and GPU values, a node running node-info emits a `node/updated` on most probe cycles as utilization moves. -The three engine ports are compiled in: only the node-info leg's port can be moved, via `tls_port`. A remote engine on a non-default port is not discovered. +`pair_node` holds across a failure episode rather than being recomputed per probe. One missed node-info answer is routine across an overlay network, and without that tolerance the gap would probe the peer's proxy facades in plaintext and withdraw it from every consumer for a cycle. It reverts past `probeFailThreshold` consecutive failures, so a node that genuinely stops being a PAIR node is not remembered as one. ## Shutdown diff --git a/services/nvpair-node-info/README.md b/services/nvpair-node-info/README.md index 8ed76948..f4017e46 100644 --- a/services/nvpair-node-info/README.md +++ b/services/nvpair-node-info/README.md @@ -11,7 +11,7 @@ A Go service that exposes this machine's hardware inventory (GPUs, CPU, physical Two surfaces: -- **HTTP(S)** — serves the node inventory at `/v1/node-info`. Plaintext HTTP on `:14318` by default; optional HTTPS (with optional mTLS) on `:14319` when a cert/key pair is supplied. +- **HTTP(S)** — serves the node inventory and this node's service map at `/v1/node-info`. Plaintext HTTP on `:14318` by default; optional HTTPS (with optional mTLS) on `:14319` when a cert/key pair is supplied. - **stdio JSON-RPC 2.0** — newline-delimited, used for lifecycle/control (`log/set-level`, `nodeinfo:set-cluster-identity`) and shutdown via stdin EOF. The service is normally launched as a subprocess by the broker. It does **not** advertise itself over mDNS. Discovery is centralized in the `nvpair-node-scanner` daemon: the broker registers this service's `ni` port with the daemon, which carries it on the node's one `_nvpair-node` record and fetches `/v1/node-info` over plain HTTP to enrich each node. @@ -71,7 +71,8 @@ Returns the merged static identity (collected once at startup) and the latest dy "used_bytes": 12884901888 }, "hostUuid": "8661676a-0d1c-4bd3-ac5e-4d370e6f1a9c", - "clusterUuid": "" + "clusterUuid": "", + "services": { "ni": 14318, "ol": 11434, "lm": 1234, "em": 14322, "ec": 14323, "cl": 14321 } } ``` @@ -81,6 +82,8 @@ Field notes: - `telemetryValid` and `msSince` describe the node-wide GPU utilization snapshot. `telemetryValid` is `true` after the collector has produced a usable GPU sample; `msSince` is that sample's age in milliseconds at response time. A failed collection retains the last usable sample and lets its age increase. Before the first usable sample, and on platforms without dynamic GPU telemetry, the response reports `telemetryValid:false` and `msSince:0`; consumers must ignore the age while validity is false. - `clusterUuid` is the cluster principal this node currently holds. It has three distinct states on the wire: **absent** means unknown, **present and empty** means this node belongs to no cluster, and a value is that principal. A consumer must not read absent as unclustered — that is how a node too old to report the field answers, and also how this node answers before its parent has told it anything, so acting on it would clear a correct annotation elsewhere in the fleet. - Under the broker, `clusterUuid` is pushed in over stdin (`nodeinfo:set-cluster-identity`) because node-info is spawned with no cluster dir and so cannot read membership itself; the field stays absent until the first push arrives. Standalone with `--cluster-dir`, it reads membership from the trust store per request instead and is therefore always known. The two sources are mutually exclusive by deployment, not a fallback chain. +- `services` is this node's `{service key: port}` set — the same set the node-scanner carries on this host's mDNS record, keyed by the same compact `nvpair-shared/noderec` service keys. It exists for the same reason `clusterUuid` does, and more so: that record is the only other place the set lives, and multicast does not cross a routed or overlay network. A peer that reached this node by a typed address reads it here and learns that this is a PAIR node and where each of its services listens. Its presence is what tells such a peer to route through this node's proxies over mutual TLS rather than probing its engine ports in plaintext. Absent means the parent has not pushed the set yet — not that this node runs nothing. +- Under the broker, `services` is pushed in over stdin (`nodeinfo:set-services`) on spawn and on every registration change, from the same cache the broker replays to the node-scanner, so the HTTP answer and the mDNS record are one derivation rather than two. The set is always sent whole: a service that stopped is expressed by its key being absent. - `clusterUuid` exists so a peer can learn this node's membership without its mDNS record. Membership otherwise travels only as the `cluster-uuid=` TXT key, which a consumer reads once per record *change*; a consumer that misses that change keeps the previous value indefinitely, and one still holding a departed node's principal will suppress the invite that would bring it back. - All dynamic fields and the `cpu` / `memory` objects use `omitempty`: a value the service couldn't read is dropped from the JSON entirely rather than reported as a misleading literal zero. A genuinely idle CPU renders the same as "unknown" — that ambiguity is intentional and benign. - `vram_bytes` is reported through DXGI on Windows, `nvidia-smi` on Linux, and IORegistry on macOS. On a unified-memory NVIDIA GPU such as DGX Spark, Linux uses total physical system memory for `vram_bytes` and the independently sampled system-memory usage for `vram_used_bytes`. On Apple Silicon, `vram_bytes` is total physical unified memory and `vram_used_bytes` is the GPU driver's mapped allocation (`Alloc system memory`), not whole-system RAM usage or the momentarily active subset. diff --git a/services/nvpair-ui-broker/README.md b/services/nvpair-ui-broker/README.md index d2b0e174..1e47f336 100644 --- a/services/nvpair-ui-broker/README.md +++ b/services/nvpair-ui-broker/README.md @@ -28,7 +28,7 @@ namespace: | `nvpair-job-scheduler` | Ranks nodes by pending work and GPU pressure for the proxies | _internal_ | | `nvpair-errors` | Service-error datastore and cross-node sync | `errors:*` | | `nvpair-node-settings` | Typed per-node settings store | `settings/*` | -| `nvpair-manual-nodes` | User-added nodes, merged into the discovery snapshot | `node/*`, `nodes/list` | +| `nvpair-manual-nodes` | User-added nodes, merged into the discovery snapshot; a user-added PAIR node is folded into the discovery relay as a peer | `node/*`, `nodes/list` | Only the scanner is required. Every other worker is optional: a missing binary leaves the broker running without that capability rather than failing to start. @@ -77,7 +77,7 @@ Bidirectional newline-delimited JSON-RPC 2.0 — same conventions as every other | `--settings-path ` | `./nvpair-node-settings[.exe]` in the CWD | Explicit path to the `nvpair-node-settings` binary the broker spawns for the typed settings store. Same optional semantics as `--node-info-path` | | `--cluster-manager-path ` | `./nvpair-cluster-manager[.exe]` in the CWD | Explicit path to the `nvpair-cluster-manager` binary the broker spawns for cluster pairing / membership. Same optional semantics as `--node-info-path` | | `--scheduler-path ` | `./nvpair-job-scheduler[.exe]` in the CWD | Explicit path to the `nvpair-job-scheduler` binary the broker spawns for responsive, node-wide workload and GPU-pressure ranking. Same optional semantics as `--node-info-path` | -| `--cluster-dir ` | `cluster/` in the per-user data dir (`%LocalAppData%\Nvidia Corporation\Personal AI Router` on Windows, `~/.config/Nvidia Corporation/Personal AI Router` on Linux) | Cluster config dir (`node.crt`/`node.key` + `trusted/`, minted by `nvpair-cluster-manager`). Threaded to the cluster-scoped workers (`nvpair-errors`, `nvpair-workload-manager`, `nvpair-node-scanner`, `nvpair-manual-nodes`, and `nvpair-engine-manager`), each of which derives its membership from it continuously — so a create, join, or leave takes effect in place and the broker does **not** restart them. The broker also passes the parent of this path to `nvpair-cluster-manager` as `--config-dir`, so the only writer of the cluster dir and the workers reading it cannot resolve different directories. `nvpair-node-info` is excluded — it stays plain HTTP even when clustered (see the repository-root `SECURITY.md`). Defaults so cluster mTLS auto-activates with nothing to pass; with an empty or cert-less dir this node is not a member, so it serves and dials no inter-node cluster traffic at all | +| `--cluster-dir ` | `cluster/` in the per-user data dir (`%LocalAppData%\Nvidia Corporation\Personal AI Router` on Windows, `~/.config/Nvidia Corporation/Personal AI Router` on Linux) | Cluster config dir (`node.crt`/`node.key` + `trusted/`, minted by `nvpair-cluster-manager`). Threaded to the cluster-scoped workers (`nvpair-errors`, `nvpair-workload-manager`, `nvpair-node-scanner`, `nvpair-manual-nodes`, and `nvpair-engine-manager`), each of which derives its membership from it continuously — so a create, join, or leave takes effect in place and the broker does **not** restart them. The broker also passes the parent of this path to `nvpair-cluster-manager` as `--config-dir`, so the only writer of the cluster dir and the workers reading it cannot resolve different directories. `nvpair-node-info` is excluded — it stays plain HTTP even when clustered (see the repository-root `SECURITY.md`), which is also what lets a peer that has not paired yet learn this node exists. Defaults so cluster mTLS auto-activates with nothing to pass; with an empty or cert-less dir this node is not a member, so it serves and dials no inter-node cluster traffic at all | | `--log-level ` | _(env `NVPAIR_LOG_LEVEL` or `info`)_ | `debug` \| `info` \| `warn` \| `error` | | `--version` | | Print version and exit | @@ -475,9 +475,17 @@ Any `settings/*` request is forwarded to `nvpair-node-settings` and its response #### `node/add` / `node/remove` / `nodes/list` (manual nodes) -Relayed to `nvpair-manual-nodes`. `node/add` (`{ address, name?, tls_port?, mtls? }`) registers a user-added node and probes it; `node/remove` (`{ id }`) drops it; `nodes/list` returns the tracked manual nodes. Manually added nodes also surface in the shared `discovery:get-nodes` / `discovery:nodes-changed` snapshot — the broker merges `nvpair-manual-nodes`' `node/discovered|updated|removed` into the same store the scanner feeds. A `nvpair-manual-nodes` restart loses the in-memory entries because neither that worker nor the broker persists an authoritative copy, so clients must re-add manual nodes after a restart. Error `-32000 "manual-nodes not available"` when no manual-nodes worker is supervised. +Relayed to `nvpair-manual-nodes`. `node/add` (`{ address, name?, ports?, tls_port?, mtls? }`) registers a user-added node and probes it. `address` must carry no port — `ports` (`{ node_info, cluster, ollama, lmstudio, vllm }`) moves any single service off its default; `node/remove` (`{ id }`) drops it; `nodes/list` returns the tracked manual nodes. Manually added nodes also surface in the shared `discovery:get-nodes` / `discovery:nodes-changed` snapshot — the broker merges `nvpair-manual-nodes`' `node/discovered|updated|removed` into the same store the scanner feeds. A `nvpair-manual-nodes` restart loses the in-memory entries because neither that worker nor the broker persists an authoritative copy, so clients must re-add manual nodes after a restart. Error `-32000 "manual-nodes not available"` when no manual-nodes worker is supervised. -**Manual → proxy bridge.** When the broker supervises both `nvpair-manual-nodes` and a proxy, it also bridges a manual node whose engine is reachable into that proxy via `node/add-manual` (host/port from the node's per-engine status), so inference can route to it through `proxy:node/select` / `lmstudio-proxy:node/select` just like a relay-discovered node. This is per-engine: a node whose `ollama_*` status is up is bridged into `ollama-proxy` (host/port from `ollama_port`), and one whose `lmstudio_*` status is up into `lmstudio-proxy` (from `lmstudio_port`) — a node running both is bridged into both. Manual nodes are by definition the ones that never appear via the daemon's `_nvpair-node` discovery, so this explicit add is what makes them routable. The bridge tracks reachability: an engine that goes down (or a node that is removed, or whose prober crashes) is pulled back out with `node/remove-manual`. A proxy that isn't supervised → that leg is a no-op; manual nodes still appear in the discovery snapshot as before. +**A manual node reaches inference one of two ways, never both**, decided by what `nvpair-manual-nodes` reports for it. + +**A bare inference host** (`pair_node: false`) — Ollama or LM Studio on a machine that does not run PAIR — is bridged into the proxies via `node/add-manual` (host/port from the node's per-engine status), so inference can route to it through `proxy:node/select` / `lmstudio-proxy:node/select` just like a relay-discovered node. This is per-engine: a node whose `ollama_*` status is up is bridged into `ollama-proxy` (from `ollama_port`), one whose `lmstudio_*` status is up into `lmstudio-proxy` (from `lmstudio_port`), and a node running both into both. The bridge tracks reachability: an engine that goes down (or a node that is removed, or whose prober crashes) is pulled back out with `node/remove-manual`. A proxy that isn't supervised → that leg is a no-op. + +**A PAIR node** (`pair_node: true`, its node-info reported a `services` map) is instead folded into the **discovery relay**: the broker synthesizes the `DirectoryNode` the scanner would have produced — the typed address as its canonical one, the peer's cluster principal, its service map, its hardware and its models — and applies it to the same relay every consumer subscribes to. Both proxies, the scheduler, `nvpair-engine-manager`'s remote operations, the workload relay and the errors peer sync then treat it exactly as they treat a discovered pinned peer, and it is dialed over cluster mTLS to its own proxy ports. The raw-engine bridge is *withdrawn* for such a node rather than left alongside: those ports are the peer's proxy facades and refuse plaintext from anything but their own loopback, so a second plaintext candidate could only ever 403. + +The synthesis stands down in three cases, so it never fights another writer or points at this machine: the node's `hostUuid` is this node's own (a manual entry naming ourselves must not become a routing target), the scanner already claims that key (the daemon's record is authoritative and carries evidence the synthesis cannot), or the node is not a PAIR node. A withdrawal removes only what the broker itself synthesized. + +This is what makes a peer on a network that carries no multicast — a Tailscale tailnet, a WireGuard tunnel, a routed link — an ordinary cluster member. See [Running PAIR across a Tailscale tailnet](../../docs/remote-networks.mdx). #### `cluster:` / `nodes:` (generic relay) diff --git a/services/versions.json b/services/versions.json index 29d8c230..25a20c2b 100644 --- a/services/versions.json +++ b/services/versions.json @@ -1,19 +1,19 @@ { "$comment": "Single source of truth for all version numbers. See VERSIONING.md for bump rules.", - "product": "0.91.7", - "installer": "0.91.7", + "product": "0.92.0", + "installer": "0.92.0", "components": { - "ollama-proxy": "0.26.2", - "lmstudio-proxy": "0.16.2", - "nvpair-node-info": "0.13.3", - "nvpair-node-scanner": "0.20.3", - "nvpair-manual-nodes": "0.11.1", - "nvpair-workload-manager": "0.13.3", - "nvpair-errors": "0.7.4", + "ollama-proxy": "0.27.0", + "lmstudio-proxy": "0.17.0", + "nvpair-node-info": "0.14.0", + "nvpair-node-scanner": "0.20.4", + "nvpair-manual-nodes": "0.12.0", + "nvpair-workload-manager": "0.13.4", + "nvpair-errors": "0.7.5", "nvpair-node-settings": "1.0.4", - "nvpair-ui-broker": "0.40.2", + "nvpair-ui-broker": "0.41.0", "nvpair-engine-manager": "0.17.4", - "nvpair-cluster-manager": "1.1.4", + "nvpair-cluster-manager": "1.1.5", "nvpair-job-scheduler": "0.4.1", "nvpair-tui": "0.7.2" } From f432ce1985a61e8eb5af93abde5cbe0388233bd1 Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 05:50:21 +0100 Subject: [PATCH 7/8] desktop: pin that a manual PAIR peer renders as a discovered one The synthesized directory record is shape-identical to a discovered one, so the renderer needs no special case for it -- which is exactly the property worth a regression gate, because it is invisible until it breaks. Drives the bridge state with the record the broker synthesizes for a peer added by address, and asserts the whole surface a node card is built from: the card itself, trust and membership, per-engine models with their loaded state, the node-info poll target, and the hardware that comes back from it. The address is a MagicDNS name throughout, since that is the part that is new: nothing on this path may require an IP literal, and an empty address is what a node with nowhere to be dialed looks like. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- .../modular/manual-pair-peer-state.test.ts | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 desktop/tests/modular/manual-pair-peer-state.test.ts diff --git a/desktop/tests/modular/manual-pair-peer-state.test.ts b/desktop/tests/modular/manual-pair-peer-state.test.ts new file mode 100644 index 00000000..588a7e44 --- /dev/null +++ b/desktop/tests/modular/manual-pair-peer-state.test.ts @@ -0,0 +1,130 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ BrowserWindow: { getAllWindows: () => [] } })) +vi.mock('@/electron/window', () => ({ createOverviewWindow: vi.fn() })) + +import { getModularBridgeState } from '@/electron/service-bridge/modular-state' + +// A peer added by address on a network that discovers nothing arrives here as an +// ordinary directory node: the broker synthesizes the record the scanner would +// have produced. These tests pin that the renderer's view of it is the view of a +// discovered peer, with no special case anywhere — same card, same engines, same +// models, same telemetry — and that its address surviving as a host name rather +// than a literal changes none of it. +describe('a manually added PAIR peer in the bridge state', () => { + it('renders with the card, engines, models and telemetry a discovered peer gets', () => { + const state = getModularBridgeState() + state.handleNotification({ + source: 'broker', + method: 'discovery:nodes-changed', + params: { + nodes: [ + { + hostUuid: 'uuid-tailnet-peer', + name: 'gpu-box.tail1234.ts.net', + // A MagicDNS name, not a literal. Nothing downstream may + // require an IP: on a tailnet the name is what survives a + // node changing address. + ipAddress: 'gpu-box.tail1234.ts.net', + port: 14318, + trusted: true, + clustered: true, + models: ['llama3.2:latest', 'qwen2.5-7b-instruct'], + modelsByEngine: { + ollama: ['llama3.2:latest'], + lmstudio: ['qwen2.5-7b-instruct'] + }, + loadedByEngine: { ollama: ['llama3.2:latest'] } + } + ] + } + }) + + // The card: keyed by the peer's stable identity, named and addressed by + // what the operator typed. + const { nodes } = state.getNodesInitial() + const card = nodes['uuid-tailnet-peer'] + expect(card).toBeDefined() + expect(card.name).toBe('gpu-box.tail1234.ts.net') + expect(card.ipAddress).toBe('gpu-box.tail1234.ts.net') + + // Trust and membership, which is what distinguishes a paired peer from a + // stranger anywhere it is shown. + const available = state.getAvailableNodes().find(node => node.id === 'uuid-tailnet-peer') + expect(available).toMatchObject({ trusted: true, clustered: true }) + expect(available?.ipAddress).toBe('gpu-box.tail1234.ts.net') + + // Models, attributed to the engine that serves each one. + const engineState = state.getEngineInitialState() + const ollama = engineState.models.find( + entry => entry.nodeId === 'uuid-tailnet-peer' && entry.engineType === 'ollama' + ) + const lmStudio = engineState.models.find( + entry => entry.nodeId === 'uuid-tailnet-peer' && entry.engineType === 'lm-studio' + ) + expect(ollama?.models.map(model => model.name)).toEqual(['llama3.2:latest']) + expect(lmStudio?.models.map(model => model.name)).toEqual(['qwen2.5-7b-instruct']) + // Loaded state travels too, so a remote card can show what is resident. + expect(ollama?.models[0]?.status).toBe('loaded') + + // Telemetry: the peer is polled for its hardware at the address it was + // added by, on its node-info port, exactly as a discovered peer is. + const target = state + .getNodeInfoPollTargets() + .find(entry => entry.id === 'uuid-tailnet-peer') + expect(target).toBeDefined() + expect(target?.hosts).toContain('gpu-box.tail1234.ts.net') + expect(target?.port).toBe(14318) + expect(card.status).toBe('active') + + // And what comes back lands on the card. + state.mergeNodeInfoResponse('uuid-tailnet-peer', { + hostUuid: 'uuid-tailnet-peer', + GPUs: [ + { + name: 'NVIDIA GeForce RTX 4090', + vram_bytes: 25_769_803_776, + vram_used_bytes: 8_589_934_592, + utilization_percent: 37 + } + ], + cpu: { name: 'AMD Ryzen 9 5900X', cores: 12, utilization_percent: 9 }, + memory: { total_bytes: 68_719_476_736, used_bytes: 17_179_869_184 } + }) + const withHardware = state.getNodesInitial().nodes['uuid-tailnet-peer'] + expect(withHardware.topology.cpu.model).toBe('AMD Ryzen 9 5900X') + expect(withHardware.topology.cpu.cores).toBe(12) + expect(withHardware.topology.gpus.map(gpu => gpu.name)).toEqual(['NVIDIA GeForce RTX 4090']) + expect(withHardware.topology.gpus[0].vramTotal).toBe(25_769_803_776) + expect(withHardware.topology.ram).toBe(68_719_476_736) + }) + + it('keeps a host-name address rather than blanking it', () => { + const state = getModularBridgeState() + state.handleNotification({ + source: 'broker', + method: 'discovery:nodes-changed', + params: { + nodes: [ + { + hostUuid: 'uuid-name-only', + name: 'name-only.tail1234.ts.net', + ipAddress: 'name-only.tail1234.ts.net', + port: 14318, + trusted: true, + clustered: true + } + ] + } + }) + + const card = state.getNodesInitial().nodes['uuid-name-only'] + expect(card.ipAddress).toBe('name-only.tail1234.ts.net') + // Nothing collapses the address to an empty string on the way through: an + // empty address is what a node with nowhere to be dialed looks like. + expect(card.ipAddress).not.toBe('') + }) +}) From e6f8b80ad5effd26db46c91fa346f5987ebe32a8 Mon Sep 17 00:00:00 2001 From: Can GULDOGAN Date: Fri, 4 Sep 2026 06:05:09 +0100 Subject: [PATCH 8/8] tests: wait for the broker's fixed pairing port before pairing The broker's cluster-manager port is a compiled-in constant, so only one broker on a machine can hold it, and a previous test's broker can still be tearing its workers down when the next one starts. When that happened this test's own cluster-manager silently failed to bind and the pairing completion was posted to whichever process did hold the port, which read as a failed pairing. Wait for the port to be released before starting, and for this broker's own listener to exist before inviting, instead of sleeping a second and hoping. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Can GULDOGAN --- services/tests/manual_pair_peer_test.go | 51 +++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/services/tests/manual_pair_peer_test.go b/services/tests/manual_pair_peer_test.go index 2a73bba2..7ac36bf1 100644 --- a/services/tests/manual_pair_peer_test.go +++ b/services/tests/manual_pair_peer_test.go @@ -40,6 +40,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "sync/atomic" "testing" "time" @@ -147,6 +148,46 @@ type brokerProc struct { nextID int } +// clusterManagerPort is the broker-owned cluster-manager port. It is a +// compiled-in constant, so only one broker on this machine can hold it, and a +// test that needs its *own* broker's pairing listener has to wait for whatever +// held it last to let go. +const clusterManagerPort = 14321 + +// awaitPortFree blocks until nothing accepts on addr. The broker's ports are +// fixed constants and a previous test's broker can still be tearing its workers +// down when the next one starts; without this the new broker's cluster-manager +// silently fails to bind and a pairing completion is posted to the wrong process. +func awaitPortFree(t *testing.T, addr string) { + t.Helper() + deadline := time.Now().Add(30 * time.Second) + for time.Now().Before(deadline) { + conn, err := net.DialTimeout("tcp", addr, time.Second) + if err != nil { + return + } + _ = conn.Close() + time.Sleep(200 * time.Millisecond) + } + t.Fatalf("%s was still held after 30s; a previous test's broker has not exited", addr) +} + +// awaitPortListening blocks until addr accepts, so a pairing is not sent before +// the listener that has to answer it exists. +func awaitPortListening(t *testing.T, addr string) { + t.Helper() + deadline := time.Now().Add(30 * time.Second) + for time.Now().Before(deadline) { + conn, err := net.DialTimeout("tcp", addr, time.Second) + if err == nil { + _ = conn.Close() + return + } + time.Sleep(200 * time.Millisecond) + } + t.Fatalf("nothing is listening on %s after 30s", addr) +} + func startBrokerWithClusterDir(t *testing.T, clusterDir string) *brokerProc { t.Helper() cfg := t.TempDir() @@ -280,13 +321,15 @@ func TestManualPairPeerBehavesLikeADiscoveredPeer(t *testing.T) { cmB := startCM(t, baseB, portB) t.Cleanup(cmB.stop) - // A is a real broker with its own real cluster-manager. + // A is a real broker with its own real cluster-manager. Its pairing port is a + // compiled-in constant, so wait for a previous test's broker to release it + // before starting, and for this one's listener to exist before pairing. + awaitPortFree(t, net.JoinHostPort("127.0.0.1", strconv.Itoa(clusterManagerPort))) brokerA := startBrokerWithClusterDir(t, dirA) brokerA.call("discovery:subscribe", nil) brokerA.call("proxy:subscribe", nil) - - // The listeners need a moment to bind before pairing. - time.Sleep(time.Second) + awaitPortListening(t, net.JoinHostPort("127.0.0.1", strconv.Itoa(clusterManagerPort))) + awaitPortListening(t, net.JoinHostPort("127.0.0.1", strconv.Itoa(portB))) // Pair A to B by address and port alone — no nodeId, because A has not // discovered B and never will. This is the invite an operator sends after