🐛 Report ACP readiness on AgentRun and probe the sandbox ACP port

[ibolton336]

Why

AgentRun.status.phase=Running is set the moment the Sandbox object exists — before the pod is scheduled, the image pulled, the repo cloned, goose serve started, or the harness bound :4000. The sandbox pod has no readiness probe, so pod Ready and Sandbox Ready both flip true when the process starts, and nothing tells a client when :4000 will accept. Every ACP client (the hub relay single-dials and closes the browser socket on failure; tackle2-ui and the earlier hub-shim grew dial-retry loops) has been papering over the same window.

Two facts were hiding in one field. This PR makes them two fields, the way pods separate phase from the Ready condition.

Fixes #130. Refs #65 (cancel: Pending can now have a live Sandbox — see below).

What

• phase follows the agent process. Pending until the sandbox pod is Running, then Running (one-way), then Succeeded/Failed when the Sandbox reports Finished. StartTime is the pod's start (container startedAt, else pod startTime, else Sandbox creation), so Duration is the run's wall time. A run that finishes before the controller sees its pod run may go straight from Pending to a terminal phase. This is already a fix vs. today (Running at Sandbox-object creation).
• ACPReady condition (new) says whether the agent's ACP endpoint accepts connections:
  • the sandbox pod's agent container declares port acp/4000 and a tcpSocket:4000 readiness probe (periodSeconds: 2). Readiness only gates the pod's Ready condition; it never restarts the container. Expect kubelet Unhealthy events during startup — that window is the point.
  • the Sandbox Ready condition (agent-sandbox v0.5.0: pod Running + Ready + podIP, and the headless Service present) turns ACPReady=True / reason Listening with the address in the message; otherwise False / NotListening carrying the sandbox's own message ("Pod is Running but not Ready" …); False / Finished once the run ends.
  • clients dial on ACPReady=True, never on phase. The constant is exported as v1alpha1.AgentRunConditionACPReady.
