Add optional mDNS/DNS-SD auto discovery of nocc servers - #36
Open
dkulp wants to merge 2 commits into
Open
Conversation
Keeping NOCC_SERVERS in sync by hand is a chore on a small network of build machines. A server started with -advertise-mdns now announces itself as _nocc._tcp.local., and a daemon run with NOCC_DISCOVER_MDNS=1 browses for such announcements and appends them to whatever NOCC_SERVERS already holds. It's the same model as distcc's zeroconf mode. Both sides are off by default: compilation ships source code to whoever answers, so joining a build cluster stays an explicit decision rather than a side effect of being on the same network. Three properties the discovery path has to preserve, since the daemon shards .cpp files across the server list by index (fnv(basename) % len(servers)): - Static hosts keep their spelling and position; discovered ones are only appended, so enabling discovery cannot re-shard an existing setup. - The discovered list is sorted, and a server announcing several addresses resolves to one deterministically chosen address (preferring one on a subnet this machine is also on, which is also what makes it reachable). Picking whichever record arrived first would alternate between a multi-homed server's addresses across daemon restarts and throw away its src cache. - A server reachable under two spellings — a hostname in NOCC_SERVERS, an address over mDNS — is added once. A missed duplicate is not a connection error; it is one machine holding two slots of the sharding wheel and taking a double share of the build. Results are cached briefly (NOCC_DISCOVER_CACHE_TTL, 1 minute), because the daemon exits ~15s after the last invocation and would otherwise pay the full browse timeout again on every restart. A stale entry costs a failed connection and a local fallback, exactly like a dead host in a static list. Also stop treating os.Args[2] as a hostname when it starts with '-', so that `nocc -check-servers -discover-mdns` checks the discovered servers instead of trying to dial a flag, and log the server list (not just its count) on daemon start, since with discovery on that list is in no config file. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
A build script that wants to point NOCC_SERVERS at whatever is on the LAN had to either screen-scrape -check-servers or browse mDNS itself with avahi-browse. The second option looks easy and is quietly wrong: a helper with two interfaces announces two addresses, so a naive browse lists it twice — giving that machine a double share of the sharding wheel — and may pick the address that isn't routable from the client. Both Pi 5s here announce on two subnets, so this is the normal case, not a corner one. The daemon already resolves that when it discovers servers for itself. This exposes the same answer: one "host:port" per line on stdout, nothing else, then exit. It always browses live rather than reading the discovery cache, since a script asking what's on the network now means now. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in mDNS/DNS-SD (“zeroconf”) mechanism so nocc-daemon can discover nocc-server instances on a LAN without manually maintaining NOCC_SERVERS, while keeping both advertising and discovery disabled by default for safety.
Changes:
- Introduces
internal/discoverywith mDNS advertise/browse, deterministic address selection, dedup/merge rules, and short-lived caching. - Adds daemon/server CLI/env hooks for advertising (
nocc-server -advertise-mdns) and discovery (NOCC_DISCOVER_MDNS,-discover-mdns,-print-discovered-servers). - Updates logging and documentation to surface the effective server list and discovery configuration.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/discovery/merge.go | Merges discovered servers after static NOCC_SERVERS while attempting to deduplicate across spellings. |
| internal/discovery/merge_test.go | Unit tests for merge ordering/dedup behavior. |
| internal/discovery/mdns.go | mDNS advertise + browse implementation, deterministic address pick, and dedup of browse results. |
| internal/discovery/mdns_test.go | Unit + end-to-end advertise/browse tests and cache tests. |
| internal/discovery/cache.go | On-disk cache for browse results to avoid repeated timeouts across daemon restarts. |
| internal/client/daemon.go | Logs the full server list and updates the “no servers” error to mention discovery. |
| cmd/nocc-server/main.go | Adds flags/env for mDNS advertising and starts the advertiser. |
| cmd/nocc-daemon/main.go | Adds discovery flags/env, merge of discovered servers, and -print-discovered-servers. |
| go.mod | Adds github.com/libp2p/zeroconf/v2 dependency. |
| go.sum | Adds checksums for zeroconf and transitive deps. |
| docs/configuration.md | Documents discovery env vars and server advertise flags. |
| docs/architecture.md | Notes that server lists may be extended by mDNS discovery. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+72
to
+86
| keys := []string{strings.ToLower(host) + ":" + port} | ||
| // a bare ".local" name and its short form denote the same machine on a zeroconf LAN | ||
| if shortHost := strings.TrimSuffix(strings.ToLower(host), ".local"); shortHost != strings.ToLower(host) { | ||
| keys = append(keys, shortHost+":"+port) | ||
| } | ||
|
|
||
| if net.ParseIP(host) == nil { | ||
| ctx, cancel := context.WithTimeout(context.Background(), resolveTimeout) | ||
| defer cancel() | ||
| if addrs, err := net.DefaultResolver.LookupHost(ctx, host); err == nil { | ||
| for _, addr := range addrs { | ||
| keys = append(keys, addr+":"+port) | ||
| } | ||
| } | ||
| } |
Comment on lines
+101
to
+125
| entries := make(chan *zeroconf.ServiceEntry, 16) | ||
| found := make([]ServerInfo, 0, 8) | ||
| done := make(chan struct{}) | ||
|
|
||
| go func() { | ||
| defer close(done) | ||
| for entry := range entries { | ||
| if info, ok := serviceEntryToServerInfo(entry); ok { | ||
| found = append(found, info) | ||
| } | ||
| } | ||
| }() | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() | ||
|
|
||
| // zeroconf.Browse closes `entries` itself once ctx expires | ||
| if err := zeroconf.Browse(ctx, ServiceType, ServiceDomain, entries); err != nil { | ||
| return nil, err | ||
| } | ||
| <-done | ||
|
|
||
| sort.Slice(found, func(i, j int) bool { return found[i].HostPort < found[j].HostPort }) | ||
| return dedupSameHostPort(found), nil | ||
| } |
Comment on lines
+147
to
+162
| for _, kv := range entry.Text { | ||
| key, value, found := strings.Cut(kv, "=") | ||
| if !found { | ||
| continue | ||
| } | ||
| switch key { | ||
| case "v": | ||
| info.Version = value | ||
| case "cpus": | ||
| info.NumCPU, _ = strconv.Atoi(value) | ||
| case "arch": | ||
| info.GOARCH = value | ||
| case "os": | ||
| info.GOOS = value | ||
| } | ||
| } |
Comment on lines
+123
to
+133
| // control: the very same call with an expired ttl must NOT return the cached list | ||
| if err := os.Chtimes(cachePath, time.Now(), time.Now()); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| stale, err := readCache(cachePath) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if time.Since(stale.savedAt) > time.Minute { | ||
| t.Fatal("test setup: cache should be fresh") | ||
| } |
Comment on lines
+119
to
139
| if *discoverMdns || *printDiscoveredServers { | ||
| cacheTTL := *discoverCacheTTL | ||
| if *printDiscoveredServers { | ||
| cacheTTL = 0 // a script asking what's on the network right now wants a live answer, not a cached one | ||
| } | ||
| // `-check-servers` is the command people run to find out what discovery sees, so let it be verbose | ||
| found := browseNoccServers(*discoverTimeout, cacheTTL, *checkServersAndExit) | ||
|
|
||
| if *printDiscoveredServers { | ||
| for _, info := range found { | ||
| fmt.Println(info.HostPort) | ||
| } | ||
| os.Exit(0) | ||
| } | ||
| remoteNoccHosts = discovery.MergeWithStaticHosts(remoteNoccHosts, found) | ||
| } | ||
|
|
||
| if *showVersionAndExit || *showVersionAndExitShort { | ||
| fmt.Println(common.GetVersion()) | ||
| os.Exit(0) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds opt-in zeroconf discovery of nocc servers, so a small network of build machines doesn't have to keep
NOCC_SERVERSin sync by hand. It's the same model as distcc's zeroconf mode:nocc-server -advertise-mdnsannounces_nocc._tcp.local., and a daemon run withNOCC_DISCOVER_MDNS=1browses for those announcements.Both sides are off by default, deliberately: compilation ships source code to whoever answers, so joining a build cluster should be a decision rather than a side effect of being on the same network.
Design notes
The daemon shards
.cppfiles across the server list by index (fnv(basename) % len(servers)), so discovery has to be careful about what that list looks like:NOCC_SERVERSalways wins. Discovered servers are appended, never reordered into it, so enabling discovery cannot re-shard an existing setup. A server present in both is added once — the comparison looks past spelling, sinceNOCC_SERVERSis usually written with hostnames while mDNS answers with addresses. A missed duplicate isn't a connection error; it's one machine occupying two slots of the sharding wheel.Results are cached briefly (
NOCC_DISCOVER_CACHE_TTL, default 1 minute), because the daemon exits ~15s after the last invocation and would otherwise pay the full browse timeout again on every restart. A stale entry costs a failed connection and a local fallback — the same as a dead host in a static list.nocc -print-discovered-servers(second commit) writes onehost:portper line for scripts that want to build the list themselves, rather than having them screen-scrape-check-serversor browse mDNS on their own and get the multi-homed case wrong.Cost
One new direct dependency,
github.com/libp2p/zeroconf/v2(which pullsmiekg/dns). Everything else is confined to a newinternal/discoverypackage plus three call sites (cmd/nocc-daemon/main.go,cmd/nocc-server/main.go, and a server-list line in the daemon's startup log). I'm happy to swap it for a hand-rolled responder ongolang.org/x/net/dns/dnsmessageif the dependency is the sticking point — that was the main alternative considered.Testing
Unit tests cover the TXT/address parsing, the multi-interface dedup, the merge rules, and the cache; one test does a real advertise-and-browse over the multicast socket. Verified on a mixed-architecture LAN (64-bit servers, 32-bit clients): a client with no
NOCC_SERVERSat all discovered both servers and compiled through them, coexisting with a running avahi-daemon on port 5353. Servers there announce on two subnets, and the address preference picked the routable one.Also run under a 32-bit
GOARCH, since nocc clients include armv7 boards.