fix: detect -allow-connected-on-save support instead of assuming it

cmd/ateom-gvisor/runsc.go

@@ -17,6 +17,7 @@
 package main
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -25,6 +26,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"sync"
 
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 
@@ -113,27 +115,111 @@ func (r *runsc) cmdCreate(ctx context.Context, out io.Writer, containerName stri
 	return nil
 }
 
+// allowConnectedOnSaveFlag lets runsc checkpoint a sandbox whose network
+// connections are still established, so an actor keeps them across
+// suspend/resume.
+//
+// It was introduced in runsc release-20260622.0. Builds older than that reject
+// it on `runsc start` with "flag provided but not defined", which fails every
+// actor start, so it is probed for below.
+//
+// There is no equivalent on those older builds. They have only
+// -net-disconnect-ok, which defaults to true and *drops* open connections on
+// save, and neither -allow-live-tcp-migration nor a working
+// -save-restore-netstack (deprecated and inert on current builds, and about
+// netstack save/restore generally rather than connection preservation). So
+// dropping the flag is not a fallback path: it silently gives up connection
+// preservation, which is why the probe below warns loudly when it has to.
+const allowConnectedOnSaveFlag = "allow-connected-on-save"
+
+// minRunscVersionForAllowConnectedOnSave is the first runsc release that
+// defines allowConnectedOnSaveFlag. Named here for the operator-facing warning;
+// the probe reads the binary rather than trusting a version string.
+const minRunscVersionForAllowConnectedOnSave = "release-20260622.0"
+
+// runscFlagSupport memoizes capability probes keyed by runsc binary path.
+// Probing shells out, and cmdStart runs on every container start.
+var runscFlagSupport sync.Map // map[string]bool
+
+// supportsAllowConnectedOnSave reports whether the runsc binary at path defines
+// -allow-connected-on-save, probing it once per path so that a build which does
+// not define it starts actors instead of failing them.
+//
+// A false result is a degraded mode, not a supported configuration: actors will
+// lose established connections on suspend. It is reported at Error once per
+// binary rather than swallowed, and upgrading runsc is the actual fix.
+func supportsAllowConnectedOnSave(ctx context.Context, path string) bool {
+	if v, ok := runscFlagSupport.Load(path); ok {
+		return v.(bool)
+	}
+	supported := probeAllowConnectedOnSave(ctx, path)
+	if !supported {
+		slog.ErrorContext(ctx, "runsc does not support -"+allowConnectedOnSaveFlag+
+			"; actors on this build will lose established network connections across suspend/resume. Upgrade runsc to "+
+			minRunscVersionForAllowConnectedOnSave+" or later.",
+			slog.String("runsc", path),
+			slog.String("minVersion", minRunscVersionForAllowConnectedOnSave))
+	}
+	runscFlagSupport.Store(path, supported)
+	return supported
+}
+
+// probeAllowConnectedOnSave shells out to `runsc flags` and looks for the flag
+// in the listing. A probe that cannot run at all assumes the flag is present:
+// that reproduces the pre-probe behavior, so a build that does define the flag
+// keeps it, and one that does not fails `runsc start` with runsc's own error
+// rather than being silently downgraded on the strength of a failed probe.
+//
+// It must be `runsc flags`, not `runsc help start`: -allow-connected-on-save is
+// a top-level flag, and the per-subcommand usage that `help start` prints lists
+// only -h and -help even on builds that define it. Probing `help start` would
+// therefore report "unsupported" on every build and silently stop passing the
+// flag where it does work.
+func probeAllowConnectedOnSave(ctx context.Context, path string) bool {
+	cmd := exec.CommandContext(ctx, path, "flags")
+	// Usage goes to stdout on some builds and stderr on others; capture both.
+	var usage bytes.Buffer
+	cmd.Stdout = &usage
+	cmd.Stderr = &usage
+
+	if err := cmd.Run(); err != nil {
+		slog.WarnContext(ctx, "Could not probe runsc for -"+allowConnectedOnSaveFlag+"; assuming it is supported",
+			slog.String("runsc", path),
+			slog.Any("error", err))
+		return true
+	}
+
+	supported := bytes.Contains(usage.Bytes(), []byte(allowConnectedOnSaveFlag))
+	slog.InfoContext(ctx, "Probed runsc for -"+allowConnectedOnSaveFlag,
+		slog.String("runsc", path),
+		slog.Bool("supported", supported))
+	return supported
+}
+
 func (r *runsc) cmdStart(ctx context.Context, out io.Writer, containerName string) error {
 	reapLock.RLock()
 	defer reapLock.RUnlock()
 
 	slog.InfoContext(ctx, "About to run runsc start", slog.String("container", containerName))
 
-	cmd := exec.CommandContext(
-		ctx,
-		r.path,
+	args := []string{
 		"-log-format", "json",
 		"--alsologtostderr",
 		// "-debug",
 		// "-debug-log", ateompath.RunscDebugLogDir(r.actorUID, containerName)+"/",
 		// "-debug-to-user-log",
 		// "-log-packets",
 		// "-strace",
-		"-allow-connected-on-save",
+	}
+	if supportsAllowConnectedOnSave(ctx, r.path) {
+		args = append(args, "-"+allowConnectedOnSaveFlag)
+	}
+	args = append(args,
 		"-root", ateompath.RunSCStateDir(r.actorUID),
 		"start",
 		containerName, // Name of the container
 	)
+	cmd := exec.CommandContext(ctx, r.path, args...)
 	cmd.Stdout = out
 	cmd.Stderr = out
 

cmd/ateom-gvisor/runsc_test.go

@@ -0,0 +1,127 @@
+//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 (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// fakeRunsc writes an executable stub at a unique path and returns it. Each stub
+// gets its own path so the supportsAllowConnectedOnSave memo cannot leak between
+// test cases.
+func fakeRunsc(t *testing.T, script string) string {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "runsc")
+	if err := os.WriteFile(path, []byte("#!/bin/sh\n"+script), 0o755); err != nil {
+		t.Fatalf("writing fake runsc: %v", err)
+	}
+	return path
+}
+
+func TestProbeAllowConnectedOnSave(t *testing.T) {
+	tests := []struct {
+		name   string
+		script string
+		want   bool
+	}{
+		{
+			name:   "flag in stdout usage",
+			script: "echo '  -allow-connected-on-save  allow checkpoint with connected sockets'\n",
+			want:   true,
+		},
+		{
+			name:   "flag in stderr usage",
+			script: "echo '  -allow-connected-on-save' >&2\n",
+			want:   true,
+		},
+		{
+			name:   "flag absent from usage",
+			script: "echo '  -detach  detach from the container'\n",
+			want:   false,
+		},
+		{
+			// A build that cannot be probed is assumed to support the flag,
+			// preserving the behavior from before capability detection.
+			name:   "probe failure assumes supported",
+			script: "echo 'unknown command' >&2\nexit 1\n",
+			want:   true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := probeAllowConnectedOnSave(t.Context(), fakeRunsc(t, tt.script))
+			if got != tt.want {
+				t.Errorf("probeAllowConnectedOnSave = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+// The probe must ask runsc for its top-level flags. -allow-connected-on-save is
+// not a subcommand flag, so the per-subcommand usage `runsc help start` prints
+// lists only -h and -help even on builds that define it — probing that way
+// reports "unsupported" on every build and silently drops the flag where it
+// actually works. Real runsc builds were checked against this: the listing from
+// `runsc flags` matches the flag's presence in the binary exactly.
+func TestProbeAllowConnectedOnSaveUsesFlagsSubcommand(t *testing.T) {
+	dir := t.TempDir()
+	argsFile := filepath.Join(dir, "args")
+	path := filepath.Join(dir, "runsc")
+	script := "#!/bin/sh\nprintf '%s' \"$*\" > " + argsFile + "\n"
+	if err := os.WriteFile(path, []byte(script), 0o755); err != nil {
+		t.Fatalf("writing fake runsc: %v", err)
+	}
+
+	probeAllowConnectedOnSave(t.Context(), path)
+
+	got, err := os.ReadFile(argsFile)
+	if err != nil {
+		t.Fatalf("probe did not invoke runsc: %v", err)
+	}
+	if string(got) != "flags" {
+		t.Errorf("probe invoked `runsc %s`, want `runsc flags`", got)
+	}
+}
+
+func TestProbeAllowConnectedOnSaveMissingBinary(t *testing.T) {
+	missing := filepath.Join(t.TempDir(), "does-not-exist")
+	if !probeAllowConnectedOnSave(t.Context(), missing) {
+		t.Error("probeAllowConnectedOnSave on a missing binary = false, want true (fail open)")
+	}
+}
+
+// The probe shells out, and cmdStart runs on every container start, so the
+// result must be cached per binary path.
+func TestSupportsAllowConnectedOnSaveMemoizes(t *testing.T) {
+	path := fakeRunsc(t, "echo '  -"+allowConnectedOnSaveFlag+"'\n")
+	t.Cleanup(func() { runscFlagSupport.Delete(path) })
+
+	if !supportsAllowConnectedOnSave(t.Context(), path) {
+		t.Fatal("first call = false, want true")
+	}
+
+	// Replacing the stub with one that reports no such flag must not change the
+	// answer: a cached result means the binary is never re-probed.
+	if err := os.WriteFile(path, []byte("#!/bin/sh\necho '  -detach'\n"), 0o755); err != nil {
+		t.Fatalf("rewriting fake runsc: %v", err)
+	}
+	if !supportsAllowConnectedOnSave(t.Context(), path) {
+		t.Error("second call = false, want the memoized true")
+	}
+}