ateom-gvisor: terminate a workload whose runsc containers are already gone

FORK.md

@@ -53,6 +53,7 @@ Everything on `giantswarm` that is not in the pin (`git log v0.0.26..giantswarm`
 | The atelet restore is bounded and retried, and an actor whose restore keeps exceeding its budget is failed with the new reason `RESTORE_TIMED_OUT` (`--actor-restore-budget`, default 90 s per attempt with a cold pull and unpack included, 0 = the workflow deadline only; three attempts with a doubling pause within `--actor-workflow-deadline`, the layers an interrupted pull fetched staying cached so a retry makes progress; then the crash directive: CRASHED, worker released, deletable, `ate.actor.crashes` labelled) | the Restore step had one attempt and no budget of its own: a restore that ran out of time came back unclassified (reason UNKNOWN), was not retried and left the actor `RESUMING` on a claimed worker that nothing reclaimed, undeletable (FailedPrecondition) until a resume succeeded ([#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 35) | `95ce0c43`, [#22](https://github.com/giantswarm/substrate/pull/22) | with the row above on `upstream/actor-lifecycle-budgets` (`861e41d1`); agent-substrate/substrate#1562 (atenet retries `ResumeActor` on gRPC status only) and #1518 (an actor stays `RESUMING` on a runsc error) are its upstream tracks |
 | ateom keeps re-asserting its capacity report (`internal/ateomcapacity.Report` re-sends the accepted report every 10 s through `reassertReport`, each re-assertion retried like the first, 500 ms → 30 s; atelet's per-call `Recorded worker capacity` line at Debug; ate-api's `Worker reported its capacity` stays the line that marks a change) | a Worker record replaced under a running ateom — the Syncer's re-registration after a pod-IP change (#20), a store restored from a backup — is created without capacity (`CreateWorker` leaves it to the ateom) and never received one, so `AssignWorker` answered `no free workers available` on every resume until the pool was rolled (the lab proof of giantswarm/giantswarm#37762 box 3, 2026-09-12; [#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 38). An unchanged report costs ate-api a read and no write (100 workers ≈ 10 reads/s); the ≤ 10 s gap after a re-registration sits inside the router's 30 s parking budget | `5fccbf08`, [#25](https://github.com/giantswarm/substrate/pull/25) (rebase-merged 2026-09-12; first release v0.0.27-gs.7) | to file: the upstream-shaped patch is branch [`upstream/ateom-reassert-capacity`](https://github.com/giantswarm/substrate/tree/upstream/ateom-reassert-capacity) here (`cedac921`, on the mirror `main` @ `007eb1ee`; its message is the pull-request text; the `syncer.go` comment hunk, which annotates #20's path, is not on it). agent-substrate/substrate#1473 (open since 2026-09-04: a `RecycleWorker` RPC keeps the record of a Worker whose ateom container was replaced *because* an ateom reports its capacity once per process and a re-registered Worker would stay unschedulable) designs around the constraint this patch removes — complementary, not cherry-picked; a recycled record's re-assertion is a no-op read. No upstream issue on the capacity loss itself (searched 2026-09-12). A team member opens the pull request with DCO sign-off once #37742 has reviewed it. Keep at a re-pin onto a release that carries #1473: a recreated sandbox has a new IP and stays a re-registration under #20 |
 | The two ServiceAccounts take annotations and ate-api-server takes `extraEnv` (`atelet.serviceAccount.annotations`, `ateApiServer.serviceAccount.annotations`, `ateApiServer.extraEnv`; all empty by default, the rendered manifests unchanged) | an installation whose snapshot store is not the bundled RustFS binds a pod identity to the pods that reach it (IRSA `eks.amazonaws.com/role-arn`, GKE Workload Identity `iam.gke.io/gcp-service-account`) and hands ate-api-server the store's endpoint; atelet took `extraEnv`, ate-api-server only the `ate-api-server-envvars` ConfigMap the chart itself creates, and neither ServiceAccount an annotation. The meta chart's snapshot-store block sets all three ([giantswarm/agent-platform#411](https://github.com/giantswarm/agent-platform/issues/411); [#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 40) | `8fbf9e60`, [#27](https://github.com/giantswarm/substrate/pull/27) (rebase-merged 2026-09-13; first release v0.0.27-gs.8) | to file: the upstream-shaped patch is branch [`upstream/serviceaccount-annotations-extraenv`](https://github.com/giantswarm/substrate/tree/upstream/serviceaccount-annotations-extraenv) here (`c5f2a3d4`, on the mirror `main` @ `007eb1ee`; its message is the pull-request text). No upstream issue or pull request covers it (searched kagent-dev/substrate and agent-substrate/substrate, 2026-09-13: agent-substrate/substrate#1277 documents the GKE Workload Identity grants `setup-gcp` creates out of band, kagent-dev/substrate#31 makes the ServiceAccount names configurable). A team member opens the pull request with DCO sign-off once #37742 has reviewed it |
+| Terminate a workload whose runsc containers are already gone (`cleanupContainers` lists the containers runsc knows under the actor's state root — `runsc list -quiet`, nothing for a root without state — and checks and deletes only those; a container that is gone is logged and skipped, and a `runsc delete` that fails once its container is gone counts as done; application containers first, the pause container last, as before) | `runsc state` is fatal on a container runsc does not know (`loading container: file does not exist`, exit 128), so a terminate whose first attempt had removed the container but failed after that could never succeed: when a golden boot crashes, the Run-failure cleanup detaches the bundle's rootfs overlay (its own `runsc delete -force pause` is killed by the 30 s cleanup budget), the first terminate's `runsc delete -force pause` destroys the container state and then fails on the filestore file that lived on the overlay (`destroying container: failed to delete filestore file … no such file or directory`), and every retry fails at `runsc state pause` — atelet never reset the actor's directories, ateapi never released the worker, the actor stayed `DELETING` with its worker assigned until the pod was replaced. On gazelle (2026-09-13, control plane `0.0.27-gs.8`, worker `0.0.27-gs.7`) two crashed golden actors of superseded `AgentTemplate` revisions pinned two of the pool's four workers while kagent's controller logged `failed to collect runtime revision … runsc state: exit status 128` every minute per template (giantswarm/giantswarm#37773; [#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 43). Reproduced and proven in agentlab on 2026-09-13: on `0.0.27-gs.7` the superseded revision's template was never collected (56 min, the same three errors), on this patch's dev build the next sweep after the template change collected it and freed the worker | `c3e452c9`, [#30](https://github.com/giantswarm/substrate/pull/30) (rebase-merged 2026-09-13; first release v0.0.27-gs.9) | to file: the upstream-shaped patch is branch [`upstream/ateom-terminate-gone-containers`](https://github.com/giantswarm/substrate/tree/upstream/ateom-terminate-gone-containers) here (`b043db2e`, on the mirror `main` @ `007eb1ee`; its message is the pull-request text). No upstream issue or pull request on the terminate path (searched kagent-dev/substrate and agent-substrate/substrate, 2026-09-13); agent-substrate/substrate#1518 (open) is the sibling symptom — a `runsc restore` error leaves the actor `RESUMING` with its worker assigned — and agent-substrate/substrate#641 the node-directory leak of a delete that never reaches the node. A team member opens the pull request with DCO sign-off once #37742 has reviewed it |
 | Fork infrastructure: this file, the README pointer, `CODEOWNERS`, `.github/workflows/publish.yaml`, `.github/workflows/sync-upstream.yaml`, `.trivyignore`, and the branch triggers of `pr-workflow.yaml`, `helm-e2e.yaml`, `govulncheck.yaml` (`main` → `giantswarm`, govulncheck also on pull requests) | the line's CI, publishing and sync | the `giantswarm` branch history | not for upstream |
 
 Ten patches change Substrate ahead of upstream — egress for an actor while it resumes, without which no

cmd/ateom-gvisor/cleanup_test.go

@@ -0,0 +1,202 @@
+//go:build linux
+
+// Copyright 2026 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/agent-substrate/substrate/internal/ateompath"
+	"github.com/agent-substrate/substrate/internal/ocispec"
+	"github.com/agent-substrate/substrate/internal/proto/ateompb"
+)
+
+func TestListArgs(t *testing.T) {
+	r := &runsc{
+		path:     "/usr/bin/runsc",
+		actorUID: "test-actor-123",
+	}
+
+	got := r.listArgs()
+	want := []string{
+		"-log-format", "json",
+		"--alsologtostderr",
+		"-root", ateompath.RunSCStateDir("test-actor-123"),
+		"list",
+		"-quiet",
+	}
+
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("listArgs() = %v, want %v", got, want)
+	}
+}
+
+// fakeRunscBehaviour configures the runsc stand-in fakeRunsc writes.
+type fakeRunscBehaviour struct {
+	// known are the containers runsc knows at the start.
+	known []string
+	// listFails makes `list` fail with exit status 128.
+	listFails bool
+	// deleteFailsAfterRemoving are the containers whose `delete` removes them
+	// and then fails with exit status 128, the way `runsc delete -force`
+	// does when the filestore file it cleans up last is already gone.
+	deleteFailsAfterRemoving []string
+	// deleteFailsKeeping are the containers whose `delete` fails with exit
+	// status 128 and leaves them in place.
+	deleteFailsKeeping []string
+}
+
+// fakeRunsc writes a runsc stand-in: `list -quiet` prints the ids of the
+// containers it currently knows, `state` and `delete` of one of them succeed
+// (`delete` forgets it), and of any other fail with exit status 128 the way
+// runsc's fatal errors do. Every invocation is appended to the returned log
+// file as one line: the subcommand and its arguments.
+func fakeRunsc(t *testing.T, b fakeRunscBehaviour) (path, log string) {
+	t.Helper()
+	dir := t.TempDir()
+	path = filepath.Join(dir, "runsc")
+	log = filepath.Join(dir, "invocations")
+	known := filepath.Join(dir, "known")
+	if err := os.WriteFile(known, []byte(strings.Join(b.known, "\n")+"\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	listExit := "0"
+	if b.listFails {
+		listExit = "128"
+	}
+	script := fmt.Sprintf(`#!/bin/sh
+known=%q
+while [ $# -gt 0 ]; do
+  case "$1" in
+    -root|-log-format) shift 2 ;;
+    --alsologtostderr) shift ;;
+    *) break ;;
+  esac
+done
+echo "$*" >> %q
+sub=$1; shift
+for arg in "$@"; do id=$arg; done
+case "$sub" in
+  list)
+    grep -v '^$' "$known"
+    exit %s ;;
+  state)
+    grep -qx "$id" "$known" && exit 0
+    echo "loading container: file does not exist" >&2; exit 128 ;;
+  delete)
+    grep -qx "$id" "$known" || { echo "loading container: file does not exist" >&2; exit 128; }
+    case " %s " in *" $id "*) exit 128 ;; esac
+    grep -vx "$id" "$known" > "$known.new"; mv "$known.new" "$known"
+    case " %s " in *" $id "*) echo "destroying container: failed to delete filestore file" >&2; exit 128 ;; esac
+    exit 0 ;;
+  *) echo "unexpected subcommand $sub" >&2; exit 128 ;;
+esac
+`, known, log, listExit, strings.Join(b.deleteFailsKeeping, " "), strings.Join(b.deleteFailsAfterRemoving, " "))
+	if err := os.WriteFile(path, []byte(script), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	return path, log
+}
+
+func invocations(t *testing.T, log string) []string {
+	t.Helper()
+	data, err := os.ReadFile(log)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		t.Fatal(err)
+	}
+	return strings.Split(strings.TrimSpace(string(data)), "\n")
+}
+
+func TestCleanupContainers(t *testing.T) {
+	const pause = ocispec.PauseContainer
+	containers := []*ateompb.Container{{Name: "kagent"}}
+	for _, tc := range []struct {
+		name      string
+		behaviour fakeRunscBehaviour
+		want      []string
+		wantErr   string
+	}{
+		{
+			// A terminate retried after the containers are gone: nothing to
+			// check or delete, and no error to fail the terminate on.
+			name: "every container already gone",
+			want: []string{"list -quiet"},
+		},
+		{
+			// The application container went with a failed boot, the sandbox
+			// is still there.
+			name:      "application container gone, pause container present",
+			behaviour: fakeRunscBehaviour{known: []string{pause}},
+			want:      []string{"list -quiet", "state " + pause, "delete -force " + pause},
+		},
+		{
+			name:      "every container present",
+			behaviour: fakeRunscBehaviour{known: []string{pause, "kagent"}},
+			want: []string{
+				"list -quiet",
+				"state kagent", "state " + pause,
+				"delete -force kagent", "delete -force " + pause,
+			},
+		},
+		{
+			// The first terminate of a crashed boot: `runsc delete -force`
+			// destroys the sandbox's state and then fails on the filestore
+			// file of the detached rootfs overlay. The container is gone, so
+			// the terminate succeeds in this pass.
+			name:      "delete fails after removing the container",
+			behaviour: fakeRunscBehaviour{known: []string{pause}, deleteFailsAfterRemoving: []string{pause}},
+			want:      []string{"list -quiet", "state " + pause, "delete -force " + pause, "list -quiet"},
+		},
+		{
+			name:      "delete fails with the container still there",
+			behaviour: fakeRunscBehaviour{known: []string{pause}, deleteFailsKeeping: []string{pause}},
+			want:      []string{"list -quiet", "state " + pause, "delete -force " + pause, "list -quiet"},
+			wantErr:   "while deleting \"" + pause + "\" container: while running `runsc delete`",
+		},
+		{
+			name:      "listing fails",
+			behaviour: fakeRunscBehaviour{known: []string{pause, "kagent"}, listFails: true},
+			want:      []string{"list -quiet"},
+			wantErr:   "while listing containers: while running `runsc list`",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			path, log := fakeRunsc(t, tc.behaviour)
+			r := &runsc{path: path, actorUID: "test-actor-123"}
+
+			err := r.cleanupContainers(context.Background(), containers)
+
+			if tc.wantErr == "" && err != nil {
+				t.Fatalf("cleanupContainers() = %v, want nil", err)
+			}
+			if tc.wantErr != "" && (err == nil || !strings.Contains(err.Error(), tc.wantErr)) {
+				t.Fatalf("cleanupContainers() = %v, want an error containing %q", err, tc.wantErr)
+			}
+			if got := invocations(t, log); !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("runsc invocations = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}

cmd/ateom-gvisor/main.go

@@ -45,6 +45,7 @@ import (
 	"github.com/agent-substrate/substrate/internal/childreap"
 	"github.com/agent-substrate/substrate/internal/contextlogging"
 	"github.com/agent-substrate/substrate/internal/imagecache"
+	"github.com/agent-substrate/substrate/internal/ocispec"
 	"github.com/agent-substrate/substrate/internal/otlprelay"
 	"github.com/agent-substrate/substrate/internal/proto/ateompb"
 	"github.com/agent-substrate/substrate/internal/readyz"
@@ -870,27 +871,53 @@ func (r *runsc) stopContainers(ctx context.Context, containers []*ateompb.Contai
 	_ = r.cmdWait(ctx, "pause")
 }
 
+// cleanupContainers deletes the sandbox's runsc containers, the application
+// containers first and the pause container (the sandbox) last.
+//
+// Idempotent: a container runsc no longer knows is already cleaned up and is
+// skipped. `runsc state` is fatal on a missing container, so checking it first
+// made every retry of a terminate fail for good once its first attempt had
+// removed the container but failed after that -- `runsc delete -force` of a
+// stopped sandbox whose rootfs overlay was already detached destroys the
+// container's state and then fails on the missing filestore file -- and the
+// actor kept its worker until the pod was replaced.
 func (r *runsc) cleanupContainers(ctx context.Context, containers []*ateompb.Container) error {
-	// Check state of all containers to mimic containerd.
-	//
-	// Without this, `runsc delete` occasionally throws an error.
-	if err := r.cmdState(ctx, "pause"); err != nil {
-		return fmt.Errorf("while checking state of pause container: %w", err)
+	known, err := r.cmdList(ctx)
+	if err != nil {
+		return fmt.Errorf("while listing containers: %w", err)
 	}
-	for _, ctr := range containers {
-		if err := r.cmdState(ctx, ctr.GetName()); err != nil {
-			return fmt.Errorf("while checking state of %q application container: %w", ctr.GetName(), err)
+	var present []string
+	for _, name := range append(containerNames(containers), ocispec.PauseContainer) {
+		if !known[name] {
+			slog.InfoContext(ctx, "runsc container is already gone, nothing to clean up", slog.String("container", name))
+			continue
 		}
+		present = append(present, name)
 	}
 
-	for _, ctr := range containers {
-		if err := r.cmdDelete(ctx, ctr.GetName()); err != nil {
-			return fmt.Errorf("while deleting %q application container: %w", ctr.GetName(), err)
+	// Check state of all containers to mimic containerd.
+	//
+	// Without this, `runsc delete` occasionally throws an error.
+	for _, name := range present {
+		if err := r.cmdState(ctx, name); err != nil {
+			return fmt.Errorf("while checking state of %q container: %w", name, err)
 		}
 	}
 
-	if err := r.cmdDelete(ctx, "pause"); err != nil {
-		return fmt.Errorf("while deleting pause container: %w", err)
+	for _, name := range present {
+		if err := r.cmdDelete(ctx, name); err != nil {
+			// `runsc delete -force` destroys the container's state before it
+			// cleans up the rest, and reports what fails after that -- the
+			// filestore file of a rootfs overlay a failed boot's cleanup already
+			// detached -- as fatal. Once the container is gone the delete has
+			// done its job; failing the terminate would only make the next
+			// attempt find nothing left to delete.
+			if known, lerr := r.cmdList(ctx); lerr == nil && !known[name] {
+				slog.InfoContext(ctx, "runsc delete failed after removing the container, nothing left to clean up", slog.String("container", name), slog.Any("err", err))
+				continue
+			}
+			return fmt.Errorf("while deleting %q container: %w", name, err)
+		}
 	}
 
 	return nil

cmd/ateom-gvisor/runsc.go

@@ -17,6 +17,7 @@
 package main
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -25,6 +26,7 @@ import (
 	"os"
 	"os/exec"
 	"slices"
+	"strings"
 	"syscall"
 
 	"github.com/agent-substrate/substrate/internal/ateompath"
@@ -323,6 +325,37 @@ func (r *runsc) cmdDelete(ctx context.Context, containerName string) error {
 	return nil
 }
 
+// listArgs builds the argv for `runsc list -quiet`: the ids of the containers
+// runsc knows under this actor's state root, one per line. Factored out so the
+// argument construction can be unit-tested without executing runsc.
+func (r *runsc) listArgs() []string {
+	return []string{
+		"-log-format", "json",
+		"--alsologtostderr",
+		"-root", ateompath.RunSCStateDir(r.actorUID),
+		"list",
+		"-quiet",
+	}
+}
+
+// cmdList returns the containers runsc knows under this actor's state root. A
+// root that holds no container state -- never created, or reset by atelet --
+// lists nothing.
+func (r *runsc) cmdList(ctx context.Context) (map[string]bool, error) {
+	cmd := exec.CommandContext(ctx, r.path, r.listArgs()...)
+	var out bytes.Buffer
+	cmd.Stdout = &out
+	cmd.Stderr = os.Stderr
+	if err := reaper.RunCommand(cmd); err != nil {
+		return nil, fmt.Errorf("while running `runsc list`: %w", err)
+	}
+	known := make(map[string]bool)
+	for _, id := range strings.Fields(out.String()) {
+		known[id] = true
+	}
+	return known, nil
+}
+
 func (r *runsc) cmdState(ctx context.Context, containerName string) error {
 	cmd := exec.CommandContext(
 		ctx,