diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml new file mode 100644 index 00000000..4d670bb6 --- /dev/null +++ b/.gitea/workflows/release.yml @@ -0,0 +1,88 @@ +# Attaches binaries to a release when a version tag is pushed. +# +# Without this a release is a git tag, and installing llmgw on a node means +# having a Go toolchain and the source. The onboarding script copies a binary +# from wherever it is run, which works and quietly makes whoever runs it the +# build server. +name: Release + +on: + push: + tags: ["v*"] + workflow_dispatch: + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + # Tags are what the version string is built from, and a shallow clone + # has none of them. + fetch-depth: 0 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + - name: Build + run: | + set -euo pipefail + version="${GITHUB_REF_NAME:-$(git describe --tags --always)}" + mkdir -p dist + + # CGO_ENABLED=0 is the whole point: the binary has to run on a node + # whose libc is older than the builder's, and llmgw has no C + # dependency — modernc.org/sqlite is pure Go precisely so this stays + # true. + for target in linux/amd64 linux/arm64 darwin/amd64 darwin/arm64; do + os="${target%/*}"; arch="${target#*/}" + name="llmgw-${version}-${os}-${arch}" + echo "building $name" + CGO_ENABLED=0 GOOS="$os" GOARCH="$arch" go build \ + -trimpath \ + -ldflags "-s -w -X gitea.mynodes.xyz/ilker/llmgw/internal/version.Version=${version}" \ + -o "dist/${name}" ./cmd/llmgw + (cd dist && sha256sum "${name}" >> checksums.txt) + done + + # Proof the artifact runs at all before it is published. It cannot + # test the cross-compiled ones, which is why the native build is + # checked rather than assumed. + ./dist/llmgw-${version}-linux-amd64 --version + cat dist/checksums.txt + + - name: Attach to the release + env: + TOKEN: ${{ secrets.GITEA_TOKEN }} + run: | + set -euo pipefail + if [ -z "${TOKEN:-}" ]; then + echo "No GITEA_TOKEN secret, so nothing can be uploaded." + echo "Add one in the repository's Actions settings with package or" + echo "repository write scope; the binaries are in dist/ meanwhile." + exit 1 + fi + tag="${GITHUB_REF_NAME}" + api="${GITHUB_SERVER_URL}/api/v1/repos/${GITHUB_REPOSITORY}" + + # The release usually exists already, created by hand with its notes. + id=$(curl -sS -H "Authorization: token $TOKEN" "$api/releases/tags/$tag" \ + | sed -n 's/.*"id":\([0-9]*\).*/\1/p' | head -1) + if [ -z "$id" ]; then + echo "creating release $tag" + id=$(curl -sS -X POST -H "Authorization: token $TOKEN" \ + -H "Content-Type: application/json" \ + -d "{\"tag_name\":\"$tag\",\"name\":\"$tag\"}" \ + "$api/releases" | sed -n 's/.*"id":\([0-9]*\).*/\1/p' | head -1) + fi + [ -n "$id" ] || { echo "could not find or create the release" >&2; exit 1; } + + for f in dist/*; do + echo "uploading $(basename "$f")" + curl -sS -X POST -H "Authorization: token $TOKEN" \ + -F "attachment=@$f" \ + "$api/releases/$id/assets?name=$(basename "$f")" >/dev/null + done + echo "attached $(ls dist | wc -l) files to $tag" diff --git a/deploy/bootstrap/onboard-node.sh b/deploy/bootstrap/onboard-node.sh index 767cadf3..7e230ad2 100755 --- a/deploy/bootstrap/onboard-node.sh +++ b/deploy/bootstrap/onboard-node.sh @@ -38,7 +38,10 @@ usage: $0 --node ID --addr HOST --server URL [options] driver installs routinely put it somewhere else. --ib-root DIR the sysfs InfiniBand directory (default /sys/class/infiniband) --pki DIR where the authority lives (default /etc/llmgw/pki) - --binary PATH llmgw binary to install (default: the one running this) + --binary PATH llmgw binary to install (default: the one running this). + Releases carry static linux/amd64 and linux/arm64 builds; + downloading one is better than copying whatever happens to + be on the machine you are running this from. --ssh-user NAME (default ilker) USAGE exit 2 diff --git a/docs/node-onboarding.md b/docs/node-onboarding.md index 21279767..8c22e437 100644 --- a/docs/node-onboarding.md +++ b/docs/node-onboarding.md @@ -21,6 +21,12 @@ It imports a cloud image and lets cloud-init do the rest, which is why a node takes a minute rather than an afternoon. It refuses to touch an existing VMID: adopting a machine is not a decision a provisioning script should make. +Releases carry static `linux/amd64` and `linux/arm64` binaries. Download one and +pass it with `--binary` rather than copying whatever happens to be on the +machine you are running onboarding from — otherwise whoever runs the script +quietly becomes the build server, and the fleet ends up running whatever was in +their working tree. + ## 2. The identity On the host that holds the certificate authority — this is the part that cannot @@ -70,6 +76,27 @@ a laptop. llmgw has no answer for it yet; that is a known gap, not an oversight. The `sim` engine needs neither, which is what makes it useful for exercising everything else. +## Checking an engine after an upgrade + +Engines rename and remove flags between versions, and when that happens the +failure arrives minutes into a model load, in the engine's own error message, on +a node nobody was watching. + +``` +llmgw ctl engine-check vllm +llmgw ctl engine-check llamacpp --help-output "$(llama-server --help 2>&1)" +``` + +It builds the widest command llmgw would ever produce — a multi-node one, since +a flag that only appears there is exactly the one nobody notices has gone — and +checks every flag against the engine's own `--help`. No GPU required: a CLI +prints its help on any machine. + +Give it the **whole** help. Truncating llama.cpp's 725 lines to 400 reports five +flags as missing that the running cluster uses; the command warns when the text +it was given does not document `--help` itself, which is the cheapest sign it +was cut short. + ## Fabrics `--fabric` is how a node says which InfiniBand fabric it is attached to, and diff --git a/internal/agent/supervisor/supervisor_test.go b/internal/agent/supervisor/supervisor_test.go index af1785f4..64c81533 100644 --- a/internal/agent/supervisor/supervisor_test.go +++ b/internal/agent/supervisor/supervisor_test.go @@ -557,15 +557,27 @@ func TestSidecarRunsBeforeEngine(t *testing.T) { t.Fatal(err) } - deadline := time.Now().Add(3 * time.Second) - for time.Now().Before(deadline) { - if b, err := os.ReadFile(order); err == nil && strings.Count(string(b), "\n") == 2 { - if got := strings.Fields(string(b)); got[0] != "sidecar" || got[1] != "engine" { - t.Fatalf("start order was %v", got) - } - break + // Generous, because this waits on two processes starting under the race + // detector while every other package is competing for the same cores. Three + // seconds was enough alone and not in a full run. + var started []string + deadline := time.Now().Add(20 * time.Second) + for time.Now().Before(deadline) && len(started) < 2 { + if b, err := os.ReadFile(order); err == nil { + started = strings.Fields(string(b)) } - time.Sleep(20 * time.Millisecond) + if len(started) < 2 { + time.Sleep(20 * time.Millisecond) + } + } + // Asserted rather than broken out of: the original fell through on timeout, + // so a slow machine turned the ordering check into a no-op and the failure + // surfaced later as something else. + if len(started) != 2 { + t.Fatalf("only %d of 2 processes started: %v", len(started), started) + } + if started[0] != "sidecar" || started[1] != "engine" { + t.Fatalf("start order was %v", started) } // The shard's pid is the engine's, not the sidecar's: the engine's exit is @@ -574,7 +586,7 @@ func TestSidecarRunsBeforeEngine(t *testing.T) { t.Fatal("no engine pid recorded") } // Both processes appear in one log tail, distinguishable by prefix. - deadline = time.Now().Add(3 * time.Second) + deadline = time.Now().Add(20 * time.Second) for time.Now().Before(deadline) { logs := strings.Join(sh.Logs(20), "\n") if strings.Contains(logs, "[sidecar] hello-from-sidecar") && diff --git a/internal/cli/ctl.go b/internal/cli/ctl.go index c5b62a10..d3c68443 100644 --- a/internal/cli/ctl.go +++ b/internal/cli/ctl.go @@ -109,6 +109,7 @@ func newCtlCmd() *cobra.Command { // cert works on files rather than the admin API, so --server does not // apply to it; it lives here because this is where operators look. newBenchCmd(&asJSON), + newEngineCheckCmd(), newCertCmd(), ) return cmd diff --git a/internal/cli/enginecheck.go b/internal/cli/enginecheck.go new file mode 100644 index 00000000..390db068 --- /dev/null +++ b/internal/cli/enginecheck.go @@ -0,0 +1,187 @@ +package cli + +import ( + "context" + "fmt" + "os/exec" + "sort" + "strings" + "time" + + "github.com/spf13/cobra" + + "gitea.mynodes.xyz/ilker/llmgw/internal/agent/engines" +) + +func newEngineCheckCmd() *cobra.Command { + var ( + help string + nodes int + tp int + perNode int + ) + + cmd := &cobra.Command{ + Use: "engine-check ", + Short: "Check that an engine still accepts the flags llmgw builds for it", + Long: "llmgw's launch arguments are read from each engine's documentation, and\n" + + "engines rename and remove flags between versions. When that happens the\n" + + "failure arrives minutes into a model load, in an engine's own error\n" + + "message, on a node nobody was watching.\n\n" + + "This asks the engine itself. It runs the engine's --help and checks every\n" + + "flag llmgw would pass against it, which needs no GPU — a CLI prints its\n" + + "help on any machine. Run it after upgrading an engine.\n\n" + + "With --help-output it reads a saved --help instead of running anything,\n" + + "so the check works from a laptop against help captured on a node.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + name := args[0] + reg := engines.NewRegistry(engines.Config{}) + eng, err := reg.Get(name) + if err != nil { + return err + } + + spec := representativeShard(name, nodes, tp, perNode) + cmds, err := eng.Commands(spec) + if err != nil { + return fmt.Errorf("building a command for %s: %w", name, err) + } + + text := help + if text == "" { + if text, err = runHelp(cmd.Context(), cmds); err != nil { + return err + } + } + + // A truncated help produces confident nonsense: every flag below the + // cut looks missing. Real help text documents --help itself, which + // is the cheapest signal that we are looking at all of it. + if !mentions(text, "--help") { + cmd.PrintErrln("warning: this help text does not document --help, " + + "which usually means it was truncated. Flags below the cut will " + + "look missing.") + } + + var unknown []string + seen := map[string]bool{} + for _, c := range cmds { + for _, a := range c.Args { + flag, _, _ := strings.Cut(a, "=") + if !strings.HasPrefix(flag, "--") || seen[flag] { + continue + } + seen[flag] = true + if !mentions(text, flag) { + unknown = append(unknown, flag) + } + } + } + sort.Strings(unknown) + + out := cmd.OutOrStdout() + fmt.Fprintf(out, "%s: %d flags checked\n", name, len(seen)) + for _, c := range cmds { + fmt.Fprintf(cmd.ErrOrStderr(), " %s %s\n", c.Path, strings.Join(c.Args, " ")) + } + if len(unknown) == 0 { + fmt.Fprintln(out, "every flag llmgw builds is accepted by this engine") + return nil + } + fmt.Fprintf(out, "\n%d flags this engine does not mention:\n", len(unknown)) + for _, f := range unknown { + fmt.Fprintf(out, " %s\n", f) + } + return fmt.Errorf("%s does not accept %d of the flags llmgw builds", name, len(unknown)) + }, + } + + f := cmd.Flags() + f.StringVar(&help, "help-output", "", "a saved --help to check against, instead of running the engine") + f.IntVar(&nodes, "nodes", 2, "shape the command for a replica of this many nodes") + f.IntVar(&tp, "tp", 8, "tensor parallel degree") + f.IntVar(&perNode, "gpus-per-node", 4, "GPUs each node contributes") + return cmd +} + +// representativeShard is the widest command llmgw would ever build, because a +// flag that only appears in the multi-node case is exactly the one nobody +// notices has been renamed. +func representativeShard(engine string, nodes, tp, perNode int) engines.ShardSpec { + if nodes < 1 { + nodes = 1 + } + return engines.ShardSpec{ + ReplicaID: "check", ShardID: "check-r0", Rank: 0, NNodes: nodes, + LeaderAddr: "10.0.0.1", LeaderPort: 20001, + GPUIndices: seq(perNode), HCAs: []string{"mlx5_0"}, + MgmtInterface: "eth0", HTTPPort: 20000, + Profile: engines.Profile{ + Name: "check", Engine: engine, TP: tp, Nodes: nodes, GPUsPerNode: perNode, + ModelPath: "/models/x", ServedModelName: "x", + }, + } +} + +func seq(n int) []int { + out := make([]int, n) + for i := range out { + out[i] = i + } + return out +} + +// runHelp asks the engine to describe itself. +// +// The last command is the one that serves; the sidecars before it are a +// different program with a different vocabulary, so its help would be the wrong +// help to check against. +func runHelp(ctx context.Context, cmds []engines.Command) (string, error) { + c := cmds[len(cmds)-1] + + // Everything up to the first flag, plus --help. Passing the real arguments + // would start the engine. + var args []string + for _, a := range c.Args { + if strings.HasPrefix(a, "-") { + break + } + args = append(args, a) + } + args = append(args, "--help") + + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + + out, err := exec.CommandContext(ctx, c.Path, args...).CombinedOutput() + if err != nil && len(out) == 0 { + return "", fmt.Errorf("running %s %s: %w\n"+ + "(is the engine installed here? use --help-output to check against a saved one)", + c.Path, strings.Join(args, " "), err) + } + return string(out), nil +} + +// mentions reports whether the help text documents a flag. +// +// Matched on a word boundary, so --port does not satisfy --port-range and a +// missing flag cannot hide behind a longer one that happens to contain it. +func mentions(help, flag string) bool { + for i := 0; ; { + j := strings.Index(help[i:], flag) + if j < 0 { + return false + } + end := i + j + len(flag) + if end == len(help) || !isFlagChar(help[end]) { + return true + } + i = end + } +} + +func isFlagChar(b byte) bool { + return b == '-' || b == '_' || (b >= 'a' && b <= 'z') || + (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') +} diff --git a/internal/cli/enginecheck_test.go b/internal/cli/enginecheck_test.go new file mode 100644 index 00000000..becd8e02 --- /dev/null +++ b/internal/cli/enginecheck_test.go @@ -0,0 +1,71 @@ +package cli + +import ( + "testing" +) + +func TestAFlagIsNotSatisfiedByALongerOne(t *testing.T) { + // The failure this guards against is silent: --port would be reported as + // present because --port-range is documented, and a renamed flag would sail + // through the check that exists to catch renamed flags. + help := " --port-range N a range\n --model-name S a name\n" + + if mentions(help, "--port") { + t.Error("--port was satisfied by --port-range") + } + if mentions(help, "--model") { + t.Error("--model was satisfied by --model-name") + } + if !mentions(help, "--port-range") { + t.Error("--port-range was not found") + } +} + +func TestFlagsAreFoundInTheShapesHelpTextUses(t *testing.T) { + help := ` + -m, --model FNAME model path + --host HOST ip address + --parallel, -np N slots + --metrics enable prometheus + --alias=NAME served name +` + for _, flag := range []string{"--model", "--host", "--parallel", "--metrics", "--alias"} { + if !mentions(help, flag) { + t.Errorf("%s was not found in a help text that documents it", flag) + } + } + for _, flag := range []string{"--nonsense", "--mode", "--para"} { + if mentions(help, flag) { + t.Errorf("%s was found in a help text that does not document it", flag) + } + } +} + +func TestTheRepresentativeShardIsTheWidestOne(t *testing.T) { + // A flag that only appears in the multi-node case is exactly the one nobody + // notices has been renamed, so the command built for checking has to be the + // widest llmgw would ever produce. + spec := representativeShard("vllm", 2, 8, 4) + if spec.NNodes != 2 || spec.Profile.Nodes != 2 { + t.Errorf("not a multi-node shard: %+v", spec) + } + if len(spec.GPUIndices) != 4 { + t.Errorf("%d GPUs, want 4", len(spec.GPUIndices)) + } + if spec.LeaderAddr == "" || spec.LeaderPort == 0 { + t.Error("no rendezvous point, so the flags that carry it will not appear") + } +} + +func TestATruncatedHelpIsSuspected(t *testing.T) { + // Found by truncating llama.cpp's 725-line help to 400 lines and being told, + // confidently, that five flags the running cluster uses do not exist. + full := " --help show this\n --model F path\n" + if !mentions(full, "--help") { + t.Fatal("the guard's own signal is missing from a complete help") + } + truncated := " --model F path\n" + if mentions(truncated, "--help") { + t.Error("a truncated help was taken for a complete one") + } +} diff --git a/llmgw-plan.md b/llmgw-plan.md index c6e27cc6..e59b1022 100644 --- a/llmgw-plan.md +++ b/llmgw-plan.md @@ -1407,9 +1407,12 @@ The launch path is now built too: that NCCL picks the right HCA, or that the generated environment makes a real collective run. Those need hardware. The GPUs here are an inventory, the fabric is a sysfs-shaped directory, and the engine speaks no NCCL. -- [ ] **Confirm the engine flags on real hardware.** The launch *sequence* is - now exercised; the launch *arguments* for vLLM and SGLang are still read - from their documentation rather than from a running engine. +- [x] `ctl engine-check`, which asks an engine whether it still accepts the + flags llmgw builds. No GPU needed — a CLI prints its help anywhere. + Verified against llama.cpp b10991: all seven flags accepted. +- [ ] **Run `engine-check` against vLLM and SGLang.** The launch *sequence* is + exercised on the simulated cluster and the *arguments* now have a way to + be checked; what is missing is an installed engine to check them against. - **Exit criterion (met on the simulated cluster):** a 2-node replica on each fabric; killing a node rebuilds the replica within one fabric and never @@ -1457,6 +1460,8 @@ The launch path is now built too: rather than printed - [ ] OTel tracing - [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 ### Phase 6 — v2 ideas