• Controller plumbing: the reconciler reads the sandbox pod (named after the Sandbox) and watches Pods via the konveyor.io/agentrun label it already stamps on the PodTemplate; the manager's Pod cache is restricted to that label (controller.SandboxPodCacheOptions()), so this does not mean caching every pod in the cluster. RBAC gains pods: get/list/watch. Finished is still checked first, so a crash before binding goes Pending → Failed.
• Field docs + CRD regenerated to say what phase, StartTime, Ready and ACPReady mean.
• E2E: the kind e2e stub (images/agentic-controller-agent) used to sleep infinity and never listen; nothing asserted phase. It now serves :4000 like a real agent (python3 -m http.server after a configurable startup delay — STUB_ACP_DELAY_SECONDS, 12 s in hack/e2e/resources.yaml), and hack/run-e2e.sh asserts the contract: pod Running + :4000 closed reads phase=Running / ACPReady=False (NotListening) (dialed by pod IP so no DNS negative cache is seeded; SKIP, never a false pass, if the window isn't observable); ACPReady turns True together with pod Ready + Sandbox Ready; pod Ready did not precede the stub's own "listening" line (timing-independent); a single dial of the pod's :4000 right after ACPReady returns 200 from an in-cluster curl pod that was Ready before the run existed — no retry; and <sandbox>.<ns>.svc:4000 answers within a bounded 10 s window (its A record rides EndpointSlice → CoreDNS, which nothing in the status chain waits for). The dialer reuses the already-loaded e2e agent image (no registry pull); hack/setup-e2e.sh restarts the manager so reruns on a reused cluster test the rebuilt image.

Resulting status on a live run:

phase: Running
Ready=False (Running): Agent is running
ACPReady=True (Listening): ACP endpoint e2e-run.default.svc:4000 accepts connections

What changes for clients

Dial on conditions[type=ACPReady].status == "True". A refused dial after that is a real error, not startup lag. Two honest caveats: (1) the sandbox Service is headless without publishNotReadyAddresses, so before readiness the name has no A record — a client that dials by DNS early sees "no such host" instead of connection refused; (2) the Service name's record lands via EndpointSlice → CoreDNS, independent of the Sandbox → AgentRun status chain, so a DNS-name dial within ~a second of the flip can in principle still race (pod-IP dials cannot). The multi-second startup race is closed; that sub-second DNS tail is not something this controller can promise.

phase keeps meaning "executing": a run whose listener never binds (tee bind failure — harness README updated) shows Running with ACPReady=False (NotListening) instead of silently looking dialable. Runs whose pod never starts (ImagePullBackOff under restartPolicy: Never, unschedulable) stay Pending with reason PodNotRunning instead of reporting Running. Cancel (#65, ADR 0006) assumed Pending implies no Sandbox; after this change Pending can have a live Sandbox, so cancel must delete the Sandbox in Pending too.

Docs

ADRs are immutable and none records the old Running trigger; ADR 0003's flow (wait, then dial svc:4000) is what ACPReady makes true. The contract lives in the AgentRun field docs/CRD, the exported condition constant, the changelog fragment, and a clause in harness/README.md.

Verified

• make lint (0 issues), make test (envtest controller suite green), make manifests/generate clean. The contract itself is covered by the kind e2e rather than envtest: it needs a real kubelet probe and the sandbox controller to mean anything.
• CONTAINER_TOOL=docker make e2e on a fresh kind cluster + agent-sandbox v0.5.0: 16/16 PASS, 0 skipped. The same script against a controller built from origin/main fails the readiness assertions (no ACPReady, Running before the port accepts).
• Context from the real harness on OpenShift (✨ Add Agent endpoints. tackle2-hub#1119 findings): the pre-fix window measured ~9–10 s past Running, 4 dial attempts from the client; this is what lets the UI/hub retry loops go.

Note: #157 edits the adjacent container-spec lines; the two merge cleanly in either order.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1ecce35a-4b74-401f-85ce-39384131d933

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jortel]

Q: Is it true (correct) that sandbox accepting ACP means Phase=running? Or would it be more correct for Phase=Running when the sandbox pod phase=Running and there be a condition that indicates READY (or ALIVE, LISTENING, etc)?

[ibolton336]

Agreed — that's the better shape, and the PR now does exactly that (force-pushed as f30937c; title/body updated):

• phase=Running when the sandbox pod phase is Running (agent process executing). Still a fix vs. today, where Running fires when the Sandbox object exists.
• New condition ACPReady (v1alpha1.AgentRunConditionACPReady): True/Listening once the Sandbox is Ready — the agent container's new tcpSocket:4000 readiness probe passes and the sandbox Service exists — else False/NotListening (carrying the sandbox's message, e.g. "Pod is Running but not Ready"), and False/Finished at the end. Clients dial on this, never on phase.

The kind e2e asserts the split: pod Running + :4000 closed ⇒ phase=Running, ACPReady=False (NotListening); ACPReady flips True together with pod/Sandbox Ready; first dial after that succeeds, no retry. Happy to rename the condition if you'd rather Listening/Ready-something — the existing Ready condition already means "run outcome", which is why I didn't reuse it.

[djzager]

ACK

[djzager]

Approving — this fixes a real, measured client-facing bug (#130, the Running-before-dialable window that's been forcing retry loops in the hub relay and tackle2-ui), and the core moves are right: splitting the live-endpoint fact out of phase is the correct instinct, ACPReady is a genuinely orthogonal condition, and the pod-read plumbing (label-restricted cache, resolve-by-name, pods: get/list/watch RBAC) is exactly what the status story needs. The e2e that asserts pod-Running + :4000-closed reads Running/ACPReady=False is a nice touch.

Heads-up on what's coming next so it's not a surprise, not a request to change anything here: when I implement #119 (LimitReached + terminationData, ADR 0011), I'm going to introduce a Knative/Job/Tekton-style Succeeded condition (True/False/Unknown + reason) as the terminal signal, and retire Ready-as-terminal into it. Reasoning: AgentRun is a run-to-completion object, and the k8s primitive it resembles is Job (which uses Complete/Failed conditions), not Pod — so Ready (a Pod steady-state 'serving traffic' property) is the wrong shape for 'did this run finish cleanly, and if not why' (Failed vs LimitReached vs, later, Refused/NoChanges). It also matches the Tekton alignment we already took with $(params.x) substitution.

What that means concretely, and why it doesn't touch this PR:

• ACPReady stays exactly as you built it — it's the one legitimately Pod-Ready-like fact (the live ACP window), orthogonal to the terminal outcome. No change.
• phase stays as the coarse compat mirror (ADR 0012 froze it; waitForRunning/isTerminalPhase key off it). #119 keeps it in lockstep; a later ADR supersedes 0012's phase clause and drops phase + Ready once clients read Succeeded.
• #119 builds on this PR's pod-read — it just extends the read to pull container exitCode + terminationMessage, maps exit 2 → Succeeded{False, LimitReached} + phase Failed, and stores opaque terminationData.

So: ship the readiness fix now; I'll do the Ready→Succeeded terminal correction in #119 before more clients pile onto Ready. Flagging early so we're aligned on the direction rather than discovering it in the #119 diff.