Skip to content

feat(substrate): render BYO extraContainers in the SandboxAgent ActorTemplate - #2799

Open
carlochessamp wants to merge 4 commits into
kagent-dev:release/v0.10.xfrom
carlochessamp:feat/substrate-extra-containers-combined
Open

feat(substrate): render BYO extraContainers in the SandboxAgent ActorTemplate#2799
carlochessamp wants to merge 4 commits into
kagent-dev:release/v0.10.xfrom
carlochessamp:feat/substrate-extra-containers-combined

Conversation

@carlochessamp

Copy link
Copy Markdown

Supersedes #2691 and #2750 — this is the same change, combined into one PR so it can be reviewed and landed as a unit. Both earlier PRs are closed in favor of this one.

Problem

byo.deployment.extraContainers (and the declarative deployment.extraContainers) render into the translator's pod template and work on the plain Deployment path, but on substrate buildSandboxAgentActorTemplate picked out only the kagent container and built a one-element Containers slice — every extra container was silently dropped. Nothing fails: the CRD accepts them, the ActorTemplate CRD supports up to ten containers, the sidecars are just gone at runtime.

Concrete case: an agent plus a credential-brokering forward proxy on 127.0.0.1 — on substrate the proxy never starts and the agent loses all outbound API access.

Fix

Render the pod template's remaining containers as additional ActorTemplate containers, appended after the agent container:

  • Index-based agent lookup (findKagentContainerIndex) — the agent container stays Containers[0], so the durableDir session-store mount (applyDurableDirSessionStore) remains agent-private.
  • Image must be digest-pinned (pinImageRef, per container — snapshots key on the image).
  • Command must be explicit — command and args are concatenated; the combined command must be non-empty (atelet copies Command verbatim into the OCI Process.Args with no shell and no image-entrypoint fallback).
  • Env is fail-closed: literals and secretKeyRef survive; envFrom and any other valueFrom source (configMapKeyRef, fieldRef, ...) are rejected with a named-container error instead of silently dropped — a sidecar whose credential source vanished mid-flight is worse than a rejected apply.
  • VolumeMounts are rejected — ActorTemplate volumes only support the single durableDir already consumed by the agent's /data session store.
  • HTTP readiness probes map to Readyz, so actor readiness keeps gating on the sidecar; named ports resolve against the extra container's own containerPorts (the translator only registers the agent container's http port). Resources, container ports, and TCP/exec probes have no ActorTemplate equivalent and are ignored.
  • At most 10 containers (the CRD's MaxItems).
  • The extras feed actorTemplateShapeHash, so adding/removing a sidecar fans out blue-green to a new template + actors like any spec change.

Resolution of the two approaches

#2691 contributed the structure (index refactor, Readyz mapping, named-port resolution, 10-container cap, shape-hash assertion); #2750 contributed the fail-closed env policy and the concatenated command/args emptiness check. Two semantic deltas from #2691 as originally written, both toward #2750's stricter behavior:

  • configMapKeyRef/other valueFrom env is now rejected rather than silently dropped;
  • the command check runs on the concatenated command+args, so an args-only sidecar is accepted instead of being required to set command.

Scoped to the SandboxAgent path; the AgentHarness substrate path has no extra-container field today. Based on the current release/v0.10.x tip (both earlier PRs were behind).

Testing

  • TestBuildSandboxAgentActorTemplateExtraContainers: happy path (command/args concat, literal + secretKeyRef env, agent stays container 0 with the durableDir mount, readyz mapped, no mounts), named readiness-port resolution, and the failure modes (unpinned image, missing command/args, envFrom, configMapKeyRef, fieldRef, volume mounts, unresolvable named port, >10 containers) — plus the shape-hash template-name change.
  • go test ./core/pkg/sandboxbackend/... green (4 packages), go vet clean.

carlochessamp and others added 4 commits September 3, 2026 21:29
…Template

buildSandboxAgentActorTemplate only ever emitted the agent container, so
byo.deployment.extraContainers (and the declarative equivalent) silently
disappeared for SandboxAgents on substrate — the translator pod template
carries them, but every SandboxAgent reconciles through substrate on 0.10,
where there is no pod.

Render them as additional ActorTemplate containers. The gVisor sandbox
gives them the sidecar semantics a Kubernetes pod would: one shared
loopback network namespace, separate rootfs/mount namespaces. Sidecars
like a credential-brokering forward proxy (127.0.0.1:<port>) work
unchanged.

Substrate containers are a strict subset of corev1.Container, so the
conversion is explicit and fail-closed:
- image must be digest-pinned (pinImageRef, per container)
- command must be explicit (same rule as the BYO agent cmd)
- env keeps literals and secretKeyRef; envFrom/configMapKeyRef drop
- an HTTP readiness probe maps to Readyz (named ports resolve against
  the container's own containerPorts)
- volumeMounts are rejected: ActorTemplate volumes only support a single
  durableDir, already consumed by the agent's /data session store
- at most 10 containers (CRD MaxItems)

Extras append after the agent container; applyDurableDirSessionStore
keeps mounting /data on Containers[0].
…emplate

buildSandboxAgentActorTemplate constructed a single-container ActorTemplate
from the pod template's kagent container, silently dropping every other
container the translator had already appended (SharedDeploymentSpec.
ExtraContainers). A BYO agent with extra containers therefore worked on
the Deployment path but lost its sidecars the moment it ran on substrate —
e.g. a credential-brokering proxy whose injected env and secretKeyRef never
reached the actor.

Render the pod template's remaining containers as additional ActorTemplate
containers. The ActorTemplate container surface is restricted, so
unsupported constructs fail the build loudly instead of being silently
dropped: the image must be digest-pinned, the command must be explicit
(substrate copies Command verbatim into the OCI Process.Args with no
entrypoint fallback), env supports literal values and secretKeyRef only
(envFrom, fieldRef/configMapKeyRef, volumeMounts are rejected). Resources,
ports and probes have no ActorTemplate equivalent and are ignored.

The agent container stays first, so the durableDir session-store mount
remains agent-private, and the shape hash picks up the extra containers so
config changes keep fanning out blue-green to a new template.
… mapping

Combines the two open PRs into one: kagent-dev#2691's structure (index-based agent
lookup, readiness-probe -> Readyz mapping with named-port resolution,
max-10 CRD cap, shape-hash fan-out) with kagent-dev#2750's fail-closed env policy
(envFrom and any non-secretKeyRef valueFrom rejected with a named-container
error instead of silently dropped) and the concatenated command/args
emptiness check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant