From a40fc53e09640cf7050e6ab36be37bf94a4c1d70 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Mon, 28 Sep 2026 12:14:22 +0200 Subject: [PATCH] Wait for a new machine's own apt, and retry a failed install sooner Found by building a node from nothing. Its agent tried to install libgomp1 while cloud-init and unattended-upgrades were running apt, and failed with "Unable to lock directory /var/lib/apt/lists/": DPkg::Lock::Timeout covers dpkg's lock, not the one `apt-get update` takes on its lists. The node was correctly kept out of placement and said why, and then sat idle for the ten minutes until the next pass. A package command that fails only on a held lock is now retried for up to ten minutes, reported as "waiting for another apt-get to finish" rather than as a failure. A pass that does fail is retried after a minute, backing off to ten. The weights directory is created when the agent starts rather than by the first download, so a new node reports its disk from its first heartbeat. Onboarding takes several control plane addresses, comma separated, so a new node gets the same failover list as the rest. Co-Authored-By: Claude Opus 5.5 (1M context) --- deploy/bootstrap/onboard-node.sh | 12 +++++- internal/agent/agent.go | 24 ++++++++++-- internal/agent/provision/install.go | 48 +++++++++++++++++++++++- internal/agent/provision/install_test.go | 30 +++++++++++++++ 4 files changed, 107 insertions(+), 7 deletions(-) diff --git a/deploy/bootstrap/onboard-node.sh b/deploy/bootstrap/onboard-node.sh index 33790e8..5fbc27d 100755 --- a/deploy/bootstrap/onboard-node.sh +++ b/deploy/bootstrap/onboard-node.sh @@ -24,7 +24,8 @@ usage: $0 --node ID --addr HOST --server URL [options] --node ID node id, hostname and certificate common name --addr HOST how to reach the machine over ssh - --server URL the control plane's agent endpoint, e.g. https://10.0.0.140:7443 + --server URL the control plane's agent endpoint, e.g. https://10.0.0.140:7443; + several, comma separated, for a node that fails over --fabric NAME InfiniBand fabric this node is attached to; repeatable as --fabric NAME:hca1,hca2 . Omit for a node with no fabric, which can host single-node replicas and will never be given @@ -169,7 +170,14 @@ NODE_IS_HOSTNAME=no # below is written only when it was given, as an override. { echo "# $NODE, written by onboard-node.sh." - echo "server: \"$SERVER\"" + # Several addresses, comma separated, are every control plane the node may + # fail over to; a standby refuses it and it moves on to the next. + if [ "${SERVER#*,}" != "$SERVER" ]; then + echo "servers:" + printf '%s\n' "$SERVER" | tr ',' '\n' | sed 's/^ *//; s/^/ - "/; s/$/"/' + else + echo "server: \"$SERVER\"" + fi echo echo "tls:" echo " ca: /etc/llmgw/ca.pem" diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 10def64..fe694ea 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -409,6 +409,11 @@ func (a *Agent) Run(ctx context.Context) error { a.log.Error("hardware discovery failed", "err", err) } a.recoverOrphans() + // Made now rather than by the first download, so a new node reports its + // disk from its first heartbeat instead of only once it holds a model. + if err := os.MkdirAll(a.cfg.WeightsCache, 0o755); err != nil { + a.log.Warn("cannot create the weights directory", "dir", a.cfg.WeightsCache, "err", err) + } go a.provisionLoop(ctx) attempt := 0 @@ -1265,14 +1270,17 @@ func (a *Agent) setProvision(p *llmgwv1.Provision) { // provisionLoop installs and re-checks the wanted engines, one pass at a time. func (a *Agent) provisionLoop(ctx context.Context) { - t := time.NewTicker(provisionEvery) - defer t.Stop() + // 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 <-t.C: + case <-time.After(next): } a.mu.Lock() wants := slices.Clone(a.provWants) @@ -1283,6 +1291,16 @@ func (a *Agent) provisionLoop(ctx context.Context) { } 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 { diff --git a/internal/agent/provision/install.go b/internal/agent/provision/install.go index d5f13d4..1c71cc7 100644 --- a/internal/agent/provision/install.go +++ b/internal/agent/provision/install.go @@ -283,13 +283,57 @@ func (in *Installer) packages(ctx context.Context, byFamily map[string][]string, } in.progress(st, "installing %s with %s", strings.Join(missing, ", "), tool) for _, c := range cmds { - if out, err := in.run(ctx, pm.env, c[0], c[1:]...); err != nil { - return fmt.Errorf("%s: %v: %s", strings.Join(c, " "), err, lastLine(out)) + if err := in.runWaitingForLock(ctx, pm.env, c, st); err != nil { + return err } } return nil } +// lockWait bounds how long a package command waits for another to finish. +const lockWait = 10 * time.Minute + +// lockRetry is how often a locked package manager is tried again. +var lockRetry = 10 * time.Second + +// runWaitingForLock runs a package command, waiting while another holds the +// package manager's lock. +// +// A machine a minute old is running cloud-init's and unattended-upgrades' own +// apt. apt's DPkg::Lock::Timeout covers dpkg's lock and not the one `apt-get +// update` takes on its lists, so the first install on a new node failed with +// "Unable to lock directory /var/lib/apt/lists/" — found by building one. +func (in *Installer) runWaitingForLock(ctx context.Context, env, c []string, st *Status) error { + deadline := time.Now().Add(lockWait) + for { + out, err := in.run(ctx, env, c[0], c[1:]...) + if err == nil { + return nil + } + if !lockHeld(string(out)) || time.Now().After(deadline) { + return fmt.Errorf("%s: %v: %s", strings.Join(c, " "), err, lastLine(out)) + } + in.progress(st, "waiting for another %s to finish (a new machine runs its own updates first)", c[0]) + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(lockRetry): + } + } +} + +// lockHeld reports whether a package command failed only because another +// held the lock. +func lockHeld(out string) bool { + for _, s := range []string{"Unable to lock", "Could not get lock", "is locked by another process", + "waiting for process with pid", "System management is locked"} { + if strings.Contains(out, s) { + return true + } + } + return false +} + // packageManager is how one packaging family asks and installs. type packageManager struct { // query exits zero when the package named after it is installed. diff --git a/internal/agent/provision/install_test.go b/internal/agent/provision/install_test.go index 0e01c31..a3c4050 100644 --- a/internal/agent/provision/install_test.go +++ b/internal/agent/provision/install_test.go @@ -16,6 +16,7 @@ import ( "strings" "sync" "testing" + "time" "gitea.mynodes.xyz/ilker/llmgw/internal/agent/discovery" ) @@ -89,6 +90,8 @@ type fakeSystem struct { tools map[string]bool lddOut string calls []string + // locked is how many more package commands find the lock held. + locked int } var errExit = errors.New("exit 1") @@ -110,6 +113,10 @@ func (f *fakeSystem) run(_ context.Context, _ []string, name string, args ...str } return nil, errExit case name == "apt-get" || name == "dnf": + if f.locked > 0 { + f.locked-- + return []byte("E: Unable to lock directory /var/lib/apt/lists/\n"), errExit + } for i, a := range args { if a == "install" { for _, p := range args[i+1:] { @@ -300,3 +307,26 @@ func TestASystemTooOldForTheBuildSaysWhatItLacks(t *testing.T) { t.Errorf("the same shortfall was listed more than once: %q", got[0].Detail) } } + +func TestAPackageManagerBusyWithItsOwnUpdatesIsWaitedFor(t *testing.T) { + // A new node's first minutes: cloud-init and unattended-upgrades hold + // apt's lists lock, which DPkg::Lock::Timeout does not cover. + lockRetry = time.Millisecond + t.Cleanup(func() { lockRetry = 10 * time.Second }) + + rel := llamaRelease(t) + sys := &fakeSystem{installed: map[string]bool{}, locked: 3} + in, seen := installer(t, rel.serve(t), sys, ubuntu) + + got := in.Converge(t.Context(), []Want{{Engine: "llamacpp", Version: "b1"}}) + if got[0].State != Ready { + t.Fatalf("gave up on a lock another apt held: %+v", got[0]) + } + waited := false + for _, s := range *seen { + waited = waited || strings.Contains(s.Detail, "waiting for another apt-get") + } + if !waited { + t.Error("the wait was not reported") + } +}