From fb84eeb4ee560e1036863310cd2be19b1483e447 Mon Sep 17 00:00:00 2001 From: blublinsky Date: Sun, 30 Aug 2026 10:13:51 +0100 Subject: [PATCH] OLS-4070: Fix sandbox-claim mode: add Sandbox resource watcher and timeout handler --- .ai/spec/how/project-structure.md | 2 +- .ai/spec/how/reconciler.md | 15 +- .ai/spec/what/run-lifecycle.md | 4 +- .ai/spec/what/sandbox-execution.md | 10 +- .../integration-test-scenarios.yaml | 39 ++++- .../agentic-operator-e2e-pipeline.yaml | 11 ++ .../scripts/install-operator.sh | 19 +++ Makefile | 2 +- cmd/main.go | 24 ++- config/rbac/role.yaml | 6 + controller/agenticrun/pod_handler.go | 147 ++++++----------- controller/agenticrun/reconciler.go | 14 +- controller/agenticrun/reconciler_test.go | 2 +- controller/agenticrun/sandbox_manager.go | 55 +++++-- controller/agenticrun/sandbox_manager_test.go | 2 +- controller/agenticrun/timeout_handler.go | 148 ++++++++++++++++++ pkg/configuration/config.go | 9 +- test/e2e/execution_test.go | 31 +++- test/e2e/failure_test.go | 2 +- 19 files changed, 407 insertions(+), 135 deletions(-) create mode 100644 controller/agenticrun/timeout_handler.go diff --git a/.ai/spec/how/project-structure.md b/.ai/spec/how/project-structure.md index 8ec6e87c..e24426ec 100644 --- a/.ai/spec/how/project-structure.md +++ b/.ai/spec/how/project-structure.md @@ -9,7 +9,7 @@ | `api/v1alpha1/` | `AgenticRun`, `Agent`, `LLMProvider`, `ApprovalPolicy`, `AgenticRunApproval`, result types, `DerivePhase` | CRD type definitions, phase derivation, CEL markers, deepcopy | | `cmd/main.go` | `main`, `scheme` | Operator binary entry point | | `cmd/oc-agentic/main.go` | `main` | CLI binary entry point | -| `controller/agenticrun/` | `AgenticRunReconciler`, `SandboxAgentCaller`, `SandboxManager`, `SandboxLifecycle`, `PodSpecBuilder`, `PodEventHandler` | AgenticRun reconciler, unified sandbox management (SA, RBAC, ConfigMap, pod), pod event handler, timeout loop, results | +| `controller/agenticrun/` | `AgenticRunReconciler`, `SandboxAgentCaller`, `SandboxManager`, `SandboxLifecycle`, `PodSpecBuilder`, `PodEventHandler`, `TimeoutHandler` | AgenticRun reconciler, unified sandbox management (SA, RBAC, ConfigMap, pod), unified pod event handler (pod_handler.go handles both bare-pod labels and sandbox-claim ownerRef chain), mode-dispatching timeout loop (timeout_handler.go), results | | `controller/console/` | `EnsureAgenticConsole`, `AgenticConsoleConfig` | Console plugin deployment (Deployment, Service, ConfigMap, ConsolePlugin CR) | | `controller/sandbox/` | Legacy bootstrap helpers | SA creation inlined into `cmd/main.go` | | `pkg/configuration/` | `Config`, `Cache`, `OnConfigMapChange` | ConfigMap-driven config cache (sandbox mode, PodSpec, OTEL, MCP) | diff --git a/.ai/spec/how/reconciler.md b/.ai/spec/how/reconciler.md index 75d12347..b7651470 100644 --- a/.ai/spec/how/reconciler.md +++ b/.ai/spec/how/reconciler.md @@ -31,7 +31,8 @@ Audience: AI agents. Behavioral rules and phase semantics live in **what/** spec | `agent.go` | `AgentCaller`, `StubAgentCaller`; `AnalysisOutput`, `ExecutionOutput`, `VerificationOutput`, `EscalationOutput` | Interface methods on `StubAgentCaller` | | `sandbox_manager.go` | `SandboxManager` | `NewSandboxManager`, `Create`, `Release`, `createBarePod`, `createSandboxClaim`, `releaseBarePod`, `releaseSandboxClaim`, `ensureSA`, `setSAOwner`, `buildInputConfigMap`, `createInputConfigMap`, `podSpecToUnstructured` | | `sandbox_agent.go` | `SandboxLifecycle` interface; `SandboxAgentCaller` | `Analyze`, `Execute`, `Verify`, `Escalate`, `ReleaseSandboxes`, `launchSandbox`, `patchSandboxInfo`, `buildAgentContext`, `collectFailedResults`, `stepString` | -| `pod_handler.go` | Pod watch handler (methods on `AgenticRunReconciler`); timeout background goroutine | `handlePodEvent`, `completeStep`, `patchStepCondition`, `patchStepResult`, `releaseSandbox`, `runTimeoutLoop`, `handleTimeEvent`, `stepConditionType`, `fetchResultCR`, `podFailMessage` | +| `pod_handler.go` | Unified pod watch handler for both bare-pod and sandbox-claim modes; shared step helpers | `handlePodEvent`, `resolveBarePodMetadata`, `resolveSandboxPodMetadata`, `completeStep`, `patchStepCondition`, `patchStepResult`, `releaseSandbox`, `stepConditionType`, `validateResultCR`, `podFailMessage`, `podTerminatedInfo` | +| `timeout_handler.go` | Mode-dispatching timeout background goroutine; shared timeout helpers | `runTimeoutLoop`, `handleTimeEvent`, `listBarePods`, `listSandboxPods`, `isSandboxClaimMode`, `startTimedOut`, `overallTimedOut` | | `podspec_builder.go` | `PodSpecBuilder`; label constants (`LabelManaged`, `LabelRun`, etc.); MCP env DTOs (`mcpServerEnvEntry`, `mcpHeaderEnvEntry`) | `Build`, `buildSkills`, `buildMCPServers`, `buildRequiredSecrets`, `addProviderSpecificEnv`, `credentialsSecretName`, `providerURL`, `providerTypeString` | | `schemas.go` | Package vars: default/minimal analysis schemas, execution/verification/escalation schemas; `defaultOutputSchemas`, `builtInPropertyJSON` | `init` (precompute property JSON), `injectBuiltInProperty`, `outputSchemaForStep` | | `rbac.go` | `readerBindings atomic.Value` (cached CRB names) | `ensureExecutionRBAC`, `cleanupExecutionRBAC`, `resolveReaderBindings`, `addReaderSubject`, `removeReaderSubject`, `addSubjectToBinding`, `removeSubjectFromBinding`, `annotatedRBACNamespaces`, `deleteIfExists`, `rbacTargetNamespaces`, `truncateK8sName`, `sandboxSAName`, `executionRoleName`, `clusterRoleName`, `rbacLabels`, `rbacRulesToPolicyRules`, `normalizeCoreAPIGroup` | @@ -128,10 +129,12 @@ Unified sandbox lifecycle manager. Fully encapsulates SA, RBAC, ConfigMap, and p The `AgentHTTPClient`, `AgentHTTPClientInterface`, `agentRunRequest`, `agentRunResponse`, `ClientFactory`, and `client.go` are removed under OLS-3066. The operator no longer makes HTTP calls to sandbox pods. All I/O is via ConfigMap (input) and Result CR (output). -## `PodEventHandler` and timeout loop [OLS-3794] +## Event handlers and timeout loop [OLS-3794, OLS-4070] -- **`PodEventHandler`:** Registered via `Watches(&Pod{}, handler.EnqueueRequestsFromMapFunc)` in `SetupWithManager`. When a sandbox pod terminates (Succeeded/Failed), it: (a) reads the Result CR to determine agent success/failure, (b) patches the step condition on the `AgenticRun`, (c) calls `releaseSandbox` for cleanup. This drives the async lifecycle without reconciler polling. -- **`runTimeoutLoop`:** Background goroutine started by the reconciler via `mgr.Add`. Periodically lists in-progress runs and checks per-step sandbox timeouts. When a timeout is detected, patches the step condition to `False` with reason `SandboxTimeout` and releases the sandbox. +Two handlers — a unified pod watcher and a timeout loop. + +- **`handlePodEvent` (pod_handler.go):** Registered via `Watches(&Pod{}, handler.EnqueueRequestsFromMapFunc)`. Handles both bare-pod and sandbox-claim modes. In bare-pod mode, `resolveBarePodMetadata` reads `LabelRun`/`LabelStep` labels directly from the pod. In sandbox-claim mode, `resolveSandboxPodMetadata` follows the pod's ownerRef chain (Pod → Sandbox → SandboxClaim) to find the SandboxClaim which carries operator labels and `AnnotationRunName`. Both paths feed into `completeStep` when the pod terminates (Succeeded/Failed): (a) reads the Result CR to determine agent success/failure, (b) patches the step condition on the `AgenticRun`, (c) calls `releaseSandbox` for cleanup. For in-progress pods, patches step reason (`Running` / `WaitingForSandbox`). +- **`runTimeoutLoop` (timeout_handler.go):** Background goroutine started via `mgr.Add`. Calls `handleTimeEvent` on each tick, which dispatches to `listBarePods` (labels) or `listSandboxPods` (ownerRef chain resolution) based on `isSandboxClaimMode()`. Both paths check per-step timeouts via `startTimedOut` / `overallTimedOut` and retry completion for terminal pods whose step condition patch failed earlier. --- @@ -196,8 +199,8 @@ AgenticRunReconciler.Reconcile │ └─ patchSandboxInfo → return (watch-driven re-entry) ├─ Re-entry: check Result CR (Completed condition) → process result │ └─ Update run conditions, append result ref - ├─ PodEventHandler: pod terminated → process result → patch condition → releaseSandbox - ├─ runTimeoutLoop: periodic check → timeout → patch condition → releaseSandbox + ├─ handlePodEvent (both modes): pod terminated → resolveBarePodMetadata or resolveSandboxPodMetadata → process result → patch condition → releaseSandbox + ├─ runTimeoutLoop → handleTimeEvent: dispatches to listBarePods or listSandboxPods based on mode └─ Sandbox.Release (terminal phases, deletion) → GC SA/CM + remove reader subjects + execution RBAC ``` diff --git a/.ai/spec/what/run-lifecycle.md b/.ai/spec/what/run-lifecycle.md index 01e8be20..d9e8fe4f 100644 --- a/.ai/spec/what/run-lifecycle.md +++ b/.ai/spec/what/run-lifecycle.md @@ -42,8 +42,8 @@ Behavioral specification for the `AgenticRun` resource lifecycle. **Approval gat | Status | Reason | Meaning | |---|---|---| -| `Unknown` | `WaitingForSandbox` | Pod/SandboxClaim created, waiting for pod to start | -| `Unknown` | `Running` | Pod is running, agent is working | +| `Unknown` | `WaitingForSandbox` | Pod/SandboxClaim created, waiting for pod to start. In sandbox-claim mode, derived from absence of `Ready=True` on the `Sandbox` resource. | +| `Unknown` | `Running` | Pod is running, agent is working. In sandbox-claim mode, derived from `Ready=True` on the `Sandbox` resource. | | `True` | `Succeeded` | Result CR exists with `success: true` | | `False` | `AgentFailed` | Result CR exists with `success: false` for a non-timeout agent failure | | `False` | `AgentTimeout` | [PLANNED: OLS-3743] Sandbox cooperatively stopped the agent at its configured execution budget and published a Result CR | diff --git a/.ai/spec/what/sandbox-execution.md b/.ai/spec/what/sandbox-execution.md index 016b00af..5210f986 100644 --- a/.ai/spec/what/sandbox-execution.md +++ b/.ai/spec/what/sandbox-execution.md @@ -15,7 +15,7 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes 8. **[OLS-3066] Output delivery — Result CR via `oc`**: The sandbox MUST create the Result CR in two steps: (a) `oc create -f ` using the pre-filled template with `spec` fields, then (b) `oc patch --type=merge --subresource=status` with the agent output in `status` fields (options, diagnosis, actionRequired, actionsTaken, checks, conditions, failureReason as applicable per step). The Result CR `status.conditions` MUST include a `Completed` condition set to `True` as part of the status patch — this is the operator's readiness signal (see rule 8b). On agent failure, the sandbox MUST still create the Result CR with `status.failureReason` populated and exit 0 (the sandbox succeeded; the agent failed). [PLANNED: OLS-3743] A cooperative timeout MUST use `Completed=True` with reason `AgentTimeout`; other agent failures use reason `Failed`. The operator maps these reasons to distinct step conditions. On sandbox failure (cannot read input, `oc create` fails, etc.), the sandbox MUST write an error message to `/dev/termination-log` (max 4096 bytes) and exit non-zero. 8a. **[OLS-3066] Sandbox RBAC for Result CRs**: Each step gets its own per-step ServiceAccount (`ls-{step}-{namespace}-{runUID}`). The per-step SA MUST have `create` and `patch` (with `status` subresource) permissions on only its specific Result CRD (e.g. analysis SA can only create `AnalysisResult`). The execution SA additionally receives execution-specific Roles/ClusterRoles for the approved remediation. 8b. **[OLS-3066] Result CR readiness signal**: The operator MUST only process a Result CR when its `status.conditions` includes `Completed=True`. A Result CR without this condition indicates the sandbox has called `oc create` but has not yet patched the status — the operator MUST wait for the status update (which triggers another `Owns()` watch event). This guards against the race between `oc create` and `oc patch --subresource=status`. -9. **[OLS-3066] Watch-driven async**: The controller MUST use watch-based event delivery instead of synchronous polling. `SetupWithManager` MUST `Owns()` Pods (bare-pod mode), SandboxClaims (sandbox-claim mode), ConfigMaps, and all Result CR types (AnalysisResult, ExecutionResult, VerificationResult, EscalationResult). Pod watches are for **failure detection only** (Pod `Failed`, `ImagePullBackOff`). Result CR watches are for **completion detection** (Result CR created with `Completed` condition). Every in-progress step MUST return `RequeueAfter(30s)` as a safety net for missed watch events. +9. **[OLS-3066, OLS-4070] Watch-driven async**: The controller MUST use watch-based event delivery instead of synchronous polling. `SetupWithManager` registers a single `Watches(&Pod{}, handlePodEvent)` handler that handles both modes. In bare-pod mode, `resolveBarePodMetadata` reads `LabelRun`/`LabelStep` labels directly from the pod. In sandbox-claim mode, `resolveSandboxPodMetadata` follows the pod's ownerRef chain (Pod → Sandbox → SandboxClaim) to find the SandboxClaim which carries operator labels (`LabelRun`, `LabelStep`) and `AnnotationRunName`. Both paths feed into the same `completeStep` logic — pod `Succeeded` or `Failed` triggers step completion. Result CR watches (via `Owns()`) handle completion detection. Every in-progress step MUST return `RequeueAfter(30s)` as a safety net for missed watch events. 10. **Output schema selection**: The `output-schema` key in the input ConfigMap MUST be the step-specific JSON schema computed by the operator: analysis schema depends on `spec.analysisOutput.mode`, whether execution/verification steps exist in the run, and optional injected `components` sub-schema from `spec.analysisOutput.schema`; other steps use fixed schemas for their response shapes. 11. **Analysis query payload**: The `query` string MUST encode the user request or revision-augmented request. [PLANNED: OLS-3491] Workflow flags and role/rules (prefer mounted skill when matching, fall back to kubectl/oc, inspect before diagnosing, remediation script shape, RBAC derivation) MUST live in `system-prompt` / `instructions`, not in `query`. Until OLS-3491, those instructions MAY still be template-rendered into `query`. The analysis instructions MUST instruct the agent to prefer a mounted skill when one matches the investigation and fall back to kubectl/oc for read-only inspection when no skill applies, inspect cluster state before diagnosing, produce a concrete remediation script of executable bash commands (mutations and waits only — pre-checks and post-checks are excluded because analysis already inspected the cluster and verification is a separate step), and derive RBAC for mutations and subresource access only (the execution environment already has cluster-wide read access to standard resources). 12. **Execution query payload**: The `query` MUST include JSON describing the approved remediation option, which contains a concrete remediation script (ordered bash commands). [PLANNED: OLS-3491] Execution role/rules MUST live in `system-prompt` / `instructions`. Until OLS-3491, those MAY still be template-rendered into `query`. The execution instructions MUST instruct the agent to follow the script exactly, execute commands in order without substitution, dry-run every mutation command with `--dry-run=server` before applying, and fix syntax errors only (no semantic changes). The execution instructions MUST NOT instruct the agent to perform verification — that is the verification step's responsibility. @@ -73,7 +73,7 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes ### Reconcile SLO and Timeout [OLS-3066] 39. **Reconcile duration SLO**: Each `Reconcile` invocation MUST complete within **30 seconds** wall-clock time. This excludes `RequeueAfter` sleep between invocations. No synchronous polling loops or blocking HTTP calls within a single Reconcile. -40. **[PLANNED: OLS-3743] Layered timeout enforcement**: The selected `Agent` provides one user-facing execution budget for the active step. The operator resolves omitted fields to 600 seconds for analysis, execution, and escalation, and 1800 seconds for verification. It passes the effective value as `LIGHTSPEED_AGENT_TIMEOUT_SECONDS`; the sandbox applies it cooperatively around the complete agent invocation. The operator independently enforces: (a) a fixed five-minute startup deadline measured from Pod creation in bare-pod mode or SandboxClaim creation in sandbox-claim mode; and (b) a hard running deadline measured from the main container `startedAt`, equal to the effective agent budget plus one minute. This supersedes OLS-3066's original single ten-minute deadline measured from Pod creation. +40. **[PLANNED: OLS-3743] Layered timeout enforcement**: The selected `Agent` provides one user-facing execution budget for the active step. The operator resolves omitted fields to 600 seconds for analysis, execution, and escalation, and 1800 seconds for verification. It passes the effective value as `LIGHTSPEED_AGENT_TIMEOUT_SECONDS`; the sandbox applies it cooperatively around the complete agent invocation. The operator independently enforces: (a) a fixed five-minute startup deadline measured from Pod creation in bare-pod mode or SandboxClaim creation in sandbox-claim mode; and (b) a hard running deadline measured from the main container `startedAt`, equal to the effective agent budget plus one minute. This supersedes OLS-3066's original single ten-minute deadline measured from Pod creation. [OLS-4070] The timeout handler (`runTimeoutLoop` → `handleTimeEvent`) dispatches to `listBarePods` (bare-pod: lists pods by label) or `listSandboxPods` (sandbox-claim: lists all pods and resolves ownerRef chain Pod → Sandbox → SandboxClaim) based on the current mode. 40a. **Startup timeout**: If the main container has not started before the startup deadline, the operator MUST release/delete the Pod or SandboxClaim, delete the input ConfigMap, set the step condition to `False` with reason `SandboxStartupTimeout`, and fail the run. 40b. **Agent timeout**: When the sandbox publishes a Result CR with `Completed=True`, reason `AgentTimeout`, the operator MUST set the step condition to `False` with reason `AgentTimeout`, preserve the sandbox failure message, clean up the sandbox and input ConfigMap, and fail the run. 40c. **Hard running timeout**: If the running deadline expires before a complete Result CR is processed, the operator MUST release/delete the Pod or SandboxClaim, delete the input ConfigMap, set the step condition to `False` with reason `SandboxTimeout`, and fail the run. @@ -100,8 +100,9 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes ### Sandbox Mode 31. **Sandbox mode selection**: The sandbox mode (`bare-pod` or `sandbox-claim`) is read from the `sandbox-mode` key in the `lightspeed-agentic-configuration` ConfigMap (produced by lightspeed-operator). When the key is omitted or empty, the operator MUST default to `bare-pod` mode. There is no CLI flag — the ConfigMap is the single source of truth. -32. **Unified sandbox lifecycle**: `SandboxManager.Create` fully encapsulates sandbox setup for every step: (a) creates a per-step ServiceAccount (`ls-{step}-{namespace}-{runUID}`) with owner reference to the pod/claim, (b) adds the per-step SA to all reader ClusterRoleBindings, (c) for execution: creates cross-namespace Roles/ClusterRoles and persists the RBAC namespaces annotation, (d) builds and creates the input ConfigMap with owner reference, (e) reads the base PodSpec from the config cache, overlays agent-specific configuration via `PodSpecBuilder.Build`, and creates either a bare Pod or a SandboxClaim+SandboxTemplate depending on the configured mode. Resource name MUST follow the pattern `ls-{step}-{agenticRunName}` truncated to 63 characters. `Release` encapsulates sandbox teardown: deletes the pod/claim (GC cascades to SA, ConfigMap, result RBAC via owner refs), removes the per-step SA from reader CRBs, and for execution: explicitly cleans up cross-namespace Roles/ClusterRoles. Every per-run Pod or SandboxClaim MUST carry a controller `ownerReference` to its AgenticRun (`controller: true`, `blockOwnerDeletion: true`). A reusable derived SandboxTemplate MUST NOT carry an owner reference to any individual run; its garbage collection is governed by rules 3–4. [OLS-3066] `WaitReady` is removed — the operator does not poll for pod readiness; it watches for pod completion and Result CR creation. -33. **[OLS-3066] Bare pod completion**: In `bare-pod` mode with the batch execution model (rule 6), the controller does NOT poll for pod readiness or extract an endpoint. Instead, the controller watches for pod phase transitions via `Owns(&Pod{})`. Pod `Succeeded` or `Failed` triggers a reconcile. The controller then checks for the Result CR per the re-entry logic (rule 43). If the pod is `NotFound` during re-entry, the controller MUST treat it as a terminal error. If the pod has a non-zero `DeletionTimestamp`, the controller MUST treat it as terminal. +32. **Unified sandbox lifecycle**: `SandboxManager.Create` fully encapsulates sandbox setup for every step: (a) creates a per-step ServiceAccount (`ls-{step}-{namespace}-{runUID}`) with owner reference to the pod/claim, (b) adds the per-step SA to all reader ClusterRoleBindings, (c) for execution: creates cross-namespace Roles/ClusterRoles and persists the RBAC namespaces annotation, (d) builds and creates the input ConfigMap with owner reference, (e) reads the base PodSpec from the config cache, overlays agent-specific configuration via `PodSpecBuilder.Build`, and creates either a bare Pod or a SandboxClaim+SandboxTemplate depending on the configured mode. Resource name MUST follow the pattern `ls-{step}-{agenticRunName}` truncated to 63 characters. `Release` encapsulates sandbox teardown: deletes the pod/claim (GC cascades to SA, ConfigMap, result RBAC via owner refs), removes the per-step SA from reader CRBs, and for execution: explicitly cleans up cross-namespace Roles/ClusterRoles. Both resource types MUST carry controller `ownerReferences` to their `AgenticRun` (`controller: true`, `blockOwnerDeletion: true`). [OLS-3066] `WaitReady` is removed — the operator does not poll for pod readiness; it watches for pod completion and Result CR creation. +33. **[OLS-3066, OLS-4070] Bare pod completion**: In `bare-pod` mode, `handlePodEvent` watches pods by `LabelRun`/`LabelStep` labels. Pod `Succeeded` or `Failed` triggers `completeStep`. The controller then checks for the Result CR per the re-entry logic (rule 43). If the pod is `NotFound` during re-entry, the controller MUST treat it as a terminal error. If the pod has a non-zero `DeletionTimestamp`, the controller MUST treat it as terminal. +33a. **[OLS-4070] Sandbox-claim completion**: In `sandbox-claim` mode, `handlePodEvent` detects sandbox-managed pods via `resolveSandboxPodMetadata`, which follows the pod's ownerRef chain: Pod → Sandbox → SandboxClaim. The SandboxClaim carries operator labels (`LabelRun`, `LabelStep`) and `AnnotationRunName`, mapping the pod back to the `AgenticRun`. Pod `Succeeded` or `Failed` triggers `completeStep` — the same path as bare-pod mode. Pods created by the Sandbox operator do NOT carry operator labels directly — the ownerRef chain resolution is required to discover the AgenticRun association. 34. **[OLS-3066] No endpoint construction**: With the batch execution model, the operator does not construct agent HTTP URLs. There is no HTTP communication between operator and sandbox. Input is delivered via ConfigMap mount (rule 7); output is delivered via Result CR creation (rule 8). 35. **Sandbox release**: `Release(ctx, run, step)` handles full teardown for one step. In `bare-pod` mode it deletes the Pod with zero grace; in `sandbox-claim` mode it deletes the SandboxClaim and its active backing workload. Per-run release MUST NOT delete a derived SandboxTemplate because templates can be shared by live claims; shared-template garbage collection is a separate operation governed by rule 4. Deletion cascades via owner references to the per-step SA, input ConfigMap, and result RBAC resources. Additionally, `Release` removes the sandbox ServiceAccount from all reader ClusterRoleBindings, and for the execution step explicitly deletes cross-namespace Roles/ClusterRoles. Both paths are idempotent — NotFound is treated as success — and cleanup callers MUST requeue on other errors. 36. **PodSpecBuilder**: `PodSpecBuilder` takes a base `*corev1.PodSpec` (from the config cache) and overlays agent-specific configuration: LLM env vars, credential mounts, skills volumes, MCP config, required secrets, input ConfigMap volume mount [OLS-3066], SA token mounting, security context. It produces a single typed `corev1.PodSpec`. In `bare-pod` mode, this PodSpec is used directly to create a Pod. In `sandbox-claim` mode, it is converted to an unstructured map and embedded in a `SandboxTemplate`. [OLS-3066] HTTP readiness/liveness probes are no longer set (see rule 30). @@ -148,3 +149,4 @@ Behavioral specification for how workflow steps run inside ephemeral **sandboxes - [PLANNED: OLS-3661] Token usage aggregation — operator reads `status.tokenUsage` from completed Result CRs and accumulates into `AgenticRun.status.tokenUsage`. See rule 43.1 and `crd-api.md` rules 6c–6e. - [PLANNED: OLS-3743] Layer Agent-configured cooperative execution budgets under fixed operator sandbox startup and hard running deadlines; wire `maxTurns`; distinguish timeout sources in status. - [PLANNED: OLS-3298, OLS-4018] Shared hard-stop cleanup: zero-grace Pod deletion, SandboxClaim/backing-workload deletion, sandbox access revocation, dual resource discovery, idempotency, and retries after terminal status. See `agentic-run-termination.md`. +- [DONE: OLS-4070] Dual-mode pod handler — `pod_handler.go` handles both bare-pod and sandbox-claim modes via a single `handlePodEvent` watcher. Bare-pod mode reads labels directly (`resolveBarePodMetadata`); sandbox-claim mode resolves Pod → Sandbox → SandboxClaim ownerRef chain (`resolveSandboxPodMetadata`). Both paths feed into `completeStep`. `timeout_handler.go` provides a mode-dispatching timeout loop with `listBarePods` / `listSandboxPods` helpers. See rules 9, 33, 33a, 40. diff --git a/.tekton/integration-tests/integration-test-scenarios.yaml b/.tekton/integration-tests/integration-test-scenarios.yaml index 92ad55ca..b0505a75 100644 --- a/.tekton/integration-tests/integration-test-scenarios.yaml +++ b/.tekton/integration-tests/integration-test-scenarios.yaml @@ -1,8 +1,11 @@ -# IntegrationTestScenario for lightspeed-agentic-operator e2e. +# IntegrationTestScenario resources for lightspeed-agentic-operator e2e. # Triggers on lightspeed-agentic-operator component builds. # Provisions an ephemeral Hypershift cluster, deploys operator from SNAPSHOT, # runs make test-e2e with the fixed mock agent image. # +# Two scenarios: bare-pod mode (default) and sandbox-claim mode. +# Both use the same pipeline; sandbox-mode param selects the mode. +# # Apply to tenant namespace: kubectl apply -f .tekton/integration-tests/integration-test-scenarios.yaml --- apiVersion: appstudio.redhat.com/v1beta2 @@ -22,6 +25,40 @@ spec: value: agentic-e2e-tests - name: namespace value: openshift-lightspeed + - name: sandbox-mode + value: bare-pod + - name: openshift-version-prefix + value: "4.19." + resolverRef: + resolver: git + resourceKind: pipeline + params: + - name: url + value: https://github.com/openshift/lightspeed-agentic-operator + - name: revision + value: main + - name: pathInRepo + value: .tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml +--- +apiVersion: appstudio.redhat.com/v1beta2 +kind: IntegrationTestScenario +metadata: + name: agentic-operator-e2e-sandbox-claim + namespace: crt-nshift-lightspeed-tenant + labels: + test.appstudio.openshift.io/optional: "true" +spec: + application: ols + contexts: + - description: Agentic operator e2e (sandbox-claim mode) + name: component_lightspeed-agentic-operator + params: + - name: test-name + value: agentic-e2e-sandbox-claim + - name: namespace + value: openshift-lightspeed + - name: sandbox-mode + value: sandbox-claim - name: openshift-version-prefix value: "4.19." resolverRef: diff --git a/.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml b/.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml index c5befff1..4f2ae9a2 100644 --- a/.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml +++ b/.tekton/integration-tests/pipelines/agentic-operator-e2e-pipeline.yaml @@ -21,6 +21,10 @@ spec: description: 'Namespace to deploy the operator into' default: 'openshift-lightspeed' type: string + - name: sandbox-mode + description: 'Sandbox mode: bare-pod (default) or sandbox-claim' + default: 'bare-pod' + type: string - name: openshift-version-prefix description: 'Minor line prefix for eaas-get-latest-openshift-version-by-prefix (include trailing dot, e.g. 4.19.)' default: '4.19.' @@ -97,6 +101,8 @@ spec: type: string - name: namespace type: string + - name: sandbox-mode + type: string results: - name: commit value: "$(steps.install-operator.results.commit)" @@ -144,6 +150,8 @@ spec: value: "$(params.namespace)" - name: SANDBOX_IMAGE value: "quay.io/openshift-lightspeed/ols-qe:lightspeed-mock-agent1" + - name: SANDBOX_MODE + value: "$(params.sandbox-mode)" image: registry.redhat.io/openshift4/ose-cli:latest script: | set -euo pipefail @@ -169,6 +177,7 @@ spec: IMG="${OPERATOR_IMAGE}" \ OPERATOR_NAMESPACE="${NAMESPACE}" \ SANDBOX_IMAGE="${SANDBOX_IMAGE}" \ + SANDBOX_MODE="${SANDBOX_MODE}" \ bash .tekton/integration-tests/scripts/install-operator.sh - name: run-e2e-tests resources: @@ -261,6 +270,8 @@ spec: value: $(params.SNAPSHOT) - name: namespace value: "$(params.namespace)" + - name: sandbox-mode + value: "$(params.sandbox-mode)" finally: - name: export-logs-for-retention taskRef: diff --git a/.tekton/integration-tests/scripts/install-operator.sh b/.tekton/integration-tests/scripts/install-operator.sh index 2cbc2b8a..53a82986 100755 --- a/.tekton/integration-tests/scripts/install-operator.sh +++ b/.tekton/integration-tests/scripts/install-operator.sh @@ -37,6 +37,25 @@ oc create namespace "${OPERATOR_NAMESPACE}" --dry-run=client -o yaml | oc apply echo "Installing CRDs..." make install +# Install Agent Sandbox operator when sandbox-claim mode is requested. +if [ "${SANDBOX_MODE}" = "sandbox-claim" ]; then + AGENT_SANDBOX_VERSION="${AGENT_SANDBOX_VERSION:-v1.0.0}" + AGENT_SANDBOX_RELEASE_BASE="https://github.com/kubernetes-sigs/agent-sandbox/releases/download" + echo "Installing Agent Sandbox operator ${AGENT_SANDBOX_VERSION}..." + oc apply -f "${AGENT_SANDBOX_RELEASE_BASE}/${AGENT_SANDBOX_VERSION}/sandbox.yaml" + oc apply -f "${AGENT_SANDBOX_RELEASE_BASE}/${AGENT_SANDBOX_VERSION}/extensions.yaml" + echo "Waiting for Sandbox CRDs to be established..." + oc wait --for=condition=Established crd/sandboxes.agents.x-k8s.io --timeout=60s + oc wait --for=condition=Established crd/sandboxclaims.extensions.agents.x-k8s.io --timeout=60s + oc wait --for=condition=Established crd/sandboxtemplates.extensions.agents.x-k8s.io --timeout=60s + oc wait --for=condition=Established crd/sandboxwarmpools.extensions.agents.x-k8s.io --timeout=60s + echo "Waiting for Agent Sandbox controller to be ready..." + oc rollout status deployment/agent-sandbox-controller -n agent-sandbox-system --timeout=120s + echo "Agent Sandbox operator installed" +else + echo "Skipping Agent Sandbox operator install (SANDBOX_MODE=${SANDBOX_MODE})" +fi + # Deploy operator (kustomize-based). echo "Deploying operator..." make deploy IMG="${IMG}" OPERATOR_NAMESPACE="${OPERATOR_NAMESPACE}" SANDBOX_MODE="${SANDBOX_MODE}" diff --git a/Makefile b/Makefile index 23eda095..ed517020 100644 --- a/Makefile +++ b/Makefile @@ -118,7 +118,7 @@ SANDBOX_MODE ?= bare-pod AGENT_IMAGE ?= quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/lightspeed-agentic-sandbox:main # kubernetes-sigs/agent-sandbox release reference (used only for documentation links). -AGENT_SANDBOX_VERSION ?= v0.4.5 +AGENT_SANDBOX_VERSION ?= v1.0.0 AGENT_SANDBOX_RELEASE_BASE ?= https://github.com/kubernetes-sigs/agent-sandbox/releases/download # Image name under the current oc project for deploy-local (OpenShift integrated registry only). diff --git a/cmd/main.go b/cmd/main.go index f80e040a..4bfd0dd3 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -13,11 +13,13 @@ import ( uberzap "go.uber.org/zap" corev1 "k8s.io/api/core/v1" + apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/healthz" @@ -95,7 +97,15 @@ func main() { } }() - cfgCache := &configuration.Cache{} + // --- Check for Sandbox CRDs (determines sandbox-claim mode availability) --- + sandboxCRDs := sandboxCRDInstalled(cfg) + if sandboxCRDs { + log.Info("Sandbox CRDs detected, sandbox-claim mode available") + } else { + log.Info("Sandbox CRDs not found, defaulting to bare-pod mode") + } + + cfgCache := &configuration.Cache{ForceBareMode: !sandboxCRDs} cfgCache.SetOTELProvider(telemetryProvider) // Eagerly read ConfigMap if it already exists (operator restart). @@ -196,3 +206,15 @@ func main() { os.Exit(1) } } + +// sandboxCRDInstalled queries the apiextensions API directly to check +// whether the Sandbox CRD exists, bypassing any cached REST mapper. +func sandboxCRDInstalled(cfg *rest.Config) bool { + apiext, err := apiextensionsclient.NewForConfig(cfg) + if err != nil { + return false + } + _, err = apiext.ApiextensionsV1().CustomResourceDefinitions().Get( + context.Background(), "sandboxes.agents.x-k8s.io", metav1.GetOptions{}) + return err == nil +} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 67dbfa84..e6c6e431 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -102,6 +102,12 @@ rules: - get - list - watch +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - get - apiGroups: - extensions.agents.x-k8s.io resources: diff --git a/controller/agenticrun/pod_handler.go b/controller/agenticrun/pod_handler.go index f30e70f0..643a5249 100644 --- a/controller/agenticrun/pod_handler.go +++ b/controller/agenticrun/pod_handler.go @@ -4,11 +4,11 @@ import ( "context" "fmt" "sync" - "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -26,42 +26,37 @@ var stepCondMu sync.Mutex // Pod event handler // --------------------------------------------------------------------------- -// handlePodEvent is the Watches handler for sandbox pods. It evaluates -// the step FSM on every pod event and acts on the outcome: -// -// Completed → cleanup pod+CM, enqueue reconcile (phase routing picks up Result CR) -// Failed → patch step condition, cleanup pod+CM -// Running → patch step reason (WaitingForSandbox / Running) +// handlePodEvent is the Watches handler for sandbox pods. It resolves +// the owning AgenticRun using either labels (bare-pod) or the ownership +// chain Pod → Sandbox → SandboxClaim (sandbox-claim mode). func (r *AgenticRunReconciler) handlePodEvent(ctx context.Context, obj client.Object) []ctrl.Request { pod, ok := obj.(*corev1.Pod) if !ok { return nil } - // Not a sandbox pod — ignore. - step := pod.Labels[LabelStep] - runName := pod.Annotations[AnnotationRunName] - if pod.Labels[LabelRun] == "" || step == "" || runName == "" { + var step, runName string + if r.isSandboxClaimMode() { + step, runName, _ = resolveSandboxPodMetadata(ctx, r.Client, pod) + } else { + step, runName = resolveBarePodMetadata(pod) + } + if step == "" || runName == "" { return nil } - // Look up the owning AgenticRun. Gone → nothing to do. var run agenticv1alpha1.AgenticRun if err := r.Get(ctx, client.ObjectKey{Name: runName, Namespace: r.Namespace}, &run); err != nil { return nil } - // Resolve once: condition type, claim name, and enrich the logger. condType := stepConditionType(step) claimName := sandboxClaimName(&run, step) - runUID := string(run.UID) ctx = logf.IntoContext(ctx, logf.FromContext(ctx).WithValues( LogKeyName, pod.Name, LogKeyStep, step, - LogKeyClaim, claimName, "runUID", runUID, LogKeyCondition, condType, + LogKeyClaim, claimName, "runUID", string(run.UID), LogKeyCondition, condType, )) - // Pod still in progress — lightweight reason update, no FSM needed. - // Edge cases (node death, force delete) are caught by the timeout ticker. if pod.Status.Phase == corev1.PodRunning || pod.Status.Phase == corev1.PodPending || pod.Status.Phase == corev1.PodUnknown { reason := ReasonRunning if pod.Status.Phase != corev1.PodRunning { @@ -71,13 +66,49 @@ func (r *AgenticRunReconciler) handlePodEvent(ctx context.Context, obj client.Ob return nil } - // Pod terminated — phase tells us the outcome. if err := r.completeStep(ctx, &run, pod, step, condType, ""); err != nil { return nil } return nil } +// resolveBarePodMetadata reads step and run name from pod labels/annotations. +func resolveBarePodMetadata(pod *corev1.Pod) (step, runName string) { + if pod.Labels[LabelRun] == "" { + return "", "" + } + return pod.Labels[LabelStep], pod.Annotations[AnnotationRunName] +} + +// resolveSandboxPodMetadata follows the ownership chain: +// Pod → Sandbox (ownerRef) → SandboxClaim (ownerRef) → our labels. +func resolveSandboxPodMetadata(ctx context.Context, c client.Client, pod *corev1.Pod) (step, runName string, err error) { + for _, podRef := range pod.OwnerReferences { + if podRef.Kind != "Sandbox" { + continue + } + sb := &unstructured.Unstructured{} + sb.SetGroupVersionKind(smSandboxGVK) + if err := c.Get(ctx, client.ObjectKey{Name: podRef.Name, Namespace: pod.Namespace}, sb); err != nil { + return "", "", err + } + for _, sbRef := range sb.GetOwnerReferences() { + if sbRef.Kind != "SandboxClaim" { + continue + } + claim := &unstructured.Unstructured{} + claim.SetGroupVersionKind(smClaimGVK) + if err := c.Get(ctx, client.ObjectKey{Name: sbRef.Name, Namespace: pod.Namespace}, claim); err != nil { + return "", "", err + } + labels := claim.GetLabels() + annotations := claim.GetAnnotations() + return labels[LabelStep], annotations[AnnotationRunName], nil + } + } + return "", "", nil +} + // completeStep handles step completion: patches the step condition (and result ref // on success), emits audit events, and releases the sandbox. When timeoutMsg is // non-empty the pod phase is ignored and a timeout failure is recorded. Returns an @@ -292,72 +323,7 @@ func appendResultRef(run *agenticv1alpha1.AgenticRun, step, name string, outcome func (r *AgenticRunReconciler) releaseSandbox(ctx context.Context, run *agenticv1alpha1.AgenticRun, step string) { if err := r.Agent.ReleaseSandbox(ctx, run, step); err != nil { - logf.FromContext(ctx).Error(err, "pod handler: failed to release sandbox", LogKeyStep, step) - } -} - -// --------------------------------------------------------------------------- -// Timeout ticker — background goroutine for time-driven timeout checks -// --------------------------------------------------------------------------- - -const sandboxTimeoutCheckInterval = 1 * time.Minute - -// runTimeoutLoop runs handleTimeEvent in a loop. -// Stopped when ctx is cancelled (manager shutdown). -func (r *AgenticRunReconciler) runTimeoutLoop(ctx context.Context) error { - for { - select { - case <-ctx.Done(): - return nil - case <-time.After(sandboxTimeoutCheckInterval): - r.handleTimeEvent(ctx) - } - } -} - -// handleTimeEvent checks all sandbox pods for start/overall timeouts. -func (r *AgenticRunReconciler) handleTimeEvent(ctx context.Context) { - log := logf.FromContext(ctx).WithName("sandbox-timeout") - var pods corev1.PodList - if err := r.List(ctx, &pods, client.InNamespace(r.Namespace), client.HasLabels{LabelRun, LabelStep}); err != nil { - log.Error(err, "failed to list sandbox pods") - return - } - - now := time.Now() - for i := range pods.Items { - pod := &pods.Items[i] - step := pod.Labels[LabelStep] - runName := pod.Annotations[AnnotationRunName] - condType := stepConditionType(step) - if runName == "" { - continue - } - - var run agenticv1alpha1.AgenticRun - if err := r.Get(ctx, client.ObjectKey{Name: runName, Namespace: r.Namespace}, &run); err != nil { - continue - } - - // Retry: pod already terminal but step condition still pending (patch failed earlier). - phase := pod.Status.Phase - if (phase == corev1.PodSucceeded || phase == corev1.PodFailed) && isStepInProgress(&run, condType) { - log.Info("retrying completion for terminal pod", LogKeyName, pod.Name, LogKeyStep, step) - _ = r.completeStep(ctx, &run, pod, step, condType, "") - continue - } - - created := pod.CreationTimestamp.Time - var message string - if startTimedOut(phase, created, now, podStartTimeout) { - message = fmt.Sprintf("sandbox pod did not start within %s", podStartTimeout) - } else if overallTimedOut(created, now, stepTimeout(step)) { - message = fmt.Sprintf("sandbox exceeded timeout %s", stepTimeout(step)) - } else { - continue - } - - _ = r.completeStep(ctx, &run, pod, step, condType, message) + logf.FromContext(ctx).Error(err, "failed to release sandbox", LogKeyStep, step) } } @@ -502,19 +468,6 @@ func podFailMessage(pod *corev1.Pod) string { return "sandbox pod failed" } -// startTimedOut returns true if the pod has not reached Running within the start deadline. -func startTimedOut(phase corev1.PodPhase, created, now time.Time, timeout time.Duration) bool { - if phase == corev1.PodRunning || phase == corev1.PodSucceeded || phase == corev1.PodFailed { - return false - } - return now.Sub(created) > timeout -} - -// overallTimedOut returns true if the pod has exceeded the step deadline. -func overallTimedOut(created, now time.Time, timeout time.Duration) bool { - return now.Sub(created) > timeout -} - // podTerminatedInfo returns the first terminated container's message and exit code. func podTerminatedInfo(pod *corev1.Pod) (msg string, exitCode *int32) { if pod == nil || len(pod.Status.ContainerStatuses) == 0 { diff --git a/controller/agenticrun/reconciler.go b/controller/agenticrun/reconciler.go index 74cc67b2..e10395ed 100644 --- a/controller/agenticrun/reconciler.go +++ b/controller/agenticrun/reconciler.go @@ -10,12 +10,14 @@ import ( "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" "github.com/openshift/lightspeed-agentic-operator/pkg/configuration" @@ -47,6 +49,7 @@ type AgenticRunReconciler struct { TempLog TempLogCleaner } +// +kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=get // +kubebuilder:rbac:groups=agentic.openshift.io,resources=agenticruns,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=agentic.openshift.io,resources=agenticruns/status,verbs=get;update;patch // +kubebuilder:rbac:groups=agentic.openshift.io,resources=agenticruns/finalizers,verbs=update @@ -261,10 +264,13 @@ func (r *AgenticRunReconciler) SetupWithManager(mgr ctrl.Manager) error { return err } - return ctrl.NewControllerManagedBy(mgr). + builder := ctrl.NewControllerManagedBy(mgr). For(&agenticv1alpha1.AgenticRun{}). Owns(&agenticv1alpha1.AgenticRunApproval{}). - Watches(&corev1.Pod{}, handler.EnqueueRequestsFromMapFunc(r.handlePodEvent)). + Watches(&corev1.Pod{}, handler.EnqueueRequestsFromMapFunc(r.handlePodEvent), + builder.WithPredicates(predicate.NewPredicateFuncs(func(obj client.Object) bool { + return obj.GetNamespace() == r.Namespace + }))). Watches(&agenticv1alpha1.ApprovalPolicy{}, handler.EnqueueRequestsFromMapFunc(fanOutToActiveRuns)). Watches(&agenticv1alpha1.AgenticOLSConfig{}, handler.EnqueueRequestsFromMapFunc(fanOutToActiveRuns)). Watches(&corev1.ConfigMap{}, handler.EnqueueRequestsFromMapFunc( @@ -274,7 +280,9 @@ func (r *AgenticRunReconciler) SetupWithManager(mgr ctrl.Manager) error { } return fanOutToActiveRuns(ctx, obj) }, - )). + )) + + return builder. Named("agenticrun"). WithOptions(controller.Options{MaxConcurrentReconciles: maxConcurrent}). Complete(r) diff --git a/controller/agenticrun/reconciler_test.go b/controller/agenticrun/reconciler_test.go index 3c870b91..490650dc 100644 --- a/controller/agenticrun/reconciler_test.go +++ b/controller/agenticrun/reconciler_test.go @@ -484,7 +484,7 @@ func approveAgenticRunWithOption(t *testing.T, fc client.WithWatch, name string, func fakeBaseTemplate() *unstructured.Unstructured { return &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": "extensions.agents.x-k8s.io/v1alpha1", + "apiVersion": "extensions.agents.x-k8s.io/v1beta1", "kind": "SandboxTemplate", "metadata": map[string]any{ "name": "lightspeed-agent", diff --git a/controller/agenticrun/sandbox_manager.go b/controller/agenticrun/sandbox_manager.go index f42162c1..2e88b9c8 100644 --- a/controller/agenticrun/sandbox_manager.go +++ b/controller/agenticrun/sandbox_manager.go @@ -45,7 +45,11 @@ const ( // +kubebuilder:rbac:groups=agents.x-k8s.io,resources=sandboxes,verbs=get;list;watch var smClaimGVK = schema.GroupVersionKind{ - Group: "extensions.agents.x-k8s.io", Version: "v1alpha1", Kind: "SandboxClaim", + Group: "extensions.agents.x-k8s.io", Version: "v1beta1", Kind: "SandboxClaim", +} + +var smSandboxGVK = schema.GroupVersionKind{ + Group: "agents.x-k8s.io", Version: "v1beta1", Kind: "Sandbox", } // SandboxManager manages sandbox lifecycle: create, wait-ready, release. @@ -404,7 +408,7 @@ func (m *SandboxManager) createSandboxClaim( template := &unstructured.Unstructured{ Object: map[string]any{ - "apiVersion": "extensions.agents.x-k8s.io/v1alpha1", + "apiVersion": "extensions.agents.x-k8s.io/v1beta1", "kind": "SandboxTemplate", "metadata": map[string]any{ "name": name, @@ -413,23 +417,45 @@ func (m *SandboxManager) createSandboxClaim( LabelRun: string(run.UID), LabelStep: step, }, - "annotations": map[string]any{ - AnnotationRunName: run.Name, - }, "ownerReferences": []any{ownerRef}, }, "spec": map[string]any{ + "networkPolicyManagement": "Unmanaged", "podTemplate": map[string]any{ "spec": podSpecMap, }, }, }, } - if err := m.client.Create(ctx, template); err != nil && !apierrors.IsAlreadyExists(err) { return "", "", fmt.Errorf("%s: %w", errEnsureAgentTemplate, err) } + pool := &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "extensions.agents.x-k8s.io/v1beta1", + "kind": "SandboxWarmPool", + "metadata": map[string]any{ + "name": name, + "namespace": m.namespace, + "labels": map[string]any{ + LabelRun: string(run.UID), + LabelStep: step, + }, + "ownerReferences": []any{ownerRef}, + }, + "spec": map[string]any{ + "replicas": int64(0), + "sandboxTemplateRef": map[string]any{ + "name": name, + }, + }, + }, + } + if err := m.client.Create(ctx, pool); err != nil && !apierrors.IsAlreadyExists(err) { + return "", "", fmt.Errorf("create SandboxWarmPool for %s: %w", step, err) + } + claim := &unstructured.Unstructured{ Object: map[string]any{ "apiVersion": smClaimGVK.Group + "/" + smClaimGVK.Version, @@ -447,7 +473,7 @@ func (m *SandboxManager) createSandboxClaim( "ownerReferences": []any{ownerRef}, }, "spec": map[string]any{ - "sandboxTemplateRef": map[string]any{ + "warmPoolRef": map[string]any{ "name": name, }, "lifecycle": map[string]any{ @@ -557,9 +583,20 @@ func (m *SandboxManager) releaseSandboxClaim(ctx context.Context, claimName stri return fmt.Errorf("%s %q: %w", errDeleteSandboxClaim, claimName, err) } + pool := &unstructured.Unstructured{} + pool.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "extensions.agents.x-k8s.io", Version: "v1beta1", Kind: "SandboxWarmPool", + }) + pool.SetName(claimName) + pool.SetNamespace(m.namespace) + + if err := m.client.Delete(ctx, pool); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("delete SandboxWarmPool %q: %w", claimName, err) + } + tmpl := &unstructured.Unstructured{} tmpl.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "extensions.agents.x-k8s.io", Version: "v1alpha1", Kind: "SandboxTemplate", + Group: "extensions.agents.x-k8s.io", Version: "v1beta1", Kind: "SandboxTemplate", }) tmpl.SetName(claimName) tmpl.SetNamespace(m.namespace) @@ -568,7 +605,7 @@ func (m *SandboxManager) releaseSandboxClaim(ctx context.Context, claimName stri return fmt.Errorf("delete SandboxTemplate %q: %w", claimName, err) } - log.Info("Released SandboxClaim and SandboxTemplate", LogKeyClaim, claimName) + log.Info("Released SandboxClaim, SandboxWarmPool, and SandboxTemplate", LogKeyClaim, claimName) return nil } diff --git a/controller/agenticrun/sandbox_manager_test.go b/controller/agenticrun/sandbox_manager_test.go index 2be19ae0..33917155 100644 --- a/controller/agenticrun/sandbox_manager_test.go +++ b/controller/agenticrun/sandbox_manager_test.go @@ -340,7 +340,7 @@ func TestCreate_OTELEnvVars_SandboxClaim(t *testing.T) { tmpl := &unstructured.Unstructured{} tmpl.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "extensions.agents.x-k8s.io", Version: "v1alpha1", Kind: "SandboxTemplate", + Group: "extensions.agents.x-k8s.io", Version: "v1beta1", Kind: "SandboxTemplate", }) if err := fc.Get(context.Background(), types.NamespacedName{Name: name, Namespace: "test-ns"}, tmpl); err != nil { t.Fatalf("SandboxTemplate not found: %v", err) diff --git a/controller/agenticrun/timeout_handler.go b/controller/agenticrun/timeout_handler.go new file mode 100644 index 00000000..79c65a7a --- /dev/null +++ b/controller/agenticrun/timeout_handler.go @@ -0,0 +1,148 @@ +package agenticrun + +import ( + "context" + "fmt" + "time" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" +) + +const sandboxTimeoutCheckInterval = 1 * time.Minute + +// isSandboxClaimMode returns true if the current configuration selects +// sandbox-claim mode. When Sandbox CRDs are not installed, the config +// cache forces bare-pod mode so this naturally returns false. +func (r *AgenticRunReconciler) isSandboxClaimMode() bool { + cfg := r.Config.Get() + return cfg != nil && cfg.Sandbox.Mode == sandboxModeSandboxClaim +} + +// runTimeoutLoop dispatches to the mode-appropriate timeout handler. +// Stopped when ctx is cancelled (manager shutdown). +func (r *AgenticRunReconciler) runTimeoutLoop(ctx context.Context) error { + for { + select { + case <-ctx.Done(): + return nil + case <-time.After(sandboxTimeoutCheckInterval): + r.handleTimeEvent(ctx) + } + } +} + +// handleTimeEvent collects sandbox pods based on the current mode and +// checks each for start/overall timeouts, retrying completion for +// terminal pods whose step condition patch failed earlier. +type podEntry struct { + pod *corev1.Pod + step string + runName string +} + +func (r *AgenticRunReconciler) handleTimeEvent(ctx context.Context) { + log := logf.FromContext(ctx).WithName("sandbox-timeout") + + var entries []podEntry + if r.isSandboxClaimMode() { + entries = r.listSandboxPods(ctx, log) + } else { + entries = r.listBarePods(ctx, log) + } + + now := time.Now() + for _, e := range entries { + condType := stepConditionType(e.step) + + var run agenticv1alpha1.AgenticRun + if err := r.Get(ctx, client.ObjectKey{Name: e.runName, Namespace: r.Namespace}, &run); err != nil { + continue + } + + if !isStepInProgress(&run, condType) { + continue + } + + phase := e.pod.Status.Phase + if phase == corev1.PodSucceeded || phase == corev1.PodFailed { + log.Info("retrying completion for terminal pod", LogKeyName, e.pod.Name, LogKeyStep, e.step) + _ = r.completeStep(ctx, &run, e.pod, e.step, condType, "") + continue + } + + var message string + created := e.pod.CreationTimestamp.Time + if startTimedOut(phase, created, now, podStartTimeout) { + message = fmt.Sprintf("sandbox pod did not start within %s", podStartTimeout) + } else if overallTimedOut(created, now, stepTimeout(e.step)) { + message = fmt.Sprintf("sandbox exceeded timeout %s", stepTimeout(e.step)) + } else { + continue + } + + _ = r.completeStep(ctx, &run, e.pod, e.step, condType, message) + } +} + +// --------------------------------------------------------------------------- +// Pod listing by mode +// --------------------------------------------------------------------------- + +func (r *AgenticRunReconciler) listBarePods(ctx context.Context, log logr.Logger) []podEntry { + var pods corev1.PodList + if err := r.List(ctx, &pods, client.InNamespace(r.Namespace), client.HasLabels{LabelRun, LabelStep}); err != nil { + log.Error(err, "failed to list sandbox pods") + return nil + } + var entries []podEntry + for i := range pods.Items { + pod := &pods.Items[i] + step, runName := resolveBarePodMetadata(pod) + if step == "" || runName == "" { + continue + } + entries = append(entries, podEntry{pod, step, runName}) + } + return entries +} + +func (r *AgenticRunReconciler) listSandboxPods(ctx context.Context, log logr.Logger) []podEntry { + var pods corev1.PodList + if err := r.List(ctx, &pods, client.InNamespace(r.Namespace), client.HasLabels{"agents.x-k8s.io/sandbox-name-hash"}); err != nil { + log.Error(err, "failed to list sandbox pods") + return nil + } + var entries []podEntry + for i := range pods.Items { + pod := &pods.Items[i] + step, runName, _ := resolveSandboxPodMetadata(ctx, r.Client, pod) + if step == "" || runName == "" { + continue + } + entries = append(entries, podEntry{pod, step, runName}) + } + return entries +} + +// --------------------------------------------------------------------------- +// Shared timeout helpers +// --------------------------------------------------------------------------- + +// startTimedOut returns true if the resource has not reached Running within the deadline. +// For bare-pod mode, phase is checked to skip already-terminal pods. +// For sandbox-claim mode, pass an empty string as phase (caller checks Ready separately). +func startTimedOut(phase corev1.PodPhase, created, now time.Time, timeout time.Duration) bool { + if phase == corev1.PodRunning || phase == corev1.PodSucceeded || phase == corev1.PodFailed { + return false + } + return now.Sub(created) > timeout +} + +func overallTimedOut(created, now time.Time, timeout time.Duration) bool { + return now.Sub(created) > timeout +} diff --git a/pkg/configuration/config.go b/pkg/configuration/config.go index a7b09226..4acaae13 100644 --- a/pkg/configuration/config.go +++ b/pkg/configuration/config.go @@ -55,8 +55,9 @@ type Config struct { // Components that need to react to config changes (e.g. OTEL provider) // are registered via SetOTELProvider and invoked from OnConfigMapChange. type Cache struct { - config atomic.Pointer[Config] - otelProvider *Provider + config atomic.Pointer[Config] + otelProvider *Provider + ForceBareMode bool } // Get returns the current config, or nil if the ConfigMap has not been seen. @@ -105,6 +106,10 @@ func (c *Cache) update(cm *corev1.ConfigMap) error { if err != nil { return err } + if c.ForceBareMode && cfg.Sandbox.Mode != "bare-pod" { + logf.Log.Info("Sandbox CRDs not installed, overriding sandbox-mode to bare-pod", "requested", cfg.Sandbox.Mode) + cfg.Sandbox.Mode = "bare-pod" + } c.config.Store(cfg) return nil } diff --git a/test/e2e/execution_test.go b/test/e2e/execution_test.go index 4936a5c8..f7ef97b8 100644 --- a/test/e2e/execution_test.go +++ b/test/e2e/execution_test.go @@ -8,8 +8,10 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" @@ -43,17 +45,36 @@ func TestExecutionFlow_ProposedToVerifying(t *testing.T) { waitForPhase(t, c, prop.Name, agenticv1alpha1.AgenticRunPhaseExecuting) t.Log("Phase reached: Executing — checking RBAC") - // --- Verify: RBAC created --- + // --- Verify: RBAC created (poll — RBAC is created inside Execute which + // runs after the Executed=Unknown status patch) --- roleName := "ls-exec-" + runUID var role rbacv1.Role - if err := c.Get(ctx, types.NamespacedName{Name: roleName, Namespace: "staging"}, &role); err != nil { - t.Fatalf("get Role %s in staging: %v", roleName, err) + if err := wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, true, func(ctx context.Context) (bool, error) { + err := c.Get(ctx, types.NamespacedName{Name: roleName, Namespace: "staging"}, &role) + if err == nil { + return true, nil + } + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + }); err != nil { + t.Fatalf("timed out waiting for Role %s in staging: %v", roleName, err) } t.Logf("RBAC Role %s exists in staging namespace", roleName) var binding rbacv1.RoleBinding - if err := c.Get(ctx, types.NamespacedName{Name: roleName, Namespace: "staging"}, &binding); err != nil { - t.Fatalf("get RoleBinding %s in staging: %v", roleName, err) + if err := wait.PollUntilContextTimeout(ctx, pollInterval, pollTimeout, true, func(ctx context.Context) (bool, error) { + err := c.Get(ctx, types.NamespacedName{Name: roleName, Namespace: "staging"}, &binding) + if err == nil { + return true, nil + } + if apierrors.IsNotFound(err) { + return false, nil + } + return false, err + }); err != nil { + t.Fatalf("timed out waiting for RoleBinding %s in staging: %v", roleName, err) } t.Logf("Verified: RoleBinding %s exists in staging", roleName) diff --git a/test/e2e/failure_test.go b/test/e2e/failure_test.go index 011ea3ad..64eb40d3 100644 --- a/test/e2e/failure_test.go +++ b/test/e2e/failure_test.go @@ -116,7 +116,7 @@ func TestSandboxTimeout(t *testing.T) { t.Logf("AgenticRun created: %s/%s", testNS, prop.Name) t.Log("Waiting for phase: Failed (sandbox timeout — this takes ~11 minutes)") - updated := waitForPhaseWithTimeout(t, c, prop.Name, agenticv1alpha1.AgenticRunPhaseFailed, 12*time.Minute) + updated := waitForPhaseWithTimeout(t, c, prop.Name, agenticv1alpha1.AgenticRunPhaseFailed, 15*time.Minute) t.Log("Phase reached: Failed") assertStepCondition(t, updated.Status.Conditions, agenticv1alpha1.AgenticRunConditionAnalyzed,