Skip to content

feat(agent): publish pod deletion cost from in-flight turns - #3144

Open
daryllimyt wants to merge 1 commit into
feat/agent-sandbox-cgroup-limitsfrom
feat/agent-executor-pod-deletion-cost
Open

feat(agent): publish pod deletion cost from in-flight turns#3144
daryllimyt wants to merge 1 commit into
feat/agent-sandbox-cgroup-limitsfrom
feat/agent-executor-pod-deletion-cost

Conversation

@daryllimyt

@daryllimyt daryllimyt commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Stacked on #3142 — base is feat/agent-sandbox-cgroup-limits; GitHub will retarget to main when that merges. Review only the top commit here.

Why

Kubernetes ReplicaSet scale-down picks pods by its default ranking (unready first, then youngest), not by load. With KEDA scaling the agent-executor on queue depth, a scale-down can delete the pod running ten in-flight 30-minute agent turns while an idle pod survives — and pod termination is irreversible, so those turns spend up to 31 minutes draining for nothing.

What

The worker now publishes its in-flight run_agent_activity count to its own pod's controller.kubernetes.io/pod-deletion-cost annotation, so the ReplicaSet always prefers deleting the emptiest pod.

  • tracecat/agent/executor/deletion_cost.py: best-effort publisher. Direct httpx PATCH (application/merge-patch+json) against the in-cluster API — no kubernetes dependency. Service-account token re-read per request (projected tokens rotate), CA-verified TLS, 5s timeout.
  • Enabled only when all of: TRACECAT__AGENT_EXECUTOR_POD_DELETION_COST_ENABLED (default true), KUBERNETES_SERVICE_HOST set, SA token file present, and TRACECAT__K8S_POD_NAME/TRACECAT__K8S_POD_NAMESPACE provided via the downward API. Anywhere else (compose, dev, Fargate) every call is a no-op.
  • Publishes only on count change (K8s docs warn against frequent deletion-cost updates); concurrent changes coalesce to the latest count behind a lock.
  • Failure policy: any API error logs a warning and never touches the activity; three consecutive failures (e.g. RBAC not deployed yet) permanently disable the publisher for the process with one final warning. Verified explicitly for 403s in tests.
  • run_agent_activity increments on entry and decrements in finally. Probe activities are not counted (millisecond noise).

Chart counterpart (downward-API env + namespaced Role/RoleBinding with get/patch on pods): TracecatHQ/k8s#77. Without it this code self-disables harmlessly.

Testing

  • 260-line unit suite: all disabled paths (flag/env/token/identity missing), PATCH shape (URL, content type, string annotation value), count coalescing under rapid start/stop, 403 → exactly three requests then silent disablement, success resetting the failure counter; plus activity lifecycle coverage in test_agent_activities.py.
  • ruff, basedpyright --warnings, and the touched test files all clean (64 passed).

Summary by cubic

Publish the agent-executor pod’s deletion cost based on its in-flight turns so scale-down prefers the emptiest pod and avoids draining long-running work. This reduces wasted time when KEDA scales down pods.

  • New Features

    • Publishes the in-flight run_agent_activity count to the pod’s controller.kubernetes.io/pod-deletion-cost via httpx PATCH (CA-verified TLS, 5s timeout). Updates only when the count changes and coalesces concurrent updates.
    • Failure policy: warn on errors; disable after 3 consecutive failures (e.g., missing RBAC). run_agent_activity now increments on start and decrements in finally; probe activities are ignored.
  • Migration

    • Kubernetes-only setup: set TRACECAT__K8S_POD_NAME and TRACECAT__K8S_POD_NAMESPACE via the downward API, ensure KUBERNETES_SERVICE_HOST and the service-account token are present, and grant the SA get/patch on pods. Feature flag TRACECAT__AGENT_EXECUTOR_POD_DELETION_COST_ENABLED defaults to true; otherwise this is a no-op.

Written for commit e61e453. Summary will update on new commits.

Review in cubic

Kubernetes ReplicaSet scale-down ranks pods by readiness and age, not
load, so KEDA can delete an agent-executor pod with ten in-flight
30-minute turns while an idle pod survives. Publish the in-flight
run_agent_activity count to the pod's
controller.kubernetes.io/pod-deletion-cost annotation so scale-down
prefers the emptiest pod.

Best-effort by design: PATCHes the pod via the in-cluster API with the
service-account token, updates only when the count changes, and after
three consecutive failures (e.g. missing RBAC) disables itself for the
process lifetime with a single warning.
@daryllimyt daryllimyt added enhancement New feature or request engine Improvements or additions to the workflow engine labels Jul 28, 2026
@zeropath-ai

zeropath-ai Bot commented Jul 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to e61e453.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► tracecat/agent/executor/activity.py
Delete     + Add get_pod_deletion_cost_publisher import and integrate deletion cost publishing in run_agent_activity
► tracecat/agent/executor/deletion_cost.py
Add new PodDeletionCostPublisher and helper get_pod_deletion_cost_publisher, including metrics, HTTP interactions, and publishing logic
► tests/unit/test_agent_activities.py
Update tests to assert increment/decrement publishing calls on success and on execution error
► tests/unit/test_agent_executor_deletion_cost.py
Add new tests for PodDeletionCostPublisher behavior and HTTP interactions
► tracecat/config.py
Add new configuration and environment variable handling for agent executor pod deletion cost publishing
► NOTE: Minor integration adjustments to import and use get_pod_deletion_cost_publisher in activity flow

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e61e453e3e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +97 to +99
if not self._enabled or self._count == count:
self._publishing = False
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Retry a failed stable-count publication

When the PATCH for a stable count fails transiently, published remains false but _count == count still stops the publishing loop. With the default single concurrent activity, a failed 0→1 update is therefore never retried until that long-running turn ends, leaving the pod advertised as idle and preferred for scale-down for the entire turn. Retry unsuccessful publications with bounded backoff instead of treating an unchanged count as successfully settled.

Useful? React with 👍 / 👎.

Comment on lines +163 to +164
# Lazy singleton - no lifespan required.
_pod_deletion_cost_publisher: PodDeletionCostPublisher | None = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset stale deletion cost when the worker starts

When this container process restarts inside an existing pod after publishing a positive cost, Kubernetes retains that pod annotation, but the lazy singleton starts its local count at zero without publishing it. If the restarted worker remains idle, the stale positive cost persists indefinitely and can cause ReplicaSet scale-down to delete a genuinely busy lower-cost pod instead; initialize the publisher during worker startup and publish zero before accepting activities.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine Improvements or additions to the workflow engine enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant