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) <noreply@anthropic.com>

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"

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 {

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.

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")
+	}
+}