diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 000000000..4b752ffa8 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,165 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Adapted from NVIDIA/Personal-AI-Router@b4a0cc5. PR code runs only on +# disposable GitHub-hosted runners, without secrets or release automation. +name: CI + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +env: + GOTOOLCHAIN: local + +jobs: + headers: + name: SPDX headers + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-node@v5.0.0 + with: + node-version: '25.5.0' + - run: node scripts/spdx-headers.mjs + + desktop: + name: Desktop checks + runs-on: ubuntu-latest + timeout-minutes: 15 + defaults: + run: + working-directory: desktop + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-node@v5.0.0 + with: + node-version: '25.5.0' + cache: npm + cache-dependency-path: desktop/package-lock.json + - uses: actions/setup-go@v6.0.0 + with: + go-version: '1.26.7' + cache-dependency-path: services/**/go.sum + # The license checker is deliberately offline and enumerates every + # release target, including Windows-only dependencies. A native + # build alone would leave those dependencies out of the cache. + - name: Populate Go license dependency cache + working-directory: . + shell: bash + run: | + for mod in services/*/go.mod; do + (cd "$(dirname "$mod")" && go mod download) + done + git diff --exit-code -- services + - run: npm ci --prefer-offline + - run: npm run typecheck + - run: npm run lint -- --max-warnings=0 + - run: npm run test:unit + - run: npm run dead-code:check + - run: npm run service-contracts:check + - run: npm run licenses:check + + services: + name: Go modules (${{ matrix.os }}) + # Pin the supported arm64 image: macos-latest failed runner acquisition + # during the macOS 26 rollout, before any test step could execute. + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + strategy: + fail-fast: false + matrix: + os: [macos-15, ubuntu-latest] + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-go@v6.0.0 + with: + go-version: '1.26.7' + cache-dependency-path: services/**/go.sum + - name: Linux test tools + if: runner.os == 'Linux' + run: sudo apt-get update -qq && sudo apt-get install -y -qq lsof iproute2 + - name: Check Go formatting + shell: bash + run: | + unformatted=$(gofmt -l services) + if [ -n "$unformatted" ]; then + printf '%s\n' "$unformatted" + exit 1 + fi + - name: Vet and test every service module (including libraries) + shell: bash + run: | + failed=0 + for mod in services/*/go.mod; do + dir=$(dirname "$mod") + # Cross-process tests run in their own job (Linux and macOS). + if [ "$dir" = services/tests ]; then continue; fi + echo "==> $dir" + (cd "$dir" && go vet ./... && go test -count=1 -timeout=5m -v ./...) || failed=1 + done + exit "$failed" + + cross-process: + name: Cross-process tests (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 20 + strategy: + fail-fast: false + # Linux runners can send mDNS multicast, so discovery is exercised + # there. Hosted macOS VMs cannot (every send to 224.0.0.251 fails + # with "no route to host"); on macOS the discovery tests skip with + # that reason named, and everything else runs. + matrix: + os: [ubuntu-latest, macos-15] + steps: + - uses: actions/checkout@v5.0.0 + with: + persist-credentials: false + - uses: actions/setup-go@v6.0.0 + with: + go-version: '1.26.7' + cache-dependency-path: services/**/go.sum + - name: Install port-owner tools + if: runner.os == 'Linux' + run: sudo apt-get update -qq && sudo apt-get install -y -qq lsof iproute2 + # Run the source guard before executing any application binary. + # All runtime helpers must use per-test disposable app-data roots. + - name: Check process isolation + working-directory: services/tests + run: go test -count=1 -run '^Test(ProcessHelpersRequireIsolation|CommandEnvironmentIsPerTest)$' . + - name: Vet cross-process suite + working-directory: services/tests + run: go vet ./... + # Leave five minutes for setup and artifact upload. The Go deadline + # prints the active test and goroutines before the job is killed. + - name: Run isolated cross-process suite with diagnostics + working-directory: services/tests + shell: bash + run: | + set -o pipefail + go test -v -count=1 -timeout=15m -json ./... 2>&1 \ + | tee "$RUNNER_TEMP/cross-process.jsonl" \ + | jq --unbuffered -j 'select(.Output != null) | .Output' + - name: Upload cross-process diagnostics + if: always() + uses: actions/upload-artifact@v4.6.2 + with: + name: cross-process-${{ matrix.os }} + path: ${{ runner.temp }}/cross-process.jsonl + if-no-files-found: warn diff --git a/services/nvpair-engine-manager/README.md b/services/nvpair-engine-manager/README.md index fab6671e8..6b380a5d1 100644 --- a/services/nvpair-engine-manager/README.md +++ b/services/nvpair-engine-manager/README.md @@ -156,7 +156,11 @@ version only decides what a fresh install starts from. `install.creates` names `/Applications/oMLX.app`: an install that finds an app already there opens it but does not record it as PAIR's (and drops any earlier claim), so it is never offered for uninstall. A claim records the created app's -file identity and covers that exact bundle only. A copy put there later is a +file identity and covers that exact bundle only. On macOS this includes birth +time (stable across the install's rename); on Linux it includes ctime so a reused +inode cannot inherit the claim. Linux metadata changes also invalidate the claim. +Older device/inode-only claims no longer authorize removal; reinstall through +PAIR to establish a new claim. A copy put there later is a different file, and so is a bundle oMLX's own updater swaps in: after an in-app update PAIR no longer offers uninstall, and the app is removed from the Finder. Uninstalling an oMLX PAIR installed quits the app and removes `/Applications/oMLX.app` and the two shims it writes (`omlx`, diff --git a/services/nvpair-engine-manager/fileid_darwin.go b/services/nvpair-engine-manager/fileid_darwin.go new file mode 100644 index 000000000..0b69c23fd --- /dev/null +++ b/services/nvpair-engine-manager/fileid_darwin.go @@ -0,0 +1,11 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "syscall" + +// Birth time survives the staging-to-destination rename and metadata changes. +func fileGenerationTime(st *syscall.Stat_t) syscall.Timespec { + return st.Birthtimespec +} diff --git a/services/nvpair-engine-manager/fileid_darwin_test.go b/services/nvpair-engine-manager/fileid_darwin_test.go new file mode 100644 index 000000000..42c216544 --- /dev/null +++ b/services/nvpair-engine-manager/fileid_darwin_test.go @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "syscall" + "testing" +) + +// Reusing a device/inode must not transfer an install claim to a later bundle. +func TestFileIdentityDistinguishesReusedInode(t *testing.T) { + st := syscall.Stat_t{Dev: 1, Ino: 2, Birthtimespec: syscall.Timespec{Sec: 3, Nsec: 4}} + original := fileIdentityFromStat(&st) + if got := fileIdentityFromStat(&st); got != original { + t.Fatal("unchanged bundle identity is unstable") + } + st.Ctimespec = syscall.Timespec{Sec: 5, Nsec: 6} + if got := fileIdentityFromStat(&st); got != original { + t.Fatal("metadata changes must not invalidate a Darwin bundle's identity") + } + st.Birthtimespec.Nsec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different birth nanosecond retained the old identity") + } + st.Birthtimespec.Nsec-- + st.Birthtimespec.Sec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different birth second retained the old identity") + } +} diff --git a/services/nvpair-engine-manager/fileid_linux.go b/services/nvpair-engine-manager/fileid_linux.go new file mode 100644 index 000000000..1c0edc76a --- /dev/null +++ b/services/nvpair-engine-manager/fileid_linux.go @@ -0,0 +1,12 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "syscall" + +// Stat_t has no birth time on Linux. ctime distinguishes reused inodes and +// stays stable while the installed bundle is unchanged. +func fileGenerationTime(st *syscall.Stat_t) syscall.Timespec { + return st.Ctim +} diff --git a/services/nvpair-engine-manager/fileid_linux_test.go b/services/nvpair-engine-manager/fileid_linux_test.go new file mode 100644 index 000000000..45caecc87 --- /dev/null +++ b/services/nvpair-engine-manager/fileid_linux_test.go @@ -0,0 +1,27 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "syscall" + "testing" +) + +// Linux can immediately reuse the inode of a deleted bundle directory. +func TestFileIdentityDistinguishesReusedInode(t *testing.T) { + st := syscall.Stat_t{Dev: 1, Ino: 2, Ctim: syscall.Timespec{Sec: 3, Nsec: 4}} + original := fileIdentityFromStat(&st) + if got := fileIdentityFromStat(&st); got != original { + t.Fatal("unchanged bundle identity is unstable") + } + st.Ctim.Nsec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different change nanosecond retained the old identity") + } + st.Ctim.Nsec-- + st.Ctim.Sec++ + if got := fileIdentityFromStat(&st); got == original { + t.Fatal("reused inode with a different change second retained the old identity") + } +} diff --git a/services/nvpair-engine-manager/fileid_unix.go b/services/nvpair-engine-manager/fileid_unix.go index b71b98040..256403e5b 100644 --- a/services/nvpair-engine-manager/fileid_unix.go +++ b/services/nvpair-engine-manager/fileid_unix.go @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//go:build !windows +//go:build darwin || linux package main @@ -11,8 +11,9 @@ import ( "syscall" ) -// fileIdentity is a path's device and inode, without following a final link, -// or "" when it cannot be read. +// fileIdentity combines device, inode, and a generation timestamp without +// following a final link. The timestamp prevents inode reuse from transferring +// an install claim to a replacement bundle. It returns "" on read failure. func fileIdentity(path string) string { info, err := os.Lstat(path) if err != nil { @@ -22,5 +23,10 @@ func fileIdentity(path string) string { if !ok { return "" } - return fmt.Sprintf("%d:%d", st.Dev, st.Ino) + return fileIdentityFromStat(st) +} + +func fileIdentityFromStat(st *syscall.Stat_t) string { + stamp := fileGenerationTime(st) + return fmt.Sprintf("%d:%d:%d:%d", st.Dev, st.Ino, stamp.Sec, stamp.Nsec) } diff --git a/services/nvpair-node-info/sampling_test.go b/services/nvpair-node-info/sampling_test.go index 2c4738453..dac53231c 100644 --- a/services/nvpair-node-info/sampling_test.go +++ b/services/nvpair-node-info/sampling_test.go @@ -15,11 +15,11 @@ func TestPolicyTable(t *testing.T) { epoch := time.Unix(1000000, 0) cases := []struct { - name string - setup func(p *SamplingPolicy, now time.Time) - evalAt time.Duration - wantInterval time.Duration - wantMode SamplingMode + name string + setup func(p *SamplingPolicy, now time.Time) + evalAt time.Duration + wantInterval time.Duration + wantMode SamplingMode }{ { name: "clean idle: no leases, no demand", @@ -189,9 +189,9 @@ func TestPollIntentClassification(t *testing.T) { {"foreground", PollIntentForeground}, {"scheduling", PollIntentScheduling}, {"background", PollIntentBackground}, - {"", PollIntentForeground}, // missing header behaves as foreground - {"unknown", PollIntentForeground}, // unknown header behaves as foreground - {"FOREGROUND", PollIntentForeground},// unknown/cased behaves as foreground + {"", PollIntentForeground}, // missing header behaves as foreground + {"unknown", PollIntentForeground}, // unknown header behaves as foreground + {"FOREGROUND", PollIntentForeground}, // unknown/cased behaves as foreground } for _, tc := range cases { if got := parsePollIntent(tc.header); got != tc.want { diff --git a/services/nvpair-ui-broker/nodeinfo_demand_test.go b/services/nvpair-ui-broker/nodeinfo_demand_test.go index 62457a631..fe1f8a664 100644 --- a/services/nvpair-ui-broker/nodeinfo_demand_test.go +++ b/services/nvpair-ui-broker/nodeinfo_demand_test.go @@ -36,9 +36,9 @@ func TestWriteDemandFrame(t *testing.T) { } var frame struct { - JSONRPC string `json:"jsonrpc"` - Method string `json:"method"` - ID json.RawMessage `json:"id"` + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + ID json.RawMessage `json:"id"` Params noderec.DemandParams `json:"params"` } if err := json.Unmarshal([]byte(line), &frame); err != nil { @@ -78,7 +78,7 @@ func TestBrokerDemandOnWorkloadTransitions(t *testing.T) { continue } var f struct { - Method string `json:"method"` + Method string `json:"method"` Params noderec.DemandParams `json:"params"` } if err := json.Unmarshal([]byte(l), &f); err == nil && f.Method == "stats/set-demand" { @@ -205,7 +205,7 @@ func TestBrokerDemandOnNodeInfoRestart(t *testing.T) { t.Fatalf("expected 1 frame on restart, got %d: %q", len(lines), buf.String()) } var f struct { - Method string `json:"method"` + Method string `json:"method"` Params noderec.DemandParams `json:"params"` } if err := json.Unmarshal([]byte(lines[0]), &f); err != nil { diff --git a/services/nvpair-ui-broker/supervisor.go b/services/nvpair-ui-broker/supervisor.go index a178068e1..b272f4172 100644 --- a/services/nvpair-ui-broker/supervisor.go +++ b/services/nvpair-ui-broker/supervisor.go @@ -79,6 +79,16 @@ func (p hangPolicy) withDefaults() hangPolicy { return p } +// startProbe transfers cancellation ownership to watch, which cancels on +// completion, deadline, or any exit. A buffered result lets a late probe finish +// without blocking after watch has moved on to a replacement handle. +func (p hangPolicy) startProbe() (context.Context, context.CancelFunc, <-chan error) { + ctx, cancel := context.WithTimeout(context.Background(), p.deadline) + result := make(chan error, 1) + go func() { result <- p.probe(ctx) }() + return ctx, cancel, result +} + // restartPolicy configures the hybrid auto-restart behaviour the broker // applies to a crashed worker. The defaults (see defaultRestartPolicy) // implement: exponential backoff between attempts, a bounded budget so a @@ -535,10 +545,8 @@ func (s *supervisor) watch(h supervisedHandle, onStable func(), onHealth func(er if probeC != nil { continue // Never overlap probes for the same live handle. } - probeCtx, cancel = context.WithTimeout(context.Background(), policy.deadline) - result := make(chan error, 1) - probeC, deadlineC = result, probeCtx.Done() - go func(ctx context.Context) { result <- policy.probe(ctx) }(probeCtx) + probeCtx, cancel, probeC = policy.startProbe() + deadlineC = probeCtx.Done() case err := <-probeC: if probeCtx.Err() != nil { err = probeCtx.Err() // A late success cannot beat the deadline. diff --git a/services/nvpair-ui-broker/supervisor_test.go b/services/nvpair-ui-broker/supervisor_test.go index 23aee3306..ce9dd7eff 100644 --- a/services/nvpair-ui-broker/supervisor_test.go +++ b/services/nvpair-ui-broker/supervisor_test.go @@ -32,6 +32,55 @@ func (h *fakeHandle) Stop() { h.stopOnce.Do(func() { close(h.stoppedC) }) } // crash closes the handle's done channel, simulating an unexpected exit. func (h *fakeHandle) crash() { close(h.done) } +func TestWatchCancelsInFlightProbeOnExit(t *testing.T) { + for _, exit := range []string{"stop", "restart", "worker-exit"} { + t.Run(exit, func(t *testing.T) { + sup := newSupervisor("cancel-test", restartPolicy{}, nil) + h := newFakeHandle() + started := make(chan context.Context, 1) + sup.hang = hangPolicy{ + interval: time.Millisecond, deadline: time.Hour, + probe: func(ctx context.Context) error { + started <- ctx + <-ctx.Done() + return ctx.Err() + }, + } + defer sup.stopOnce.Do(func() { close(sup.stopCh) }) + done := make(chan watchResult, 1) + go func() { done <- sup.watch(h, nil, nil) }() + var probeCtx context.Context + select { + case probeCtx = <-started: + case <-time.After(2 * time.Second): + t.Fatal("probe did not start") + } + want := watchStopped + switch exit { + case "stop": + sup.stopOnce.Do(func() { close(sup.stopCh) }) + case "restart": + sup.restartCh <- struct{}{} + want = watchRestart + case "worker-exit": + h.crash() + want = watchExited + } + select { + case got := <-done: + if got != want { + t.Fatalf("watch = %v, want %v", got, want) + } + case <-time.After(2 * time.Second): + t.Fatal("watch did not exit") + } + if !errors.Is(probeCtx.Err(), context.Canceled) { + t.Fatalf("in-flight probe not canceled on %s: %v", exit, probeCtx.Err()) + } + }) + } +} + func TestRestartPolicyBackoff(t *testing.T) { p := restartPolicy{baseDelay: time.Second, maxDelay: 16 * time.Second} cases := []struct { diff --git a/services/readme.md b/services/readme.md index 955c7404e..1c49f8592 100644 --- a/services/readme.md +++ b/services/readme.md @@ -280,8 +280,19 @@ cd tests go test ./... ``` +Every runtime launch uses a per-test temporary app-data root for `HOME`, +`USERPROFILE`, `XDG_CONFIG_HOME`, `APPDATA`, and `LOCALAPPDATA`, inherited by +supervised workers. Restart tests explicitly reuse their own temporary root. +Runtime commands inherit the remaining environment with those five roots +overridden; build commands keep the original home/config roots so Go's caches +are not redirected into disposable test state. The +`TestProcessHelpersRequireIsolation` source guard rejects launch helpers that +bypass the isolated command factory or replace its environment. + Expect this to take a few minutes. Run it before opening a merge request, and -after any change to a JSON-RPC method, payload, or notification. +after any change to a JSON-RPC method, payload, or notification. CI runs the +cross-process suite on macOS arm64; component vet/tests run on both macOS and +Linux. ### Tests that skip themselves diff --git a/services/shared/splitlisten/splitlisten_test.go b/services/shared/splitlisten/splitlisten_test.go index 2843d3f1e..f8cd78fcc 100644 --- a/services/shared/splitlisten/splitlisten_test.go +++ b/services/shared/splitlisten/splitlisten_test.go @@ -247,11 +247,11 @@ func testServerTLSConfig(t *testing.T) *tls.Config { // scriptedListener is a net.Listener whose Accept behavior is driven by acceptFn. type scriptedListener struct { - addr net.Addr - acceptFn func(call int) (net.Conn, error) - mu sync.Mutex - calls int - closed chan struct{} + addr net.Addr + acceptFn func(call int) (net.Conn, error) + mu sync.Mutex + calls int + closed chan struct{} closeOnce sync.Once } diff --git a/services/tests/broker_management_test.go b/services/tests/broker_management_test.go index 3342878ed..967165749 100644 --- a/services/tests/broker_management_test.go +++ b/services/tests/broker_management_test.go @@ -149,6 +149,10 @@ func TestBrokerBridgesManualNodeIntoProxy(t *testing.T) { stopOllama := fakeOllama(t) // skips if 11434 unavailable t.Cleanup(stopOllama) + // Own node-info as well as the engine: a running local PAIR must not + // supply its cluster principal/UUID to this deliberately unclustered node. + t.Cleanup(fakeNodeInfo(t, "")) + stdin, msgs, _, cleanup := startBrokerWith(t, "--manual-nodes-path", manualNodesBin, "--proxy-path", proxyBin, @@ -257,6 +261,7 @@ func TestBrokerBridgesManualNodeIntoLMStudioProxy(t *testing.T) { configDir := persistLMStudioProxyPort(t, freePort(t)) stopLM := fakeLMStudio(t) // skips if 1234 unavailable t.Cleanup(stopLM) + t.Cleanup(fakeNodeInfo(t, "")) stdin, msgs, _, cleanup := startBrokerWithConfigDir(t, configDir, "--manual-nodes-path", manualNodesBin, diff --git a/services/tests/broker_supervision_test.go b/services/tests/broker_supervision_test.go index 03266b25b..07f5db174 100644 --- a/services/tests/broker_supervision_test.go +++ b/services/tests/broker_supervision_test.go @@ -15,7 +15,6 @@ import ( "io" "net" "os" - "os/exec" "path/filepath" "regexp" "runtime" @@ -75,18 +74,9 @@ func startBrokerWithDirsAndEnv(t *testing.T, configDir, clusterDir string, extra func startSupervisedBrokerInDir(t *testing.T, workDir, configDir, clusterDir string, extraEnv []string, extraArgs ...string) (stdin io.WriteCloser, msgs <-chan jsonrpc.Message, stderrLines <-chan string, cleanup func()) { t.Helper() args := append([]string{"--scanner-path", scannerBin, "--cluster-dir", clusterDir}, extraArgs...) - cmd := exec.Command(brokerBin, args...) + cmd := testCommandInDir(t, configDir, brokerBin, args...) cmd.Dir = workDir - // Every supervised worker must use disposable per-test config. In - // particular, managed-port startup can persist an Ollama backend port; a - // cross-process test must never read or rewrite the developer's real file. - cmd.Env = append(os.Environ(), - "HOME="+configDir, - "XDG_CONFIG_HOME="+configDir, - "APPDATA="+configDir, - "LOCALAPPDATA="+configDir, - ) - cmd.Env = append(cmd.Env, extraEnv...) + addTestEnv(t, cmd, extraEnv...) stdinPipe, err := cmd.StdinPipe() if err != nil { @@ -199,19 +189,42 @@ const methodErrorsUpdate = "errors:update" var proxyPidRe = regexp.MustCompile(`proxy started.*\bpid=(\d+)`) -// portBusy reports whether a TCP port on localhost is already bound — used +// portBusy probes the same wildcard address the workers bind, not just IPv4 +// loopback (which can coexist with an IPv6 listener on macOS). It is used // to skip tests whose supervised workers need a fatal-on-conflict listener // (nvpair-errors --peer-sync on 14319, ollama-proxy on 11435) when something // else on the host already holds it. func portBusy(port int) bool { - ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) - if err != nil { - return true + for _, host := range []string{"", "127.0.0.1", "::1"} { + if host == "::1" { + // An unavailable IPv6 loopback is not an occupied port. Probe + // support independently before interpreting a bind failure. + probe, err := net.Listen("tcp6", "[::1]:0") + if err != nil { + continue + } + probe.Close() + } + ln, err := net.Listen("tcp", net.JoinHostPort(host, strconv.Itoa(port))) + if err != nil { + return true + } + ln.Close() } - ln.Close() return false } +func TestPortBusyDetectsIPv6Listener(t *testing.T) { + ln, err := net.Listen("tcp6", "[::1]:0") + if err != nil { + t.Skipf("IPv6 unavailable: %v", err) + } + defer ln.Close() + if !portBusy(ln.Addr().(*net.TCPAddr).Port) { + t.Fatal("IPv6 listener was missed by the worker-port preflight") + } +} + // TestBrokerCrashSurfacingAndRestart kills the broker's supervised proxy // and asserts the crash is surfaced as supervisor:subprocess-crashed:proxy // through the nvpair-errors pipeline (errors:update relay + errors:get-initial), @@ -291,7 +304,7 @@ func TestBrokerShutsDownOnSignal(t *testing.T) { // other tests' teardown already exercises. t.Skip("signalling a specific process is not supported on Windows") } - cmd := exec.Command(brokerBin, + cmd := testCommand(t, brokerBin, "--scanner-path", scannerBin, "--proxy-path", proxyBin, "--cluster-dir", t.TempDir(), @@ -424,20 +437,11 @@ func TestBrokerProxySetPortRebinds(t *testing.T) { if portBusy(11435) { t.Skip("ollama-proxy default port 11435 already in use; skipping") } - cfg := t.TempDir() - cmd := exec.Command(brokerBin, + cmd := testCommand(t, brokerBin, "--scanner-path", scannerBin, "--proxy-path", proxyBin, "--engine-manager-path", engineMgrBin, ) - // Isolate per-user state (proxy-port.json, any engine override) to a - // temp dir so the test leaves the real config untouched. - 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 { @@ -583,7 +587,7 @@ func waitWorkerRunning(t *testing.T, stdin io.Writer, msgs <-chan jsonrpc.Messag func signalWorker(t *testing.T, pid int, signal string) { t.Helper() - if output, err := exec.Command("kill", "-"+signal, strconv.Itoa(pid)).CombinedOutput(); err != nil { + if output, err := testCommand(t, "kill", "-"+signal, strconv.Itoa(pid)).CombinedOutput(); err != nil { t.Fatalf("kill -%s %d: %v %s", signal, pid, err, output) } } @@ -597,7 +601,7 @@ func TestHungSchedulerRestarts(t *testing.T) { waitForMethod(t, msgs, "app:ready", 10*time.Second) old := waitWorkerRunning(t, stdin, msgs, "scheduler").PID signalWorker(t, old, "STOP") - t.Cleanup(func() { _ = exec.Command("kill", "-CONT", strconv.Itoa(old)).Run() }) + t.Cleanup(func() { _ = testCommand(t, "kill", "-CONT", strconv.Itoa(old)).Run() }) waitWorkerState(t, msgs, "scheduler", "degraded", 8*time.Second) waitWorkerState(t, msgs, "scheduler", "restarting", 10*time.Second) status := waitWorkerState(t, msgs, "scheduler", "running", 5*time.Second) @@ -609,7 +613,7 @@ func TestHungSchedulerRestarts(t *testing.T) { tick := time.NewTicker(20 * time.Millisecond) defer tick.Stop() for { - if exec.Command("kill", "-0", strconv.Itoa(old)).Run() != nil { + if testCommand(t, "kill", "-0", strconv.Itoa(old)).Run() != nil { break } select { @@ -632,7 +636,7 @@ func TestHungEngineManagerOnlyDegrades(t *testing.T) { Workers []workerHealthStatus `json:"workers"` } signalWorker(t, pid, "STOP") - t.Cleanup(func() { _ = exec.Command("kill", "-CONT", strconv.Itoa(pid)).Run() }) + t.Cleanup(func() { _ = testCommand(t, "kill", "-CONT", strconv.Itoa(pid)).Run() }) waitWorkerState(t, msgs, "engine-manager", "degraded", 8*time.Second) timer := time.NewTimer(10 * time.Second) defer timer.Stop() diff --git a/services/tests/broker_test.go b/services/tests/broker_test.go index 22944f5b2..c947be6fd 100644 --- a/services/tests/broker_test.go +++ b/services/tests/broker_test.go @@ -27,7 +27,6 @@ import ( "fmt" "io" "os" - "os/exec" "testing" "time" @@ -59,7 +58,7 @@ type subscriptionResult struct { func startBroker(t *testing.T) (stdin io.WriteCloser, msgs <-chan jsonrpc.Message, cleanup func()) { t.Helper() - cmd := exec.Command(brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()) + cmd := testCommand(t, brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() @@ -192,6 +191,7 @@ func waitForPushContaining(t *testing.T, msgs <-chan jsonrpc.Message, instance s } func TestCrossProcessBrokerSubscription(t *testing.T) { + requireMulticast(t) const ( inst1 = "xproc-broker-1" inst2 = "xproc-broker-2" diff --git a/services/tests/cluster_manager_test.go b/services/tests/cluster_manager_test.go index bec9cb999..704304423 100644 --- a/services/tests/cluster_manager_test.go +++ b/services/tests/cluster_manager_test.go @@ -36,10 +36,8 @@ func startCM(t *testing.T, configDir string, port int) *cmProc { // cancel-vs-Completion commit-window delay). func startCMEnv(t *testing.T, configDir string, port int, extraEnv []string) *cmProc { t.Helper() - cmd := exec.Command(clusterMgrBin, "--config-dir", configDir, "--port", strconv.Itoa(port)) - if len(extraEnv) > 0 { - cmd.Env = append(os.Environ(), extraEnv...) - } + cmd := testCommand(t, clusterMgrBin, "--config-dir", configDir, "--port", strconv.Itoa(port)) + addTestEnv(t, cmd, extraEnv...) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("stdin pipe: %v", err) diff --git a/services/tests/cluster_restore_test.go b/services/tests/cluster_restore_test.go index a8d37123c..e5261c47b 100644 --- a/services/tests/cluster_restore_test.go +++ b/services/tests/cluster_restore_test.go @@ -13,7 +13,6 @@ import ( "fmt" "io" "os" - "os/exec" "testing" "time" @@ -26,13 +25,7 @@ import ( func startBrokerInDir(t *testing.T, configDir string, extraArgs ...string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() args := append([]string{"--scanner-path", scannerBin, "--cluster-dir", t.TempDir()}, extraArgs...) - cmd := exec.Command(brokerBin, args...) - cmd.Env = append(os.Environ(), - "HOME="+configDir, - "XDG_CONFIG_HOME="+configDir, - "APPDATA="+configDir, - "LOCALAPPDATA="+configDir, - ) + cmd := testCommandInDir(t, configDir, brokerBin, args...) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("broker stdin pipe: %v", err) @@ -85,6 +78,9 @@ func waitForResponseID(t *testing.T, msgs <-chan jsonrpc.Message, id int, timeou // config dir, and asserts cluster:get-node-id reports the restored clusterId // (rather than the "" a fresh cluster-manager comes up with). func TestBrokerRestoresClusterIdentityAfterRestart(t *testing.T) { + if portBusy(14321) { + t.Skip("cluster-manager inter-node port 14321 already in use; skipping") + } configDir := t.TempDir() const clusterID = "restore-test-cluster-0001" const friendly = "Restore Test Lab" diff --git a/services/tests/discovery_relay_test.go b/services/tests/discovery_relay_test.go index 7ac1510a9..fd1ed52f6 100644 --- a/services/tests/discovery_relay_test.go +++ b/services/tests/discovery_relay_test.go @@ -17,6 +17,7 @@ import ( // ni=14318. This is the new consolidated discovery record (distinct from // node-info's own transitional _nvpair-node-info advertisement). func TestBrokerAdvertisesNodeRecord(t *testing.T) { + requireMulticast(t) _, msgs, stderr, cleanup := startBrokerWith(t, "--node-info-path", nodeInfoBin) t.Cleanup(cleanup) // Drain the broker's stdout/stderr so it doesn't stall on a full pipe while diff --git a/services/tests/discovery_test.go b/services/tests/discovery_test.go index cd9a6c05f..59da2fc18 100644 --- a/services/tests/discovery_test.go +++ b/services/tests/discovery_test.go @@ -15,6 +15,7 @@ import ( const testDomain = "local." func TestInProcessDiscovery(t *testing.T) { + requireMulticast(t) const ( service = "_test-disc._tcp" instance = "in-proc-test" @@ -71,6 +72,7 @@ func TestInProcessDiscovery(t *testing.T) { } func TestInProcessMultipleInstances(t *testing.T) { + requireMulticast(t) const service = "_test-multi._tcp" instances := []string{"node-alpha", "node-beta", "node-gamma"} @@ -121,6 +123,7 @@ func TestInProcessMultipleInstances(t *testing.T) { } func TestInProcessRemoval(t *testing.T) { + requireMulticast(t) const ( service = "_test-rmv._tcp" instance = "removable-node" diff --git a/services/tests/errors_test.go b/services/tests/errors_test.go index 16d291a44..0e9a16bc7 100644 --- a/services/tests/errors_test.go +++ b/services/tests/errors_test.go @@ -9,7 +9,6 @@ import ( "encoding/json" "io" "os" - "os/exec" "sort" "strconv" "sync" @@ -36,7 +35,7 @@ import ( // - clear → update broadcast with empty list // - re-report after clear → resurrect (ack-until-reemit) func TestErrorsBrokerPipeline_EndToEnd(t *testing.T) { - cmd := exec.Command(errorsBin) + cmd := testCommand(t, errorsBin) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("stdin pipe: %v", err) diff --git a/services/tests/ghost_node_test.go b/services/tests/ghost_node_test.go index 1cff4d68c..a50bae8c1 100644 --- a/services/tests/ghost_node_test.go +++ b/services/tests/ghost_node_test.go @@ -10,7 +10,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "strconv" "strings" "sync/atomic" @@ -42,6 +41,7 @@ func TestScannerEvictsRecordSupersededAtItsAddress(t *testing.T) { if testing.Short() { t.Skip("multicast + miss-threshold timing; skipped under -short") } + requireMulticast(t) const ( peerInstance = "ghost-peer-host" @@ -129,7 +129,7 @@ func TestScannerEvictsRecordSupersededAtItsAddress(t *testing.T) { // real cluster identity on the LAN while the test runs. func startScannerForGhostTest(t *testing.T) <-chan jsonrpc.Message { t.Helper() - cmd := exec.Command(scannerBin, "--cluster-dir", t.TempDir()) + cmd := testCommand(t, scannerBin, "--cluster-dir", t.TempDir()) cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() diff --git a/services/tests/go.mod b/services/tests/go.mod index 38bfb3a9c..f61403558 100644 --- a/services/tests/go.mod +++ b/services/tests/go.mod @@ -4,6 +4,7 @@ go 1.25.0 require ( github.com/grandcat/zeroconf v1.0.0 + golang.org/x/net v0.58.0 nvpair-shared v0.0.0-00010101000000-000000000000 ) @@ -14,7 +15,6 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/miekg/dns v1.1.55 // indirect golang.org/x/mod v0.38.0 // indirect - golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/tools v0.48.0 // indirect diff --git a/services/tests/isolation_test.go b/services/tests/isolation_test.go new file mode 100644 index 000000000..192b70192 --- /dev/null +++ b/services/tests/isolation_test.go @@ -0,0 +1,182 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tests + +import ( + "encoding/json" + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +// TestProcessHelpersRequireIsolation prevents new launch helpers from bypassing +// the sole runtime command constructor. goBuild deliberately keeps the real Go +// cache environment: it builds, but never executes, an application binary. +func TestProcessHelpersRequireIsolation(t *testing.T) { + files, err := filepath.Glob("*.go") + if err != nil { + t.Fatal(err) + } + fset := token.NewFileSet() + for _, path := range files { + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatal(err) + } + execNames := map[string]bool{} + osNames := map[string]bool{} + for _, imp := range file.Imports { + if imp.Path.Value == `"os"` { + name := "os" + if imp.Name != nil { + name = imp.Name.Name + } + if name == "." { + t.Errorf("%s: dot import bypasses isolation guard", path) + } + osNames[name] = true + } + if imp.Path.Value == `"os/exec"` { + name := "exec" + if imp.Name != nil { + name = imp.Name.Name + } + if name == "." { + t.Errorf("%s: dot import bypasses isolation guard", path) + } + execNames[name] = true + } + } + // *exec.Cmd annotations are harmless; values and constructors must + // come from the isolated factory, including new(exec.Cmd) or literals. + pointerTypes := map[*ast.SelectorExpr]bool{} + ast.Inspect(file, func(node ast.Node) bool { + if ptr, ok := node.(*ast.StarExpr); ok { + if sel, ok := ptr.X.(*ast.SelectorExpr); ok { + pointerTypes[sel] = true + } + } + return true + }) + for _, decl := range file.Decls { + fn, isFunc := decl.(*ast.FuncDecl) + allowed := isFunc && path == "main_test.go" && (fn.Name.Name == "goBuild" || fn.Name.Name == "testCommandInDir") + envAllowed := allowed || (isFunc && path == "main_test.go" && fn.Name.Name == "addTestEnv") + ast.Inspect(decl, func(node ast.Node) bool { + if assign, ok := node.(*ast.AssignStmt); ok && !envAllowed { + for _, lhs := range assign.Lhs { + if sel, ok := lhs.(*ast.SelectorExpr); ok && sel.Sel.Name == "Env" { + t.Errorf("%s: use addTestEnv instead of replacing isolated Env", fset.Position(sel.Pos())) + } + } + } + sel, ok := node.(*ast.SelectorExpr) + if !ok { + return true + } + pkg, ok := sel.X.(*ast.Ident) + if ok && osNames[pkg.Name] && sel.Sel.Name == "StartProcess" { + t.Errorf("%s: use testCommand instead of os.StartProcess", fset.Position(sel.Pos())) + } + if ok && execNames[pkg.Name] && !pointerTypes[sel] && !allowed { + t.Errorf("%s: use testCommand/testCommandInDir instead of os/exec.%s", fset.Position(sel.Pos()), sel.Sel.Name) + } + return true + }) + } + } +} + +// Ported from NVIDIA/Personal-AI-Router@937cf6c, using this fork's +// standalone LM Studio proxy rather than the upstream unified proxy facade. +func TestLMStudioChildPersistsUnderPrivateBase(t *testing.T) { + root := t.TempDir() + cmd := testCommandInDir(t, root, lmstudioProxyBin, "--port", "0", "--ignore-persisted-port") + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = stdin.Close(); _ = cmd.Process.Kill(); _ = cmd.Wait() }) + msgs := startMsgReader(stdout) + waitForMethod(t, msgs, "ready", 10*time.Second) + var port int + for attempt := 0; attempt < 8; attempt++ { + port = freePort(t) + sendLine(t, stdin, map[string]any{ + "jsonrpc": "2.0", "id": attempt + 1, "method": "set-port", + "params": map[string]int{"port": port}, + }) + resp := waitForResponse(t, msgs, 5*time.Second) + if resp.Error == nil { + break + } + if !strings.HasPrefix(resp.Error.Message, "failed to bind port") { + t.Fatalf("set-port: %+v", resp.Error) + } + if attempt == 7 { + t.Fatal("all probed ports were taken before bind") + } + } + base := root + if runtime.GOOS == "darwin" { + base = filepath.Join(root, "Library", "Application Support") + } + path := filepath.Join(base, "Nvidia Corporation", "Personal AI Router", "lmstudio-proxy-port.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("child did not persist inside its private root: %v", err) + } + var saved struct { + Port int `json:"port"` + } + if err := json.Unmarshal(data, &saved); err != nil { + t.Fatal(err) + } + if saved.Port != port { + t.Fatalf("saved port = %d, want %d", saved.Port, port) + } +} + +func TestCommandEnvironmentIsPerTest(t *testing.T) { + var roots []string + for _, name := range []string{"first", "second"} { + t.Run(name, func(t *testing.T) { + cmd := testCommand(t, "not-executed") + values := map[string]string{} + for _, entry := range cmd.Environ() { + key, value, _ := strings.Cut(entry, "=") + values[key] = value + } + root := values["HOME"] + if root == "" || root == os.Getenv("HOME") { + t.Fatal("HOME is not isolated") + } + if _, err := os.Stat(root); err != nil { + t.Fatal(err) + } + for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA", "USERPROFILE"} { + if values[key] != root { + t.Errorf("%s = %q, want %q", key, values[key], root) + } + } + roots = append(roots, root) + }) + } + if len(roots) == 2 && roots[0] == roots[1] { + t.Fatal("tests share an app-data root") + } +} diff --git a/services/tests/log_level_test.go b/services/tests/log_level_test.go index c1a1dfb0d..452601de4 100644 --- a/services/tests/log_level_test.go +++ b/services/tests/log_level_test.go @@ -8,7 +8,6 @@ import ( "bytes" "encoding/json" "io" - "os/exec" "strings" "sync" "testing" @@ -47,7 +46,7 @@ func (b *safeBuffer) Reset() { func startProxyWithLog(t *testing.T, level string) (stdin io.WriteCloser, msgs <-chan jsonrpc.Message, stderr *safeBuffer, cleanup func()) { t.Helper() - cmd := exec.Command(proxyBin, "--log-level", level) + cmd := testCommand(t, proxyBin, "--log-level", level) stderrBuf := &safeBuffer{} cmd.Stderr = stderrBuf @@ -185,8 +184,8 @@ func TestLogSetLevelViaRPC(t *testing.T) { // TestLogLevelEnvFallback verifies NVPAIR_LOG_LEVEL is honoured when --log-level // is not passed. func TestLogLevelEnvFallback(t *testing.T) { - cmd := exec.Command(proxyBin) - cmd.Env = append(cmd.Environ(), "NVPAIR_LOG_LEVEL=debug") + cmd := testCommand(t, proxyBin) + addTestEnv(t, cmd, "NVPAIR_LOG_LEVEL=debug") stderrBuf := &safeBuffer{} cmd.Stderr = stderrBuf diff --git a/services/tests/main_test.go b/services/tests/main_test.go index 52171a1cc..7b5f76e2d 100644 --- a/services/tests/main_test.go +++ b/services/tests/main_test.go @@ -12,6 +12,7 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" "testing" "time" @@ -164,6 +165,41 @@ func goBuild(srcDir, output string) error { return cmd.Run() } +var isolatedHomeKeys = []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA", "USERPROFILE"} + +// testCommand is the only runtime command factory. Like upstream's post-build +// isolation, this leaves goBuild's module/build caches alone, but gives each +// test child (and all its supervised workers) disposable app-data state. +func testCommand(t *testing.T, binary string, args ...string) *exec.Cmd { + t.Helper() + return testCommandInDir(t, t.TempDir(), binary, args...) +} + +// An explicit root lets restart tests reuse their own persisted state. +func testCommandInDir(t *testing.T, root, binary string, args ...string) *exec.Cmd { + t.Helper() + cmd := exec.Command(binary, args...) + cmd.Env = os.Environ() + for _, key := range isolatedHomeKeys { + cmd.Env = append(cmd.Env, key+"="+root) + } + return cmd +} + +// addTestEnv permits scenario-specific overrides without undoing isolation. +func addTestEnv(t *testing.T, cmd *exec.Cmd, entries ...string) { + t.Helper() + for _, entry := range entries { + key, _, _ := strings.Cut(entry, "=") + for _, isolated := range isolatedHomeKeys { + if strings.EqualFold(key, isolated) { + t.Fatalf("test environment cannot override %s", isolated) + } + } + } + cmd.Env = append(cmd.Env, entries...) +} + // --- JSON-RPC helpers --- // The on-wire JSON-RPC frame and error envelope are the shared diff --git a/services/tests/models_http_test.go b/services/tests/models_http_test.go index baf5601de..20e3708e3 100644 --- a/services/tests/models_http_test.go +++ b/services/tests/models_http_test.go @@ -24,6 +24,7 @@ import ( ) func TestModelsHTTPEnrichment(t *testing.T) { + requireMulticast(t) // Stand-in engine-manager HTTP surface serving the model list. srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/v1/models" { diff --git a/services/tests/models_refresh_test.go b/services/tests/models_refresh_test.go index 608787983..9bd318cf3 100644 --- a/services/tests/models_refresh_test.go +++ b/services/tests/models_refresh_test.go @@ -55,6 +55,7 @@ func (s *swapModels) handler() http.HandlerFunc { } func TestModelsPeriodicRefreshConvergesWithoutMDNSChange(t *testing.T) { + requireMulticast(t) // Start with an empty inventory (engine present but serving no models yet). stub := &swapModels{} stub.set(map[string]any{"models": []string{}}) diff --git a/services/tests/multicast_probe_test.go b/services/tests/multicast_probe_test.go new file mode 100644 index 000000000..05828e0dd --- /dev/null +++ b/services/tests/multicast_probe_test.go @@ -0,0 +1,185 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tests + +import ( + "encoding/binary" + "errors" + "fmt" + "net" + "strings" + "sync" + "syscall" + "testing" + "time" + + "golang.org/x/net/ipv4" +) + +var ( + multicastOnce sync.Once + multicastSkip string + multicastFailed error + multicastNotes []string +) + +// requireMulticast skips a test that needs real mDNS discovery when this host +// cannot send IPv4 multicast through any interface. GitHub's hosted macOS +// runners are such a host: every send to 224.0.0.251 fails with "no route to +// host", so no node is ever discovered. Like discovery, the probe tries every +// eligible interface, so a host that can multicast on any of them runs the +// test. An unexpected socket error fails the test rather than skipping it. +func requireMulticast(t *testing.T) { + t.Helper() + multicastOnce.Do(func() { + multicastSkip, multicastFailed, multicastNotes = decideMulticast(probeInterfaces()) + }) + for _, note := range multicastNotes { + t.Logf("multicast probe: %s", note) + } + if multicastFailed != nil { + t.Fatalf("multicast probe: %v", multicastFailed) + } + if multicastSkip != "" { + t.Skipf("environment: this host cannot send mDNS multicast (%s); this test needs real mDNS discovery", multicastSkip) + } +} + +// interfaceProbe is one eligible interface's send result. +type interfaceProbe struct { + name string + err error +} + +// probeInterfaces sends one mDNS PTR query, for a name nothing answers, out of +// every up, multicast-capable, non-loopback interface with an IPv4 address — +// the interfaces discovery sends on. +func probeInterfaces() []interfaceProbe { + ifaces, err := net.Interfaces() + if err != nil { + return []interfaceProbe{{name: "interfaces", err: err}} + } + var out []interfaceProbe + for i := range ifaces { + ifi := ifaces[i] + if ifi.Flags&net.FlagUp == 0 || ifi.Flags&net.FlagMulticast == 0 || ifi.Flags&net.FlagLoopback != 0 { + continue + } + src := firstIPv4(&ifi) + if src == nil { + continue + } + out = append(out, interfaceProbe{name: ifi.Name, err: sendProbe(&ifi, src)}) + } + return out +} + +func firstIPv4(ifi *net.Interface) net.IP { + addrs, err := ifi.Addrs() + if err != nil { + return nil + } + for _, a := range addrs { + if ipnet, ok := a.(*net.IPNet); ok { + if ip4 := ipnet.IP.To4(); ip4 != nil { + return ip4 + } + } + } + return nil +} + +func sendProbe(ifi *net.Interface, src net.IP) error { + conn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: src}) + if err != nil { + return err + } + defer conn.Close() + pc := ipv4.NewPacketConn(conn) + if err := pc.SetMulticastInterface(ifi); err != nil { + return err + } + _ = pc.SetMulticastTTL(255) + _ = conn.SetWriteDeadline(time.Now().Add(2 * time.Second)) + _, err = conn.WriteToUDP(mdnsProbeQuery(), &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353}) + return err +} + +// decideMulticast turns the probe results into a decision. Every result is +// inspected: an unexpected error on any interface is reported as a note even +// when another interface sent, so nothing is masked. The test runs when any +// interface sent, since discovery tolerates per-interface failures too. It +// skips, naming why, only when none could send for lack of a multicast route +// (or there is no eligible interface), and fails when none could send and an +// error was anything else. +func decideMulticast(probes []interfaceProbe) (skip string, failed error, notes []string) { + if len(probes) == 0 { + return "no up, multicast-capable IPv4 interface", nil, nil + } + sent := false + var unexpected error + var reasons []string + for _, p := range probes { + if p.err == nil { + sent = true + continue + } + reasons = append(reasons, fmt.Sprintf("%s: %v", p.name, p.err)) + if !noMulticastRoute(p.err) { + unexpected = errors.Join(unexpected, fmt.Errorf("%s: %w", p.name, p.err)) + notes = append(notes, fmt.Sprintf("unexpected send error on %s: %v", p.name, p.err)) + } + } + switch { + case sent: + return "", nil, notes + case unexpected != nil: + return "", unexpected, notes + default: + return strings.Join(reasons, "; "), nil, notes + } +} + +// noMulticastRoute reports whether a send failed because the host has no +// route for multicast out of that interface, the environment limitation the +// skip exists for. +func noMulticastRoute(err error) bool { + return errors.Is(err, syscall.EHOSTUNREACH) || errors.Is(err, syscall.ENETUNREACH) || errors.Is(err, syscall.EADDRNOTAVAIL) +} + +func mdnsProbeQuery() []byte { + msg := make([]byte, 12) + binary.BigEndian.PutUint16(msg[4:], 1) // one question + for _, label := range []string{"_nvpair-ci-probe", "_tcp", "local"} { + msg = append(msg, byte(len(label))) + msg = append(msg, label...) + } + msg = append(msg, 0, 0, 12, 0, 1) // root, type PTR, class IN + return msg +} + +func TestDecideMulticast(t *testing.T) { + noRoute := &net.OpError{Op: "write", Net: "udp4", Err: syscall.EHOSTUNREACH} + odd := &net.OpError{Op: "write", Net: "udp4", Err: syscall.EPERM} + for _, tc := range []struct { + name string + probes []interfaceProbe + wantSkip bool + wantFail bool + wantNotes int + }{ + {"no eligible interface", nil, true, false, 0}, + {"one interface can send", []interfaceProbe{{"en0", nil}}, false, false, 0}, + {"any interface sending wins", []interfaceProbe{{"en0", noRoute}, {"en1", nil}}, false, false, 0}, + {"no route anywhere", []interfaceProbe{{"en0", noRoute}, {"utun3", noRoute}}, true, false, 0}, + {"an unexpected error with no sender fails", []interfaceProbe{{"en0", noRoute}, {"en1", odd}}, false, true, 1}, + {"success then unexpected error is noted", []interfaceProbe{{"en0", nil}, {"en1", odd}}, false, false, 1}, + {"unexpected error then success is noted", []interfaceProbe{{"en1", odd}, {"en0", nil}}, false, false, 1}, + } { + skip, failed, notes := decideMulticast(tc.probes) + if (skip != "") != tc.wantSkip || (failed != nil) != tc.wantFail || len(notes) != tc.wantNotes { + t.Errorf("%s: skip=%q failed=%v notes=%q", tc.name, skip, failed, notes) + } + } +} diff --git a/services/tests/node_settings_test.go b/services/tests/node_settings_test.go index 77221a64e..bd22e0319 100644 --- a/services/tests/node_settings_test.go +++ b/services/tests/node_settings_test.go @@ -7,7 +7,6 @@ import ( "encoding/json" "io" "os" - "os/exec" "path/filepath" "strconv" "sync/atomic" @@ -35,7 +34,7 @@ import ( func startNodeSettings(t *testing.T, settingsPath string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() - cmd := exec.Command(nodeSettingsBin, "--settings", settingsPath) + cmd := testCommand(t, nodeSettingsBin, "--settings", settingsPath) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() diff --git a/services/tests/protocol_v2_test.go b/services/tests/protocol_v2_test.go index 7969a324b..36fb250c5 100644 --- a/services/tests/protocol_v2_test.go +++ b/services/tests/protocol_v2_test.go @@ -9,7 +9,6 @@ import ( "encoding/json" "errors" "io" - "os" "os/exec" "strings" "testing" @@ -40,15 +39,11 @@ type protocolV2Broker struct { func startProtocolV2Broker(t *testing.T) *protocolV2Broker { t.Helper() p := &protocolV2Broker{ - cmd: exec.Command(brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()), + cmd: testCommand(t, brokerBin, "--scanner-path", scannerBin, "--cluster-dir", t.TempDir()), messages: make(chan jsonrpc.Message, 64), readError: make(chan error, 1), done: make(chan struct{}), } // Optional binaries resolve ONLY against cwd, not next to brokerBin. p.cmd.Dir = t.TempDir() - p.cmd.Env = os.Environ() - for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA"} { - p.cmd.Env = append(p.cmd.Env, key+"="+t.TempDir()) - } p.cmd.Stderr = &p.stderr p.cmd.WaitDelay = time.Second var err error diff --git a/services/tests/remote_engine_test.go b/services/tests/remote_engine_test.go index 07b3af724..c029ddd16 100644 --- a/services/tests/remote_engine_test.go +++ b/services/tests/remote_engine_test.go @@ -23,7 +23,6 @@ import ( "math/big" "net/url" "os" - "os/exec" "path/filepath" "reflect" "runtime" @@ -202,7 +201,7 @@ func TestRemoteEngineGetInstalled(t *testing.T) { // would leak into the fixture. func setRemoteEngineHome(t *testing.T, dir string) { t.Helper() - for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA"} { + for _, key := range []string{"HOME", "XDG_CONFIG_HOME", "APPDATA", "LOCALAPPDATA", "USERPROFILE"} { t.Setenv(key, dir) } } @@ -247,7 +246,7 @@ func TestRemoteEngineRejectsUntrusted(t *testing.T) { // stays alive) and drains stdout. Returns stdin and a cleanup. func startEngineManagerServer(t *testing.T, clusterDir string, controlPort int) (io.WriteCloser, func()) { t.Helper() - cmd := exec.Command(engineMgrBin, + cmd := testCommandInDir(t, clusterDir, engineMgrBin, "--control-port", fmt.Sprintf("%d", controlPort), "--cluster-dir", clusterDir, "--log-level", "warn", @@ -282,7 +281,7 @@ func startEngineManagerServer(t *testing.T, clusterDir string, controlPort int) // client. Returns its stdin, a reader over its stdout frames, and a cleanup. func startEngineManagerStdio(t *testing.T, clusterDir string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() - cmd := exec.Command(engineMgrBin, "--cluster-dir", clusterDir, "--log-level", "warn") + cmd := testCommandInDir(t, clusterDir, engineMgrBin, "--cluster-dir", clusterDir, "--log-level", "warn") cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() if err != nil { diff --git a/services/tests/scheduler_interop_test.go b/services/tests/scheduler_interop_test.go index 267c82504..89855486a 100644 --- a/services/tests/scheduler_interop_test.go +++ b/services/tests/scheduler_interop_test.go @@ -22,7 +22,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "sort" "sync" "testing" @@ -57,7 +56,7 @@ func writeStampedSchedulerNotification(t *testing.T, w io.Writer, nextSeq *int, // reader over its stdout JSON-RPC frames, and a cleanup. func startSchedulerProc(t *testing.T, args ...string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() - cmd := exec.Command(schedulerBin, args...) + cmd := testCommand(t, schedulerBin, args...) cmd.Stderr = os.Stderr stdin, err := cmd.StdinPipe() if err != nil { diff --git a/services/tests/secure_inference_test.go b/services/tests/secure_inference_test.go index f2f3e49f6..fc9ee4495 100644 --- a/services/tests/secure_inference_test.go +++ b/services/tests/secure_inference_test.go @@ -62,15 +62,11 @@ type proxyProc struct { // config. func startProxyProc(t *testing.T, clusterDir string, listenPort int) *proxyProc { t.Helper() - cfg := t.TempDir() - cmd := exec.Command(proxyBin, + cmd := testCommand(t, proxyBin, "--cluster-dir", clusterDir, "--port", strconv.Itoa(listenPort), "--ignore-persisted-port", ) - cmd.Env = append(os.Environ(), - "HOME="+cfg, "XDG_CONFIG_HOME="+cfg, "APPDATA="+cfg, "LOCALAPPDATA="+cfg, - ) stdin, err := cmd.StdinPipe() if err != nil { t.Fatalf("proxy stdin pipe: %v", err) @@ -241,6 +237,7 @@ func startFakeOllama(t *testing.T) (host string, port int, generates *int32) { } func TestSecureInferenceClusterMTLS(t *testing.T) { + requireMulticast(t) baseA, baseB, baseC := t.TempDir(), t.TempDir(), t.TempDir() clusterA := filepath.Join(baseA, "cluster") clusterB := filepath.Join(baseB, "cluster") diff --git a/services/tests/tls_test.go b/services/tests/tls_test.go index e0b7f0010..c43155ba8 100644 --- a/services/tests/tls_test.go +++ b/services/tests/tls_test.go @@ -17,7 +17,6 @@ import ( "net" "net/http" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -125,7 +124,7 @@ func startNodeInfo(t *testing.T, instanceName string, extraArgs ...string) (clea t.Helper() // node-info no longer advertises over mDNS (discovery consolidation), so // these tests connect directly to the ports they pass. - cmd := exec.Command(nodeInfoBin, extraArgs...) + cmd := testCommand(t, nodeInfoBin, extraArgs...) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() if err != nil { diff --git a/services/tests/workload_identity_interop_test.go b/services/tests/workload_identity_interop_test.go index f3879a74a..d6afd0c6b 100644 --- a/services/tests/workload_identity_interop_test.go +++ b/services/tests/workload_identity_interop_test.go @@ -158,8 +158,9 @@ func TestWorkloadCrossEngineIdentityDistinct(t *testing.T) { // hear the running workload AGAIN from the restarted manager — which only // happens if the broker rehydrated the fresh process's active set. func TestWorkloadManagerRehydratesActiveWorkloadOnRestart(t *testing.T) { - if portBusy(11435) { - t.Skip("ollama-proxy default port 11435 already in use; skipping") + requireMulticast(t) + if portBusy(11435) || portBusy(14320) { + t.Skip("ollama-proxy (11435) or workload-manager (14320) port already in use; skipping") } // Fake Ollama that accepts the request then blocks, keeping the proxied @@ -274,8 +275,9 @@ func TestWorkloadManagerRehydratesActiveWorkloadOnRestart(t *testing.T) { // state — otherwise a peer's wrongly-inferred "failed" (or a missed terminal) // can never be repaired after a restart. func TestWorkloadManagerRehydratesRecentTerminalOnRestart(t *testing.T) { - if portBusy(11435) { - t.Skip("ollama-proxy default port 11435 already in use; skipping") + requireMulticast(t) + if portBusy(11435) || portBusy(14320) { + t.Skip("ollama-proxy (11435) or workload-manager (14320) port already in use; skipping") } // Fake Ollama that completes immediately, so the workload reaches a terminal diff --git a/services/tests/workload_interop_test.go b/services/tests/workload_interop_test.go index 249f43350..bfbbc978e 100644 --- a/services/tests/workload_interop_test.go +++ b/services/tests/workload_interop_test.go @@ -29,7 +29,6 @@ import ( "net/http/httptest" "net/url" "os" - "os/exec" "strconv" "strings" "testing" @@ -80,8 +79,13 @@ func startBrokerProc(t *testing.T, args ...string) (io.WriteCloser, <-chan jsonr // cluster that directory describes (see newInterNodeCluster). func startBrokerProcInCluster(t *testing.T, clusterDir string, args ...string) (io.WriteCloser, <-chan jsonrpc.Message, func()) { t.Helper() + for _, arg := range args { + if arg == "--workload-manager-path" && portBusy(14320) { + t.Skip("workload-manager inter-node port 14320 already in use; skipping") + } + } args = append([]string{"--cluster-dir", clusterDir}, args...) - cmd := exec.Command(brokerBin, args...) + cmd := testCommand(t, brokerBin, args...) cmd.Stderr = os.Stderr stdinPipe, err := cmd.StdinPipe() @@ -278,6 +282,7 @@ func TestWorkloadOutOfOrderSuppressed(t *testing.T) { // evicts it (miss threshold + liveness probe), and the broker synthesizes the // terminal workloads:upsert(state=failed). func TestWorkloadFailedOnNodeLoss(t *testing.T) { + requireMulticast(t) // The peer's mDNS instance name and its stable HostUUID differ on purpose: // workloads are stamped/keyed by HostUUID, and the node-loss sweep // must match on that UUID, not the display name. Keying by name would leave @@ -365,6 +370,7 @@ func TestWorkloadFailedOnNodeLoss(t *testing.T) { // workload event that the broker stamps and forwards to the manager, which // broadcasts it to a discovered peer. func TestWorkloadManagerOutboundBroadcast(t *testing.T) { + requireMulticast(t) // Fake Ollama upstream the proxy forwards to (200 on any request). ollama := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { io.Copy(io.Discard, r.Body)