fix: detect -allow-connected-on-save support instead of assuming it - #713
Closed
Maya Wang (mayawang) wants to merge 1 commit into
Closed
fix: detect -allow-connected-on-save support instead of assuming it#713Maya Wang (mayawang) wants to merge 1 commit into
Maya Wang (mayawang) wants to merge 1 commit into
Conversation
2 tasks
ateom passes `runsc start -allow-connected-on-save` unconditionally. Builds older than runsc release-20260622.0 do not define it and reject the invocation with "flag provided but not defined", which fails every actor start on those builds. Probe the binary once per path with `runsc flags` and pass the flag only when that build lists it, so an old runsc starts actors rather than failing them. The result is memoized per binary path, since cmdStart runs on every container start and probing shells out. Dropping the flag is a degraded mode, not a fallback path. Older builds have no equivalent: only -net-disconnect-ok, which defaults to true and drops open connections on save. -allow-live-tcp-migration does not exist there either, and -save-restore-netstack is about netstack save/restore generally — deprecated and inert on current builds. So an actor on an old runsc keeps its connections today and loses them across suspend/resume, silently. The probe logs that at Error, once per binary, naming the minimum version rather than swallowing it. A probe that cannot run at all assumes the flag is present. That reproduces the pre-probe behavior: a build that defines the flag keeps it, and one that does not fails with runsc's own error rather than being downgraded on the strength of a failed probe. It has to 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 do define it. Probing there would report "unsupported" everywhere and silently stop passing a flag that works.
Maya Wang (mayawang)
force-pushed
the
fix/runsc-allow-connected-detection
branch
from
August 3, 2026 22:32
7853bfa to
781b7f3
Compare
Collaborator
Author
|
Closing this. I just learnt that the gVisor team has ongoing work on runsc save/restore flags, so rather than probing for capability on our side we'll move to a recent runsc build and pin a minimum supported version during GA. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ateom passes
runsc start -allow-connected-on-saveunconditionally. Some runscbuilds do not define the flag and reject the invocation with
flag provided but not defined, which fails every actor start on those builds.This probes the binary instead of assuming, so ateom works across runsc builds
with no operator configuration.
Changes
cmd/ateom-gvisor/runsc.go—supportsAllowConnectedOnSaveshells out torunsc flagsonce per binary path (memoized inrunscFlagSupport, sincecmdStartruns on every container start) and passes the flag only when thatbuild lists it. A probe that cannot run at all assumes the flag is supported,
preserving today's behavior wherever detection is inconclusive.
It has to be
runsc flags, notrunsc help start.-allow-connected-on-saveis a top-level flag, and the per-subcommand usage that
help startprints listsonly
-hand-helpeven on builds that define it — so probing the subcommandwould report "unsupported" everywhere and quietly stop passing a flag that
works.
runsc_test.goasserts the probe invokesflagsspecifically, becausethe stub binaries answer any argv and a regression back to
help startwouldotherwise pass.
Verification
go build ./...,go vet ./...,go test ./...— all pass.runsc_test.gocovers: flag in stdout usage, flag in stderr usage, flagabsent, probe failure fails open, missing binary fails open, per-path
memoization, and the
flags-subcommand assertion above.Validated against five real runsc binaries. The probe's verdict was
compared against ground truth — whether the flag is defined in the binary, and
whether the binary actually accepts it. It agrees on all five:
google-873132245(GKE-Sandbox)release-20260504.0release-20260622.0release-20260706.0release-20260706.0-21-g6d73c10dThe two rejecting builds fail any invocation with
flag provided but not defined— note both spellings, one build saysflags provided— so stringmatching on the error text is not a reliable substitute for the flag listing.
The original failure was hit e2e on a GKE-Sandbox build that rejects the flag.
At that time it was worked around with an explicit opt-out; the automatic
probe replaces it, validated as above.
Out of scope
The gVisor
SandboxConfigchange that pointsrunsc.urlat a specific build isdeliberately not included — it is environment-specific (a private bucket and
build SHA) and does not belong upstream. The portable finding is this probe.
Fixes #<issue_number_goes_here>