diff --git a/CHANGELOG.md b/CHANGELOG.md index 747850d..29098f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,10 +20,33 @@ is only released once its **exit criterion** is met, not when its code lands. Until v1.0.0 the minor version is the breaking one: configuration formats and Go APIs change between minors without a deprecation period. -## Unreleased +## v0.10.4 — 2026-09-30 -Changes since v0.10.3, published as release candidates (v0.10.4-rc.N) until -there are enough for v0.10.4. +Published first as release candidates v0.10.4-rc.1 to rc.9. + +After upgrading, run `llmgw ctl restart --all` once: CPU replicas take the new +thread count below only when they start. + +### CPU replicas no longer fight over the CPUs + +llama.cpp computes on every CPU of the machine unless told otherwise, and the +agent did not tell it. Two busy CPU replicas on one node then spin against each +other: on an 8-vCPU node, 7B generation fell from 10 tokens a second alone to +0.8 each with both busy, in two rounds of the same test. The agent now starts a +CPU replica on half the machine's CPUs, where one alone makes 6.5 and two busy +make 5.5 each. `llamacpp.threads` in the agent's configuration sets it for a +node — all of them suits a node that only ever runs one CPU replica — and a +profile's own `--threads` still wins. Measured on VMs whose host is +oversubscribed; the proportions, not the numbers, are the point. + +### Tracing + +With `tracing.endpoint` set (or `OTEL_EXPORTER_OTLP_ENDPOINT`), each inference +request is traced over OTLP/HTTP: a span for the request, carrying its +`X-Request-Id`, with admission, scheduling and each attempt at an engine under +it. A caller's own `traceparent` is continued, and the engine is sent one for +its attempt. `tracing.sample_ratio` traces a share of the requests that arrive +without a decision. Off, it costs nothing. ### Restarting replicas @@ -157,6 +180,27 @@ half again plus two" could not see. links.** Archives are unpacked through `os.Root`, and a link may only point at a file beside it. Every llama.cpp b10991 archive, CPU and CUDA, already follows that rule. +- **Onboarding made every node trust the cluster's root for everything.** It + was added to the system store, so a certificate the control plane signed + would pass for any site with every TLS client on the node. The agent trusts + it through its own configuration only; onboarding no longer adds it, and + takes it out of a node that an older version onboarded. +- **Any node could manage a mirror running mutual TLS**, since one root signs + every node's certificate. `llmgw mirror serve --api-client NAME` answers only + the certificate with that common name, and the mirror warns when mutual TLS + runs without one. +- **The administrator's password stayed in `init.log`.** It is shown once, on + the screen, and the log keeps a placeholder. +- **Auth off on a public address was silent.** The server now warns at start + when the API listens beyond loopback without keys. +- **The actor recorded with a change was whatever the caller sent**, line + breaks included. It is cut to a short printable name, and `sudo llmgw ctl` + records the person who ran sudo rather than root. +- **vLLM's Ray head ran its dashboard**, whose job API runs code for anyone who + reaches it. It is off. The head itself still has no authentication, and + SGLang still takes its key as a flag; docs/install.md says what to do about + both. The monitoring README says how to keep Prometheus's queries to the + control plane. ### Fixes @@ -165,6 +209,8 @@ half again plus two" could not see. loaded runner. - `ctl` reported a change the API had queued (202 Accepted) as an error, as `ctl restart` did on its first real use. +- The status each failure is answered with — 499, 502, 503 with and without + Retry-After, 504, 429 — is now tested case by case. ### Releases diff --git a/deploy/bootstrap/onboard-node.sh b/deploy/bootstrap/onboard-node.sh index 36e951d..03e808e 100755 --- a/deploy/bootstrap/onboard-node.sh +++ b/deploy/bootstrap/onboard-node.sh @@ -122,9 +122,10 @@ TMP="$(mktemp -d)" trap 'rm -rf "$TMP"' EXIT "$BINARY" ctl cert issue-client "$NODE" --dir "$PKI" --out "$TMP" >/dev/null -# 2 and 3. Identity, and trusting the root so future tooling on that machine -# does not treat our certificates as strangers. -say "installing the certificate and trusting the root" +# 2 and 3. Identity. The root is trusted by the agent's configuration only, +# never system-wide: it signs whatever the control plane asks, and every +# TLS client on the machine would take it for any site. +say "installing the certificate" scp -q $SSHOPTS "$PKI/ca.pem" "$TMP/$NODE.pem" "$TMP/$NODE-key.pem" "$SSH_USER@$ADDR:/tmp/" $SSH "set -e sudo install -d -m 0755 /etc/llmgw @@ -132,8 +133,10 @@ $SSH "set -e sudo install -m 0644 /tmp/$NODE.pem /etc/llmgw/agent.pem sudo install -m 0600 /tmp/$NODE-key.pem /etc/llmgw/agent-key.pem sudo chown root:root /etc/llmgw/* - sudo install -m 0644 /tmp/ca.pem /usr/local/share/ca-certificates/llmgw-ca.crt - sudo update-ca-certificates >/dev/null 2>&1 || true + if [ -f /usr/local/share/ca-certificates/llmgw-ca.crt ]; then + sudo rm -f /usr/local/share/ca-certificates/llmgw-ca.crt + sudo update-ca-certificates --fresh >/dev/null 2>&1 || true + fi rm -f /tmp/ca.pem /tmp/$NODE.pem /tmp/$NODE-key.pem" # 4. The binary, the configuration and the unit. diff --git a/deploy/monitoring/README.md b/deploy/monitoring/README.md index ea7a752..2483daa 100644 --- a/deploy/monitoring/README.md +++ b/deploy/monitoring/README.md @@ -52,6 +52,13 @@ install -m 0644 alertmanager.yml /etc/prometheus/alertmanager.yml # client; one Alertmanager needs no cluster port. echo 'ARGS="--web.listen-address=127.0.0.1:9093 --cluster.listen-address="' \ > /etc/default/prometheus-alertmanager +# Prometheus itself is read-only as installed: its admin and lifecycle APIs +# stay off (no --web.enable-admin-api, no --web.enable-lifecycle). It still +# answers any query from anyone who reaches :9090, which says which models and +# nodes there are and how busy each is. Its readers are the control plane +# (monitoring.prometheus) and Grafana beside it; to keep it to those, allow +# the control plane and drop the rest, e.g. with nftables: +# nft add rule inet filter input tcp dport 9090 ip saddr != { 127.0.0.1, CONTROL_PLANE_IP } drop promtool check config /etc/prometheus/prometheus.yml systemctl restart prometheus prometheus-alertmanager diff --git a/docs/install.md b/docs/install.md index f327ecf..43b0845 100644 --- a/docs/install.md +++ b/docs/install.md @@ -1124,3 +1124,11 @@ requests. On a slow model, set `timeouts.generation` higher. has them. Single-node placement on real GPUs is the well-trodden path; if you are the first to run a TP group across real cards, run `llmgw doctor --json` on each node first and keep the reports. +- **Two engine setups are weaker than the rest.** An engine's key is passed + in its environment, where only root and its own user can read it, except + SGLang's, which takes it only as a flag: other users on the node can see it + in the process list. Mount `/proc` with `hidepid=2` on nodes where that + matters. vLLM's `launch_mode: ray` starts a Ray head without + authentication, so anyone who can reach its port can run code on the + node. The dashboard is off, but firewall the port to the other nodes in the + replica, or use the default `mp` launch mode, which needs no Ray at all. diff --git a/docs/reference.md b/docs/reference.md index d2688ba..bce4b1a 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -81,6 +81,7 @@ rather than repeated. | `notice` | `string` | [Landing](#landing) | | `bin` | `string` | [LlamaCppConfig](#llamacppconfig) | | `parallel` | `int` | [LlamaCppConfig](#llamacppconfig) | +| `threads` | `int` | [LlamaCppConfig](#llamacppconfig) | | `api` | `string` | [Mirror](#mirror) | | `tls` | `tlsconf.Config` | [Mirror](#mirror) | | `registry` | `string` | [Mirror](#mirror) | @@ -167,6 +168,7 @@ rather than repeated. | `landing` | `Landing` | [Server](#server) | | `agent_tls` | `tlsconf.Config` | [Server](#server) | | `monitoring` | `Monitoring` | [Server](#server) | +| `tracing` | `Tracing` | [Server](#server) | | `mirror` | `Mirror` | [Server](#server) | | `engines` | `map[string]string` | [Server](#server) | | `backup_dir` | `string` | [Server](#server) | @@ -184,6 +186,8 @@ rather than repeated. | `first_token` | `time.Duration` | [Timeouts](#timeouts) | | `idle` | `time.Duration` | [Timeouts](#timeouts) | | `total` | `time.Duration` | [Timeouts](#timeouts) | +| `endpoint` | `string` | [Tracing](#tracing) | +| `sample_ratio` | `float64` | [Tracing](#tracing) | | `bin` | `string` | [VLLMConfig](#vllmconfig) | | `launch_mode` | `string` | [VLLMConfig](#vllmconfig) | | `ray_bin` | `string` | [VLLMConfig](#vllmconfig) | @@ -634,6 +638,20 @@ Parallel is how many requests a server handles at once, when a profile does not say. llama.cpp defaults to one, which would make every second request wait behind the first no matter what the router decided. +### `threads` + +`int` + +Threads is how many CPU threads a shard with no GPU computes on. Zero +takes half this machine's CPUs. + +Not all of them, which is llama.cpp's own default: two busy servers +that each spin on every CPU fight for them, and on an 8-vCPU node +that made 7B generation fall from 10 tokens a second to under 1 each. +At half, one server alone is about a third slower and two together +keep 5.5 each. A node that only ever runs one CPU replica does better +with all of them; say so here. + ## Mirror ### `api` @@ -1246,6 +1264,16 @@ Monitoring is where the dashboard reads its history from. Takes a [Monitoring](#monitoring) block. +### `tracing` + +`Tracing` + +Tracing sends a trace of each inference request to an OpenTelemetry +collector. Off unless an endpoint is set here or in +OTEL_EXPORTER_OTLP_ENDPOINT. + +Takes a [Tracing](#tracing) block. + ### `mirror` `Mirror` @@ -1366,6 +1394,22 @@ about a hundred tokens on a slow model. `time.Duration` +## Tracing + +### `endpoint` + +`string` + +Endpoint is the collector, for example http://otel.internal:4318. +Empty falls back to OTEL_EXPORTER_OTLP_ENDPOINT, and to no tracing. + +### `sample_ratio` + +`float64` + +SampleRatio is the share of requests traced, 0 to 1, when the caller +did not decide it with a sampled traceparent of its own. Zero takes 1. + ## VLLMConfig ### `bin` diff --git a/go.mod b/go.mod index ba02d99..5c59dba 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,10 @@ require ( github.com/prometheus/common v0.71.0 github.com/spf13/cobra v1.10.2 github.com/yuin/goldmark v1.8.6 + go.opentelemetry.io/otel v1.46.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0 + go.opentelemetry.io/otel/sdk v1.46.0 + go.opentelemetry.io/otel/trace v1.46.0 golang.org/x/crypto v0.57.0 golang.org/x/net v0.59.0 golang.org/x/sync v0.23.0 @@ -29,6 +33,7 @@ require ( require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/beorn7/perks v1.0.1 // indirect + github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/charmbracelet/colorprofile v0.4.1 // indirect github.com/charmbracelet/x/ansi v0.11.6 // indirect github.com/charmbracelet/x/cellbuf v0.0.15 // indirect @@ -39,8 +44,11 @@ require ( github.com/dustin/go-humanize v1.0.1 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect - github.com/go-viper/mapstructure/v2 v2.4.0 // indirect + github.com/go-logr/logr v1.4.4 // indirect + github.com/go-logr/stdr v1.2.2 // indirect + github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/grpc-ecosystem/grpc-gateway/v2 v2.30.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect @@ -61,9 +69,16 @@ require ( github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect + go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.46.0 // indirect + go.opentelemetry.io/otel/metric v1.46.0 // indirect + go.opentelemetry.io/proto/otlp v1.11.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/sys v0.48.0 // indirect golang.org/x/text v0.42.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260819154853-08b0e4226688 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260819154853-08b0e4226688 // indirect + google.golang.org/grpc v1.83.1 // indirect modernc.org/libc v1.75.7 // indirect modernc.org/mathutil v1.7.1 // indirect modernc.org/memory v1.12.1 // indirect diff --git a/go.sum b/go.sum index 87da3b1..626b7e0 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,8 @@ github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiE github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= +github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= @@ -34,14 +36,23 @@ github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/fsnotify/fsnotify v1.9.0 h1:2Ml+OJNzbYCTzsxtv8vKSFD9PbJjmhYF14k/jKC7S9k= github.com/fsnotify/fsnotify v1.9.0/go.mod h1:8jBTzvmWwFyi3Pb8djgCCO5IBqzKJ/Jwo8TRcHyHii0= -github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= -github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= +github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= +github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro= +github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3 h1:LMLX+LgTNWpfvCBdFebv6EsYotImrt/Ppc5cXIriCSo= github.com/google/pprof v0.0.0-20260802141513-ef3492d7dac3/go.mod h1:jl5iWTm0/hd5PjEYEOuwAJ57L/CibdZfrqZ5XA5GrCk= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.30.0 h1:/Tnpcb2E0Pz/tN9s3bfEY2Q8ePCEX9iuS+cneUwncnw= +github.com/grpc-ecosystem/grpc-gateway/v2 v2.30.0/go.mod h1:zOBXOsUaBSjKgmH4OGzV1esUpR3oUSCPYVd2cUBjKYY= github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -117,6 +128,24 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/yuin/goldmark v1.8.6 h1:d0VcaP1sx9GkFVkoW+KtggpGi2KZ965i14b0+bDQST4= github.com/yuin/goldmark v1.8.6/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= +go.opentelemetry.io/otel v1.46.0 h1:FHt5/CDyVxi/8IM1CH7VE/rRgq3kLHa2mSTVMO8AWyc= +go.opentelemetry.io/otel v1.46.0/go.mod h1:Gj3SEScelsNC45tp4nSxRYlS+f5iez7W8XPMCt905kE= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.46.0 h1:OFnwLJr+pF3iHrlGSzbxyuo6/6HyBlnlN1CWEJmBVcw= +go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.46.0/go.mod h1:716wFneO0ov19A2beH5hjfh9AK5z/VWNAtDijp1Y0/g= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0 h1:KrC1YrQeSt46ITMWAbgQx1M1eV1/1TKzttrBzymPmss= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0/go.mod h1:zDSEzoEqsOrgBeGvH66KRgxh90VonFyJqBHA0Pk3+rM= +go.opentelemetry.io/otel/metric v1.46.0 h1:yBnkXvgV7AXFILZc5K6IZe/CBFF3OS7BJ8ov6/lj0K8= +go.opentelemetry.io/otel/metric v1.46.0/go.mod h1:iPmdWqifKUdzziPkvvzIJXITl56fQx2mGM/DHLB3/2o= +go.opentelemetry.io/otel/sdk v1.46.0 h1:h5CNQQjEbuQXY/JfZtgt3i7HVFV3aHPO2OAwO2eTYPI= +go.opentelemetry.io/otel/sdk v1.46.0/go.mod h1:GAERFXFt5SYCEB+YiKUbMBeza6UaDH7GmGOZEfh2gSM= +go.opentelemetry.io/otel/sdk/metric v1.46.0 h1:0piZ26EG4RBfebb2jhDH6ERCYHoVWduc3kLgPCwSnSE= +go.opentelemetry.io/otel/sdk/metric v1.46.0/go.mod h1:I1PbKrdVc8Qu8HYVDNtqVIwLwjNrhsV/uFuxfwg8mO4= +go.opentelemetry.io/otel/trace v1.46.0 h1:OULy7ccdJnZtJ0UDYFOIGaCmiWzJ8Vi2G/Rsu60qs1c= +go.opentelemetry.io/otel/trace v1.46.0/go.mod h1:J7GAXweO77XSFkB/rmAqk9D6ihszhFjLU+d9WuUxDLI= +go.opentelemetry.io/proto/otlp v1.11.0 h1:5rrYs0Ykyj50sdU/JU0x8etU+LubXWb+gED6TbEdMIk= +go.opentelemetry.io/proto/otlp v1.11.0/go.mod h1:SmVizdCOAm3XBtG1g1NnOdhW6jtddT72hLMhv8VwA8E= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.yaml.in/yaml/v2 v2.4.4 h1:tuyd0P+2Ont/d6e2rl3be67goVK4R6deVxCUX5vyPaQ= @@ -141,6 +170,14 @@ golang.org/x/text v0.42.0 h1:JbOZXgfeCPU9gacVtYliJqOhD+zhrEqK4LfdpmlUZqI= golang.org/x/text v0.42.0/go.mod h1:ojzP1Z+2QtioaF8DTtO8K5q7JWVVYwZKenzujK0Zd0E= golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/genproto/googleapis/api v0.0.0-20260819154853-08b0e4226688 h1:ax2KzoSRIZU/M0cIxri3pKxy99vniH1PVxWC6si/eZI= +google.golang.org/genproto/googleapis/api v0.0.0-20260819154853-08b0e4226688/go.mod h1:1RJ9BQGyNdZwkGc1eTqkErfRZ6RJyYPHZo73BZ1vQqI= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260819154853-08b0e4226688 h1:cYNAzI2sUwhmCcoj9TxvihSrqsxt6uIkj3rDRhSDmW4= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260819154853-08b0e4226688/go.mod h1:DjtHYE8FKJLivXcBEjGwndXfIC23G0VpXiXKqG179uA= +google.golang.org/grpc v1.83.1 h1:HIO0+BEtBP6soyqvqC8sNUjZ7bTs+0hFQuFF+RAy++Y= +google.golang.org/grpc v1.83.1/go.mod h1:kDyl6SKsiHKt0uylY5gtn5cEjkrIOhQOGDgIc4JGwzQ= google.golang.org/protobuf v1.36.12 h1:pJOKDDOyeXErUroCihFAd5LQuwXBSpVnKGrj5o/fwxc= google.golang.org/protobuf v1.36.12/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 62e570f..4801fa5 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -8,21 +8,15 @@ import ( "encoding/hex" "errors" "fmt" - "gitea.mynodes.xyz/ilker/llmgw/internal/release" "io" "log/slog" "maps" "math/rand/v2" "net" "net/http" - "net/url" "os" - "os/exec" "path/filepath" - "runtime" "slices" - "strconv" - "strings" "sync" "time" @@ -36,278 +30,9 @@ import ( "gitea.mynodes.xyz/ilker/llmgw/internal/agent/supervisor" "gitea.mynodes.xyz/ilker/llmgw/internal/agent/weights" "gitea.mynodes.xyz/ilker/llmgw/internal/control/agentsvc" - "gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory" "gitea.mynodes.xyz/ilker/llmgw/internal/tlsconf" ) -// Config configures the agent. -type Config struct { - // NodeID defaults to the hostname. It must equal the common name of the - // agent's certificate, so set it only where the two differ. - NodeID string `koanf:"node_id"` - // Server is the control plane, and Servers is the same thing when there is - // more than one. Both are accepted: every configuration written before - // there could be a second control plane names one address, and none of - // them should have to change. - // - // The order is tried in order, and a refusal moves to the next. Only the - // leader accepts an agent, so a standby is skipped in the time it takes to - // be told so — which is what makes failover a reconnect rather than an - // edit to every node's configuration. - Server string `koanf:"server"` - Servers []string `koanf:"servers"` - // Hostname defaults to the machine's. - Hostname string `koanf:"hostname"` - // MgmtAddr is the address engines are reached on. It defaults to this - // machine's address on the route to the control plane, which is the one - // the control plane and the router can reach. - MgmtAddr string `koanf:"mgmt_addr"` - // Labels are free-form facts about a node for a profile's node_selector. - // Nothing needs them: memory, GPUs, engines and disk are measured, and a - // label restating one of those goes stale the day the machine changes. - Labels map[string]string `koanf:"labels"` - - // MgmtInterface is the ethernet interface NCCL bootstraps over. It - // defaults to the interface holding MgmtAddr, then eth0. - MgmtInterface string `koanf:"mgmt_interface"` - // Fabrics labels this node's HCAs. It is explicit on purpose: OpenSM's - // default subnet prefix is the same on every fabric, so nothing can be - // inferred from the wire (plan §5.2). - Fabrics []inventory.FabricAttachment `koanf:"fabrics"` - - HeartbeatInterval time.Duration `koanf:"heartbeat_interval"` - // ReconnectBase is the first reconnect delay; it backs off from there. - ReconnectBase time.Duration `koanf:"reconnect_base"` - ReconnectMax time.Duration `koanf:"reconnect_max"` - - // AdoptReadyTimeout bounds how long an adopted engine is given to answer a - // health check before the agent stops waiting on it. - // - // Shorter than a startup timeout on purpose: this engine is already - // running, so it is not loading a model, it is either serving or it is not. - AdoptReadyTimeout time.Duration `koanf:"adopt_ready_timeout"` - - // TLS authenticates both ends of the control-plane stream. Leaving it unset - // keeps the plaintext behaviour, which only belongs on a trusted network. - TLS tlsconf.Config `koanf:"tls"` - - // ModelDirs are the directories this node keeps model weights in, beyond - // WeightsCache, which is always looked in. Storage differs at every installation, - // so llmgw observes it rather than owning it: a Lustre mount, an image - // exported over the fabric, local NVMe and a copy made by hand all look - // identical from here, and placement only needs to know whether the file - // is present. - ModelDirs []string `koanf:"model_dirs"` - - // EnginesDir is where the agent installs engines, one directory per - // build. Defaults to /opt/llmgw/engines. - EnginesDir string `koanf:"engines_dir"` - - // WeightsCache is where a remote source is downloaded to. Defaults to the - // first entry of ModelDirs, because a node that says where its models live - // has already answered this question, and otherwise to - // /var/lib/llmgw-weights. - // - // Fetching is the simple arrangement: every node keeps its own copy. The - // exotic ones need no code — weights on a shared filer or an image exported - // over the fabric are simply present, and present means nothing is fetched. - WeightsCache string `koanf:"weights_cache"` - - Discovery discovery.Options `koanf:"discovery"` - Engines engines.Config `koanf:"engines"` - Superv supervisor.Config `koanf:"supervisor"` - - Version string -} - -// DefaultConfig returns sensible defaults. -func DefaultConfig() Config { - // No MgmtInterface: normalize finds the one holding the management - // address, and falls back to eth0 only when it cannot. Defaulted here, it - // was never found — every node said eth0 unless told otherwise, and a node - // on ens3 or bond0 had NCCL bootstrap over an interface it does not have. - return Config{ - HeartbeatInterval: 5 * time.Second, - ReconnectBase: time.Second, - ReconnectMax: 30 * time.Second, - AdoptReadyTimeout: 60 * time.Second, - Superv: supervisor.DefaultConfig(), - // NVML gives NVLink topology, so prefer it when this build has it. - Discovery: discovery.Options{PreferNVML: true}, - } -} - -func (c *Config) normalize() error { - d := DefaultConfig() - if err := c.TLS.Validate(); err != nil { - return err - } - if err := c.normalizeServerURL(); err != nil { - return err - } - // Everything below is found rather than written. A file that has to say - // which machine it is on, and at what address, is a file that differs on - // every node — and a copy taken from one node to the next registers the - // second as the first. - host, _ := os.Hostname() - if c.NodeID == "" { - c.NodeID = host - } - if c.NodeID == "" { - return errors.New("node_id is not set and the hostname could not be read") - } - if c.Hostname == "" { - c.Hostname = host - } - if c.MgmtAddr == "" { - c.MgmtAddr = addrToward(c.Server) - } - if c.MgmtInterface == "" { - c.MgmtInterface = interfaceOf(c.MgmtAddr) - } - if c.MgmtInterface == "" { - c.MgmtInterface = "eth0" - } - // The directory downloads land in is always one that is looked in. When - // they were two settings, a node could fill its disk with fetched weights - // that placement never saw — which is what every node here was doing. - if c.WeightsCache == "" && len(c.ModelDirs) > 0 { - c.WeightsCache = c.ModelDirs[0] - } - if c.WeightsCache == "" { - c.WeightsCache = DefaultWeightsCache - } - if !slices.Contains(c.ModelDirs, c.WeightsCache) { - c.ModelDirs = append(c.ModelDirs, c.WeightsCache) - } - if c.HeartbeatInterval <= 0 { - c.HeartbeatInterval = d.HeartbeatInterval - } - if c.ReconnectBase <= 0 { - c.ReconnectBase = d.ReconnectBase - } - if c.AdoptReadyTimeout <= 0 { - c.AdoptReadyTimeout = d.AdoptReadyTimeout - } - if c.ReconnectMax < c.ReconnectBase { - c.ReconnectMax = max(c.ReconnectBase, d.ReconnectMax) - } - return nil -} - -// normalizeServerURL makes the configured address a URL connect can use, and -// refuses the combination that would quietly undo the certificates: a scheme -// that says plaintext while TLS is configured. Silently connecting anyway would -// mean a cluster believing it has mutual authentication when it has none. -func (c *Config) normalizeServerURL() error { - // One list, however it was written. Server first when both are given, - // because an operator adding a standby to an existing file expects the - // one already there to stay preferred. - all := make([]string, 0, 1+len(c.Servers)) - if c.Server != "" { - all = append(all, c.Server) - } - all = append(all, c.Servers...) - - seen := map[string]struct{}{} - out := make([]string, 0, len(all)) - for _, s := range all { - fixed, err := normalizeOne(s, c.TLS.Enabled()) - if err != nil { - return err - } - // A duplicate would be tried twice in a row on failover, which reads - // as the rotation not working. - if _, dup := seen[fixed]; dup { - continue - } - seen[fixed] = struct{}{} - out = append(out, fixed) - } - if len(out) == 0 { - return errors.New("server is required") - } - c.Servers = out - c.Server = out[0] - return nil -} - -// DefaultWeightsCache is where fetched weights go when nothing says otherwise. -const DefaultWeightsCache = "/var/lib/llmgw-weights" - -// addrToward is this machine's address on the route to the control plane. -// -// The address engines are reached on is whichever one the control plane can -// reach, and the kernel already knows which that is: a UDP "connection" picks -// the source address for the route without sending a packet. Asking the -// interfaces instead would have to guess among docker bridges, VPNs and -// link-local addresses. -func addrToward(server string) string { - u, err := url.Parse(server) - if err != nil || u.Host == "" { - return "" - } - host := u.Hostname() - port := u.Port() - if port == "" { - port = "443" - } - conn, err := net.Dial("udp", net.JoinHostPort(host, port)) - if err != nil { - return "" - } - defer conn.Close() - if a, ok := conn.LocalAddr().(*net.UDPAddr); ok { - return a.IP.String() - } - return "" -} - -// interfaceOf names the interface that holds an address, or "" when none does. -func interfaceOf(addr string) string { - ip := net.ParseIP(addr) - if ip == nil { - return "" - } - ifaces, err := net.Interfaces() - if err != nil { - return "" - } - for _, ifc := range ifaces { - addrs, err := ifc.Addrs() - if err != nil { - continue - } - for _, a := range addrs { - if n, ok := a.(*net.IPNet); ok && n.IP.Equal(ip) { - return ifc.Name - } - } - } - return "" -} - -// normalizeOne applies the scheme rules to a single address. -func normalizeOne(addr string, secure bool) (string, error) { - switch { - case strings.HasPrefix(addr, "https://"): - if !secure { - return "", fmt.Errorf("%s is an https:// address but no tls certificates are configured", addr) - } - case strings.HasPrefix(addr, "http://"): - if secure { - return "", fmt.Errorf("tls is configured but %s is an http:// address; use https://", addr) - } - case strings.Contains(addr, "://"): - return "", fmt.Errorf("server %q must be an http:// or https:// address", addr) - case secure: - return "https://" + addr, nil - default: - return "http://" + addr, nil - } - return addr, nil -} - // Agent runs on a GPU node. type Agent struct { cfg Config @@ -588,131 +313,6 @@ func (a *Agent) settleAdopted(sh *supervisor.Shard, o supervisor.Orphan) { "shard", o.ShardID, "replica", o.ReplicaID) } -// usableFabrics reports the fabrics this node can actually carry traffic on. -// -// Loud about what it drops. A fabric silently missing from a registration looks -// identical to one never configured, and the operator who typed the adapter -// name is the only person who can fix it — so the reason is logged here, on the -// machine where both halves are visible, rather than inferred later from a -// replica that will not converge. -func (a *Agent) usableFabrics(inv discovery.Inventory) []inventory.FabricAttachment { - out := a.checkFabrics(inv.HCAs, true) - a.mu.Lock() - a.fabricsSent = fabricsFingerprint(out) - a.mu.Unlock() - return out -} - -// checkFabrics is the declared fabrics this node can carry traffic on, from -// the adapters as they are now. Loud logs every problem; the heartbeat checks -// quietly and speaks only when the answer changes. -func (a *Agent) checkFabrics(hcas []discovery.HCA, loud bool) []inventory.FabricAttachment { - declared := map[string][]string{} - for _, f := range a.cfg.Fabrics { - declared[f.Fabric] = f.HCAs - } - - var out []inventory.FabricAttachment - for _, c := range discovery.CheckFabrics(declared, hcas) { - if loud { - for _, p := range c.Problems { - a.log.Error("a declared fabric adapter cannot carry traffic", - "fabric", c.Fabric, "problem", p) - } - } - if !c.OK() { - if loud { - a.log.Error("fabric not advertised; no declared adapter on it can carry traffic", - "fabric", c.Fabric, - "consequence", "no multi-node replica will be placed here") - } - continue - } - out = append(out, inventory.FabricAttachment{ - Fabric: c.Fabric, HCAs: c.Usable, RateGbps: c.RateGbps, - }) - } - return out -} - -// fabricsFingerprint is enough to notice a fabric, an adapter or a rate -// changing. -func fabricsFingerprint(fs []inventory.FabricAttachment) string { - var b strings.Builder - for _, f := range fs { - fmt.Fprintf(&b, "%s:%s:%d;", f.Fabric, strings.Join(f.HCAs, ","), f.RateGbps) - } - return b.String() -} - -// sampleIB re-reads the adapters for the heartbeat: every port's state and -// counters, and the fabrics again when they no longer match what the control -// plane was told. -// -// Every heartbeat, because it is a few small files per port, and because a -// port that went down used to be invisible until the agent restarted — the -// node went on being offered multi-node work that would hang on its first -// collective. -func (a *Agent) sampleIB(hb *llmgwv1.Heartbeat) { - a.mu.Lock() - had := len(a.inv.HCAs) > 0 - a.mu.Unlock() - if !had && len(a.cfg.Fabrics) == 0 { - return - } - hcas, err := discovery.DiscoverIB(a.cfg.Discovery.IBRoot) - if err != nil { - return - } - a.attachSM(hcas) - for _, h := range hcas { - for _, p := range h.Ports { - ip := &llmgwv1.IBPort{ - Hca: h.Name, Port: int32(p.Num), State: p.State, PhysState: p.PhysState, - RateGbps: int32(p.RateGbps), LinkLayer: string(p.LinkLayer), Counters: p.Counters, - NodeGuid: h.NodeGUID, PortGuid: p.PortGUID, Network: p.Network, GidPrefix: p.GIDPrefix, - } - a.mu.Lock() - if p.SM != nil { - ip.SmGuid, ip.SmLid, ip.SmPriority, ip.SmState = p.SM.GUID, int32(p.SM.LID), int32(p.SM.Priority), p.SM.State - } else { - ip.SmError = a.smErr[smKey(h.Name, p.Num)] - } - for _, r := range a.sms[smKey(h.Name, p.Num)] { - ip.SubnetManagers = append(ip.SubnetManagers, &llmgwv1.SubnetManager{ - Guid: r.GUID, Lid: int32(r.LID), Priority: int32(r.Priority), State: r.State}) - } - ip.SubnetManagersError = a.smsErr[smKey(h.Name, p.Num)] - a.mu.Unlock() - hb.IbPorts = append(hb.IbPorts, ip) - } - } - a.mu.Lock() - a.inv.HCAs = hcas // a reconnect registers what is true now - a.mu.Unlock() - if len(a.cfg.Fabrics) == 0 { - return - } - now := a.checkFabrics(hcas, false) - fp := fabricsFingerprint(now) - a.mu.Lock() - changed := fp != a.fabricsSent - a.fabricsSent = fp - a.mu.Unlock() - if !changed { - return - } - // Said once, with the reasons, the moment it happens. - a.checkFabrics(hcas, true) - a.log.Warn("the fabrics this node can carry traffic on changed", "now", fp) - hb.FabricsChanged = true - for _, f := range now { - hb.Fabrics = append(hb.Fabrics, &llmgwv1.FabricAttachment{ - Fabric: f.Fabric, Hcas: f.HCAs, RateGbps: int32(f.RateGbps), - }) - } -} - // weights builds the fetcher lazily, so an agent that never starts a shard // never needs a cache directory. func (a *Agent) weights() *weights.Fetcher { @@ -1414,215 +1014,6 @@ func Client(cfg tlsconf.Config, timeout time.Duration) (*http.Client, error) { return &http.Client{Transport: &http2.Transport{TLSClientConfig: tc}}, nil } -// engines is what this node can run: what the agent installed and saw run, -// and what the configuration names and answers. Nothing merely found on the -// machine — a llama-server on PATH is somebody's, not necessarily one that -// works, and placing on it is how a replica fails on a node nobody watches. -func (a *Agent) engines(ctx context.Context) map[string]string { - a.engineOnce.Do(func() { - vers := a.reg.ProbeConfigured(ctx) - a.mu.Lock() - a.configuredVers, a.lastEngineProbe = vers, time.Now() - a.mu.Unlock() - if len(vers) > 0 { - a.log.Info("engines named in the configuration", - "engines", engines.ProbedNames(vers)) - } - }) - a.mu.Lock() - defer a.mu.Unlock() - vers := a.engineReportLocked() - // Whatever registration sends is what the control plane now knows, so a - // heartbeat repeats it only once it differs. - a.enginesSent = enginesFingerprint(vers) - return vers -} - -// engineReportLocked merges the two sources. Installed wins: it is the one -// that was seen to run. Called with mu held. -func (a *Agent) engineReportLocked() map[string]string { - out := maps.Clone(a.configuredVers) - if out == nil { - out = map[string]string{} - } - maps.Copy(out, a.installedVers) - return out -} - -// provisionEvery is how often installed engines are checked again. A library -// removed by an unrelated upgrade is found here rather than by the next -// replica to start. -const provisionEvery = 10 * time.Minute - -// setProvision takes the control plane's list of wanted engines. -func (a *Agent) setProvision(p *llmgwv1.Provision) { - wants := []provision.Want{} - for _, e := range p.GetEngines() { - wants = append(wants, provision.Want{Engine: e.GetEngine(), Version: e.GetVersion()}) - } - a.mu.Lock() - a.provWants = wants - a.mu.Unlock() - select { - case a.provKick <- struct{}{}: - default: - } -} - -// provisionLoop installs and re-checks the wanted engines, one pass at a time. -func (a *Agent) provisionLoop(ctx context.Context) { - // After a failure the next pass comes sooner, and backs off: a lock held - // by a new machine's own updates clears in a minute or two, while ten - // minutes' wait left a node idle for no reason. - next := provisionEvery - retry := time.Minute - for { - select { - case <-ctx.Done(): - return - case <-a.provKick: - case <-time.After(next): - } - a.mu.Lock() - wants := slices.Clone(a.provWants) - managed := a.provWants != nil - a.mu.Unlock() - if !managed { - continue - } - - statuses := a.prov.Converge(ctx, wants) - next = provisionEvery - for _, st := range statuses { - if st.State == provision.Failed { - next, retry = retry, min(retry*2, provisionEvery) - break - } - } - if next == provisionEvery { - retry = time.Minute - } - installed := map[string]string{} - keep := map[string]bool{} - for _, st := range statuses { - keep[st.Engine] = true - if st.State == provision.Ready { - a.reg.UseInstalled(st.Engine, st.Binary) - installed[st.Engine] = st.Version - } - } - a.mu.Lock() - if enginesFingerprint(installed) != enginesFingerprint(a.installedVers) { - a.log.Info("engines installed and running", "engines", engines.ProbedNames(installed)) - } - a.installedVers = installed - // An engine no longer asked for is no longer reported on. - for e := range a.provStatus { - if !keep[e] { - delete(a.provStatus, e) - } - } - a.mu.Unlock() - } -} - -// provisionStatus records a change in one engine's installation. -func (a *Agent) provisionStatus(st provision.Status) { - a.mu.Lock() - prev, had := a.provStatus[st.Engine] - a.provStatus[st.Engine] = st - a.mu.Unlock() - if !had || prev.State != st.State || prev.Detail != st.Detail { - lvl := slog.LevelInfo - if st.State == provision.Failed || st.State == provision.Unsupported { - lvl = slog.LevelWarn - } - a.log.Log(context.Background(), lvl, "engine installation", - "engine", st.Engine, "version", st.Version, "variant", st.Variant, - "state", st.State, "detail", st.Detail) - } -} - -func provisionFingerprint(m map[string]provision.Status) string { - var b strings.Builder - for _, k := range slices.Sorted(maps.Keys(m)) { - st := m[k] - fmt.Fprintf(&b, "%s|%s|%s|%s|%s\n", st.Engine, st.Version, st.Variant, st.State, st.Detail) - } - return b.String() -} - -// engineReprobe is how often the installed engines are looked for again. -// -// Minutes, not seconds: an engine is installed by somebody at a keyboard, so a -// minute's delay in noticing is invisible, while SGLang's probe starts a -// Python interpreter that imports it. -const engineReprobe = time.Minute - -// reprobeEngines looks for installed engines again when it is time, off the -// heartbeat goroutine so a slow probe never delays a heartbeat. -func (a *Agent) reprobeEngines() { - a.mu.Lock() - due := !a.probingEngines && !a.lastEngineProbe.IsZero() && - time.Since(a.lastEngineProbe) >= engineReprobe - if due { - a.probingEngines = true - } - a.mu.Unlock() - if !due { - return - } - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - vers := a.reg.ProbeConfigured(ctx) - - a.mu.Lock() - changed := enginesFingerprint(vers) != enginesFingerprint(a.configuredVers) - a.configuredVers, a.lastEngineProbe, a.probingEngines = vers, time.Now(), false - a.mu.Unlock() - if changed { - a.log.Info("engines named in the configuration changed", - "engines", engines.ProbedNames(vers)) - } - }() -} - -func enginesFingerprint(vers map[string]string) string { - var b strings.Builder - for _, k := range slices.Sorted(maps.Keys(vers)) { - fmt.Fprintf(&b, "%s=%s\n", k, vers[k]) - } - return b.String() -} - -// logsReply answers a request for one of this node's logs. -func (a *Agent) logsReply(req *llmgwv1.LogsRequest) *llmgwv1.LogsReply { - n := int(req.GetLines()) - if n <= 0 { - n = 200 - } - n = min(n, agentLogLines) - shards := a.sup.IDs() - slices.Sort(shards) - out := &llmgwv1.LogsReply{ - RequestId: req.GetRequestId(), - Sources: append([]string{"agent"}, shards...), - } - switch src := req.GetSource(); src { - case "", "agent": - out.Lines = a.logRing.tail(n) - default: - sh, err := a.sup.Get(src) - if err != nil { - out.Error = "no engine " + src + " runs on this node" - return out - } - out.Lines = sh.Logs(n) - } - return out -} - // localizeSpec fills in what only this node knows. // // The interface NCCL bootstraps over is this node's, not the cluster's: the @@ -1673,254 +1064,3 @@ func ResolveManagement(c Config) (addr, iface string) { } return addr, iface } - -func smKey(hca string, port int) string { return fmt.Sprintf("%s/%d", hca, port) } - -// scanFabric is discovery.ScanFabric, replaced in tests. -var scanFabric = discovery.ScanFabric - -// querySMs is discovery.QuerySMs, replaced in tests. -var querySMs = func(hca string, port int) ([]discovery.SMRecord, error) { - return discovery.QuerySMs(context.Background(), hca, port) -} - -func splitSMKey(k string) (string, int) { - i := strings.LastIndexByte(k, '/') - port, _ := strconv.Atoi(k[i+1:]) - return k[:i], port -} - -// querySM is discovery.QuerySM, replaced in tests. -var querySM = func(hca string, port int) (*discovery.SMInfo, error) { - return discovery.QuerySM(context.Background(), hca, port) -} - -// attachSM puts on each active InfiniBand port the subnet manager it sees, -// asking again once a minute and otherwise reusing the last answer. -func (a *Agent) attachSM(hcas []discovery.HCA) { - a.mu.Lock() - stale := time.Since(a.smAsked) > time.Minute || a.sm == nil - a.mu.Unlock() - if stale { - sm, errs := map[string]*discovery.SMInfo{}, map[string]string{} - _ = discovery.AttachSM(hcas, func(hca string, port int) (*discovery.SMInfo, error) { - got, err := querySM(hca, port) - if err != nil { - errs[smKey(hca, port)] = err.Error() - return nil, err - } - sm[smKey(hca, port)] = got - return got, nil - }) - // Every subnet manager on each fabric, through one port per master: - // the same list through every port of one fabric is one query too - // many for each. - sms, smsErr := map[string][]discovery.SMRecord{}, map[string]string{} - asked := map[string]bool{} - for k, v := range sm { - if asked[v.GUID] { - continue - } - asked[v.GUID] = true - hca, port := splitSMKey(k) - if recs, err := querySMs(hca, port); err != nil { - smsErr[k] = err.Error() - } else { - sms[k] = recs - } - } - a.mu.Lock() - for k, v := range sm { - if old := a.sm[k]; old != nil && old.GUID != v.GUID { - a.log.Warn("the master subnet manager changed", "port", k, "from", old.GUID, "to", v.GUID) - } - } - a.sm, a.smErr, a.smAsked = sm, errs, time.Now() - a.sms, a.smsErr = sms, smsErr - a.mu.Unlock() - return - } - a.mu.Lock() - defer a.mu.Unlock() - for i := range hcas { - for j := range hcas[i].Ports { - if sm := a.sm[smKey(hcas[i].Name, hcas[i].Ports[j].Num)]; sm != nil && hcas[i].Ports[j].Active() { - hcas[i].Ports[j].SM = sm - } - } - } -} - -// ensureDiags installs infiniband-diags on a node with adapters and without -// it. sminfo is what tells which physical fabric a port is on; without it, -// two separate fabrics given one name cannot be caught. Installed the way -// engine dependencies are, from the machine's own package sources. -func (a *Agent) ensureDiags(ctx context.Context) { - a.mu.Lock() - hasIB := len(a.inv.HCAs) > 0 - a.mu.Unlock() - if !hasIB || a.prov == nil { - return - } - if _, err := exec.LookPath("sminfo"); err == nil { - return - } - a.log.Info("installing infiniband-diags, to tell which fabric each port is on") - pkgs := map[string][]string{"deb": {"infiniband-diags"}, "rpm": {"infiniband-diags"}} - if err := a.prov.InstallPackages(ctx, pkgs); err != nil { - a.log.Warn("could not install infiniband-diags; fabrics cannot be told apart by their subnet manager", - "err", err) - } -} - -// FeatureFabricTest is the agent answering FabricTestRequest. -const FeatureFabricTest = "fabric-test" - -// runWriteBW is discovery.RunWriteBW, and haveWriteBW whether it is -// installed; both replaced in tests. -var ( - runWriteBW = discovery.RunWriteBW - haveWriteBW = func() bool { _, err := exec.LookPath("ib_write_bw"); return err == nil } -) - -// fabricTest runs this node's side of a bandwidth test with another. -func (a *Agent) fabricTest(ctx context.Context, req *llmgwv1.FabricTestRequest) *llmgwv1.FabricTestReply { - start := time.Now() - reply := &llmgwv1.FabricTestReply{RequestId: req.GetRequestId()} - defer func() { reply.TookMs = time.Since(start).Milliseconds() }() - - w := discovery.WriteBW{HCA: req.GetHca(), Port: int(req.GetPort()), GIDIndex: -1, - TCPPort: int(req.GetTcpPort()), Seconds: int(req.GetSeconds())} - if req.GetRole() == "client" { - w.Peer = req.GetPeer() - } - a.mu.Lock() - found := false - for _, h := range a.inv.HCAs { - for _, p := range h.Ports { - if h.Name == w.HCA && p.Num == w.Port { - found = true - // RoCE needs its v2 GID named, as NCCL does; InfiniBand uses the LID. - if p.LinkLayer == discovery.LinkEthernet && p.RoCEv2GID >= 0 { - w.GIDIndex = p.RoCEv2GID - } - } - } - } - a.mu.Unlock() - if !found { - reply.Error = fmt.Sprintf("this node has no port %s/%d", w.HCA, w.Port) - return reply - } - if !haveWriteBW() && a.prov != nil { - a.log.Info("installing perftest, for a bandwidth test with another node") - pkgs := map[string][]string{"deb": {"perftest"}, "rpm": {"perftest"}} - if err := a.prov.InstallPackages(ctx, pkgs); err != nil { - reply.Error = "installing perftest: " + err.Error() - return reply - } - } - gbps, out, err := runWriteBW(ctx, w) - reply.Gbps = gbps - reply.Output = tailLines(out, 12) - if err != nil { - reply.Error = err.Error() - } - return reply -} - -func tailLines(s string, n int) string { - lines := strings.Split(strings.TrimRight(s, "\n"), "\n") - if len(lines) > n { - lines = lines[len(lines)-n:] - } - return strings.Join(lines, "\n") -} - -// discoverGPUFabric and discoverIMEX are replaced in tests. -var ( - discoverGPUFabric = discovery.DiscoverGPUFabric - discoverIMEX = discovery.DiscoverIMEX -) - -// sampleNVLink reads, once a minute, which NVLink domain each GPU is joined -// to and whether IMEX is up. nvidia-smi -q takes a second or two, too long -// for every heartbeat, and a fabric that was joined an hour ago rarely -// changes in five seconds — but when a fabric manager restarts it does, for -// every card at once. -func (a *Agent) sampleNVLink(hb *llmgwv1.Heartbeat) { - a.mu.Lock() - gpus := slices.Clone(a.inv.GPUs) - due := time.Since(a.nvlAsked) > time.Minute - if due { - a.nvlAsked = time.Now() - } - a.mu.Unlock() - if len(gpus) == 0 || !due { - return - } - ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) - defer cancel() - fab, err := discoverGPUFabric(ctx, a.cfg.Discovery.NvidiaSMI) - if err != nil || len(fab) == 0 { - return - } - discovery.ApplyFabric(gpus, fab) - for _, g := range gpus { - if g.Fabric == nil { - continue - } - hb.GpuFabric = append(hb.GpuFabric, &llmgwv1.GPUFabricState{ - Index: int32(g.Index), State: g.Fabric.State, Status: g.Fabric.Status, - ClusterUuid: g.Fabric.ClusterUUID, CliqueId: g.Fabric.CliqueID, Bandwidth: g.Fabric.Bandwidth, - }) - } - m := discoverIMEX(ctx) - hb.Imex = &llmgwv1.IMEX{State: m.State, Channels: int32(m.Channels)} -} - -// upgradeExit is the exit status after an upgrade: non-zero, so systemd's -// Restart=on-failure starts the new binary, and distinct, so the journal says -// why the agent went. -const upgradeExit = 75 - -// exitForUpgrade ends the process; a variable so a test can see it called -// rather than have its own process end. -var exitForUpgrade = func() { os.Exit(upgradeExit) } - -// upgradeTarget is the binary an upgrade replaces; a variable so a test -// replaces a file of its own rather than the test binary running it. -var upgradeTarget = os.Executable - -// upgrade replaces this agent's binary with a release and restarts into it. -// -// Engines are not touched: an agent restart leaves them running and adopts -// them again, which is what makes upgrading a node something that can be done -// while it serves. -func (a *Agent) upgrade(ctx context.Context, m *llmgwv1.UpgradeAgent, out chan<- *llmgwv1.AgentMessage) { - reply := &llmgwv1.UpgradeReply{RequestId: m.GetRequestId(), Version: m.GetVersion()} - send := func() { - select { - case out <- &llmgwv1.AgentMessage{Msg: &llmgwv1.AgentMessage_Upgrade{Upgrade: reply}}: - case <-ctx.Done(): - } - } - a.log.Info("upgrading", "to", m.GetVersion(), "from", a.cfg.Version) - self, err := upgradeTarget() - if err == nil { - dctx, cancel := context.WithTimeout(ctx, 10*time.Minute) - _, err = release.Install(dctx, nil, m.GetReleases(), m.GetVersion(), runtime.GOOS, runtime.GOARCH, self) - cancel() - } - if err != nil { - reply.Error = err.Error() - a.log.Error("upgrade failed; still running the old version", "to", m.GetVersion(), "err", err) - send() - return - } - send() - a.log.Info("upgraded; restarting into the new version", "to", m.GetVersion()) - // A moment for the reply to leave before the stream goes. - time.Sleep(500 * time.Millisecond) - exitForUpgrade() -} diff --git a/internal/agent/config.go b/internal/agent/config.go new file mode 100644 index 0000000..e1593ee --- /dev/null +++ b/internal/agent/config.go @@ -0,0 +1,286 @@ +package agent + +import ( + "errors" + "fmt" + "net" + "net/url" + "os" + "slices" + "strings" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/discovery" + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/engines" + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/supervisor" + "gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory" + "gitea.mynodes.xyz/ilker/llmgw/internal/tlsconf" +) + +// Config configures the agent. +type Config struct { + // NodeID defaults to the hostname. It must equal the common name of the + // agent's certificate, so set it only where the two differ. + NodeID string `koanf:"node_id"` + // Server is the control plane, and Servers is the same thing when there is + // more than one. Both are accepted: every configuration written before + // there could be a second control plane names one address, and none of + // them should have to change. + // + // The order is tried in order, and a refusal moves to the next. Only the + // leader accepts an agent, so a standby is skipped in the time it takes to + // be told so — which is what makes failover a reconnect rather than an + // edit to every node's configuration. + Server string `koanf:"server"` + Servers []string `koanf:"servers"` + // Hostname defaults to the machine's. + Hostname string `koanf:"hostname"` + // MgmtAddr is the address engines are reached on. It defaults to this + // machine's address on the route to the control plane, which is the one + // the control plane and the router can reach. + MgmtAddr string `koanf:"mgmt_addr"` + // Labels are free-form facts about a node for a profile's node_selector. + // Nothing needs them: memory, GPUs, engines and disk are measured, and a + // label restating one of those goes stale the day the machine changes. + Labels map[string]string `koanf:"labels"` + + // MgmtInterface is the ethernet interface NCCL bootstraps over. It + // defaults to the interface holding MgmtAddr, then eth0. + MgmtInterface string `koanf:"mgmt_interface"` + // Fabrics labels this node's HCAs. It is explicit on purpose: OpenSM's + // default subnet prefix is the same on every fabric, so nothing can be + // inferred from the wire (plan §5.2). + Fabrics []inventory.FabricAttachment `koanf:"fabrics"` + + HeartbeatInterval time.Duration `koanf:"heartbeat_interval"` + // ReconnectBase is the first reconnect delay; it backs off from there. + ReconnectBase time.Duration `koanf:"reconnect_base"` + ReconnectMax time.Duration `koanf:"reconnect_max"` + + // AdoptReadyTimeout bounds how long an adopted engine is given to answer a + // health check before the agent stops waiting on it. + // + // Shorter than a startup timeout on purpose: this engine is already + // running, so it is not loading a model, it is either serving or it is not. + AdoptReadyTimeout time.Duration `koanf:"adopt_ready_timeout"` + + // TLS authenticates both ends of the control-plane stream. Leaving it unset + // keeps the plaintext behaviour, which only belongs on a trusted network. + TLS tlsconf.Config `koanf:"tls"` + + // ModelDirs are the directories this node keeps model weights in, beyond + // WeightsCache, which is always looked in. Storage differs at every installation, + // so llmgw observes it rather than owning it: a Lustre mount, an image + // exported over the fabric, local NVMe and a copy made by hand all look + // identical from here, and placement only needs to know whether the file + // is present. + ModelDirs []string `koanf:"model_dirs"` + + // EnginesDir is where the agent installs engines, one directory per + // build. Defaults to /opt/llmgw/engines. + EnginesDir string `koanf:"engines_dir"` + + // WeightsCache is where a remote source is downloaded to. Defaults to the + // first entry of ModelDirs, because a node that says where its models live + // has already answered this question, and otherwise to + // /var/lib/llmgw-weights. + // + // Fetching is the simple arrangement: every node keeps its own copy. The + // exotic ones need no code — weights on a shared filer or an image exported + // over the fabric are simply present, and present means nothing is fetched. + WeightsCache string `koanf:"weights_cache"` + + Discovery discovery.Options `koanf:"discovery"` + Engines engines.Config `koanf:"engines"` + Superv supervisor.Config `koanf:"supervisor"` + + Version string +} + +// DefaultConfig returns sensible defaults. +func DefaultConfig() Config { + // No MgmtInterface: normalize finds the one holding the management + // address, and falls back to eth0 only when it cannot. Defaulted here, it + // was never found — every node said eth0 unless told otherwise, and a node + // on ens3 or bond0 had NCCL bootstrap over an interface it does not have. + return Config{ + HeartbeatInterval: 5 * time.Second, + ReconnectBase: time.Second, + ReconnectMax: 30 * time.Second, + AdoptReadyTimeout: 60 * time.Second, + Superv: supervisor.DefaultConfig(), + // NVML gives NVLink topology, so prefer it when this build has it. + Discovery: discovery.Options{PreferNVML: true}, + } +} + +func (c *Config) normalize() error { + d := DefaultConfig() + if err := c.TLS.Validate(); err != nil { + return err + } + if err := c.normalizeServerURL(); err != nil { + return err + } + // Everything below is found rather than written. A file that has to say + // which machine it is on, and at what address, is a file that differs on + // every node — and a copy taken from one node to the next registers the + // second as the first. + host, _ := os.Hostname() + if c.NodeID == "" { + c.NodeID = host + } + if c.NodeID == "" { + return errors.New("node_id is not set and the hostname could not be read") + } + if c.Hostname == "" { + c.Hostname = host + } + if c.MgmtAddr == "" { + c.MgmtAddr = addrToward(c.Server) + } + if c.MgmtInterface == "" { + c.MgmtInterface = interfaceOf(c.MgmtAddr) + } + if c.MgmtInterface == "" { + c.MgmtInterface = "eth0" + } + // The directory downloads land in is always one that is looked in. When + // they were two settings, a node could fill its disk with fetched weights + // that placement never saw — which is what every node here was doing. + if c.WeightsCache == "" && len(c.ModelDirs) > 0 { + c.WeightsCache = c.ModelDirs[0] + } + if c.WeightsCache == "" { + c.WeightsCache = DefaultWeightsCache + } + if !slices.Contains(c.ModelDirs, c.WeightsCache) { + c.ModelDirs = append(c.ModelDirs, c.WeightsCache) + } + if c.HeartbeatInterval <= 0 { + c.HeartbeatInterval = d.HeartbeatInterval + } + if c.ReconnectBase <= 0 { + c.ReconnectBase = d.ReconnectBase + } + if c.AdoptReadyTimeout <= 0 { + c.AdoptReadyTimeout = d.AdoptReadyTimeout + } + if c.ReconnectMax < c.ReconnectBase { + c.ReconnectMax = max(c.ReconnectBase, d.ReconnectMax) + } + return nil +} + +// normalizeServerURL makes the configured address a URL connect can use, and +// refuses the combination that would quietly undo the certificates: a scheme +// that says plaintext while TLS is configured. Silently connecting anyway would +// mean a cluster believing it has mutual authentication when it has none. +func (c *Config) normalizeServerURL() error { + // One list, however it was written. Server first when both are given, + // because an operator adding a standby to an existing file expects the + // one already there to stay preferred. + all := make([]string, 0, 1+len(c.Servers)) + if c.Server != "" { + all = append(all, c.Server) + } + all = append(all, c.Servers...) + + seen := map[string]struct{}{} + out := make([]string, 0, len(all)) + for _, s := range all { + fixed, err := normalizeOne(s, c.TLS.Enabled()) + if err != nil { + return err + } + // A duplicate would be tried twice in a row on failover, which reads + // as the rotation not working. + if _, dup := seen[fixed]; dup { + continue + } + seen[fixed] = struct{}{} + out = append(out, fixed) + } + if len(out) == 0 { + return errors.New("server is required") + } + c.Servers = out + c.Server = out[0] + return nil +} + +// DefaultWeightsCache is where fetched weights go when nothing says otherwise. +const DefaultWeightsCache = "/var/lib/llmgw-weights" + +// addrToward is this machine's address on the route to the control plane. +// +// The address engines are reached on is whichever one the control plane can +// reach, and the kernel already knows which that is: a UDP "connection" picks +// the source address for the route without sending a packet. Asking the +// interfaces instead would have to guess among docker bridges, VPNs and +// link-local addresses. +func addrToward(server string) string { + u, err := url.Parse(server) + if err != nil || u.Host == "" { + return "" + } + host := u.Hostname() + port := u.Port() + if port == "" { + port = "443" + } + conn, err := net.Dial("udp", net.JoinHostPort(host, port)) + if err != nil { + return "" + } + defer conn.Close() + if a, ok := conn.LocalAddr().(*net.UDPAddr); ok { + return a.IP.String() + } + return "" +} + +// interfaceOf names the interface that holds an address, or "" when none does. +func interfaceOf(addr string) string { + ip := net.ParseIP(addr) + if ip == nil { + return "" + } + ifaces, err := net.Interfaces() + if err != nil { + return "" + } + for _, ifc := range ifaces { + addrs, err := ifc.Addrs() + if err != nil { + continue + } + for _, a := range addrs { + if n, ok := a.(*net.IPNet); ok && n.IP.Equal(ip) { + return ifc.Name + } + } + } + return "" +} + +// normalizeOne applies the scheme rules to a single address. +func normalizeOne(addr string, secure bool) (string, error) { + switch { + case strings.HasPrefix(addr, "https://"): + if !secure { + return "", fmt.Errorf("%s is an https:// address but no tls certificates are configured", addr) + } + case strings.HasPrefix(addr, "http://"): + if secure { + return "", fmt.Errorf("tls is configured but %s is an http:// address; use https://", addr) + } + case strings.Contains(addr, "://"): + return "", fmt.Errorf("server %q must be an http:// or https:// address", addr) + case secure: + return "https://" + addr, nil + default: + return "http://" + addr, nil + } + return addr, nil +} diff --git a/internal/agent/engines/llamacpp.go b/internal/agent/engines/llamacpp.go index 9424cb6..7b500af 100644 --- a/internal/agent/engines/llamacpp.go +++ b/internal/agent/engines/llamacpp.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "sort" "strconv" "strings" @@ -21,8 +22,21 @@ type LlamaCppConfig struct { // does not say. llama.cpp defaults to one, which would make every second // request wait behind the first no matter what the router decided. Parallel int `koanf:"parallel"` + // Threads is how many CPU threads a shard with no GPU computes on. Zero + // takes half this machine's CPUs. + // + // Not all of them, which is llama.cpp's own default: two busy servers + // that each spin on every CPU fight for them, and on an 8-vCPU node + // that made 7B generation fall from 10 tokens a second to under 1 each. + // At half, one server alone is about a third slower and two together + // keep 5.5 each. A node that only ever runs one CPU replica does better + // with all of them; say so here. + Threads int `koanf:"threads"` } +// numCPU is the CPUs this process may run on; a variable for tests. +var numCPU = runtime.NumCPU + // LlamaCpp builds llama.cpp launch commands. // // It is the engine that makes a cluster without GPUs useful: llama.cpp runs a @@ -118,6 +132,13 @@ func (l *LlamaCpp) Commands(s ShardSpec) ([]Command, error) { if len(s.GPUIndices) > 0 && !hasFlag(p.Args, "--n-gpu-layers", "-ngl") { args = append(args, "--n-gpu-layers", "999") } + if len(s.GPUIndices) == 0 && !hasFlag(p.Args, "--threads", "-t") { + n := l.cfg.Threads + if n <= 0 { + n = max(numCPU()/2, 1) + } + args = append(args, "--threads", strconv.Itoa(n)) + } args = append(args, p.Args...) env := baseEnv(s) diff --git a/internal/agent/engines/llamacpp_test.go b/internal/agent/engines/llamacpp_test.go index e97c933..aff2d69 100644 --- a/internal/agent/engines/llamacpp_test.go +++ b/internal/agent/engines/llamacpp_test.go @@ -261,3 +261,36 @@ func TestLlamaCppIsGivenItsKeyOutOfSightOfTheProcessList(t *testing.T) { t.Error("llama-server was not given its key") } } + +func TestACPUShardLeavesHalfTheMachineForAnother(t *testing.T) { + prev := numCPU + numCPU = func() int { return 8 } + t.Cleanup(func() { numCPU = prev }) + + threads := func(l *LlamaCpp, s ShardSpec) (string, bool) { + cmds, err := l.Commands(s) + if err != nil { + t.Fatal(err) + } + return flagValue(cmds[0].Args, "--threads") + } + if v, _ := threads(&LlamaCpp{}, llamaSpec(nil)); v != "4" { + t.Errorf("--threads %q on 8 CPUs, expected 4", v) + } + if v, _ := threads(&LlamaCpp{cfg: LlamaCppConfig{Threads: 8}}, llamaSpec(nil)); v != "8" { + t.Errorf("--threads %q with 8 configured", v) + } + if _, ok := threads(&LlamaCpp{}, llamaSpec([]int{0})); ok { + t.Error("a GPU shard was given a thread count") + } + s := llamaSpec(nil) + s.Profile.Args = []string{"-t", "2"} + cmds, _ := (&LlamaCpp{}).Commands(s) + if slices.Contains(cmds[0].Args, "--threads") { + t.Error("the profile's own -t was overridden") + } + numCPU = func() int { return 1 } + if v, _ := threads(&LlamaCpp{}, llamaSpec(nil)); v != "1" { + t.Errorf("--threads %q on one CPU", v) + } +} diff --git a/internal/agent/engines/vllm.go b/internal/agent/engines/vllm.go index 81a63d0..0d4b532 100644 --- a/internal/agent/engines/vllm.go +++ b/internal/agent/engines/vllm.go @@ -169,7 +169,10 @@ func (v *VLLM) rayCommand(s ShardSpec) (Command, bool) { args := []string{"start", "--block"} if s.Leader() { - args = append(args, "--head", "--port", strconv.Itoa(s.LeaderPort)) + // Ray's head has no authentication: whoever reaches its port can run + // code on the node. Its dashboard, and the job API behind it, is the + // easiest way in, and nothing here uses it. + args = append(args, "--head", "--port", strconv.Itoa(s.LeaderPort), "--include-dashboard=false") } else { args = append(args, "--address", fmt.Sprintf("%s:%d", s.LeaderAddr, s.LeaderPort)) } diff --git a/internal/agent/fabrics.go b/internal/agent/fabrics.go new file mode 100644 index 0000000..13b05b3 --- /dev/null +++ b/internal/agent/fabrics.go @@ -0,0 +1,345 @@ +package agent + +import ( + "context" + "fmt" + "os/exec" + "slices" + "strconv" + "strings" + "time" + + llmgwv1 "gitea.mynodes.xyz/ilker/llmgw/api/gen/llmgw/v1" + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/discovery" + "gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory" +) + +// usableFabrics reports the fabrics this node can actually carry traffic on. +// +// Loud about what it drops. A fabric silently missing from a registration looks +// identical to one never configured, and the operator who typed the adapter +// name is the only person who can fix it — so the reason is logged here, on the +// machine where both halves are visible, rather than inferred later from a +// replica that will not converge. +func (a *Agent) usableFabrics(inv discovery.Inventory) []inventory.FabricAttachment { + out := a.checkFabrics(inv.HCAs, true) + a.mu.Lock() + a.fabricsSent = fabricsFingerprint(out) + a.mu.Unlock() + return out +} + +// checkFabrics is the declared fabrics this node can carry traffic on, from +// the adapters as they are now. Loud logs every problem; the heartbeat checks +// quietly and speaks only when the answer changes. +func (a *Agent) checkFabrics(hcas []discovery.HCA, loud bool) []inventory.FabricAttachment { + declared := map[string][]string{} + for _, f := range a.cfg.Fabrics { + declared[f.Fabric] = f.HCAs + } + + var out []inventory.FabricAttachment + for _, c := range discovery.CheckFabrics(declared, hcas) { + if loud { + for _, p := range c.Problems { + a.log.Error("a declared fabric adapter cannot carry traffic", + "fabric", c.Fabric, "problem", p) + } + } + if !c.OK() { + if loud { + a.log.Error("fabric not advertised; no declared adapter on it can carry traffic", + "fabric", c.Fabric, + "consequence", "no multi-node replica will be placed here") + } + continue + } + out = append(out, inventory.FabricAttachment{ + Fabric: c.Fabric, HCAs: c.Usable, RateGbps: c.RateGbps, + }) + } + return out +} + +// fabricsFingerprint is enough to notice a fabric, an adapter or a rate +// changing. +func fabricsFingerprint(fs []inventory.FabricAttachment) string { + var b strings.Builder + for _, f := range fs { + fmt.Fprintf(&b, "%s:%s:%d;", f.Fabric, strings.Join(f.HCAs, ","), f.RateGbps) + } + return b.String() +} + +// sampleIB re-reads the adapters for the heartbeat: every port's state and +// counters, and the fabrics again when they no longer match what the control +// plane was told. +// +// Every heartbeat, because it is a few small files per port, and because a +// port that went down used to be invisible until the agent restarted — the +// node went on being offered multi-node work that would hang on its first +// collective. +func (a *Agent) sampleIB(hb *llmgwv1.Heartbeat) { + a.mu.Lock() + had := len(a.inv.HCAs) > 0 + a.mu.Unlock() + if !had && len(a.cfg.Fabrics) == 0 { + return + } + hcas, err := discovery.DiscoverIB(a.cfg.Discovery.IBRoot) + if err != nil { + return + } + a.attachSM(hcas) + for _, h := range hcas { + for _, p := range h.Ports { + ip := &llmgwv1.IBPort{ + Hca: h.Name, Port: int32(p.Num), State: p.State, PhysState: p.PhysState, + RateGbps: int32(p.RateGbps), LinkLayer: string(p.LinkLayer), Counters: p.Counters, + NodeGuid: h.NodeGUID, PortGuid: p.PortGUID, Network: p.Network, GidPrefix: p.GIDPrefix, + } + a.mu.Lock() + if p.SM != nil { + ip.SmGuid, ip.SmLid, ip.SmPriority, ip.SmState = p.SM.GUID, int32(p.SM.LID), int32(p.SM.Priority), p.SM.State + } else { + ip.SmError = a.smErr[smKey(h.Name, p.Num)] + } + for _, r := range a.sms[smKey(h.Name, p.Num)] { + ip.SubnetManagers = append(ip.SubnetManagers, &llmgwv1.SubnetManager{ + Guid: r.GUID, Lid: int32(r.LID), Priority: int32(r.Priority), State: r.State}) + } + ip.SubnetManagersError = a.smsErr[smKey(h.Name, p.Num)] + a.mu.Unlock() + hb.IbPorts = append(hb.IbPorts, ip) + } + } + a.mu.Lock() + a.inv.HCAs = hcas // a reconnect registers what is true now + a.mu.Unlock() + if len(a.cfg.Fabrics) == 0 { + return + } + now := a.checkFabrics(hcas, false) + fp := fabricsFingerprint(now) + a.mu.Lock() + changed := fp != a.fabricsSent + a.fabricsSent = fp + a.mu.Unlock() + if !changed { + return + } + // Said once, with the reasons, the moment it happens. + a.checkFabrics(hcas, true) + a.log.Warn("the fabrics this node can carry traffic on changed", "now", fp) + hb.FabricsChanged = true + for _, f := range now { + hb.Fabrics = append(hb.Fabrics, &llmgwv1.FabricAttachment{ + Fabric: f.Fabric, Hcas: f.HCAs, RateGbps: int32(f.RateGbps), + }) + } +} + +func smKey(hca string, port int) string { return fmt.Sprintf("%s/%d", hca, port) } + +// scanFabric is discovery.ScanFabric, replaced in tests. +var scanFabric = discovery.ScanFabric + +// querySMs is discovery.QuerySMs, replaced in tests. +var querySMs = func(hca string, port int) ([]discovery.SMRecord, error) { + return discovery.QuerySMs(context.Background(), hca, port) +} + +func splitSMKey(k string) (string, int) { + i := strings.LastIndexByte(k, '/') + port, _ := strconv.Atoi(k[i+1:]) + return k[:i], port +} + +// querySM is discovery.QuerySM, replaced in tests. +var querySM = func(hca string, port int) (*discovery.SMInfo, error) { + return discovery.QuerySM(context.Background(), hca, port) +} + +// attachSM puts on each active InfiniBand port the subnet manager it sees, +// asking again once a minute and otherwise reusing the last answer. +func (a *Agent) attachSM(hcas []discovery.HCA) { + a.mu.Lock() + stale := time.Since(a.smAsked) > time.Minute || a.sm == nil + a.mu.Unlock() + if stale { + sm, errs := map[string]*discovery.SMInfo{}, map[string]string{} + _ = discovery.AttachSM(hcas, func(hca string, port int) (*discovery.SMInfo, error) { + got, err := querySM(hca, port) + if err != nil { + errs[smKey(hca, port)] = err.Error() + return nil, err + } + sm[smKey(hca, port)] = got + return got, nil + }) + // Every subnet manager on each fabric, through one port per master: + // the same list through every port of one fabric is one query too + // many for each. + sms, smsErr := map[string][]discovery.SMRecord{}, map[string]string{} + asked := map[string]bool{} + for k, v := range sm { + if asked[v.GUID] { + continue + } + asked[v.GUID] = true + hca, port := splitSMKey(k) + if recs, err := querySMs(hca, port); err != nil { + smsErr[k] = err.Error() + } else { + sms[k] = recs + } + } + a.mu.Lock() + for k, v := range sm { + if old := a.sm[k]; old != nil && old.GUID != v.GUID { + a.log.Warn("the master subnet manager changed", "port", k, "from", old.GUID, "to", v.GUID) + } + } + a.sm, a.smErr, a.smAsked = sm, errs, time.Now() + a.sms, a.smsErr = sms, smsErr + a.mu.Unlock() + return + } + a.mu.Lock() + defer a.mu.Unlock() + for i := range hcas { + for j := range hcas[i].Ports { + if sm := a.sm[smKey(hcas[i].Name, hcas[i].Ports[j].Num)]; sm != nil && hcas[i].Ports[j].Active() { + hcas[i].Ports[j].SM = sm + } + } + } +} + +// ensureDiags installs infiniband-diags on a node with adapters and without +// it. sminfo is what tells which physical fabric a port is on; without it, +// two separate fabrics given one name cannot be caught. Installed the way +// engine dependencies are, from the machine's own package sources. +func (a *Agent) ensureDiags(ctx context.Context) { + a.mu.Lock() + hasIB := len(a.inv.HCAs) > 0 + a.mu.Unlock() + if !hasIB || a.prov == nil { + return + } + if _, err := exec.LookPath("sminfo"); err == nil { + return + } + a.log.Info("installing infiniband-diags, to tell which fabric each port is on") + pkgs := map[string][]string{"deb": {"infiniband-diags"}, "rpm": {"infiniband-diags"}} + if err := a.prov.InstallPackages(ctx, pkgs); err != nil { + a.log.Warn("could not install infiniband-diags; fabrics cannot be told apart by their subnet manager", + "err", err) + } +} + +// FeatureFabricTest is the agent answering FabricTestRequest. +const FeatureFabricTest = "fabric-test" + +// runWriteBW is discovery.RunWriteBW, and haveWriteBW whether it is +// installed; both replaced in tests. +var ( + runWriteBW = discovery.RunWriteBW + haveWriteBW = func() bool { _, err := exec.LookPath("ib_write_bw"); return err == nil } +) + +// fabricTest runs this node's side of a bandwidth test with another. +func (a *Agent) fabricTest(ctx context.Context, req *llmgwv1.FabricTestRequest) *llmgwv1.FabricTestReply { + start := time.Now() + reply := &llmgwv1.FabricTestReply{RequestId: req.GetRequestId()} + defer func() { reply.TookMs = time.Since(start).Milliseconds() }() + + w := discovery.WriteBW{HCA: req.GetHca(), Port: int(req.GetPort()), GIDIndex: -1, + TCPPort: int(req.GetTcpPort()), Seconds: int(req.GetSeconds())} + if req.GetRole() == "client" { + w.Peer = req.GetPeer() + } + a.mu.Lock() + found := false + for _, h := range a.inv.HCAs { + for _, p := range h.Ports { + if h.Name == w.HCA && p.Num == w.Port { + found = true + // RoCE needs its v2 GID named, as NCCL does; InfiniBand uses the LID. + if p.LinkLayer == discovery.LinkEthernet && p.RoCEv2GID >= 0 { + w.GIDIndex = p.RoCEv2GID + } + } + } + } + a.mu.Unlock() + if !found { + reply.Error = fmt.Sprintf("this node has no port %s/%d", w.HCA, w.Port) + return reply + } + if !haveWriteBW() && a.prov != nil { + a.log.Info("installing perftest, for a bandwidth test with another node") + pkgs := map[string][]string{"deb": {"perftest"}, "rpm": {"perftest"}} + if err := a.prov.InstallPackages(ctx, pkgs); err != nil { + reply.Error = "installing perftest: " + err.Error() + return reply + } + } + gbps, out, err := runWriteBW(ctx, w) + reply.Gbps = gbps + reply.Output = tailLines(out, 12) + if err != nil { + reply.Error = err.Error() + } + return reply +} + +func tailLines(s string, n int) string { + lines := strings.Split(strings.TrimRight(s, "\n"), "\n") + if len(lines) > n { + lines = lines[len(lines)-n:] + } + return strings.Join(lines, "\n") +} + +// discoverGPUFabric and discoverIMEX are replaced in tests. +var ( + discoverGPUFabric = discovery.DiscoverGPUFabric + discoverIMEX = discovery.DiscoverIMEX +) + +// sampleNVLink reads, once a minute, which NVLink domain each GPU is joined +// to and whether IMEX is up. nvidia-smi -q takes a second or two, too long +// for every heartbeat, and a fabric that was joined an hour ago rarely +// changes in five seconds — but when a fabric manager restarts it does, for +// every card at once. +func (a *Agent) sampleNVLink(hb *llmgwv1.Heartbeat) { + a.mu.Lock() + gpus := slices.Clone(a.inv.GPUs) + due := time.Since(a.nvlAsked) > time.Minute + if due { + a.nvlAsked = time.Now() + } + a.mu.Unlock() + if len(gpus) == 0 || !due { + return + } + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + fab, err := discoverGPUFabric(ctx, a.cfg.Discovery.NvidiaSMI) + if err != nil || len(fab) == 0 { + return + } + discovery.ApplyFabric(gpus, fab) + for _, g := range gpus { + if g.Fabric == nil { + continue + } + hb.GpuFabric = append(hb.GpuFabric, &llmgwv1.GPUFabricState{ + Index: int32(g.Index), State: g.Fabric.State, Status: g.Fabric.Status, + ClusterUuid: g.Fabric.ClusterUUID, CliqueId: g.Fabric.CliqueID, Bandwidth: g.Fabric.Bandwidth, + }) + } + m := discoverIMEX(ctx) + hb.Imex = &llmgwv1.IMEX{State: m.State, Channels: int32(m.Channels)} +} diff --git a/internal/agent/logs.go b/internal/agent/logs.go new file mode 100644 index 0000000..78b4909 --- /dev/null +++ b/internal/agent/logs.go @@ -0,0 +1,34 @@ +package agent + +import ( + "slices" + + llmgwv1 "gitea.mynodes.xyz/ilker/llmgw/api/gen/llmgw/v1" +) + +// logsReply answers a request for one of this node's logs. +func (a *Agent) logsReply(req *llmgwv1.LogsRequest) *llmgwv1.LogsReply { + n := int(req.GetLines()) + if n <= 0 { + n = 200 + } + n = min(n, agentLogLines) + shards := a.sup.IDs() + slices.Sort(shards) + out := &llmgwv1.LogsReply{ + RequestId: req.GetRequestId(), + Sources: append([]string{"agent"}, shards...), + } + switch src := req.GetSource(); src { + case "", "agent": + out.Lines = a.logRing.tail(n) + default: + sh, err := a.sup.Get(src) + if err != nil { + out.Error = "no engine " + src + " runs on this node" + return out + } + out.Lines = sh.Logs(n) + } + return out +} diff --git a/internal/agent/provision.go b/internal/agent/provision.go new file mode 100644 index 0000000..a0e876d --- /dev/null +++ b/internal/agent/provision.go @@ -0,0 +1,197 @@ +package agent + +import ( + "context" + "fmt" + "log/slog" + "maps" + "slices" + "strings" + "time" + + llmgwv1 "gitea.mynodes.xyz/ilker/llmgw/api/gen/llmgw/v1" + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/engines" + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/provision" +) + +// engines is what this node can run: what the agent installed and saw run, +// and what the configuration names and answers. Nothing merely found on the +// machine — a llama-server on PATH is somebody's, not necessarily one that +// works, and placing on it is how a replica fails on a node nobody watches. +func (a *Agent) engines(ctx context.Context) map[string]string { + a.engineOnce.Do(func() { + vers := a.reg.ProbeConfigured(ctx) + a.mu.Lock() + a.configuredVers, a.lastEngineProbe = vers, time.Now() + a.mu.Unlock() + if len(vers) > 0 { + a.log.Info("engines named in the configuration", + "engines", engines.ProbedNames(vers)) + } + }) + a.mu.Lock() + defer a.mu.Unlock() + vers := a.engineReportLocked() + // Whatever registration sends is what the control plane now knows, so a + // heartbeat repeats it only once it differs. + a.enginesSent = enginesFingerprint(vers) + return vers +} + +// engineReportLocked merges the two sources. Installed wins: it is the one +// that was seen to run. Called with mu held. +func (a *Agent) engineReportLocked() map[string]string { + out := maps.Clone(a.configuredVers) + if out == nil { + out = map[string]string{} + } + maps.Copy(out, a.installedVers) + return out +} + +// provisionEvery is how often installed engines are checked again. A library +// removed by an unrelated upgrade is found here rather than by the next +// replica to start. +const provisionEvery = 10 * time.Minute + +// setProvision takes the control plane's list of wanted engines. +func (a *Agent) setProvision(p *llmgwv1.Provision) { + wants := []provision.Want{} + for _, e := range p.GetEngines() { + wants = append(wants, provision.Want{Engine: e.GetEngine(), Version: e.GetVersion()}) + } + a.mu.Lock() + a.provWants = wants + a.mu.Unlock() + select { + case a.provKick <- struct{}{}: + default: + } +} + +// provisionLoop installs and re-checks the wanted engines, one pass at a time. +func (a *Agent) provisionLoop(ctx context.Context) { + // After a failure the next pass comes sooner, and backs off: a lock held + // by a new machine's own updates clears in a minute or two, while ten + // minutes' wait left a node idle for no reason. + next := provisionEvery + retry := time.Minute + for { + select { + case <-ctx.Done(): + return + case <-a.provKick: + case <-time.After(next): + } + a.mu.Lock() + wants := slices.Clone(a.provWants) + managed := a.provWants != nil + a.mu.Unlock() + if !managed { + continue + } + + statuses := a.prov.Converge(ctx, wants) + next = provisionEvery + for _, st := range statuses { + if st.State == provision.Failed { + next, retry = retry, min(retry*2, provisionEvery) + break + } + } + if next == provisionEvery { + retry = time.Minute + } + installed := map[string]string{} + keep := map[string]bool{} + for _, st := range statuses { + keep[st.Engine] = true + if st.State == provision.Ready { + a.reg.UseInstalled(st.Engine, st.Binary) + installed[st.Engine] = st.Version + } + } + a.mu.Lock() + if enginesFingerprint(installed) != enginesFingerprint(a.installedVers) { + a.log.Info("engines installed and running", "engines", engines.ProbedNames(installed)) + } + a.installedVers = installed + // An engine no longer asked for is no longer reported on. + for e := range a.provStatus { + if !keep[e] { + delete(a.provStatus, e) + } + } + a.mu.Unlock() + } +} + +// provisionStatus records a change in one engine's installation. +func (a *Agent) provisionStatus(st provision.Status) { + a.mu.Lock() + prev, had := a.provStatus[st.Engine] + a.provStatus[st.Engine] = st + a.mu.Unlock() + if !had || prev.State != st.State || prev.Detail != st.Detail { + lvl := slog.LevelInfo + if st.State == provision.Failed || st.State == provision.Unsupported { + lvl = slog.LevelWarn + } + a.log.Log(context.Background(), lvl, "engine installation", + "engine", st.Engine, "version", st.Version, "variant", st.Variant, + "state", st.State, "detail", st.Detail) + } +} + +func provisionFingerprint(m map[string]provision.Status) string { + var b strings.Builder + for _, k := range slices.Sorted(maps.Keys(m)) { + st := m[k] + fmt.Fprintf(&b, "%s|%s|%s|%s|%s\n", st.Engine, st.Version, st.Variant, st.State, st.Detail) + } + return b.String() +} + +// engineReprobe is how often the installed engines are looked for again. +// +// Minutes, not seconds: an engine is installed by somebody at a keyboard, so a +// minute's delay in noticing is invisible, while SGLang's probe starts a +// Python interpreter that imports it. +const engineReprobe = time.Minute + +// reprobeEngines looks for installed engines again when it is time, off the +// heartbeat goroutine so a slow probe never delays a heartbeat. +func (a *Agent) reprobeEngines() { + a.mu.Lock() + due := !a.probingEngines && !a.lastEngineProbe.IsZero() && + time.Since(a.lastEngineProbe) >= engineReprobe + if due { + a.probingEngines = true + } + a.mu.Unlock() + if !due { + return + } + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + vers := a.reg.ProbeConfigured(ctx) + + a.mu.Lock() + changed := enginesFingerprint(vers) != enginesFingerprint(a.configuredVers) + a.configuredVers, a.lastEngineProbe, a.probingEngines = vers, time.Now(), false + a.mu.Unlock() + if changed { + a.log.Info("engines named in the configuration changed", + "engines", engines.ProbedNames(vers)) + } + }() +} + +func enginesFingerprint(vers map[string]string) string { + var b strings.Builder + for _, k := range slices.Sorted(maps.Keys(vers)) { + fmt.Fprintf(&b, "%s=%s\n", k, vers[k]) + } + return b.String() +} diff --git a/internal/agent/upgrade.go b/internal/agent/upgrade.go new file mode 100644 index 0000000..80fa591 --- /dev/null +++ b/internal/agent/upgrade.go @@ -0,0 +1,58 @@ +package agent + +import ( + "context" + "os" + "runtime" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/release" + + llmgwv1 "gitea.mynodes.xyz/ilker/llmgw/api/gen/llmgw/v1" +) + +// upgradeExit is the exit status after an upgrade: non-zero, so systemd's +// Restart=on-failure starts the new binary, and distinct, so the journal says +// why the agent went. +const upgradeExit = 75 + +// exitForUpgrade ends the process; a variable so a test can see it called +// rather than have its own process end. +var exitForUpgrade = func() { os.Exit(upgradeExit) } + +// upgradeTarget is the binary an upgrade replaces; a variable so a test +// replaces a file of its own rather than the test binary running it. +var upgradeTarget = os.Executable + +// upgrade replaces this agent's binary with a release and restarts into it. +// +// Engines are not touched: an agent restart leaves them running and adopts +// them again, which is what makes upgrading a node something that can be done +// while it serves. +func (a *Agent) upgrade(ctx context.Context, m *llmgwv1.UpgradeAgent, out chan<- *llmgwv1.AgentMessage) { + reply := &llmgwv1.UpgradeReply{RequestId: m.GetRequestId(), Version: m.GetVersion()} + send := func() { + select { + case out <- &llmgwv1.AgentMessage{Msg: &llmgwv1.AgentMessage_Upgrade{Upgrade: reply}}: + case <-ctx.Done(): + } + } + a.log.Info("upgrading", "to", m.GetVersion(), "from", a.cfg.Version) + self, err := upgradeTarget() + if err == nil { + dctx, cancel := context.WithTimeout(ctx, 10*time.Minute) + _, err = release.Install(dctx, nil, m.GetReleases(), m.GetVersion(), runtime.GOOS, runtime.GOARCH, self) + cancel() + } + if err != nil { + reply.Error = err.Error() + a.log.Error("upgrade failed; still running the old version", "to", m.GetVersion(), "err", err) + send() + return + } + send() + a.log.Info("upgraded; restarting into the new version", "to", m.GetVersion()) + // A moment for the reply to leave before the stream goes. + time.Sleep(500 * time.Millisecond) + exitForUpgrade() +} diff --git a/internal/cli/mirror.go b/internal/cli/mirror.go index 8cda330..089dc35 100644 --- a/internal/cli/mirror.go +++ b/internal/cli/mirror.go @@ -8,6 +8,7 @@ import ( "net/http" "net/netip" "os" + "slices" "strings" "time" @@ -232,7 +233,7 @@ func newMirrorRemoveCmd(dir *string) *cobra.Command { func newMirrorServeCmd(dir *string) *cobra.Command { var listen, apiListen, base string var tlsc tlsconf.Config - var allow []string + var allow, clients []string var allowPrivate bool c := &cobra.Command{ @@ -259,6 +260,9 @@ func newMirrorServeCmd(dir *string) *cobra.Command { if err := tlsc.Validate(); err != nil { return err } + if len(clients) > 0 && !tlsc.Enabled() { + return fmt.Errorf("--api-client names a certificate, and needs --tls-ca, --tls-cert and --tls-key") + } out := cmd.OutOrStdout() log := slog.Default() @@ -282,7 +286,7 @@ func newMirrorServeCmd(dir *string) *cobra.Command { } admin := &http.Server{ Addr: apiListen, - Handler: apiGate(tlsc.Enabled(), allowed, api.Handler()), + Handler: onlyClients(clients, apiGate(tlsc.Enabled(), allowed, api.Handler())), ReadHeaderTimeout: 10 * time.Second, } @@ -294,6 +298,12 @@ func newMirrorServeCmd(dir *string) *cobra.Command { switch { case tlsc.Enabled(): fmt.Fprintf(out, "management API on %s, mutual TLS against %s\n", apiListen, tlsc.CA) + if len(clients) == 0 { + // The same root signs every node's certificate, so without a + // name any node could delete models here. + log.Warn("the management API takes any certificate from " + tlsc.CA + + ", every node's included; name the control plane's with --api-client") + } case len(allowed) > 0: fmt.Fprintf(out, "management API on %s, for %s and this machine only\n", apiListen, strings.Join(allow, ", ")) default: @@ -317,6 +327,8 @@ func newMirrorServeCmd(dir *string) *cobra.Command { "the address nodes use to reach this mirror, as it should appear in catalog sources") c.Flags().StringArrayVar(&allow, "api-allow", nil, "an address or network the management API answers, besides this machine; repeatable (default: this machine only)") + c.Flags().StringArrayVar(&clients, "api-client", nil, + "with mutual TLS, the common name of a certificate the management API answers; repeatable (default: any from --tls-ca)") c.Flags().BoolVar(&allowPrivate, "allow-private-sources", false, "let downloads reach loopback, private and link-local addresses") c.Flags().StringVar(&tlsc.CA, "tls-ca", "", "CA bundle that management clients must present a certificate from") @@ -452,6 +464,23 @@ func apiGate(mtls bool, allowed []netip.Prefix, next http.Handler) http.Handler return onlyFrom(allowed, next) } +// onlyClients answers only certificates with one of these common names, when +// any are given. The handshake has already checked the certificate against +// the root; this narrows which of the root's certificates may manage. +func onlyClients(names []string, next http.Handler) http.Handler { + if len(names) == 0 { + return next + } + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.TLS == nil || len(r.TLS.VerifiedChains) == 0 || + !slices.Contains(names, r.TLS.VerifiedChains[0][0].Subject.CommonName) { + http.Error(w, "this mirror is managed by its control plane only", http.StatusForbidden) + return + } + next.ServeHTTP(w, r) + }) +} + // onlyFrom answers only this machine and the addresses allowed: the management // API can delete models and start downloads, and a mirror on its own machine // is reached over the network by one control plane and no one else. diff --git a/internal/cli/mirror_gate_test.go b/internal/cli/mirror_gate_test.go index 746d9ee..768cf56 100644 --- a/internal/cli/mirror_gate_test.go +++ b/internal/cli/mirror_gate_test.go @@ -1,6 +1,9 @@ package cli import ( + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" "net/http" "net/http/httptest" "net/netip" @@ -37,3 +40,27 @@ func TestTheMirrorAPIAnswersOnlyThisMachineUnlessTold(t *testing.T) { t.Errorf("mTLS without an allowlist refused a client: %d", got) } } + +func TestOnlyANamedCertificateManagesTheMirror(t *testing.T) { + ok := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {}) + with := func(cn string) *http.Request { + r := httptest.NewRequest("GET", "/", nil) + if cn != "" { + r.TLS = &tls.ConnectionState{VerifiedChains: [][]*x509.Certificate{{{Subject: pkix.Name{CommonName: cn}}}}} + } + return r + } + h := onlyClients([]string{"control-plane"}, ok) + for cn, want := range map[string]int{"control-plane": 200, "node01": 403, "": 403} { + w := httptest.NewRecorder() + h.ServeHTTP(w, with(cn)) + if w.Code != want { + t.Errorf("%q: %d, expected %d", cn, w.Code, want) + } + } + w := httptest.NewRecorder() + onlyClients(nil, ok).ServeHTTP(w, with("node01")) + if w.Code != 200 { + t.Error("no names given narrowed it anyway") + } +} diff --git a/internal/cli/model.go b/internal/cli/model.go index 82bcf96..e0346a3 100644 --- a/internal/cli/model.go +++ b/internal/cli/model.go @@ -3,6 +3,7 @@ package cli import ( "encoding/json" "fmt" + "os" "os/user" "time" @@ -127,6 +128,10 @@ func newModelRemoveCmd(client func() *ctlClient) *cobra.Command { // actor names who made a change, for the audit trail on a stored model. func actor() string { + // Under sudo, the person who ran it rather than root. + if u := os.Getenv("SUDO_USER"); u != "" && os.Geteuid() == 0 { + return u + } if u, err := user.Current(); err == nil && u.Username != "" { return u.Username } diff --git a/internal/cli/server.go b/internal/cli/server.go index 91bbd0b..3a90420 100644 --- a/internal/cli/server.go +++ b/internal/cli/server.go @@ -35,6 +35,7 @@ import ( "gitea.mynodes.xyz/ilker/llmgw/internal/control/reconciler" "gitea.mynodes.xyz/ilker/llmgw/internal/hf" "gitea.mynodes.xyz/ilker/llmgw/internal/mirror" + "gitea.mynodes.xyz/ilker/llmgw/internal/obs" "gitea.mynodes.xyz/ilker/llmgw/internal/promq" "gitea.mynodes.xyz/ilker/llmgw/internal/router" storepkg "gitea.mynodes.xyz/ilker/llmgw/internal/store" @@ -90,6 +91,21 @@ func newServerCmd() *cobra.Command { "or give --endpoint for a router-only run") } + // Before the router, so its tracer is the configured one. + shutdownTracing, err := obs.SetupTracing(cmd.Context(), obs.TracingOptions{ + Endpoint: cfg.Tracing.Endpoint, SampleRatio: cfg.Tracing.SampleRatio, + Service: "llmgw-server", Version: version.Version, + }) + if err != nil { + return err + } + defer func() { + // Bounded: a collector that is gone must not hold up a shutdown. + sctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = shutdownTracing(sctx) + }() + rt, err := router.New(cfg, log) if err != nil { return err diff --git a/internal/cli/tui_init.go b/internal/cli/tui_init.go index df8c437..d1a3fdc 100644 --- a/internal/cli/tui_init.go +++ b/internal/cli/tui_init.go @@ -389,17 +389,22 @@ func (in *serverInit) runInitTUI(ctx context.Context, out io.Writer) error { return fmt.Errorf("stopped; run init again to finish what was started") } - // The whole run is kept, root's only, since it holds the password. On a - // failure it is printed too, for the lines the screen had no room for; - // otherwise the screen's last frame says it, and only what must outlive - // the screen is printed again: the password, and what to do next. + // The whole run is kept, root's only, with the password taken out: it is + // shown once, on the screen, and a file on disk would outlive that. On a + // failure the run is printed too, for the lines the screen had no room + // for; otherwise the screen's last frame says it, and only what must + // outlive the screen is printed again: the password, and what to do next. logMu.Lock() defer logMu.Unlock() whole := strings.Join(log, "\n") + "\n" + kept := whole + if fm.password != "" { + kept = strings.ReplaceAll(kept, fm.password, "[password shown once, not kept]") + } saved := "" if in.stateDir != "" { p := filepath.Join(in.stateDir, "init.log") - if err := os.WriteFile(p, []byte(whole), 0o600); err == nil { + if err := os.WriteFile(p, []byte(kept), 0o600); err == nil { saved = p } } diff --git a/internal/config/config.go b/internal/config/config.go index dc74f88..0272dab 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -67,6 +67,10 @@ type Server struct { // Monitoring is where the dashboard reads its history from. Monitoring Monitoring `koanf:"monitoring"` + // Tracing sends a trace of each inference request to an OpenTelemetry + // collector. Off unless an endpoint is set here or in + // OTEL_EXPORTER_OTLP_ENDPOINT. + Tracing Tracing `koanf:"tracing"` // Mirror points the control plane at a model mirror it can administer. // Empty means there is none, and /admin/mirror stays unmounted. @@ -120,6 +124,21 @@ type Monitoring struct { Timeout time.Duration `koanf:"timeout"` } +// Tracing is where request traces go, over OTLP/HTTP. +// +// One span per inference request, with admission, scheduling and each attempt +// at an engine beneath it, and the trace carried on to the engine in a +// traceparent header. The request's X-Request-Id is on the span, so a trace +// and `llmgw ctl request` name the same request. +type Tracing struct { + // Endpoint is the collector, for example http://otel.internal:4318. + // Empty falls back to OTEL_EXPORTER_OTLP_ENDPOINT, and to no tracing. + Endpoint string `koanf:"endpoint"` + // SampleRatio is the share of requests traced, 0 to 1, when the caller + // did not decide it with a sampled traceparent of its own. Zero takes 1. + SampleRatio float64 `koanf:"sample_ratio"` +} + // Mirror is a model mirror this control plane administers over its API. // // The mirror is a separate machine with a large disk, not part of the gateway, diff --git a/internal/control/adminapi/actor_test.go b/internal/control/adminapi/actor_test.go new file mode 100644 index 0000000..1452ce6 --- /dev/null +++ b/internal/control/adminapi/actor_test.go @@ -0,0 +1,24 @@ +package adminapi + +import ( + "net/http/httptest" + "strings" + "testing" +) + +func TestAClaimedActorIsOneShortName(t *testing.T) { + for claimed, want := range map[string]string{ + "ilker": "ilker", + "": "unknown", + "ilker\nlevel=ERROR msg=x": "ilkerlevelERRORmsgx", + strings.Repeat("a", 500): strings.Repeat("a", 64), + "ops@example.org": "ops@example.org", + "\x1b[31m": "31m", + } { + r := httptest.NewRequest("POST", "/", nil) + r.Header.Set("X-Llmgw-Actor", claimed) + if got := actorOf(r); got != want { + t.Errorf("%q: %q, expected %q", claimed, got, want) + } + } +} diff --git a/internal/control/adminapi/api.go b/internal/control/adminapi/api.go index 8aecaa3..3730ac0 100644 --- a/internal/control/adminapi/api.go +++ b/internal/control/adminapi/api.go @@ -17,6 +17,7 @@ import ( "slices" "strings" "time" + "unicode" "gitea.mynodes.xyz/ilker/llmgw/internal/agent/discovery" "gitea.mynodes.xyz/ilker/llmgw/internal/bench" @@ -685,7 +686,7 @@ func (a *API) putCapacity(w http.ResponseWriter, r *http.Request) { } if err := a.capacity.PutCapacity(r.Context(), store.StoredCapacity{ Model: c.Model, Profile: c.Profile, Measurement: body, - MeasuredAt: c.MeasuredAt, MeasuredBy: r.Header.Get("X-Llmgw-Actor"), + MeasuredAt: c.MeasuredAt, MeasuredBy: actorOf(r), }); err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return @@ -736,7 +737,7 @@ func (a *API) addModel(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "the model definition is not valid JSON: "+err.Error()) return } - if err := a.models.Add(r.Context(), m, r.Header.Get("X-Llmgw-Actor")); err != nil { + if err := a.models.Add(r.Context(), m, actorOf(r)); err != nil { // Every failure here is the definition being wrong, which is the // caller's to fix — a collision, a missing fallback, a profile that // cannot be placed. None of them are server faults. @@ -785,7 +786,7 @@ func (a *API) scaleModel(w http.ResponseWriter, r *http.Request) { name := r.PathValue("name") m, err := a.models.Scale(r.Context(), name, catalog.Scaling{Min: req.Min, Max: req.Max, IdleTimeout: req.IdleTimeout}, - r.Header.Get("X-Llmgw-Actor")) + actorOf(r)) if err != nil { // Like addModel: every refusal here is the request being wrong — a // model declared in configuration, one that was never added, a max @@ -1224,7 +1225,7 @@ func (a *API) serveModel(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "the request is not valid JSON: "+err.Error()) return } - req.By = r.Header.Get("X-Llmgw-Actor") + req.By = actorOf(r) job, err := a.serve.Start(req) if err != nil { writeError(w, http.StatusBadRequest, err.Error()) @@ -1245,3 +1246,24 @@ func (a *API) job(w http.ResponseWriter, r *http.Request) { } writeJSON(w, http.StatusOK, job) } + +// actorOf is who a change is recorded as made by: what the caller says, since +// the admin API answers only this machine and trusts whoever reaches it. Kept +// to a short printable name, so a claim cannot pass for more than one — no +// line breaks to forge a second log line, no length to bury the rest. +func actorOf(r *http.Request) string { + claimed := r.Header.Get("X-Llmgw-Actor") + var b strings.Builder + for _, c := range claimed { + if b.Len() >= 64 { + break + } + if unicode.IsLetter(c) || unicode.IsDigit(c) || strings.ContainsRune("._-@", c) { + b.WriteRune(c) + } + } + if b.Len() == 0 { + return "unknown" + } + return b.String() +} diff --git a/internal/control/reconciler/adopt.go b/internal/control/reconciler/adopt.go new file mode 100644 index 0000000..8a27c38 --- /dev/null +++ b/internal/control/reconciler/adopt.go @@ -0,0 +1,255 @@ +package reconciler + +import ( + "slices" + + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/supervisor" + "gitea.mynodes.xyz/ilker/llmgw/internal/control/inventory" +) + +// Adopt rebuilds replica state from shards an agent reports it is already +// running (plan §6.5). +// +// This is what makes a control plane restart survivable. Without it the +// reconciler sees an empty cluster, places duplicates, and the engines already +// running keep holding GPUs the inventory believes are free — so the duplicates +// fail to allocate, with an error naming the new process rather than the old. +// +// Shards whose model or profile is no longer in the catalog are returned for +// the caller to stop: nothing will ever claim them. +func (r *Reconciler) Adopt(nodeID string, shards []AdoptedShard) (unclaimed []AdoptedShard) { + defer r.forgetShardsTheAgentDoesNotHave(nodeID, shards) + + byReplica := map[string][]AdoptedShard{} + for _, s := range shards { + byReplica[s.ReplicaID] = append(byReplica[s.ReplicaID], s) + } + + r.mu.Lock() + defer r.mu.Unlock() + + for replicaID, group := range byReplica { + if existing, known := r.replicas[replicaID]; known { + // The other half of a replica we are still assembling. Merge rather + // than choose: these shards belong to the same replica as the ones + // already adopted, they are simply on a different node whose agent + // registered a moment later. Treating them as a competing claim is + // what tore multi-node replicas apart on every control plane + // restart — one half adopted and failed for being short, the other + // stopped as unclaimed when it finally arrived. + if !existing.AdoptedAt.IsZero() && len(existing.Shards) < existing.WantShards { + r.mergeAdopted(existing, group) + continue + } + // Usually this is an agent reconnecting rather than a control plane + // restart, and its status reports keep the state current. + // + // Not always. Replica ids are derived from the model name and a + // counter, so a control plane that restarts and places before the + // agents reconnect produces a *different* replica under the same + // id, on a freshly allocated port. Skipping the adoption then + // leaves the cluster pointing at a port nothing listens on, while + // the engine that is actually serving is treated as a stray. + // + // The fleet is authoritative about what is running (plan §6.5), so + // what we merely placed loses to what an agent reports — unless our + // copy has a shard that has actually come up, in which case two + // things really are running and the one we did not ask for is the + // stray. + if existing.hasLiveShard() { + // Only what we do not recognise. A reconnecting agent reports + // the shards we already know, and disowning those would tell it + // to stop the very replica that is serving. + known := map[string]struct{}{} + for _, sh := range existing.Shards { + known[sh.ID] = struct{}{} + } + var strays []AdoptedShard + for _, s := range group { + if _, ok := known[s.ShardID]; !ok { + strays = append(strays, s) + } + } + unclaimed = append(unclaimed, strays...) + r.reservePorts(strays) + continue + } + r.log.Warn("a running replica reported by the fleet replaces one we placed", + "replica", replicaID, "placed_endpoint", existing.Endpoint) + for _, s := range existing.Shards { + r.inv.Release(s.ID) + } + delete(r.replicas, replicaID) + } + + first := group[0] + m, ok := r.cat.Load().Resolve(first.Model) + if !ok { + unclaimed = append(unclaimed, group...) + r.reservePorts(group) + continue + } + prof, ok := m.ProfileByName(first.Profile) + if !ok { + unclaimed = append(unclaimed, group...) + r.reservePorts(group) + continue + } + + rep := &Replica{ + ID: replicaID, Model: m.Name, Profile: prof.Name, + Fabric: first.Fabric, Pool: first.Pool, + // From the agent, which was told it when the shard started. Empty + // from an agent too old to report it, which reads as unknown. + ProfileFP: first.ProfileFP, + Weight: prof.Weight, CreatedAt: r.now(), + Endpoint: first.Endpoint, + // From the profile, not from what was reported: the point is to + // notice when what was reported is short. + WantShards: max(prof.Nodes, 1), + AdoptedAt: r.now(), + } + r.mergeAdopted(rep, group) + r.replicas[replicaID] = rep + + } + return unclaimed +} + +// mergeAdopted folds shards an agent reports into a replica being reassembled. +// +// Called both for the first group, which creates the replica, and for every +// later group belonging to it, so a replica spanning N nodes is built up over N +// registrations. Shard ids already present are skipped: an agent that +// reconnects repeats what it is running, and counting those again would carry a +// replica past its shard count without a second node ever appearing. +func (r *Reconciler) mergeAdopted(rep *Replica, group []AdoptedShard) { + have := make(map[string]struct{}, len(rep.Shards)) + for _, sh := range rep.Shards { + have[sh.ID] = struct{}{} + } + for _, s := range group { + if _, dup := have[s.ShardID]; dup { + continue + } + have[s.ShardID] = struct{}{} + rep.Shards = append(rep.Shards, Shard{ + ID: s.ShardID, NodeID: s.NodeID, Rank: s.Rank, + GPUIndices: s.GPUIndices, HCAs: s.HCAs, + HTTPPort: s.HTTPPort, State: s.State, + }) + // Reclaim the GPUs and the memory, or the next placement hands them + // out twice. The memory comes from the catalog rather than the agent: + // what a profile asked for is not something a running process reports. + r.inv.Allocate(s.NodeID, inventory.Allocation{ + ShardID: s.ShardID, ReplicaID: rep.ID, GPUIndices: s.GPUIndices, + MemoryGB: r.profileMemory(rep.Model, rep.Profile), + }) + // The endpoint comes from the leader, which may be in any of the + // groups — the rank 0 node is not necessarily the first to register. + if s.Rank == 0 && s.Endpoint != "" { + rep.Endpoint = s.Endpoint + } + } + slices.SortFunc(rep.Shards, func(a, b Shard) int { return a.Rank - b.Rank }) + rep.deriveState(r.now()) + + r.log.Info("adopted replica from the fleet", + "replica", rep.ID, "model", rep.Model, + "shards", len(rep.Shards), "want", rep.WantShards, "state", rep.State) +} + +// forgetShardsTheAgentDoesNotHave fails shards we believe are on a node that the +// agent, having just told us everything it is running, did not mention. +// +// The mirror image of adoption, and the half that was missing. Adoption handles +// "the agent has something we do not know about"; this handles "we think the +// agent has something it does not". Both follow from the same rule: the fleet is +// authoritative about what is running (plan §6.5). +// +// Without it, an engine that dies while its agent survives leaves the control +// plane believing a replica is Ready forever. Observed on a real cluster: the +// engine was killed, the agent restarted and registered with nothing running, +// and the replica stayed Ready pointing at a dead port for as long as anyone +// watched. Traffic survived — the breaker took the endpoint out — but the +// cluster ran a third short with no sign of it. +// +// Safe against the obvious race. A registration follows a fresh connection, so +// anything we dispatched over a previous one is not running by definition; +// there is no window where a shard is legitimately started but unreported. +func (r *Reconciler) forgetShardsTheAgentDoesNotHave(nodeID string, running []AdoptedShard) { + if nodeID == "" { + return + } + reported := make(map[string]struct{}, len(running)) + for _, s := range running { + reported[s.ShardID] = struct{}{} + } + + var lost []string + r.mu.Lock() + for _, rep := range r.replicas { + if rep.State == Stopped { + continue + } + changed := false + for i := range rep.Shards { + sh := &rep.Shards[i] + if sh.NodeID != nodeID || sh.State.Terminal() { + continue + } + if _, ok := reported[sh.ID]; ok { + continue + } + sh.State = supervisor.Failed + sh.Err = "the agent reports it is not running" + r.inv.Release(sh.ID) + changed = true + lost = append(lost, sh.ID) + } + if changed { + rep.deriveState(r.now()) + } + } + r.mu.Unlock() + + for _, id := range lost { + r.log.Warn("a shard we believed was running is not; the agent does not have it", + "node", nodeID, "shard", id) + } +} + +// reservePorts records ports held by shards nothing owns. +// +// They are being stopped, but stopping is not instant and the engine keeps its +// socket until it exits. Handing the port to a new shard in the meantime makes +// that shard fail to bind, which fails its replica, which places another one on +// the same port — a loop that does not end on its own. +// +// Called with the lock held. +func (r *Reconciler) reservePorts(shards []AdoptedShard) { + for _, s := range shards { + if s.HTTPPort > 0 { + r.reservedPorts[s.HTTPPort] = struct{}{} + } + } +} + +// AdoptedShard is one running shard an agent reported at registration. +type AdoptedShard struct { + NodeID string + ReplicaID string + ShardID string + Rank int + GPUIndices []int + HCAs []string + HTTPPort int + State supervisor.State + Model string + Profile string + Pool string + Fabric string + ProfileFP string + // Endpoint is the leader's base URL, set on the rank 0 shard. + Endpoint string +} diff --git a/internal/control/reconciler/demand.go b/internal/control/reconciler/demand.go new file mode 100644 index 0000000..5de5726 --- /dev/null +++ b/internal/control/reconciler/demand.go @@ -0,0 +1,214 @@ +package reconciler + +import ( + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/control/catalog" +) + +// RecordUsage stamps replicas with when each was last sent a request. +// +// Replicas absent from the map are left alone rather than zeroed: a router that +// has just restarted knows nothing about what happened before it, and treating +// that as "never used" would make every replica look abandoned at once — +// exactly when a fleet is least able to afford acting on that. +func (r *Reconciler) RecordUsage(lastUsed map[string]time.Time) { + if len(lastUsed) == 0 { + return + } + r.mu.Lock() + defer r.mu.Unlock() + for id, at := range lastUsed { + rep, ok := r.replicas[id] + if !ok { + continue + } + // Only forwards. Two routers reporting, or a stale snapshot arriving + // late, must not move a replica's last use backwards. + if at.After(rep.LastUsedAt) { + rep.LastUsedAt = at + } + } + r.noteUsage(lastUsed) +} + +// IdleFor reports how long a replica has gone without a request, and whether +// that is known at all. +// +// The second return is not a detail. "Idle for an hour" and "nobody has told me +// yet" are different states, and anything that acts on idleness — eviction, +// most obviously — has to refuse to act on the second. +func (r *Reconciler) IdleFor(id string) (time.Duration, bool) { + r.mu.Lock() + defer r.mu.Unlock() + rep, ok := r.replicas[id] + if !ok || rep.LastUsedAt.IsZero() { + return 0, false + } + return r.now().Sub(rep.LastUsedAt), true +} + +// Want marks an on-demand model as asked for, so the next pass places it. +// +// Called by whatever fields the request — the router, in practice. It returns +// whether the model is one this applies to, so a caller can tell "being loaded" +// from "there is genuinely nothing serving this", which are different answers +// to give a client. +func (r *Reconciler) Want(model string) bool { + m, ok := r.cat.Load().Resolve(model) + if !ok || !m.Scaling.OnDemand() { + return false + } + r.mu.Lock() + r.wanted[m.Name] = r.now() + // A placement that failed before this request says nothing about now: + // forgotten, so the pass this triggers decides afresh and Unservable + // does not answer from a stale result. + if res, ok := r.lastExplain[m.Name]; ok && res.Placement == nil { + delete(r.lastExplain, m.Name) + } + r.mu.Unlock() + // Immediately, not on the next tick. Someone is waiting on this request, + // and five seconds of ticker is five seconds they spend staring at nothing. + r.Notify() + return true +} + +// desiredFor is how many replicas a model should have right now. +// +// For an ordinary model that is its configured minimum. For an on-demand one it +// is one while somebody is asking and none once they stop, which is the whole +// feature: a cluster with more models than GPUs can hold every model in its +// catalog and only the wanted ones in its cards. +func (r *Reconciler) desiredFor(m catalog.Model) int { + base := r.baseFor(m) + if base == 0 { + return 0 + } + r.mu.Lock() + extra := r.extra[m.Name] + r.mu.Unlock() + return min(base+extra, max(m.Scaling.Max, base)) +} + +// baseFor is what a model runs without load: its minimum, or for an on-demand +// model one while somebody is asking and none once they stop. +func (r *Reconciler) baseFor(m catalog.Model) int { + if !m.Scaling.OnDemand() { + return m.Scaling.Min + } + r.mu.Lock() + last, asked := r.wanted[m.Name] + r.mu.Unlock() + if !asked { + return 0 + } + if r.now().Sub(last) > r.idleTimeout(m) { + return 0 + } + return 1 +} + +// idleTimeout is how long an on-demand model stays up unasked-for. +func (r *Reconciler) idleTimeout(m catalog.Model) time.Duration { + if m.Scaling.IdleTimeout > 0 { + return m.Scaling.IdleTimeout + } + return r.cfg.IdleTimeout +} + +// noteUsage keeps an on-demand model alive while it is being used. +// +// Without this a model would be evicted exactly IdleTimeout after the request +// that loaded it, however busy it had been since — the timer would measure +// time since the first request rather than the last. +func (r *Reconciler) noteUsage(lastUsed map[string]time.Time) { + byModel := map[string]time.Time{} + for id, at := range lastUsed { + rep, ok := r.replicas[id] + if !ok { + continue + } + if at.After(byModel[rep.Model]) { + byModel[rep.Model] = at + } + } + for model, at := range byModel { + if cur, ok := r.wanted[model]; ok && at.After(cur) { + r.wanted[model] = at + } + } +} + +// Load is what a model's replicas are doing, as the router sees it. +type Load struct { + Replicas int + Running int + Waiting int +} + +// RecordLoad adds or takes away replicas between a model's min and max as its +// load says. +// +// Max was a ceiling nothing reached: every model ran exactly its minimum +// however long its queue. Now requests waiting at the engines — every slot of +// every replica busy — held for ScaleUpAfter add one replica, and a quiet spell +// of ScaleDownAfter takes one of those back. One step at a time, and not while +// the last one is still starting: on a small CPU fleet each replica is minutes +// of weights and gigabytes of memory, and a spike that is over by the time it +// loads should not leave three behind. +func (r *Reconciler) RecordLoad(loads map[string]Load) { + now := r.now() + for _, m := range r.cat.Load().Models() { + base := r.baseFor(m) + r.mu.Lock() + if base == 0 || m.Scaling.Max <= base { + delete(r.extra, m.Name) + delete(r.busySince, m.Name) + delete(r.quietSince, m.Name) + r.mu.Unlock() + continue + } + starting := false + for _, rep := range r.replicas { + if rep.Model == m.Name && (rep.State == Placing || rep.State == Starting) { + starting = true + } + } + l := loads[m.Name] + perReplica := float64(l.Waiting) / float64(max(l.Replicas, 1)) + extra := r.extra[m.Name] + switch { + case perReplica >= r.cfg.ScaleUpWaiting: + delete(r.quietSince, m.Name) + since, ok := r.busySince[m.Name] + if !ok { + r.busySince[m.Name] = now + break + } + if now.Sub(since) >= r.cfg.ScaleUpAfter && base+extra < m.Scaling.Max && !starting { + r.extra[m.Name] = extra + 1 + r.busySince[m.Name] = now + r.log.Info("scaling up for load", "model", m.Name, "replicas", base+extra+1, + "waiting_per_replica", perReplica, "for", now.Sub(since).Round(time.Second)) + } + case l.Waiting == 0 && extra > 0: + delete(r.busySince, m.Name) + since, ok := r.quietSince[m.Name] + if !ok { + r.quietSince[m.Name] = now + break + } + if now.Sub(since) >= r.cfg.ScaleDownAfter { + r.extra[m.Name] = extra - 1 + r.quietSince[m.Name] = now + r.log.Info("scaling down after a quiet spell", "model", m.Name, + "replicas", base+extra-1, "quiet_for", now.Sub(since).Round(time.Second)) + } + default: + delete(r.busySince, m.Name) + delete(r.quietSince, m.Name) + } + r.mu.Unlock() + } +} diff --git a/internal/control/reconciler/health.go b/internal/control/reconciler/health.go new file mode 100644 index 0000000..5cb145a --- /dev/null +++ b/internal/control/reconciler/health.go @@ -0,0 +1,151 @@ +package reconciler + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "strings" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/control/catalog" +) + +// parkRecord is why a model stopped being retried. +type parkRecord struct { + Since time.Time + Reason string + Failures int + // fp is the model's definition when it was parked; a different one + // unparks it. + fp string +} + +type placementNote struct { + text string + at time.Time +} + +// modelFingerprint identifies a model's definition, so a change to it can be +// told from a retry of the same thing. +func modelFingerprint(m catalog.Model) string { + b, _ := json.Marshal(m) + sum := sha256.Sum256(b) + return hex.EncodeToString(sum[:8]) +} + +// placementLogEvery is how often an unchanged placement failure is repeated. +const placementLogEvery = 10 * time.Minute + +// notePlacementFailure logs why a model could not be placed, when the reason +// is new or has not been said for a while. Said on every pass it was 250 +// identical lines in two days for one model, burying everything else. +func (r *Reconciler) notePlacementFailure(model, why string) { + r.mu.Lock() + last := r.placementLogged[model] + say := last.text != why || r.now().Sub(last.at) >= placementLogEvery + if say { + r.placementLogged[model] = placementNote{text: why, at: r.now()} + } + r.mu.Unlock() + if say { + r.log.Warn("no placement for model", "model", model, "reasons", why) + } +} + +// Retry clears a parked model and its backoff, so it is placed on the next +// pass. +func (r *Reconciler) Retry(model string) bool { + r.mu.Lock() + _, was := r.parked[model] + delete(r.parked, model) + r.attempts[model] = 0 + delete(r.nextAttempt, model) + r.mu.Unlock() + r.Notify() + return was +} + +// ModelHealth is how one model stands, for status and metrics. +type ModelHealth struct { + Model string + // Desired is how many replicas it should have now; 0 for a stopped model + // or an on-demand one nobody is asking for. + Desired int + Ready int + // Failures counts replicas that failed in a row. + Failures int + // Parked is set when Failures reached ParkAfter; Reason is the last error. + Parked bool + ParkedSince time.Time + Reason string + // Unplaceable is set when it is short of replicas and the last attempt to + // place one found nowhere; Placement says why. + Unplaceable bool + Placement string +} + +// ModelHealth reports every model in the catalog. +func (r *Reconciler) ModelHealth() []ModelHealth { + models := r.cat.Load().Models() + desired := map[string]int{} + for _, m := range models { + desired[m.Name] = r.desiredFor(m) + } + r.mu.Lock() + defer r.mu.Unlock() + ready, live := map[string]int{}, map[string]int{} + for _, rep := range r.replicas { + if rep.State == Ready { + ready[rep.Model]++ + } + if rep.State.Counts() { + live[rep.Model]++ + } + } + out := make([]ModelHealth, 0, len(models)) + for _, m := range models { + h := ModelHealth{Model: m.Name, Desired: desired[m.Name], Ready: ready[m.Name], + Failures: r.attempts[m.Name]} + if p, ok := r.parked[m.Name]; ok { + h.Parked, h.ParkedSince, h.Reason = true, p.Since, p.Reason + } + if res, ok := r.lastExplain[m.Name]; ok && res.Placement == nil && live[m.Name] < h.Desired { + h.Unplaceable, h.Placement = true, res.Explain() + } + out = append(out, h) + } + return out +} + +// Unservable says why a model cannot be served at all, so a request for it is +// told now rather than asked to retry something that will never come up. +func (r *Reconciler) Unservable(model string) (string, bool) { + m, ok := r.cat.Load().Resolve(model) + if !ok { + return "", false + } + r.mu.Lock() + defer r.mu.Unlock() + if p, ok := r.parked[m.Name]; ok { + return fmt.Sprintf("%s failed to start %d times in a row and is not being retried: %s", + m.Name, p.Failures, p.Reason), true + } + for _, rep := range r.replicas { + if rep.Model == m.Name && rep.State.Counts() { + return "", false // something is on its way + } + } + if res, ok := r.lastExplain[m.Name]; ok && res.Placement == nil { + return m.Name + " cannot be placed on any node: " + firstLine(res.Explain()), true + } + return "", false +} + +func firstLine(s string) string { + s = strings.TrimSpace(s) + if i := strings.IndexByte(s, '\n'); i >= 0 { + return strings.TrimSpace(s[:i]) + " (and others; see llmgw ctl explain-placement)" + } + return s +} diff --git a/internal/control/reconciler/reconciler.go b/internal/control/reconciler/reconciler.go index c0a3d3b..9f86bd6 100644 --- a/internal/control/reconciler/reconciler.go +++ b/internal/control/reconciler/reconciler.go @@ -3,15 +3,10 @@ package reconciler import ( "context" crypterand "crypto/rand" - "crypto/sha256" - "encoding/hex" - "encoding/json" - "errors" "fmt" "log/slog" "math/rand/v2" "slices" - "strings" "sync" "time" @@ -460,140 +455,6 @@ func (r *Reconciler) fleetHasSettled() bool { return settled } -// RecordUsage stamps replicas with when each was last sent a request. -// -// Replicas absent from the map are left alone rather than zeroed: a router that -// has just restarted knows nothing about what happened before it, and treating -// that as "never used" would make every replica look abandoned at once — -// exactly when a fleet is least able to afford acting on that. -func (r *Reconciler) RecordUsage(lastUsed map[string]time.Time) { - if len(lastUsed) == 0 { - return - } - r.mu.Lock() - defer r.mu.Unlock() - for id, at := range lastUsed { - rep, ok := r.replicas[id] - if !ok { - continue - } - // Only forwards. Two routers reporting, or a stale snapshot arriving - // late, must not move a replica's last use backwards. - if at.After(rep.LastUsedAt) { - rep.LastUsedAt = at - } - } - r.noteUsage(lastUsed) -} - -// IdleFor reports how long a replica has gone without a request, and whether -// that is known at all. -// -// The second return is not a detail. "Idle for an hour" and "nobody has told me -// yet" are different states, and anything that acts on idleness — eviction, -// most obviously — has to refuse to act on the second. -func (r *Reconciler) IdleFor(id string) (time.Duration, bool) { - r.mu.Lock() - defer r.mu.Unlock() - rep, ok := r.replicas[id] - if !ok || rep.LastUsedAt.IsZero() { - return 0, false - } - return r.now().Sub(rep.LastUsedAt), true -} - -// Want marks an on-demand model as asked for, so the next pass places it. -// -// Called by whatever fields the request — the router, in practice. It returns -// whether the model is one this applies to, so a caller can tell "being loaded" -// from "there is genuinely nothing serving this", which are different answers -// to give a client. -func (r *Reconciler) Want(model string) bool { - m, ok := r.cat.Load().Resolve(model) - if !ok || !m.Scaling.OnDemand() { - return false - } - r.mu.Lock() - r.wanted[m.Name] = r.now() - // A placement that failed before this request says nothing about now: - // forgotten, so the pass this triggers decides afresh and Unservable - // does not answer from a stale result. - if res, ok := r.lastExplain[m.Name]; ok && res.Placement == nil { - delete(r.lastExplain, m.Name) - } - r.mu.Unlock() - // Immediately, not on the next tick. Someone is waiting on this request, - // and five seconds of ticker is five seconds they spend staring at nothing. - r.Notify() - return true -} - -// desiredFor is how many replicas a model should have right now. -// -// For an ordinary model that is its configured minimum. For an on-demand one it -// is one while somebody is asking and none once they stop, which is the whole -// feature: a cluster with more models than GPUs can hold every model in its -// catalog and only the wanted ones in its cards. -func (r *Reconciler) desiredFor(m catalog.Model) int { - base := r.baseFor(m) - if base == 0 { - return 0 - } - r.mu.Lock() - extra := r.extra[m.Name] - r.mu.Unlock() - return min(base+extra, max(m.Scaling.Max, base)) -} - -// baseFor is what a model runs without load: its minimum, or for an on-demand -// model one while somebody is asking and none once they stop. -func (r *Reconciler) baseFor(m catalog.Model) int { - if !m.Scaling.OnDemand() { - return m.Scaling.Min - } - r.mu.Lock() - last, asked := r.wanted[m.Name] - r.mu.Unlock() - if !asked { - return 0 - } - if r.now().Sub(last) > r.idleTimeout(m) { - return 0 - } - return 1 -} - -// idleTimeout is how long an on-demand model stays up unasked-for. -func (r *Reconciler) idleTimeout(m catalog.Model) time.Duration { - if m.Scaling.IdleTimeout > 0 { - return m.Scaling.IdleTimeout - } - return r.cfg.IdleTimeout -} - -// noteUsage keeps an on-demand model alive while it is being used. -// -// Without this a model would be evicted exactly IdleTimeout after the request -// that loaded it, however busy it had been since — the timer would measure -// time since the first request rather than the last. -func (r *Reconciler) noteUsage(lastUsed map[string]time.Time) { - byModel := map[string]time.Time{} - for id, at := range lastUsed { - rep, ok := r.replicas[id] - if !ok { - continue - } - if at.After(byModel[rep.Model]) { - byModel[rep.Model] = at - } - } - for model, at := range byModel { - if cur, ok := r.wanted[model]; ok && at.After(cur) { - r.wanted[model] = at - } - } -} - // expireStuckReplicas fails replicas that never finished coming up. // // Every other path out of Starting depends on the agent reporting something. A @@ -884,110 +745,6 @@ func (r *Reconciler) reconcileModel(ctx context.Context, m catalog.Model) { r.retire(ctx, retiringIDs, readyLive, len(liveIDs), desired) } -// retire finishes restarting replicas. -// -// A retiring replica keeps serving until its replacement is ready, and is then -// drained like any other. When no replacement could be placed at all — -// nowhere else has the memory, which on a small CPU fleet is the usual case — -// it is stopped after DrainGrace so its replacement can take its place: a -// restart in place, with the model unserved while it loads if it had no other -// replica. Said in the log, since that is the one case with an outage. -func (r *Reconciler) retire(ctx context.Context, ids []string, readyLive, live, desired int) { - for _, id := range ids { - r.mu.Lock() - rep, ok := r.replicas[id] - var since time.Time - if ok { - since = rep.RetiringAt - } - r.mu.Unlock() - if !ok { - continue - } - switch { - case readyLive >= desired: - r.drainForRestart(ctx, id) - case live < desired && r.now().Sub(since) > r.cfg.DrainGrace: - r.log.Warn("restarting a replica in place: no room for its replacement beside it", - "replica", id, "after", r.cfg.DrainGrace) - r.drainForRestart(ctx, id) - r.stopReplica(ctx, id) - } - } -} - -func (r *Reconciler) drainForRestart(ctx context.Context, id string) { - r.mu.Lock() - rep, ok := r.replicas[id] - var shards []Shard - if ok { - rep.State, rep.DrainingAt, rep.RetiringAt = Draining, r.now(), time.Time{} - shards = slices.Clone(rep.Shards) - } - r.mu.Unlock() - for _, s := range shards { - if err := r.disp.DrainShard(ctx, s.NodeID, s.ID); err != nil { - r.log.Warn("draining a shard failed", "shard", s.ID, "err", err) - } - } -} - -// nextRestartLocked pops the next queued replica of a model that is still -// ready to be restarted. Called with mu held. -func (r *Reconciler) nextRestartLocked(model string) (string, bool) { - q := r.restartQueue[model] - for len(q) > 0 { - id := q[0] - q = q[1:] - if rep, ok := r.replicas[id]; ok && rep.State == Ready && rep.RetiringAt.IsZero() { - r.restartQueue[model] = q - return id, true - } - } - delete(r.restartQueue, model) - return "", false -} - -// ErrNoSuchReplica means no ready replica has that id. -var ErrNoSuchReplica = errors.New("no ready replica with that id") - -// Restart queues replicas to be replaced, each by a new one started beside it -// before it is stopped — to pick up an edited profile, an engine key, a new -// engine version. Replicas of one model are replaced one at a time. -func (r *Reconciler) Restart(ids ...string) error { - r.mu.Lock() - for _, id := range ids { - rep, ok := r.replicas[id] - if !ok || rep.State != Ready { - r.mu.Unlock() - return fmt.Errorf("%w: %s", ErrNoSuchReplica, id) - } - if !slices.Contains(r.restartQueue[rep.Model], id) && rep.RetiringAt.IsZero() { - r.restartQueue[rep.Model] = append(r.restartQueue[rep.Model], id) - } - } - r.mu.Unlock() - r.Notify() - return nil -} - -// RestartState says where a replica's restart stands: "queued", "replacing", -// or "" when none was asked for. -func (r *Reconciler) RestartState(id string) string { - r.mu.Lock() - defer r.mu.Unlock() - rep, ok := r.replicas[id] - switch { - case !ok: - return "" - case !rep.RetiringAt.IsZero(): - return "replacing" - case slices.Contains(r.restartQueue[rep.Model], id): - return "queued" - } - return "" -} - // scaleUp places and starts one replica per pass. // // One at a time on purpose: placement decisions depend on what is already @@ -1229,253 +986,6 @@ func (r *Reconciler) stopReplica(ctx context.Context, id string) { r.mu.Unlock() } -// Adopt rebuilds replica state from shards an agent reports it is already -// running (plan §6.5). -// -// This is what makes a control plane restart survivable. Without it the -// reconciler sees an empty cluster, places duplicates, and the engines already -// running keep holding GPUs the inventory believes are free — so the duplicates -// fail to allocate, with an error naming the new process rather than the old. -// -// Shards whose model or profile is no longer in the catalog are returned for -// the caller to stop: nothing will ever claim them. -func (r *Reconciler) Adopt(nodeID string, shards []AdoptedShard) (unclaimed []AdoptedShard) { - defer r.forgetShardsTheAgentDoesNotHave(nodeID, shards) - - byReplica := map[string][]AdoptedShard{} - for _, s := range shards { - byReplica[s.ReplicaID] = append(byReplica[s.ReplicaID], s) - } - - r.mu.Lock() - defer r.mu.Unlock() - - for replicaID, group := range byReplica { - if existing, known := r.replicas[replicaID]; known { - // The other half of a replica we are still assembling. Merge rather - // than choose: these shards belong to the same replica as the ones - // already adopted, they are simply on a different node whose agent - // registered a moment later. Treating them as a competing claim is - // what tore multi-node replicas apart on every control plane - // restart — one half adopted and failed for being short, the other - // stopped as unclaimed when it finally arrived. - if !existing.AdoptedAt.IsZero() && len(existing.Shards) < existing.WantShards { - r.mergeAdopted(existing, group) - continue - } - // Usually this is an agent reconnecting rather than a control plane - // restart, and its status reports keep the state current. - // - // Not always. Replica ids are derived from the model name and a - // counter, so a control plane that restarts and places before the - // agents reconnect produces a *different* replica under the same - // id, on a freshly allocated port. Skipping the adoption then - // leaves the cluster pointing at a port nothing listens on, while - // the engine that is actually serving is treated as a stray. - // - // The fleet is authoritative about what is running (plan §6.5), so - // what we merely placed loses to what an agent reports — unless our - // copy has a shard that has actually come up, in which case two - // things really are running and the one we did not ask for is the - // stray. - if existing.hasLiveShard() { - // Only what we do not recognise. A reconnecting agent reports - // the shards we already know, and disowning those would tell it - // to stop the very replica that is serving. - known := map[string]struct{}{} - for _, sh := range existing.Shards { - known[sh.ID] = struct{}{} - } - var strays []AdoptedShard - for _, s := range group { - if _, ok := known[s.ShardID]; !ok { - strays = append(strays, s) - } - } - unclaimed = append(unclaimed, strays...) - r.reservePorts(strays) - continue - } - r.log.Warn("a running replica reported by the fleet replaces one we placed", - "replica", replicaID, "placed_endpoint", existing.Endpoint) - for _, s := range existing.Shards { - r.inv.Release(s.ID) - } - delete(r.replicas, replicaID) - } - - first := group[0] - m, ok := r.cat.Load().Resolve(first.Model) - if !ok { - unclaimed = append(unclaimed, group...) - r.reservePorts(group) - continue - } - prof, ok := m.ProfileByName(first.Profile) - if !ok { - unclaimed = append(unclaimed, group...) - r.reservePorts(group) - continue - } - - rep := &Replica{ - ID: replicaID, Model: m.Name, Profile: prof.Name, - Fabric: first.Fabric, Pool: first.Pool, - // From the agent, which was told it when the shard started. Empty - // from an agent too old to report it, which reads as unknown. - ProfileFP: first.ProfileFP, - Weight: prof.Weight, CreatedAt: r.now(), - Endpoint: first.Endpoint, - // From the profile, not from what was reported: the point is to - // notice when what was reported is short. - WantShards: max(prof.Nodes, 1), - AdoptedAt: r.now(), - } - r.mergeAdopted(rep, group) - r.replicas[replicaID] = rep - - } - return unclaimed -} - -// mergeAdopted folds shards an agent reports into a replica being reassembled. -// -// Called both for the first group, which creates the replica, and for every -// later group belonging to it, so a replica spanning N nodes is built up over N -// registrations. Shard ids already present are skipped: an agent that -// reconnects repeats what it is running, and counting those again would carry a -// replica past its shard count without a second node ever appearing. -func (r *Reconciler) mergeAdopted(rep *Replica, group []AdoptedShard) { - have := make(map[string]struct{}, len(rep.Shards)) - for _, sh := range rep.Shards { - have[sh.ID] = struct{}{} - } - for _, s := range group { - if _, dup := have[s.ShardID]; dup { - continue - } - have[s.ShardID] = struct{}{} - rep.Shards = append(rep.Shards, Shard{ - ID: s.ShardID, NodeID: s.NodeID, Rank: s.Rank, - GPUIndices: s.GPUIndices, HCAs: s.HCAs, - HTTPPort: s.HTTPPort, State: s.State, - }) - // Reclaim the GPUs and the memory, or the next placement hands them - // out twice. The memory comes from the catalog rather than the agent: - // what a profile asked for is not something a running process reports. - r.inv.Allocate(s.NodeID, inventory.Allocation{ - ShardID: s.ShardID, ReplicaID: rep.ID, GPUIndices: s.GPUIndices, - MemoryGB: r.profileMemory(rep.Model, rep.Profile), - }) - // The endpoint comes from the leader, which may be in any of the - // groups — the rank 0 node is not necessarily the first to register. - if s.Rank == 0 && s.Endpoint != "" { - rep.Endpoint = s.Endpoint - } - } - slices.SortFunc(rep.Shards, func(a, b Shard) int { return a.Rank - b.Rank }) - rep.deriveState(r.now()) - - r.log.Info("adopted replica from the fleet", - "replica", rep.ID, "model", rep.Model, - "shards", len(rep.Shards), "want", rep.WantShards, "state", rep.State) -} - -// forgetShardsTheAgentDoesNotHave fails shards we believe are on a node that the -// agent, having just told us everything it is running, did not mention. -// -// The mirror image of adoption, and the half that was missing. Adoption handles -// "the agent has something we do not know about"; this handles "we think the -// agent has something it does not". Both follow from the same rule: the fleet is -// authoritative about what is running (plan §6.5). -// -// Without it, an engine that dies while its agent survives leaves the control -// plane believing a replica is Ready forever. Observed on a real cluster: the -// engine was killed, the agent restarted and registered with nothing running, -// and the replica stayed Ready pointing at a dead port for as long as anyone -// watched. Traffic survived — the breaker took the endpoint out — but the -// cluster ran a third short with no sign of it. -// -// Safe against the obvious race. A registration follows a fresh connection, so -// anything we dispatched over a previous one is not running by definition; -// there is no window where a shard is legitimately started but unreported. -func (r *Reconciler) forgetShardsTheAgentDoesNotHave(nodeID string, running []AdoptedShard) { - if nodeID == "" { - return - } - reported := make(map[string]struct{}, len(running)) - for _, s := range running { - reported[s.ShardID] = struct{}{} - } - - var lost []string - r.mu.Lock() - for _, rep := range r.replicas { - if rep.State == Stopped { - continue - } - changed := false - for i := range rep.Shards { - sh := &rep.Shards[i] - if sh.NodeID != nodeID || sh.State.Terminal() { - continue - } - if _, ok := reported[sh.ID]; ok { - continue - } - sh.State = supervisor.Failed - sh.Err = "the agent reports it is not running" - r.inv.Release(sh.ID) - changed = true - lost = append(lost, sh.ID) - } - if changed { - rep.deriveState(r.now()) - } - } - r.mu.Unlock() - - for _, id := range lost { - r.log.Warn("a shard we believed was running is not; the agent does not have it", - "node", nodeID, "shard", id) - } -} - -// reservePorts records ports held by shards nothing owns. -// -// They are being stopped, but stopping is not instant and the engine keeps its -// socket until it exits. Handing the port to a new shard in the meantime makes -// that shard fail to bind, which fails its replica, which places another one on -// the same port — a loop that does not end on its own. -// -// Called with the lock held. -func (r *Reconciler) reservePorts(shards []AdoptedShard) { - for _, s := range shards { - if s.HTTPPort > 0 { - r.reservedPorts[s.HTTPPort] = struct{}{} - } - } -} - -// AdoptedShard is one running shard an agent reported at registration. -type AdoptedShard struct { - NodeID string - ReplicaID string - ShardID string - Rank int - GPUIndices []int - HCAs []string - HTTPPort int - State supervisor.State - Model string - Profile string - Pool string - Fabric string - ProfileFP string - // Endpoint is the leader's base URL, set on the rank 0 shard. - Endpoint string -} - // OnFetchProgress records how far a shard's download has got. // // Separate from OnShardStatus because it is not a transition: the shard is @@ -1616,215 +1126,3 @@ func (r *Reconciler) engineKey(replicaID string) string { } return r.cfg.EngineKey(replicaID) } - -// parkRecord is why a model stopped being retried. -type parkRecord struct { - Since time.Time - Reason string - Failures int - // fp is the model's definition when it was parked; a different one - // unparks it. - fp string -} - -type placementNote struct { - text string - at time.Time -} - -// modelFingerprint identifies a model's definition, so a change to it can be -// told from a retry of the same thing. -func modelFingerprint(m catalog.Model) string { - b, _ := json.Marshal(m) - sum := sha256.Sum256(b) - return hex.EncodeToString(sum[:8]) -} - -// placementLogEvery is how often an unchanged placement failure is repeated. -const placementLogEvery = 10 * time.Minute - -// notePlacementFailure logs why a model could not be placed, when the reason -// is new or has not been said for a while. Said on every pass it was 250 -// identical lines in two days for one model, burying everything else. -func (r *Reconciler) notePlacementFailure(model, why string) { - r.mu.Lock() - last := r.placementLogged[model] - say := last.text != why || r.now().Sub(last.at) >= placementLogEvery - if say { - r.placementLogged[model] = placementNote{text: why, at: r.now()} - } - r.mu.Unlock() - if say { - r.log.Warn("no placement for model", "model", model, "reasons", why) - } -} - -// Retry clears a parked model and its backoff, so it is placed on the next -// pass. -func (r *Reconciler) Retry(model string) bool { - r.mu.Lock() - _, was := r.parked[model] - delete(r.parked, model) - r.attempts[model] = 0 - delete(r.nextAttempt, model) - r.mu.Unlock() - r.Notify() - return was -} - -// ModelHealth is how one model stands, for status and metrics. -type ModelHealth struct { - Model string - // Desired is how many replicas it should have now; 0 for a stopped model - // or an on-demand one nobody is asking for. - Desired int - Ready int - // Failures counts replicas that failed in a row. - Failures int - // Parked is set when Failures reached ParkAfter; Reason is the last error. - Parked bool - ParkedSince time.Time - Reason string - // Unplaceable is set when it is short of replicas and the last attempt to - // place one found nowhere; Placement says why. - Unplaceable bool - Placement string -} - -// ModelHealth reports every model in the catalog. -func (r *Reconciler) ModelHealth() []ModelHealth { - models := r.cat.Load().Models() - desired := map[string]int{} - for _, m := range models { - desired[m.Name] = r.desiredFor(m) - } - r.mu.Lock() - defer r.mu.Unlock() - ready, live := map[string]int{}, map[string]int{} - for _, rep := range r.replicas { - if rep.State == Ready { - ready[rep.Model]++ - } - if rep.State.Counts() { - live[rep.Model]++ - } - } - out := make([]ModelHealth, 0, len(models)) - for _, m := range models { - h := ModelHealth{Model: m.Name, Desired: desired[m.Name], Ready: ready[m.Name], - Failures: r.attempts[m.Name]} - if p, ok := r.parked[m.Name]; ok { - h.Parked, h.ParkedSince, h.Reason = true, p.Since, p.Reason - } - if res, ok := r.lastExplain[m.Name]; ok && res.Placement == nil && live[m.Name] < h.Desired { - h.Unplaceable, h.Placement = true, res.Explain() - } - out = append(out, h) - } - return out -} - -// Unservable says why a model cannot be served at all, so a request for it is -// told now rather than asked to retry something that will never come up. -func (r *Reconciler) Unservable(model string) (string, bool) { - m, ok := r.cat.Load().Resolve(model) - if !ok { - return "", false - } - r.mu.Lock() - defer r.mu.Unlock() - if p, ok := r.parked[m.Name]; ok { - return fmt.Sprintf("%s failed to start %d times in a row and is not being retried: %s", - m.Name, p.Failures, p.Reason), true - } - for _, rep := range r.replicas { - if rep.Model == m.Name && rep.State.Counts() { - return "", false // something is on its way - } - } - if res, ok := r.lastExplain[m.Name]; ok && res.Placement == nil { - return m.Name + " cannot be placed on any node: " + firstLine(res.Explain()), true - } - return "", false -} - -func firstLine(s string) string { - s = strings.TrimSpace(s) - if i := strings.IndexByte(s, '\n'); i >= 0 { - return strings.TrimSpace(s[:i]) + " (and others; see llmgw ctl explain-placement)" - } - return s -} - -// Load is what a model's replicas are doing, as the router sees it. -type Load struct { - Replicas int - Running int - Waiting int -} - -// RecordLoad adds or takes away replicas between a model's min and max as its -// load says. -// -// Max was a ceiling nothing reached: every model ran exactly its minimum -// however long its queue. Now requests waiting at the engines — every slot of -// every replica busy — held for ScaleUpAfter add one replica, and a quiet spell -// of ScaleDownAfter takes one of those back. One step at a time, and not while -// the last one is still starting: on a small CPU fleet each replica is minutes -// of weights and gigabytes of memory, and a spike that is over by the time it -// loads should not leave three behind. -func (r *Reconciler) RecordLoad(loads map[string]Load) { - now := r.now() - for _, m := range r.cat.Load().Models() { - base := r.baseFor(m) - r.mu.Lock() - if base == 0 || m.Scaling.Max <= base { - delete(r.extra, m.Name) - delete(r.busySince, m.Name) - delete(r.quietSince, m.Name) - r.mu.Unlock() - continue - } - starting := false - for _, rep := range r.replicas { - if rep.Model == m.Name && (rep.State == Placing || rep.State == Starting) { - starting = true - } - } - l := loads[m.Name] - perReplica := float64(l.Waiting) / float64(max(l.Replicas, 1)) - extra := r.extra[m.Name] - switch { - case perReplica >= r.cfg.ScaleUpWaiting: - delete(r.quietSince, m.Name) - since, ok := r.busySince[m.Name] - if !ok { - r.busySince[m.Name] = now - break - } - if now.Sub(since) >= r.cfg.ScaleUpAfter && base+extra < m.Scaling.Max && !starting { - r.extra[m.Name] = extra + 1 - r.busySince[m.Name] = now - r.log.Info("scaling up for load", "model", m.Name, "replicas", base+extra+1, - "waiting_per_replica", perReplica, "for", now.Sub(since).Round(time.Second)) - } - case l.Waiting == 0 && extra > 0: - delete(r.busySince, m.Name) - since, ok := r.quietSince[m.Name] - if !ok { - r.quietSince[m.Name] = now - break - } - if now.Sub(since) >= r.cfg.ScaleDownAfter { - r.extra[m.Name] = extra - 1 - r.quietSince[m.Name] = now - r.log.Info("scaling down after a quiet spell", "model", m.Name, - "replicas", base+extra-1, "quiet_for", now.Sub(since).Round(time.Second)) - } - default: - delete(r.busySince, m.Name) - delete(r.quietSince, m.Name) - } - r.mu.Unlock() - } -} diff --git a/internal/control/reconciler/restart.go b/internal/control/reconciler/restart.go new file mode 100644 index 0000000..6ff7e91 --- /dev/null +++ b/internal/control/reconciler/restart.go @@ -0,0 +1,113 @@ +package reconciler + +import ( + "context" + "errors" + "fmt" + "slices" + "time" +) + +// retire finishes restarting replicas. +// +// A retiring replica keeps serving until its replacement is ready, and is then +// drained like any other. When no replacement could be placed at all — +// nowhere else has the memory, which on a small CPU fleet is the usual case — +// it is stopped after DrainGrace so its replacement can take its place: a +// restart in place, with the model unserved while it loads if it had no other +// replica. Said in the log, since that is the one case with an outage. +func (r *Reconciler) retire(ctx context.Context, ids []string, readyLive, live, desired int) { + for _, id := range ids { + r.mu.Lock() + rep, ok := r.replicas[id] + var since time.Time + if ok { + since = rep.RetiringAt + } + r.mu.Unlock() + if !ok { + continue + } + switch { + case readyLive >= desired: + r.drainForRestart(ctx, id) + case live < desired && r.now().Sub(since) > r.cfg.DrainGrace: + r.log.Warn("restarting a replica in place: no room for its replacement beside it", + "replica", id, "after", r.cfg.DrainGrace) + r.drainForRestart(ctx, id) + r.stopReplica(ctx, id) + } + } +} + +func (r *Reconciler) drainForRestart(ctx context.Context, id string) { + r.mu.Lock() + rep, ok := r.replicas[id] + var shards []Shard + if ok { + rep.State, rep.DrainingAt, rep.RetiringAt = Draining, r.now(), time.Time{} + shards = slices.Clone(rep.Shards) + } + r.mu.Unlock() + for _, s := range shards { + if err := r.disp.DrainShard(ctx, s.NodeID, s.ID); err != nil { + r.log.Warn("draining a shard failed", "shard", s.ID, "err", err) + } + } +} + +// nextRestartLocked pops the next queued replica of a model that is still +// ready to be restarted. Called with mu held. +func (r *Reconciler) nextRestartLocked(model string) (string, bool) { + q := r.restartQueue[model] + for len(q) > 0 { + id := q[0] + q = q[1:] + if rep, ok := r.replicas[id]; ok && rep.State == Ready && rep.RetiringAt.IsZero() { + r.restartQueue[model] = q + return id, true + } + } + delete(r.restartQueue, model) + return "", false +} + +// ErrNoSuchReplica means no ready replica has that id. +var ErrNoSuchReplica = errors.New("no ready replica with that id") + +// Restart queues replicas to be replaced, each by a new one started beside it +// before it is stopped — to pick up an edited profile, an engine key, a new +// engine version. Replicas of one model are replaced one at a time. +func (r *Reconciler) Restart(ids ...string) error { + r.mu.Lock() + for _, id := range ids { + rep, ok := r.replicas[id] + if !ok || rep.State != Ready { + r.mu.Unlock() + return fmt.Errorf("%w: %s", ErrNoSuchReplica, id) + } + if !slices.Contains(r.restartQueue[rep.Model], id) && rep.RetiringAt.IsZero() { + r.restartQueue[rep.Model] = append(r.restartQueue[rep.Model], id) + } + } + r.mu.Unlock() + r.Notify() + return nil +} + +// RestartState says where a replica's restart stands: "queued", "replacing", +// or "" when none was asked for. +func (r *Reconciler) RestartState(id string) string { + r.mu.Lock() + defer r.mu.Unlock() + rep, ok := r.replicas[id] + switch { + case !ok: + return "" + case !rep.RetiringAt.IsZero(): + return "replacing" + case slices.Contains(r.restartQueue[rep.Model], id): + return "queued" + } + return "" +} diff --git a/internal/docgen/render.go b/internal/docgen/render.go index 6d00117..063526b 100644 --- a/internal/docgen/render.go +++ b/internal/docgen/render.go @@ -14,7 +14,7 @@ import ( // a new configuration file is a failure here rather than a silent omission. var Sources = []string{ "internal/config/config.go", - "internal/agent/agent.go", + "internal/agent/config.go", "internal/agent/discovery/discover.go", "internal/agent/engines/engine.go", "internal/agent/engines/llamacpp.go", diff --git a/internal/obs/trace.go b/internal/obs/trace.go new file mode 100644 index 0000000..334e9be --- /dev/null +++ b/internal/obs/trace.go @@ -0,0 +1,72 @@ +package obs + +import ( + "context" + "fmt" + "net/url" + "os" + "time" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" + "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/sdk/resource" + sdktrace "go.opentelemetry.io/otel/sdk/trace" +) + +// TracingOptions says where traces go. +type TracingOptions struct { + Endpoint string // http(s)://host:4318; empty reads OTEL_EXPORTER_OTLP_ENDPOINT + SampleRatio float64 // of traces the caller did not already decide; zero takes 1 + Service string + Version string +} + +// SetupTracing installs the global tracer provider and the W3C propagator, +// and returns what flushes it on the way out. With no endpoint it installs +// nothing, and every span is the no-op one: tracing that is off costs a +// function call per span. +func SetupTracing(ctx context.Context, o TracingOptions) (func(context.Context) error, error) { + noop := func(context.Context) error { return nil } + endpoint := o.Endpoint + if endpoint == "" { + endpoint = os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT") + } + if endpoint == "" { + return noop, nil + } + u, err := url.Parse(endpoint) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { + return noop, fmt.Errorf("tracing endpoint %q is not an http(s) address", endpoint) + } + opts := []otlptracehttp.Option{otlptracehttp.WithEndpoint(u.Host)} + if u.Scheme == "http" { + opts = append(opts, otlptracehttp.WithInsecure()) + } + if u.Path != "" && u.Path != "/" { + opts = append(opts, otlptracehttp.WithURLPath(u.Path)) + } + exp, err := otlptracehttp.New(ctx, opts...) + if err != nil { + return noop, err + } + ratio := o.SampleRatio + if ratio <= 0 || ratio > 1 { + ratio = 1 + } + res := resource.NewSchemaless( + attribute.String("service.name", o.Service), + attribute.String("service.version", o.Version), + ) + tp := sdktrace.NewTracerProvider( + sdktrace.WithBatcher(exp, sdktrace.WithBatchTimeout(5*time.Second)), + sdktrace.WithResource(res), + // A caller that sent a sampled traceparent has already decided; the + // ratio is for requests that arrive without one. + sdktrace.WithSampler(sdktrace.ParentBased(sdktrace.TraceIDRatioBased(ratio))), + ) + otel.SetTracerProvider(tp) + otel.SetTextMapPropagator(propagation.TraceContext{}) + return tp.Shutdown, nil +} diff --git a/internal/obs/trace_test.go b/internal/obs/trace_test.go new file mode 100644 index 0000000..ea0da69 --- /dev/null +++ b/internal/obs/trace_test.go @@ -0,0 +1,19 @@ +package obs + +import ( + "context" + "testing" +) + +func TestTracingIsOffUntilGivenSomewhereToSend(t *testing.T) { + t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "") + stop, err := SetupTracing(context.Background(), TracingOptions{Service: "t"}) + if err != nil || stop(context.Background()) != nil { + t.Fatal(err) + } + for _, bad := range []string{"otel:4318", "ftp://otel:4318", "http://"} { + if _, err := SetupTracing(context.Background(), TracingOptions{Endpoint: bad}); err == nil { + t.Errorf("%q accepted", bad) + } + } +} diff --git a/internal/onboard/onboard.go b/internal/onboard/onboard.go index af615de..3ba5337 100644 --- a/internal/onboard/onboard.go +++ b/internal/onboard/onboard.go @@ -498,14 +498,17 @@ func (r *Runner) certificate(ctx context.Context, res *Result) error { return err } } - // Trusting the root too, so other tools on the machine do not treat the - // cluster's certificates as strangers. Best effort: either family's way. - _, _ = r.Remote.Run(ctx, `if [ -d /usr/local/share/ca-certificates ]; then - sudo install -m 0644 /etc/llmgw/ca.pem /usr/local/share/ca-certificates/llmgw-ca.crt && sudo update-ca-certificates >/dev/null 2>&1 -elif [ -d /etc/pki/ca-trust/source/anchors ]; then - sudo install -m 0644 /etc/llmgw/ca.pem /etc/pki/ca-trust/source/anchors/llmgw-ca.pem && sudo update-ca-trust >/dev/null 2>&1 + // The agent trusts the cluster's root through its own configuration only. + // Not system-wide: that root signs whatever the control plane asks it to, + // and every TLS client on the machine would then take it for any site. + // A node onboarded by an older version had it added; take it back out. + _, _ = r.Remote.Run(ctx, `if [ -f /usr/local/share/ca-certificates/llmgw-ca.crt ]; then + sudo rm -f /usr/local/share/ca-certificates/llmgw-ca.crt && sudo update-ca-certificates --fresh >/dev/null 2>&1 +fi +if [ -f /etc/pki/ca-trust/source/anchors/llmgw-ca.pem ]; then + sudo rm -f /etc/pki/ca-trust/source/anchors/llmgw-ca.pem && sudo update-ca-trust >/dev/null 2>&1 fi; true`, nil) - r.emit(StepCert, Done, "certificate for "+res.Node+" installed, the cluster's root trusted", "") + r.emit(StepCert, Done, "certificate for "+res.Node+" installed", "") return nil } diff --git a/internal/router/finish_test.go b/internal/router/finish_test.go new file mode 100644 index 0000000..93e47ec --- /dev/null +++ b/internal/router/finish_test.go @@ -0,0 +1,119 @@ +package router + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "testing" + "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/accounting" + "gitea.mynodes.xyz/ilker/llmgw/internal/router/admission" + "gitea.mynodes.xyz/ilker/llmgw/internal/router/proxy" +) + +// What a client is told for each way a dispatch can end. Each status is +// something a client acts on: 503 with Retry-After is "try again", 503 alone +// is "not here", 502 is "the engine failed", and a stream already under way +// cannot be given a status at all. +func TestEachWayADispatchEndsIsToldToTheClientAsItShouldBe(t *testing.T) { + rt := newTestRouter(t, baseConfig()) + for _, c := range []struct { + name string + res dispatchResult + status int + body bool // an error body was written + retryAfter bool + }{ + {"served", dispatchResult{outcome: proxy.Outcome{Status: 200, Wrote: true}}, 200, false, false}, + {"engine refused, relayed", dispatchResult{outcome: proxy.Outcome{Status: 400, Wrote: true}}, 400, false, false}, + {"client went away", dispatchResult{err: proxy.ErrClientGone}, 499, false, false}, + {"stream cut short", dispatchResult{outcome: proxy.Outcome{Status: 200, Wrote: true}, err: errors.New("eof")}, 200, false, false}, + {"stream cut before a status", dispatchResult{outcome: proxy.Outcome{Wrote: true}, err: errors.New("eof")}, 200, false, false}, + {"still loading", dispatchResult{err: fmt.Errorf("%w: retry shortly", errLoading)}, 503, true, true}, + {"nothing serves it", dispatchResult{err: fmt.Errorf("%w: no replica", errNoCapacity)}, 503, true, false}, + {"engine failed", dispatchResult{err: errors.New("connection reset")}, 502, true, false}, + } { + t.Run(c.name, func(t *testing.T) { + w := httptest.NewRecorder() + got := rt.finish(w, &parsedRequest{Model: "m"}, c.res, time.Now()) + if got != c.status { + t.Errorf("status %d, expected %d", got, c.status) + } + if c.body { + if w.Code != c.status { + t.Errorf("written %d, expected %d", w.Code, c.status) + } + var e struct { + Error struct { + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal(w.Body.Bytes(), &e); err != nil || e.Error.Message == "" { + t.Errorf("error body %q", w.Body.String()) + } + } else if w.Body.Len() != 0 { + t.Errorf("wrote %q over a response that was not ours to write", w.Body.String()) + } + if (w.Header().Get("Retry-After") != "") != c.retryAfter { + t.Errorf("Retry-After %q", w.Header().Get("Retry-After")) + } + }) + } +} + +func TestAnUpstreamStatusIsKeptWhenTheStreamBreaks(t *testing.T) { + rt := newTestRouter(t, baseConfig()) + w := httptest.NewRecorder() + res := dispatchResult{outcome: proxy.Outcome{Status: http.StatusTooManyRequests, Wrote: true}, err: errors.New("eof")} + if got := rt.finish(w, &parsedRequest{Model: "m"}, res, time.Now()); got != 429 { + t.Errorf("status %d", got) + } +} + +func TestARefusalBeforeDispatchSaysWhetherWaitingHelps(t *testing.T) { + rt := newTestRouter(t, baseConfig()) + for _, c := range []struct { + name string + refuse func(http.ResponseWriter) + status int + retryAfter string + }{ + {"queue full", func(w http.ResponseWriter) { + rt.rejectAdmission(w, &parsedRequest{Model: "m"}, admission.ErrQueueFull, time.Now()) + }, 503, ""}, + {"queued too long", func(w http.ResponseWriter) { + rt.rejectAdmission(w, &parsedRequest{Model: "m"}, admission.ErrQueueTimeout, time.Now()) + }, 504, ""}, + {"shutting down", func(w http.ResponseWriter) { + rt.rejectAdmission(w, &parsedRequest{Model: "m"}, admission.ErrClosed, time.Now()) + }, 503, ""}, + {"client left the queue", func(w http.ResponseWriter) { + rt.rejectAdmission(w, &parsedRequest{Model: "m"}, context.Canceled, time.Now()) + }, 200, ""}, // nothing written: nobody is listening + {"rate limited", func(w http.ResponseWriter) { + rt.rejectLimit(w, &parsedRequest{Model: "m"}, + &accounting.LimitError{Err: accounting.ErrRateLimited, RetryAfter: 3 * time.Second}, time.Now()) + }, 429, "3"}, + {"budget spent", func(w http.ResponseWriter) { + rt.rejectLimit(w, &parsedRequest{Model: "m"}, &accounting.LimitError{Err: accounting.ErrBudgetSpent}, time.Now()) + }, 429, ""}, + {"not a limit at all", func(w http.ResponseWriter) { + rt.rejectLimit(w, &parsedRequest{Model: "m"}, errors.New("store down"), time.Now()) + }, 500, ""}, + } { + t.Run(c.name, func(t *testing.T) { + w := httptest.NewRecorder() + c.refuse(w) + if w.Code != c.status { + t.Errorf("status %d, expected %d", w.Code, c.status) + } + if got := w.Header().Get("Retry-After"); got != c.retryAfter { + t.Errorf("Retry-After %q, expected %q", got, c.retryAfter) + } + }) + } +} diff --git a/internal/router/handlers.go b/internal/router/handlers.go index a1f4bca..be87b30 100644 --- a/internal/router/handlers.go +++ b/internal/router/handlers.go @@ -15,6 +15,9 @@ import ( "strings" "time" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "gitea.mynodes.xyz/ilker/llmgw/internal/accounting" "gitea.mynodes.xyz/ilker/llmgw/internal/auth" "gitea.mynodes.xyz/ilker/llmgw/internal/router/admission" @@ -288,9 +291,13 @@ func (r *Router) handleInference(path string) http.HandlerFunc { // somebody asks what happened to it. rid := requestID(req.Header.Get("X-Request-Id")) w.Header().Set("X-Request-Id", rid) + ctx, span := startInference(req, path, rid) + defer span.End() + req = req.WithContext(ctx) parsed, err := r.parse(req, path) if err == nil { + span.SetAttributes(attribute.String("llmgw.model", parsed.Model)) parsed.RequestID = rid if parsed.Header == nil { parsed.Header = http.Header{} @@ -355,7 +362,9 @@ func (r *Router) handleInference(path string) http.HandlerFunc { } // Step 5 of the lifecycle: "now?" before "where?" (plan §7.1). + _, aspan := tracer().Start(req.Context(), "admission") lease, err := r.admit(req.Context(), parsed) + endSpan(aspan, err) if err != nil { r.rejectAdmission(w, parsed, err, start) return @@ -381,7 +390,13 @@ func (r *Router) handleInference(path string) http.HandlerFunc { } res := r.dispatch(req.Context(), w, parsed) - r.finish(w, parsed, res, start) + status := r.finish(w, parsed, res, start) + span.SetAttributes(attribute.Int("http.response.status_code", status), + attribute.String("llmgw.served_model", res.model), + attribute.String("llmgw.endpoint", res.endpointID)) + if status >= 500 { + span.SetStatus(codes.Error, http.StatusText(status)) + } // After the client has been served. A conversation that failed to // record is a worse outcome than a slow reply, but only just, and the // caller has their answer either way. @@ -413,7 +428,8 @@ func (r *Router) permits(p auth.Principal, requested string) bool { } // finish records metrics and, when nothing was written, produces the error body. -func (r *Router) finish(w http.ResponseWriter, req *parsedRequest, res dispatchResult, start time.Time) { +// It returns the status the request ended with. +func (r *Router) finish(w http.ResponseWriter, req *parsedRequest, res dispatchResult, start time.Time) int { status := res.outcome.Status switch { case res.err == nil: @@ -447,6 +463,7 @@ func (r *Router) finish(w http.ResponseWriter, req *parsedRequest, res dispatchR r.m.observeRequest(r.metricModel(req.Model), res.pool, status, time.Since(start), res.outcome.Usage, res.outcome.TTFT) r.account(req, res, status, time.Since(start)) r.accessLog(req, res, status, time.Since(start)) + return status } // account settles a finished request against its key. diff --git a/internal/router/router.go b/internal/router/router.go index 92d393e..bba828f 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -9,22 +9,26 @@ import ( "context" "errors" "fmt" - "gitea.mynodes.xyz/ilker/llmgw/internal/control/reconciler" "log/slog" "math" + "net" "net/http" + "net/netip" "slices" "strings" "sync/atomic" "time" "github.com/prometheus/client_golang/prometheus" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "gitea.mynodes.xyz/ilker/llmgw/internal/account" "gitea.mynodes.xyz/ilker/llmgw/internal/accounting" "gitea.mynodes.xyz/ilker/llmgw/internal/auth" "gitea.mynodes.xyz/ilker/llmgw/internal/config" "gitea.mynodes.xyz/ilker/llmgw/internal/control/catalog" + "gitea.mynodes.xyz/ilker/llmgw/internal/control/reconciler" "gitea.mynodes.xyz/ilker/llmgw/internal/router/admission" "gitea.mynodes.xyz/ilker/llmgw/internal/router/breaker" "gitea.mynodes.xyz/ilker/llmgw/internal/router/endpoint" @@ -231,6 +235,11 @@ func New(cfg config.Server, log *slog.Logger) (*Router, error) { Timeout: cfg.Router.ScrapeTimeout, }, log) + if !cfg.Auth.Enabled && !loopbackOnly(cfg.Listen) { + log.Warn("auth is off and the API listens beyond this machine: anyone who can reach "+ + cfg.Listen+" can use every model; set auth.enabled, or listen on 127.0.0.1", + "listen", cfg.Listen) + } if cfg.Auth.Enabled { st, err := storepkg.Open(context.Background(), cfg.Auth.Store) if err != nil { @@ -682,7 +691,13 @@ func (r *Router) dispatchOne(ctx context.Context, w http.ResponseWriter, req *pa if len(candidates) == 0 { break } + _, sspan := tracer().Start(ctx, "schedule", trace.WithAttributes( + attribute.String("llmgw.model", model), attribute.Int("llmgw.candidates", len(candidates)))) dec, err := profile.Schedule(sctx, candidates) + if err == nil { + sspan.SetAttributes(attribute.String("llmgw.endpoint", dec.Endpoint.ID)) + } + endSpan(sspan, err) if err != nil { last = dispatchResult{err: fmt.Errorf("%w: %v", errNoCapacity, err)} break @@ -696,15 +711,20 @@ func (r *Router) dispatchOne(ctx context.Context, w http.ResponseWriter, req *pa continue } - out, perr := r.px.Do(ctx, w, proxy.Request{ + uctx, uspan := tracer().Start(ctx, "upstream", trace.WithSpanKind(trace.SpanKindClient), + trace.WithAttributes(attribute.String("llmgw.endpoint", dec.Endpoint.ID), + attribute.String("url.full", dec.Endpoint.URL+req.Path))) + out, perr := r.px.Do(uctx, w, proxy.Request{ URL: dec.Endpoint.URL + req.Path, APIKey: dec.Endpoint.APIKey, Body: req.UpstreamBody, - Header: req.Header, + Header: withTraceparent(uctx, req.Header), Stream: req.Stream, SuppressUsageChunk: req.SuppressUsageChunk, CaptureReply: req.captureBytes(), }) + uspan.SetAttributes(attribute.Int("http.response.status_code", out.Status)) + endSpan(uspan, perr) // A retryable failure is the endpoint's fault; a client disconnect is not. release(perr == nil || errors.Is(perr, proxy.ErrClientGone)) @@ -764,3 +784,17 @@ func personalLimits(p config.Personal) *storepkg.Limits { } return &l } + +// loopbackOnly reports whether a listen address is reachable from this +// machine alone. An empty host (":8080") is every interface. +func loopbackOnly(listen string) bool { + host, _, err := net.SplitHostPort(listen) + if err != nil { + return false + } + if host == "localhost" { + return true + } + a, err := netip.ParseAddr(host) + return err == nil && a.IsLoopback() +} diff --git a/internal/router/router_listen_test.go b/internal/router/router_listen_test.go new file mode 100644 index 0000000..3998b81 --- /dev/null +++ b/internal/router/router_listen_test.go @@ -0,0 +1,14 @@ +package router + +import "testing" + +func TestOnlyALoopbackListenCountsAsPrivate(t *testing.T) { + for addr, want := range map[string]bool{ + "127.0.0.1:8080": true, "[::1]:8080": true, "localhost:8080": true, + ":8080": false, "0.0.0.0:8080": false, "10.0.0.140:8080": false, "junk": false, + } { + if got := loopbackOnly(addr); got != want { + t.Errorf("%s: %v", addr, got) + } + } +} diff --git a/internal/router/router_test.go b/internal/router/router_test.go index b245539..e6ce981 100644 --- a/internal/router/router_test.go +++ b/internal/router/router_test.go @@ -36,6 +36,8 @@ type fakeEngine struct { status atomic.Int64 // last is the body of the last completion asked for. last atomic.Value + // traceparent is the one the last completion came with. + traceparent atomic.Value } func newFakeEngine(t *testing.T, model string) *fakeEngine { @@ -49,6 +51,7 @@ func newFakeEngine(t *testing.T, model string) *fakeEngine { }) mux.HandleFunc("POST /v1/chat/completions", func(w http.ResponseWriter, r *http.Request) { f.calls.Add(1) + f.traceparent.Store(r.Header.Get("traceparent")) if b, err := io.ReadAll(r.Body); err == nil { f.last.Store(b) } diff --git a/internal/router/trace.go b/internal/router/trace.go new file mode 100644 index 0000000..0992e54 --- /dev/null +++ b/internal/router/trace.go @@ -0,0 +1,51 @@ +package router + +import ( + "context" + "net/http" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/trace" +) + +// tracer is looked up per use rather than kept, so a provider installed after +// the router was built — as tests do — is the one spans go to. +func tracer() trace.Tracer { return otel.Tracer("gitea.mynodes.xyz/ilker/llmgw/internal/router") } + +// startInference opens the span for one inference request, continuing the +// caller's trace when it sent a traceparent. +func startInference(req *http.Request, path, requestID string) (context.Context, trace.Span) { + ctx := otel.GetTextMapPropagator().Extract(req.Context(), propagation.HeaderCarrier(req.Header)) + return tracer().Start(ctx, "inference "+path, + trace.WithSpanKind(trace.SpanKindServer), + trace.WithAttributes( + attribute.String("llmgw.request_id", requestID), + attribute.String("http.route", path), + )) +} + +// endSpan closes a span, marking it failed when err is set. +func endSpan(span trace.Span, err error) { + if err != nil { + span.RecordError(err) + span.SetStatus(codes.Error, err.Error()) + } + span.End() +} + +// withTraceparent is h with the current span's context in it, for the engine. +// A copy: the request's header is sent on every attempt, each its own span. +func withTraceparent(ctx context.Context, h http.Header) http.Header { + if !trace.SpanContextFromContext(ctx).IsValid() { + return h + } + out := h.Clone() + if out == nil { + out = http.Header{} + } + otel.GetTextMapPropagator().Inject(ctx, propagation.HeaderCarrier(out)) + return out +} diff --git a/internal/router/trace_test.go b/internal/router/trace_test.go new file mode 100644 index 0000000..302f600 --- /dev/null +++ b/internal/router/trace_test.go @@ -0,0 +1,83 @@ +package router + +import ( + "testing" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/propagation" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + "go.opentelemetry.io/otel/trace/noop" + + "gitea.mynodes.xyz/ilker/llmgw/internal/config" +) + +func TestARequestIsTracedIntoTheEngine(t *testing.T) { + rec := tracetest.NewSpanRecorder() + prevTP, prevProp := otel.GetTracerProvider(), otel.GetTextMapPropagator() + otel.SetTracerProvider(sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(rec))) + otel.SetTextMapPropagator(propagation.TraceContext{}) + t.Cleanup(func() { otel.SetTracerProvider(prevTP); otel.SetTextMapPropagator(prevProp) }) + + e := newFakeEngine(t, "m") + rt := newTestRouter(t, baseConfig(config.Endpoint{ID: "a", URL: e.srv.URL, Models: []string{"m"}})) + // The caller's own trace, continued rather than replaced. + const caller = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01" + res := post(t, rt.Handler(), map[string]any{"model": "m", "messages": []any{}}, + map[string]string{"traceparent": caller, "X-Request-Id": "req-1"}) + if res.Code != 200 { + t.Fatalf("status %d: %s", res.Code, res.Body.String()) + } + + spans := map[string]sdktrace.ReadOnlySpan{} + for _, s := range rec.Ended() { + spans[s.Name()] = s + } + root, ok := spans["inference /v1/chat/completions"] + if !ok { + t.Fatalf("no inference span among %v", spans) + } + if got := root.SpanContext().TraceID().String(); got != "4bf92f3577b34da6a3ce929d0e0e4736" { + t.Errorf("the caller's trace was not continued: %s", got) + } + attrs := map[string]string{} + for _, a := range root.Attributes() { + attrs[string(a.Key)] = a.Value.Emit() + } + if attrs["llmgw.request_id"] != "req-1" || attrs["llmgw.model"] != "m" || + attrs["http.response.status_code"] != "200" || attrs["llmgw.endpoint"] != "a" { + t.Errorf("inference span attributes %v", attrs) + } + for _, name := range []string{"admission", "schedule", "upstream"} { + s, ok := spans[name] + if !ok { + t.Errorf("no %s span", name) + continue + } + if s.Parent().SpanID() != root.SpanContext().SpanID() { + t.Errorf("%s is not under the request's span", name) + } + } + up := spans["upstream"] + tp, _ := e.traceparent.Load().(string) + if want := "00-" + up.SpanContext().TraceID().String() + "-" + up.SpanContext().SpanID().String() + "-01"; tp != want { + t.Errorf("the engine got traceparent %q, expected %q", tp, want) + } +} + +func TestWithoutTracingTheEngineGetsNoTraceparent(t *testing.T) { + // What an unconfigured process has. Set explicitly, since the default + // global delegates for good to the first real provider another test set. + prevTP, prevProp := otel.GetTracerProvider(), otel.GetTextMapPropagator() + otel.SetTracerProvider(noop.NewTracerProvider()) + otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator()) + t.Cleanup(func() { otel.SetTracerProvider(prevTP); otel.SetTextMapPropagator(prevProp) }) + e := newFakeEngine(t, "m") + rt := newTestRouter(t, baseConfig(config.Endpoint{ID: "a", URL: e.srv.URL, Models: []string{"m"}})) + if res := post(t, rt.Handler(), map[string]any{"model": "m", "messages": []any{}}, nil); res.Code != 200 { + t.Fatalf("status %d", res.Code) + } + if tp, _ := e.traceparent.Load().(string); tp != "" { + t.Errorf("traceparent %q sent with tracing off", tp) + } +} diff --git a/llmgw-plan.md b/llmgw-plan.md index 3414a01..c16f1c8 100644 --- a/llmgw-plan.md +++ b/llmgw-plan.md @@ -1349,10 +1349,9 @@ work_dir: /var/lib/llmgw-agent - [x] Reconciler: desired vs actual, replica lifecycle, node expiry, backoff - [x] `EndpointSource` wired from the reconciler to the router - [x] `ctl`: nodes, models, replicas, cordon, uncordon, drain, explain-placement -- [ ] `ctl logs`: needs a request/response pair on the agent stream, which the - protocol does not have. The failure path already carries an engine log - tail with every status report, so the common case is covered; what is - missing is reading a healthy shard's output on demand. +- [x] `ctl logs`: a request/response pair on the agent stream (LogsRequest, + LogsReply) reads a node's agent log and any shard's engine output on + demand, from the CLI and from the node's page under /admin. - **Exit criterion (met against a simulated agent fleet):** shutting down a node moves its replicas elsewhere while the router keeps serving. Measured in `test/e2e`: a node hosting a live replica is killed mid-traffic; the replica is @@ -1452,32 +1451,39 @@ The launch path is now built too: - [x] Model allowlists and key→band mapping. With auth on, the band comes from the key and the header is ignored — it was trusted only while there was no identity to consult, which let any client promote itself. -- [ ] Budget fallback -- [ ] Routing profiles from configuration plus the `x-llmgw-profile` header +- [x] Budget fallback +- [x] Routing profiles from configuration plus the `x-llmgw-profile` header - [x] Approximate radix tree (§7.5 tier 1), with its thresholds and the `llmgw_prefix_decisions_total` breakdown. Measured against sim-engines whose caches were too small for the working set: 22.5% → 81.7% cache hits. It does nothing when the working set fits in one replica's cache, which is worth knowing rather than assuming. -- [ ] Fallback models and pool spillover +- [x] Fallback models +- [ ] Pool spillover - [x] `ctl bench`: a load ramp that reports capacity as the last concurrency level meeting the SLO, plus the queue depth and KV usage observed there, so `weight`, `target_waiting` and `kv_ceiling` stop being guesses - [ ] `ctl plan --target-rps N`, which needs capacity profiles stored rather than printed -- [ ] OTel tracing +- [x] OTel tracing: an inference span with admission, schedule and each + upstream attempt under it, over OTLP/HTTP when `tracing.endpoint` is + set, and the traceparent carried on to the engine - [x] systemd units and `docs/onboarding.md` - [x] Release binaries: static linux/amd64, linux/arm64 and darwin builds attached to each tag, so installing on a node does not need a Go toolchain -- [ ] Reference documentation beyond the plan itself +- [x] Reference documentation beyond the plan itself: `docs/`, with + `docs/reference.md` generated from the configuration structs ### Phase 6 — v2 ideas - [ ] Precise KV index (§7.5 tier 2) — vLLM KV events plus tokenizer integration - [ ] Dynamo-style block cost function, tuning the `softmax` picker - [ ] Admission displacement (dropping requests from lower bands) -- [ ] Autoscaling and priority-based preemption +- [x] Autoscaling on load: a replica added while requests wait, up to a + model's max, and taken back after ten quiet minutes +- [ ] Priority-based preemption - [ ] Control plane HA (Raft) -- [ ] Weight downloading and cache management +- [x] Weight downloading and cache management: the mirror, agent fetches + checked against their digest, and reclaiming what nothing uses - [ ] Prefill/decode disaggregation (§12) - [ ] LoRA adapter routing (affinity to a replica that already has the adapter) - [ ] External provider support (as a fallback)