Ship binaries with releases, and ask engines whether our flags still exist
CI / test (push) Successful in 1m31s
CI / test (push) Successful in 1m31s
A release was a git tag. Installing llmgw on a node meant having a Go toolchain, and the onboarding script copied a binary from wherever it was run — which works and quietly makes whoever runs it the build server, with the fleet ending up on whatever was in their working tree. Tags now build static linux/amd64, linux/arm64 and darwin binaries and attach them. CGO_ENABLED=0 is the point: it has to run on a node whose libc is older than the builder's, and llmgw has no C dependency because modernc.org/sqlite is pure Go precisely so this stays true. Verified: "not a dynamic executable", runs on a node, reports its version. `ctl engine-check` closes the other half of the phase 4 gap. llmgw's launch arguments are read from each engine's documentation, and engines rename flags between versions; when that happens the failure arrives minutes into a model load, in the engine's own error message, on a node nobody was watching. It builds the widest command llmgw would ever produce — multi-node, since a flag that only appears there is the one nobody notices has gone — and checks each flag against the engine's own --help. No GPU: a CLI prints its help anywhere. Verified against llama.cpp b10991, all seven flags accepted. Flags are matched on a word boundary, so --port is not satisfied by --port-range. Without that, a renamed flag would sail through the check that exists to catch renamed flags. It also warns when the help text looks truncated, which is how the first run went: llama.cpp's help cut to 400 of its 725 lines reported five flags as missing that the running cluster uses. TestSidecarRunsBeforeEngine was flaky under a full run, and worse than flaky — its wait loop fell through on timeout, so a slow machine turned the ordering assertion into a no-op and the failure surfaced later as something else. It now asserts, with a deadline that suits a machine running every other package at the same time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
7b4acd9284
commit
5b425c32e5
8 files changed
+407
-13
No files matched your search
@@ -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"
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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") &&
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <engine>",
|
||||
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')
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
+8
-3
@@ -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
|
||||
|
||||
Reference in new issue
Block a user