diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml b/openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml new file mode 100644 index 0000000..38f7628 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-22 diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/design.md b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md new file mode 100644 index 0000000..1ebab69 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/design.md @@ -0,0 +1,167 @@ +## Context + +The CLI MCP Server architecture has two binaries: the MCP server (control plane) and the sandbox agent (data plane). SANDBOX-1806/1807 delivered the agent side — persistent bash session, HTTP handlers, and entry point. SANDBOX-1810 adds the server-side session management layer — the bridge between incoming MCP tool calls (identified by `X-Session-ID`) and the agent pods that execute commands. + +The MCP server is stateless — any replica can serve any request. All durable state lives in Kubernetes labels on the sandbox pods. The session manager provides a thin caching layer (30s TTL) to avoid per-request K8s API calls while remaining correct across replicas (label-based discovery is the source of truth; cache is an optimization). + +## Goals / Non-Goals + +**Goals:** +- PodCache with thread-safe map and TTL-based expiry (~30s) +- SessionManager with `GetOrCreatePod` (label lookup, cache, idempotent create-or-get for pod + auth Secret using K8s API conflict handling, wait for ready) +- SessionManager with `ExecuteCommand` (resolve pod, HMAC-SHA256 bearer token, POST to agent, update last-activity annotation) +- SessionManager with `CleanupSession` (delete pod + Secret) +- SessionManager with `CleanupStale` (periodic, idle-timeout based) +- Pod spec includes: security context (RunAsNonRoot, drop ALL), readiness probe on :8090/health, kubeconfig volume, workspace emptyDir, correct labels and annotations +- SandboxConfig struct for all tunables (image, CPU/mem, idle timeout, HMAC key, warm pool size, namespace, service account, kubeconfig secret name) + +**Non-Goals:** +- Warm pod pool (SANDBOX-1811) +- Agent HTTP client abstraction (SANDBOX-1812 — the manager uses `net/http` directly for now) +- MCP tool registration (SANDBOX-1813) +- MCP server entry point (SANDBOX-1814) +- Server unit tests beyond those in this PR (SANDBOX-1816) +- Metrics or structured logging middleware +- Retry logic for pod creation failures (single attempt; can be added later) + +## Decisions + +### Decision 1: In-memory PodCache with short TTL (30s) + +Use a simple `sync.RWMutex`-protected map with 30-second TTL entries. + +**Rationale:** +- The stateless MCP server can't rely on per-session state across requests — but a short-lived cache avoids a K8s API call on every single command execution within a burst +- 30s TTL means stale entries self-correct quickly if a pod gets a new IP (crash/restart) +- Conditional invalidation (`Invalidate(sessionID, podIP)`) ensures we don't accidentally evict a freshly-updated entry after a reconnect +- No external dependencies (Redis, memcached) — simplicity first + +**Alternative considered:** No cache — always query K8s API +- Rejected — K8s API calls add latency (~50-100ms) to every command execution; with TTL cache, only the first request per 30s window hits the API + +**Alternative considered:** Longer TTL (5 minutes) +- Rejected — if a pod restarts and gets a new IP, requests would fail for up to 5 minutes before cache expiry forces rediscovery + +### Decision 2: Idempotent `GetOrCreatePod` using K8s API conflict handling + +Use deterministic resource names (derived from session ID) and handle `AlreadyExists` errors from the Kubernetes API to make pod + Secret creation idempotent across replicas. + +**Rationale:** +- With a stateless multi-replica server, two requests for the same `X-Session-ID` can both miss the cache and labels, then race into the create path +- By using deterministic names (`cli-mcp-sandbox-`, `cli-mcp-sandbox-auth-`) and treating `AlreadyExists` as success (fall through to discovery), only one set of resources is created +- The Kubernetes API's built-in atomicity on create operations provides the serialization point — no external locking needed +- The losing replica simply re-discovers the pod that the winning replica created + +**Alternative considered:** Distributed lock (e.g., K8s Lease) before creation +- Rejected — adds complexity, latency, and a failure mode (lock not released) for a race that rarely occurs and is harmless when handled via conflict detection + +### Decision 3: HMAC-SHA256 deterministic token derivation + +Compute per-session auth tokens as `hex(HMAC-SHA256(HMACKey, sessionID))`. + +**Rationale:** +- Any MCP server replica can derive the same token for a given session without storing or looking up tokens +- Deterministic — same key + session ID always yields same token, enabling stateless operation +- Standard cryptographic construction; 256-bit output provides ample security margin +- The token is delivered to the sandbox agent via a K8s Secret (env var injection), so the agent never computes it — it just stores and compares + +**Alternative considered:** Random token stored in Secret, looked up by MCP server on each request +- Rejected — requires a K8s API call to read the Secret on each command execution, defeating the purpose of caching + +### Decision 4: Label-based pod discovery as source of truth + +Use label selectors (`tarsy.redhat.com/session-id=, tarsy.redhat.com/component=cli-mcp-sandbox`) for pod lookup. + +**Rationale:** +- Labels are indexed in etcd — list-by-label is efficient +- Any replica can discover any session's pod without shared state +- If cache misses (TTL expired, replica restart, first request), the K8s API call provides the correct answer +- Matches the parent design doc's architecture + +**Alternative considered:** In-memory session-to-pod mapping as primary store +- Rejected — breaks stateless multi-replica operation; a different replica wouldn't know about a pod another replica created + +### Decision 5: Create auth Secret before pod + +Create the per-session Secret first, then create the pod that references it via `secretKeyRef`. + +**Rationale:** +- The pod spec references the Secret via env var injection (`SANDBOX_AUTH_TOKEN` from `secretKeyRef`). If the pod starts before the Secret exists, the container will fail to start with `CreateContainerConfigError`. +- Secret-first ordering ensures the pod's env var resolution succeeds on first attempt +- If pod creation fails, we best-effort delete the orphaned Secret + +**Alternative considered:** Create pod first, then Secret +- Rejected — race condition where the container starts before the Secret exists + +### Decision 6: Poll-based `waitForReady` with 60s deadline + +Poll the pod status every 2 seconds for up to 60 seconds until Running + PodReady condition. + +**Rationale:** +- Pod startup time is typically 3-8 seconds (image pull is fast with pre-cached layers on nodes) +- 60s deadline accommodates cold image pulls or scheduling delays +- 2s poll interval is a good balance between latency and K8s API load +- Simple to implement and reason about; watch-based would add complexity for marginal latency improvement + +**Alternative considered:** Watch-based with informer +- Rejected — adds significant complexity; session creation is not a hot path (happens once per investigation), so poll latency is acceptable + +### Decision 7: Background annotation update after ExecuteCommand + +Update `last-activity` annotation in a background goroutine (`go m.updateLastActivity()`). + +**Rationale:** +- The annotation update is best-effort — it's used for stale cleanup but not for correctness +- Executing the patch synchronously would add ~50ms latency to every command response +- If the patch fails (e.g., pod deleted between exec and patch), it's logged and harmless + +**Alternative considered:** Synchronous annotation update +- Rejected — adds latency to the critical path for non-critical metadata + +### Decision 8: SessionManager directly uses `net/http` for agent communication + +The manager constructs HTTP requests inline rather than depending on a separate agent client package. + +**Rationale:** +- SANDBOX-1812 will introduce `pkg/agent/client.go` as a proper abstraction; the manager will be refactored to use it +- For now, inline HTTP keeps the session manager self-contained with zero internal dependencies beyond `pkg/agent` types +- The HTTP call is simple: POST JSON body, bearer header, decode JSON response + +**Alternative considered:** Block on SANDBOX-1812 first +- Rejected — creates unnecessary sequencing; inline HTTP can be replaced later with a one-line refactor + +### Decision 9: `CleanupStale` is caller-driven (no internal ticker) + +The `CleanupStale` method is called externally (e.g., by a goroutine in `cmd/server/main.go` on a ticker). It does not manage its own lifecycle. + +**Rationale:** +- Keeps the SessionManager testable — tests call `CleanupStale()` directly without dealing with timers +- The caller (server entry point) owns the ticker and can cancel it on shutdown +- Consistent with the design doc's "stale pod cleanup goroutine" in `runServer` + +**Alternative considered:** Internal background goroutine with ticker +- Rejected — harder to test, harder to shut down cleanly, mixes lifecycle management with business logic + +## Risks / Trade-offs + +**Risk:** 30s cache TTL means requests can be routed to a dead pod IP for up to 30s after pod restart +→ **Mitigation:** `Invalidate()` is called on transport-level failures (connection refused, timeout, network unreachable), immediately clearing the stale entry and forcing rediscovery on the next request. Application-level HTTP errors (4xx/5xx from a reachable agent) do not invalidate the cache, since the pod is healthy. + +**Risk:** `waitForReady` polls every 2s — adds 0-2s latency to pod creation +→ **Mitigation:** Acceptable for session creation (happens once per investigation). The warm pool feature (SANDBOX-1811) eliminates this latency entirely for pre-warmed pods. + +**Risk:** Background `updateLastActivity` can silently fail +→ **Mitigation:** Logged at WARN level. Worst case: a pod gets cleaned up slightly before its true idle timeout. The pod would be recreated on the next request. + +**Trade-off:** Manager uses inline `net/http` instead of `pkg/agent/client` +→ **Accepted:** Will be refactored in SANDBOX-1812. Keeps this story self-contained. + +**Trade-off:** Concurrent commands for the same session queue at the agent +→ **Accepted:** The session manager is stateless and POSTs each command independently. The sandbox agent serializes execution via its `BashSession` mutex (single persistent bash process). If N parallel requests arrive, they queue and execute sequentially — no interleaving. This means the Nth request's wall-clock latency includes waiting for the previous N-1 commands. This is the correct behavior for a single bash process; users who need parallelism can use shell-level constructs (`&`, `&&`) within a single command. + +**Trade-off:** No retry on pod creation failure +→ **Accepted:** Single attempt returns error to caller. The MCP tool handler will propagate the error to TARSy, which can retry the tool call. + +## Open Questions + +None — all design decisions are resolved. diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md b/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md new file mode 100644 index 0000000..3332b35 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/proposal.md @@ -0,0 +1,46 @@ +## Why + +The MCP server needs to manage sandbox pod lifecycle — creating pods on demand, caching their IPs for fast routing, executing commands via the agent HTTP API, and cleaning up idle sessions. Without a session manager, the server has no way to map an incoming `X-Session-ID` header to a running sandbox pod. Without a cache, every request would require a Kubernetes API call for pod discovery. Without cleanup, abandoned pods would accumulate indefinitely. + +SANDBOX-1806/1807 delivered the sandbox agent (persistent bash + HTTP handlers). This story builds the MCP server-side counterpart that manages the agent pods from the outside. + +## What Changes + +- Add `pkg/session/config.go` with: + - `SandboxConfig` struct holding all tunables (image, CPU/mem requests/limits, idle timeout, HMAC key, warm pool size, namespace, service account, kubeconfig secret name) + - `DefaultConfig()` factory with sensible production defaults + +- Add `pkg/session/cache.go` with: + - `PodCache` struct — thread-safe `sync.RWMutex`-protected map of session ID → (pod IP, pod name, expiry) + - TTL-based expiry (~30s default) — expired entries return cache miss + - `Get`, `Set`, `Delete`, `Invalidate` (conditional on matching pod IP), `EvictExpired` + +- Add `pkg/session/manager.go` with: + - `SessionManager` struct holding Kubernetes clientset, config, cache, HTTP client, logger + - `GetOrCreatePod(ctx, sessionID)` — cache → label lookup → create pod + auth Secret → wait for ready + - `ExecuteCommand(ctx, sessionID, command, timeout)` — resolve pod, HMAC-SHA256 bearer token, POST to agent, update last-activity annotation + - `CleanupSession(ctx, sessionID)` — delete pod + Secret, clear cache + - `CleanupStale(ctx)` — iterate sandbox pods, delete those idle longer than IdleTimeout (delegates to `CleanupSession`, which removes both the pod and the matching auth Secret) + - `buildPodSpec(sessionID)` — full pod manifest with security context, readiness probe, volumes, labels, annotations + - `buildAuthSecret(sessionID, token)` — per-session Secret for HMAC token delivery + - `computeToken(sessionID)` — HMAC-SHA256(HMACKey, sessionID) deterministic token derivation + +## Capabilities + +### New Capabilities +- `pod-cache`: Defines the in-memory TTL cache for pod IP lookups with thread-safe access and conditional invalidation +- `session-manager`: Defines the session manager responsible for pod lifecycle (create, discover, proxy, cleanup) using Kubernetes labels as the source of truth + +### Modified Capabilities + + +## Impact + +- **Affected code**: + - `pkg/session/config.go` — New file with SandboxConfig + - `pkg/session/cache.go` — New file with PodCache + - `pkg/session/manager.go` — New file with SessionManager +- **API changes**: New Go API in `pkg/session` package. No external HTTP endpoints added (those come in SANDBOX-1814). +- **Dependencies**: Depends on SANDBOX-1806 (`pkg/agent` types). Depended on by SANDBOX-1811 (warm pod pool), SANDBOX-1812 (agent HTTP client — provides the transport layer the manager uses), SANDBOX-1813 (bash tool handler), SANDBOX-1814 (MCP server entry point). +- **New Go dependencies**: `k8s.io/client-go`, `k8s.io/api`, `k8s.io/apimachinery` +- **No breaking changes** — entirely new package. diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md new file mode 100644 index 0000000..e7b6562 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/pod-cache/spec.md @@ -0,0 +1,50 @@ +## ADDED Requirements + +### Requirement: PodCache provides thread-safe in-memory caching with TTL + +The `PodCache` struct SHALL map session IDs to pod IPs and names using a `sync.RWMutex`-protected map with per-entry TTL expiry. + +#### Scenario: Set and Get within TTL +- **WHEN** `Set` is called with sessionID "inv-1", podIP "10.0.0.1", podName "pod-abc" +- **AND** `Get` is called for "inv-1" within the TTL window +- **THEN** `Get` SHALL return podIP "10.0.0.1", podName "pod-abc", and ok=true + +#### Scenario: Get returns miss for unknown session +- **WHEN** `Get` is called for a session ID that was never stored +- **THEN** it SHALL return empty strings and ok=false + +#### Scenario: Get returns miss after TTL expiry +- **WHEN** `Set` is called and the TTL duration elapses +- **AND** `Get` is called for the same session ID +- **THEN** it SHALL return empty strings and ok=false + +#### Scenario: Delete removes entry unconditionally +- **WHEN** `Delete` is called with a session ID +- **THEN** subsequent `Get` calls for that session SHALL return ok=false + +#### Scenario: Invalidate removes entry only if pod IP matches +- **WHEN** `Invalidate` is called with sessionID and podIP +- **AND** the cached entry has the same podIP +- **THEN** the entry SHALL be removed + +#### Scenario: Invalidate preserves entry if pod IP differs +- **WHEN** `Invalidate` is called with sessionID and a podIP different from the cached entry +- **THEN** the cached entry SHALL NOT be removed + +#### Scenario: EvictExpired removes all expired entries +- **WHEN** `EvictExpired` is called +- **THEN** all entries whose TTL has expired SHALL be removed +- **AND** entries whose TTL has not expired SHALL be preserved +- **AND** the count of evicted entries SHALL be returned + +#### Scenario: Default TTL is 30 seconds +- **WHEN** `NewPodCache` is called with ttl <= 0 +- **THEN** it SHALL use a default TTL of 30 seconds + +#### Scenario: Concurrent access is safe +- **WHEN** `Get`, `Set`, `Delete`, and `Invalidate` are called concurrently from multiple goroutines +- **THEN** all operations SHALL be serialized via the RWMutex without data races + +#### Scenario: Set overwrites existing entries +- **WHEN** `Set` is called for a session ID that already has a cached entry +- **THEN** the entry SHALL be replaced with the new podIP, podName, and a fresh TTL diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md new file mode 100644 index 0000000..6eed863 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/specs/session-manager/spec.md @@ -0,0 +1,166 @@ +## ADDED Requirements + +### Requirement: SandboxConfig holds all tunables with sensible defaults + +The `SandboxConfig` struct SHALL hold all configurable parameters for sandbox pod lifecycle, and `DefaultConfig()` SHALL return production-ready defaults. + +#### Scenario: DefaultConfig provides production defaults +- **WHEN** `DefaultConfig()` is called +- **THEN** CPURequest SHALL be "100m", CPULimit "500m", MemoryRequest "128Mi", MemoryLimit "512Mi" +- **AND** IdleTimeout SHALL be 30 minutes +- **AND** WarmPoolSize SHALL be 0 (disabled) +- **AND** Namespace SHALL be "tarsy" +- **AND** ServiceAccountName SHALL be "cli-mcp-investigation-sa" +- **AND** KubeconfigSecret SHALL be "cli-mcp-investigation-kubeconfig" + +### Requirement: Session IDs are validated before use in Kubernetes resource names + +Session IDs are interpolated into Kubernetes resource names (pod names, Secret names) and label values. They MUST be validated or normalized before use to ensure compatibility with Kubernetes naming rules. + +#### Scenario: Valid session ID is accepted +- **WHEN** a session ID matches the pattern `^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$` (RFC 1123 DNS label) +- **THEN** it SHALL be accepted and used directly in resource names + +#### Scenario: Invalid session ID is rejected +- **WHEN** a session ID contains characters outside `[a-z0-9-]`, starts/ends with a hyphen, or exceeds 63 characters +- **THEN** `GetOrCreatePod` SHALL return a validation error without making any K8s API calls + +### Requirement: GetOrCreatePod resolves or creates a sandbox pod for a session + +The `GetOrCreatePod` method SHALL implement a three-tier lookup: cache → label-based K8s API discovery → idempotent create-or-get new pod (handling `AlreadyExists` conflicts from concurrent replicas). + +#### Scenario: Cache hit returns cached pod IP +- **WHEN** `GetOrCreatePod` is called for a session with a valid cache entry +- **THEN** it SHALL return the cached pod IP without querying the K8s API + +#### Scenario: Cache miss triggers label-based discovery +- **WHEN** `GetOrCreatePod` is called for a session with no cache entry +- **AND** a Ready pod exists with matching session-id label (PodReady condition is true) +- **THEN** it SHALL return the pod's IP and cache the result +- **AND** if multiple matching Ready pods exist, it SHALL select the oldest by creation timestamp for deterministic behavior + +#### Scenario: Cache miss finds non-Ready pod +- **WHEN** `GetOrCreatePod` is called for a session with no cache entry +- **AND** a non-terminal pod exists with matching session-id label (Pending or Running, but PodReady is false) +- **AND** no Ready pod exists for the session +- **THEN** it SHALL wait for the existing pod to become Ready (reusing `waitForReady` with the 60s deadline) +- **AND** return the pod's IP and cache the result once Ready + +#### Scenario: No existing pod triggers creation +- **WHEN** `GetOrCreatePod` is called for a session with no cache entry +- **AND** no non-terminal pod exists with matching session-id label (either no pods, or only Failed/Succeeded pods) +- **THEN** it SHALL create an auth Secret, create a sandbox pod, wait for ready, cache the result, and return the pod IP + +#### Scenario: Concurrent create is idempotent via conflict handling +- **WHEN** two replicas simultaneously attempt to create a pod for the same session ID +- **AND** one replica's K8s create call returns `AlreadyExists` +- **THEN** the losing replica SHALL fall through to label-based discovery and return the existing pod's IP +- **AND** no duplicate pods or Secrets SHALL be created + +#### Scenario: Pod creation creates auth Secret first +- **WHEN** a new sandbox pod is created for a session +- **THEN** the per-session auth Secret SHALL be created before the Pod +- **AND** the Secret name SHALL be `cli-mcp-sandbox-auth-` +- **AND** the Secret SHALL contain the HMAC-derived token in the `token` key + +#### Scenario: Pod creation waits for readiness +- **WHEN** a new sandbox pod is created +- **THEN** `GetOrCreatePod` SHALL poll the pod status every 2 seconds +- **AND** return success only when the pod is Running with PodReady condition true +- **AND** timeout after 60 seconds with an error + +### Requirement: Pod spec follows security and operational standards + +The pod manifest built by `buildPodSpec` SHALL include all required security context, volumes, probes, and labels from the design doc. + +#### Scenario: Pod has correct labels and annotations +- **WHEN** `buildPodSpec` is called with a session ID +- **THEN** the pod SHALL have label `tarsy.redhat.com/session-id` = sessionID +- **AND** label `tarsy.redhat.com/component` = "cli-mcp-sandbox" +- **AND** annotation `tarsy.redhat.com/created-at` = current UTC time in RFC3339 +- **AND** annotation `tarsy.redhat.com/last-activity` = current UTC time in RFC3339 + +#### Scenario: Pod has restrictive security context +- **WHEN** `buildPodSpec` is called +- **THEN** the pod-level SecurityContext SHALL set RunAsNonRoot=true, RunAsUser=1001, RunAsGroup=1001 +- **AND** the container-level SecurityContext SHALL set AllowPrivilegeEscalation=false +- **AND** the container-level Capabilities SHALL Drop ALL + +#### Scenario: Pod has readiness probe on agent health endpoint +- **WHEN** `buildPodSpec` is called +- **THEN** the container SHALL have an HTTP readiness probe on /health port 8090 +- **AND** InitialDelaySeconds SHALL be 2 +- **AND** PeriodSeconds SHALL be 10 + +#### Scenario: Pod has kubeconfig and workspace volumes +- **WHEN** `buildPodSpec` is called +- **THEN** the pod SHALL have a "kubeconfig" volume from the configured Secret (read-only mount at /config) +- **AND** a "workspace" emptyDir volume (writable mount at /workspace) + +#### Scenario: Pod env delivers auth token via SecretKeyRef +- **WHEN** `buildPodSpec` is called for a session +- **THEN** the SANDBOX_AUTH_TOKEN env var SHALL reference the per-session Secret's "token" key +- **AND** KUBECONFIG SHALL be "/config/kubeconfig" +- **AND** HOME SHALL be "/workspace" + +### Requirement: ExecuteCommand proxies commands to the sandbox agent + +The `ExecuteCommand` method SHALL resolve the pod, authenticate with HMAC, POST to the agent, and return the response. + +#### Scenario: Successful command execution +- **WHEN** `ExecuteCommand` is called with a valid session ID and command +- **THEN** it SHALL POST to `http://:8090/exec` with JSON body containing command and timeout +- **AND** include `Authorization: Bearer ` header +- **AND** return the decoded `ExecResponse` + +**Trust boundary note:** HTTP (not HTTPS) is used because the MCP server and sandbox agent pods communicate over the Kubernetes pod network within the same cluster. The bearer token guards against unauthorized callers within the cluster network, not against network-level eavesdropping. If cross-cluster or external-network communication is needed in future, this should be upgraded to TLS/mTLS. + +#### Scenario: HMAC token derivation is deterministic +- **WHEN** `computeToken` is called with the same session ID and HMAC key +- **THEN** it SHALL always return the same hex-encoded HMAC-SHA256 value +- **AND** different session IDs SHALL produce different tokens + +#### Scenario: Agent unreachable invalidates cache +- **WHEN** the HTTP request to the agent fails with a transport-level error (connection refused, timeout, DNS resolution failure, or network unreachable) +- **THEN** the cache entry SHALL be invalidated for that session and pod IP +- **AND** an error SHALL be returned +- **AND** application-level HTTP errors (4xx/5xx responses from a reachable agent) SHALL NOT trigger cache invalidation + +#### Scenario: Concurrent commands for the same session are serialized at the agent +- **WHEN** multiple `ExecuteCommand` calls arrive concurrently for the same session ID +- **THEN** the session manager SHALL POST each request independently to the agent (no server-side queuing) +- **AND** the sandbox agent SHALL serialize execution via its bash session mutex — each command runs to completion before the next begins +- **AND** each queued command's timeout SHALL start when it begins executing, not when the HTTP request was received + +#### Scenario: Last-activity annotation is updated in background +- **WHEN** a command executes successfully +- **THEN** the pod's `tarsy.redhat.com/last-activity` annotation SHALL be patched with the current UTC timestamp +- **AND** the patch SHALL run in a background goroutine (not blocking the response) + +### Requirement: CleanupSession deletes a session's pod and Secret + +The `CleanupSession` method SHALL remove all Kubernetes resources for a session and clear the cache. + +#### Scenario: Full session cleanup +- **WHEN** `CleanupSession` is called with a session ID +- **THEN** all pods with matching session-id label SHALL be deleted +- **AND** the auth Secret `cli-mcp-sandbox-auth-` SHALL be deleted +- **AND** the cache entry SHALL be removed + +### Requirement: CleanupStale removes idle sessions exceeding the timeout + +The `CleanupStale` method SHALL iterate all sandbox pods and clean up those whose last-activity exceeds IdleTimeout. + +#### Scenario: Stale pod is cleaned up +- **WHEN** `CleanupStale` is called +- **AND** a pod's `last-activity` annotation indicates idle time exceeding IdleTimeout +- **THEN** the session SHALL be cleaned up via `CleanupSession` + +#### Scenario: Fresh pod is preserved +- **WHEN** `CleanupStale` is called +- **AND** a pod's `last-activity` annotation indicates idle time within IdleTimeout +- **THEN** the pod SHALL NOT be deleted + +#### Scenario: Pod without last-activity uses created-at as fallback +- **WHEN** `CleanupStale` encounters a pod with no `last-activity` annotation +- **THEN** it SHALL use the `created-at` annotation for idle time calculation diff --git a/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md new file mode 100644 index 0000000..1c226f5 --- /dev/null +++ b/openspec/changes/2026-06-22-pod-cache-session-manager/tasks.md @@ -0,0 +1,58 @@ +## 1. SandboxConfig + +- [ ] 1.1 Create `pkg/session/config.go` with `SandboxConfig` struct (Image, CPURequest, CPULimit, MemoryRequest, MemoryLimit, IdleTimeout, HMACKey, WarmPoolSize, Namespace, ServiceAccountName, KubeconfigSecret) +- [ ] 1.2 Implement `DefaultConfig()` returning sensible defaults (100m/500m CPU, 128Mi/512Mi mem, 30m idle, tarsy namespace) + +## 2. PodCache + +- [ ] 2.1 Create `pkg/session/cache.go` with `PodCache` struct (`sync.RWMutex`, `map[string]*podEntry`, `ttl`) +- [ ] 2.2 Define `podEntry` struct (podIP, podName, expiresAt) +- [ ] 2.3 Implement `NewPodCache(ttl)` — default to 30s if ttl <= 0 +- [ ] 2.4 Implement `Get(sessionID)` — return pod IP and name if entry exists and not expired +- [ ] 2.5 Implement `Set(sessionID, podIP, podName)` — store with fresh TTL +- [ ] 2.6 Implement `Delete(sessionID)` — unconditional removal +- [ ] 2.7 Implement `Invalidate(sessionID, podIP)` — conditional removal (only if stored IP matches) +- [ ] 2.8 Implement `EvictExpired()` — remove all expired entries, return count +- [ ] 2.9 Implement `Len()` — return entry count (including expired but not yet evicted) + +## 3. SessionManager — Pod Discovery and Creation + +- [ ] 3.1 Create `pkg/session/manager.go` with `SessionManager` struct (clientset, config, cache, httpClient, logger) +- [ ] 3.2 Implement `NewSessionManager(clientset, config, logger)` — initialize cache and HTTP client +- [ ] 3.3 Implement `discoverPod(ctx, sessionID)` — list pods by label selector, return oldest Ready pod's IP (PodReady condition true, deterministic by creation timestamp). If no Ready pod but a non-terminal pod exists (Pending/Running), wait for it via `waitForReady`. Skip terminal pods (Failed/Succeeded). +- [ ] 3.4 Implement `buildPodSpec(sessionID)` — construct Pod manifest with: + - Name `cli-mcp-sandbox-` (deterministic, required for idempotent create-or-get via `AlreadyExists` handling) + - Labels: `tarsy.redhat.com/session-id`, `tarsy.redhat.com/component` + - Annotations: `tarsy.redhat.com/created-at`, `tarsy.redhat.com/last-activity` + - SecurityContext: RunAsNonRoot, RunAsUser 1001, RunAsGroup 1001 + - Container security: AllowPrivilegeEscalation=false, Drop ALL capabilities + - ReadinessProbe: HTTPGet /health:8090, initialDelay 2s, period 10s + - VolumeMounts: kubeconfig (ro at /config), workspace (rw at /workspace) + - Env: KUBECONFIG, HOME, SANDBOX_AUTH_TOKEN (from SecretKeyRef) +- [ ] 3.5 Implement `buildAuthSecret(sessionID, token)` — create Secret with token in StringData +- [ ] 3.6 Implement `computeToken(sessionID)` — HMAC-SHA256(HMACKey, sessionID), hex-encoded +- [ ] 3.7 Implement `waitForReady(ctx, podName)` — poll every 2s for up to 60s until Running + PodReady +- [ ] 3.8 Implement `createSandboxPod(ctx, sessionID)` — create Secret, create Pod, wait for ready +- [ ] 3.9 Implement `validateSessionID(sessionID)` — reject IDs not matching RFC 1123 DNS label format (`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$`) +- [ ] 3.10 Implement `GetOrCreatePod(ctx, sessionID)` — validate session ID, then cache hit → label discovery → idempotent create-or-get (handle `AlreadyExists` conflicts) + +## 4. SessionManager — Command Execution + +- [ ] 4.1 Implement `ExecuteCommand(ctx, sessionID, command, timeoutSec)` — resolve pod, compute token, POST to agent /exec +- [ ] 4.2 On transport-level error (connection refused, timeout, network unreachable), call `cache.Invalidate(sessionID, podIP)` before returning error — application-level HTTP errors (4xx/5xx) SHALL NOT invalidate cache +- [ ] 4.3 On success, decode `agent.ExecResponse` from JSON body +- [ ] 4.4 Launch background `updateLastActivity(sessionID)` goroutine after successful exec +- [ ] 4.5 Implement `updateLastActivity(sessionID)` — patch pod annotation with current RFC3339 timestamp + +## 5. SessionManager — Cleanup + +- [ ] 5.1 Implement `CleanupSession(ctx, sessionID)` — delete cache entry, list+delete pods by label, delete auth Secret +- [ ] 5.2 Implement `CleanupStale(ctx)` — list all sandbox pods, parse last-activity annotation, cleanup those exceeding IdleTimeout + +## 6. Unit Tests + +- [ ] 6.1 Cache tests: set/get, TTL expiry, delete, invalidate (match/mismatch), evict expired, concurrent access, overwrite +- [ ] 6.2 Manager tests: cache hit, discover existing pod, create pod spec validation, auth secret structure, token determinism +- [ ] 6.3 Manager tests: cleanup session (pod + secret deleted, cache cleared) +- [ ] 6.4 Manager tests: cleanup stale (stale pod deleted, fresh pod preserved) +- [ ] 6.5 Manager tests: execute command via httptest server (token sent, request proxied, response decoded)