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:
committed by
GitHub
parent
6301c21bea
commit
29eefd6ae5
@@ -133,6 +133,13 @@ sequenceDiagram
|
||||
Suspend deletes only the pod. The PVC keeps conversations, sessions, and config, so resume
|
||||
reattaches exactly the same state.
|
||||
|
||||
**Ownership boundary.** The gateway shares its namespace with claims created outside it — the
|
||||
example's resource-layer walkthrough applies `dsh-alice` by hand, and every claim is named
|
||||
`dsh-<user>`. Claims the gateway creates carry `aaas.example.com/managed-by=dsh-gateway`; the
|
||||
sweeper skips everything else, so it can never suspend a sandbox an operator is using. Without
|
||||
that marker the sweeper would suspend a hand-applied claim 60s after first sighting and break
|
||||
`kubectl logs`/`port-forward` against its pod.
|
||||
|
||||
### WebSocket
|
||||
|
||||
```mermaid
|
||||
@@ -210,11 +217,15 @@ sequenceDiagram
|
||||
### ADR-5: Claims as the user record
|
||||
|
||||
- **Decision:** a user is a `SandboxClaim` named `dsh-<user>`; the bearer-token SHA-256 lives in
|
||||
a claim annotation.
|
||||
a claim annotation, and the gateway marks its own claims with
|
||||
`aaas.example.com/managed-by=dsh-gateway`.
|
||||
- **Why:** no database, no separate account service; deleting the claim is account deletion and
|
||||
cascades everything.
|
||||
cascades everything. The ownership annotation keeps a namespaced claim list safe to share with
|
||||
objects created by hand or by other controllers.
|
||||
- **Cost:** no password reset, no admin API, no audit log of its own; Kubernetes API load grows
|
||||
with users. Fine for a reference pattern, not for a product.
|
||||
with users. Claims created before the ownership marker existed must be annotated
|
||||
(`kubectl annotate sandboxclaim ...`) to keep being swept. Fine for a reference pattern, not
|
||||
for a product.
|
||||
|
||||
### ADR-6: Gateway cookie session instead of bearer tokens in the browser
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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. |
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user