1
0
mirror of https://github.com/sredevopsorg/deepseek-kubernetes-sandbox.git synced 2026-11-03 01:13:24 +00:00

Fix idle sweeper suspending claims the gateway does not own (#4)

The sweeper listed every SandboxClaim in NAMESPACE and treated each
dsh-<user> name as one of its own users. A claim applied by hand from
the resource-layer manifests (e.g. 30-claim-alice.yaml) therefore looked
exactly like a gateway user: the idle clock started on first sighting
and the pod was suspended after IDLE_TIMEOUT. That deletes the pod the
README's step 3 logs/port-forward targets, so the walkthrough broke as
soon as the gateway was deployed next to it.

Mark claims created by create_claim() with
aaas.example.com/managed-by=dsh-gateway and skip everything else in
sweep_once() before any idle bookkeeping happens. Document the
ownership boundary in the example docs, the manifest and ADR-5, add
unit coverage for the marker and the skipped sweep, and assert in
run-test-kind.sh that alice's hand-applied claim stays Running while
bob is correctly idle-suspended.

Co-authored-by: SREDevOpsAgents <agents@sredevops.org>
This commit is contained in:
SREDevOps.org Agent
2026-10-21 13:27:19 -03:00
committed by GitHub
parent 6301c21bea
commit 29eefd6ae5
7 changed files with 107 additions and 7 deletions

View File

@@ -101,7 +101,10 @@ spec:
value: dsh-pool
# Seconds a sandbox may sit idle before the sweeper suspends it
# (pod deleted, PVC kept), and the max time a proxy request waits
# for a suspended sandbox to become Ready again.
# for a suspended sandbox to become Ready again. Only claims this
# gateway created are swept: a claim applied by hand from the
# resource-layer manifests carries no aaas.example.com/managed-by
# annotation and is never suspended.
- name: IDLE_TIMEOUT
value: "60"
- name: WAKE_TIMEOUT

View File

@@ -130,6 +130,12 @@ kubectl -n dsh-demo port-forward "pod/$SB" 18081:3080
> never `3080`. `localhost` is always in DSH's trusted-host list, so the forwarded request is
> accepted.
> **Why the pod stays up:** this claim was applied by hand, so it carries no gateway ownership
> annotation (`aaas.example.com/managed-by`). The gateway's idle sweeper only suspends claims it
> created through `POST /users`; the resource-layer claims from these manifests are never
> touched. You can deploy the gateway (step 8) and still keep a sandbox you are port-forwarding
> to.
## 4. Write state on the PVC
```bash
@@ -307,6 +313,7 @@ redirects `/u/<user>` to the trailing-slash form with `308` so relative URLs res
| WebSocket closes immediately (`4401`) | Missing/expired gateway cookie, or the upgrade's `Origin` does not match `Host` (CSWSH check); DSH rejecting `Host` instead surfaces as close `1011` — add the host to `DSH_TRUSTED_HOSTS` |
| Signup is slow / `202` then `Waking` | Warm pool exhausted (`kubectl get sandboxes -l agents.x-k8s.io/warm-pool-sandbox`); scale the pool |
| `403` from the Kubernetes API in gateway logs | The gateway ServiceAccount is missing claim/sandbox/pod-log verbs (see `50-gateway.yaml`) |
| Step 3: `pod "$SB" not found` after deploying the gateway | The claim was suspended by the idle sweeper. Claims created by the gateway carry `aaas.example.com/managed-by=dsh-gateway`; hand-applied claims (steps 1–7) do not and are never swept. Upgrade the gateway to a build that honors the marker, or resume the sandbox: `kubectl -n dsh-demo patch sandbox "$SB" --type merge -p '{"spec":{"operatingMode":"Running"}}'` |
## Limitations

View File

@@ -112,7 +112,7 @@ can also be injected into `create_app` for tests.
| --- | --- | --- |
| `NAMESPACE` | `dsh-demo` | Namespace holding the claims, sandboxes and pods. |
| `POOL` | `dsh-pool` | `warmPoolRef.name` for new claims. |
| `IDLE_TIMEOUT` | `60` | Seconds of inactivity before the sweeper suspends a sandbox. |
| `IDLE_TIMEOUT` | `60` | Seconds of inactivity before the sweeper suspends a sandbox. Only claims the gateway created (annotation `aaas.example.com/managed-by=dsh-gateway`) are ever suspended. |
| `WAKE_TIMEOUT` | `120` | Seconds to wait for a sandbox to become Ready (signup and wake-on-connect). |
| `DSH_PORT` | `3080` | Upstream DSH port. |
| `DSH_CONTAINER` | `dsh-web` | Container whose logs carry the launch token. |

View File

@@ -69,6 +69,12 @@ from starlette.websockets import WebSocket
GROUP, VERSION = "extensions.agents.x-k8s.io", "v1beta1"
CORE_GROUP = "agents.x-k8s.io"
TOKEN_ANNOTATION = "aaas.example.com/token-sha256"
# Ownership marker: the gateway shares its namespace with claims applied by
# hand (the resource-layer walkthrough in the example README). Every claim is
# named "dsh-<user>", so without an explicit marker the sweeper cannot tell its
# own users from a claim someone created manually.
MANAGED_ANNOTATION = "aaas.example.com/managed-by"
MANAGED_BY = "dsh-gateway"
POD_NAME_ANNOTATION = "agents.x-k8s.io/pod-name"
USER_LABEL = "sandbox.users.io/dsh-user"
@@ -149,6 +155,18 @@ def claim_user(claim: dict) -> str:
return claim["metadata"]["name"].removeprefix("dsh-")
def gateway_managed(claim: dict) -> bool:
"""True only for claims this gateway created.
The sweeper must not suspend claims it does not own: a claim applied from
a resource-layer manifest (e.g. ``30-claim-alice.yaml``) has the same
``dsh-<user>`` name but no ownership marker, and suspending it deletes a
pod the operator is using.
"""
annotations = (claim.get("metadata") or {}).get("annotations") or {}
return annotations.get(MANAGED_ANNOTATION) == MANAGED_BY
def condition(obj: dict, ctype: str) -> bool:
for c in (obj.get("status") or {}).get("conditions", []):
if c.get("type") == ctype:
@@ -372,7 +390,10 @@ class Kubernetes:
"kind": "SandboxClaim",
"metadata": {
"name": claim_name(user),
"annotations": {TOKEN_ANNOTATION: token_hash},
"annotations": {
TOKEN_ANNOTATION: token_hash,
MANAGED_ANNOTATION: MANAGED_BY,
},
},
"spec": {
"warmPoolRef": {"name": pool},
@@ -1169,6 +1190,11 @@ against the JSON API. The browser session is an HttpOnly cookie.</p>
async def sweep_once(self) -> None:
claims = await self.k8s.list_claims()
for claim in claims:
# Only suspend claims this gateway created. Everything else in
# the namespace (resource-layer claims from the README walkthrough,
# other controllers' objects) is not ours to manage.
if not gateway_managed(claim):
continue
user = claim_user(claim)
try:
sandbox = await self._sandbox_of(claim)

View File

@@ -29,6 +29,7 @@ from gateway import (
derive_state,
extract_dsh_cookies,
filtered_query,
gateway_managed,
parse_dsh_launch_token,
rewrite_location,
same_origin,
@@ -318,6 +319,15 @@ def test_claim_name_and_user_roundtrip():
assert claim_user({"metadata": {"name": "dsh-alice"}}) == "alice"
def test_gateway_managed_requires_ownership_annotation():
managed = {"metadata": {"annotations": {"aaas.example.com/managed-by": "dsh-gateway"}}}
manual = {"metadata": {"name": "dsh-alice"}}
other = {"metadata": {"annotations": {"aaas.example.com/managed-by": "someone"}}}
assert gateway_managed(managed) is True
assert gateway_managed(manual) is False
assert gateway_managed(other) is False
def test_parse_dsh_launch_token_from_real_log_line():
log = (
"2026-10-21T10:00:00Z starting\n"
@@ -443,6 +453,10 @@ async def test_kubernetes_create_claim_body(monkeypatch):
body = args[4]
assert body["metadata"]["name"] == "dsh-alice"
assert body["metadata"]["annotations"]["aaas.example.com/token-sha256"] == "hash"
assert (
body["metadata"]["annotations"]["aaas.example.com/managed-by"]
== "dsh-gateway"
)
assert body["spec"]["warmPoolRef"]["name"] == "test-pool"
assert body["spec"]["additionalPodMetadata"]["labels"][
"sandbox.users.io/dsh-user"
@@ -1159,7 +1173,10 @@ def test_ws_upstream_connect_failure_closes_1011(env):
def ready_claim(user="alice"):
return {
"metadata": {"name": claim_name(user)},
"metadata": {
"name": claim_name(user),
"annotations": {"aaas.example.com/managed-by": "dsh-gateway"},
},
"status": {"sandbox": {"name": f"dsh-{user}-sbx"}},
}
@@ -1172,6 +1189,33 @@ def ready_sandbox(user="alice"):
}
@pytest.mark.asyncio
async def test_sweeper_skips_claim_it_does_not_own(env):
"""A hand-applied claim (e.g. README 30-claim-alice.yaml) must not be swept.
The gateway shares the namespace with the resource-layer walkthrough; those
claims are named dsh-<user> too, but carry no ownership annotation. The
sweeper used to suspend them 60s after first sighting, deleting the pod that
README step 3 port-forwards to.
"""
env.k8s.claims["alice"] = {
"metadata": {"name": claim_name("alice")},
"status": {"sandbox": {"name": "dsh-alice-sbx"}},
}
env.k8s.sandboxes["dsh-alice-sbx"] = ready_sandbox()
# The sweeper never starts an idle clock for a claim it does not own...
await env.gateway.sweep_once()
assert "alice" not in env.gateway.state.last_activity
# ...and does not suspend it even when it looks long-idle.
env.gateway.state.last_activity["alice"] = (
time.time() - env.settings.idle_timeout - 10
)
await env.gateway.sweep_once()
assert env.k8s.patches == []
@pytest.mark.asyncio
async def test_sweeper_suspends_idle_ready_sandbox(env):
env.k8s.claims["alice"] = ready_claim()

View File

@@ -228,6 +228,15 @@ PY
[ "$STATE" = "Suspended" ] || fail "bob was not idle-suspended (state=$STATE)"
pass "idle sweeper suspended bob"
# Regression: alice's claim was applied by hand in step 2, so it has no
# aaas.example.com/managed-by annotation. The sweeper must leave it alone -
# even though the same idle window just suspended bob. If it does not, README
# step 3 (logs/port-forward against alice's pod) breaks.
ALICE_MODE=$(kubectl -n "$NS" get sandbox "$SB" -o jsonpath='{.spec.operatingMode}')
[ "$ALICE_MODE" = "Running" ] \
|| fail "sweeper suspended a claim it does not own (alice: $ALICE_MODE)"
pass "idle sweeper left the hand-applied alice claim Running"
T0=$(date +%s)
curl -sf -m 150 -b "$JAR" "http://$GW/u/bob/" >/dev/null \
|| fail "wake-on-connect request failed"