fix(sandbox-kubernetes): make SandboxClaim ownership configurable so release can keep the pod alive - #2779
Open
Buktal wants to merge 2 commits into
Open
Conversation
…release can keep the pod alive
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
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.
AgentScope-Java Version
2.0.1 (developed against current
main,bf7b7dae)Description
Fixes #2778.
Background.
KubernetesSandboxClient.create()hardcodesclaimOwned = true, soKubernetesSandbox.shutdown()terminates the claim on every per-call release. With a shared isolation scope (USER/AGENT/GLOBAL) and the default no-opSandboxExecutionGuard, two concurrent calls resume the same live claim and share one pod; the first call's release then terminates that pod underneath the other in-flight call, whose subsequent exec / file operations all fail. There is currently no supported way to opt out:claimOwnedis only ever set insidecreate().Change. Claim ownership becomes a client option instead of a hardcoded constant — the existing
claimOwned = falsecode path (shutdown()only closes the connection, pod stays alive) becomes reachable through configuration:KubernetesSandboxClientOptions.claimOwned(Boolean,null= defaulttrue), merged like every other option (call-level overrides defaults)KubernetesFilesystemSpec.claimOwned(boolean)fluent settercreate()seeds the state from the merged option (state init extracted into package-privateinitState(...)so the wiring is unit-testable without a cluster)resume()applies an explicitly configured option to the resumed state (applyConfiguredClaimOwnership), so states persisted before the option existed — including slots shared through a distributedAgentStateStore— also honour an operator opting out; when unset, the persisted flag winsIsolationScope's concurrency note previously claimed concurrent same-scope calls "each get their own running container", which is not what the implementation does; it now describes the actual behaviour and points at the two mitigations (execution guard, non-owned claims) — split into its owndocs(harness)commitclaimOwnedrow in theKubernetesFilesystemSpecoptions tableDefault behaviour is unchanged (
claimOwned = true). Opting in tofalsemoves claim deletion to the caller (e.g. an idle-eviction policy), which is what same-scope concurrent sharing needs.How to test. New
KubernetesSandboxClientTest(7 cases) covers default-owned, option-drivenfalse, call-level override, resume-time ownership application in both directions, and the spec passthrough;KubernetesSandboxTestgains direct coverage of the shutdown branch (false→closeConnection()only,true→terminate());KubernetesSandboxStateSerdeTestgains aclaimOwned = falseround-trip. Module suite:mvn -pl agentscope-extensions/agentscope-extensions-sandbox/agentscope-extensions-sandbox-kubernetes test— all passing; full reactormvn testalso green.Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)