From 288b5480da23cc511064abc5e360e3ec618fb218 Mon Sep 17 00:00:00 2001 From: sebas_correa Date: Thu, 10 Sep 2026 21:58:15 -0300 Subject: [PATCH] feat(agent): default worker idleTTL to 30m so stale workers get reaped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NP_WORKER_IDLE_TTL was unset by default, which disables the agent's own worker-idle reaper entirely — Deployments left behind by an old or removed package revision accumulate in the cluster forever instead of being cleaned up. Default to "30m"; still fully overridable via var.worker.idleTTL (including "" to restore the previous disabled behavior). Co-Authored-By: Claude Sonnet 5 --- nullplatform/agent/README.md | 2 +- nullplatform/agent/locals.tf | 7 +++++ .../agent/tests/agent_values.tftest.hcl | 26 +++++++++++++++++++ nullplatform/agent/variables.tf | 15 ++++++----- 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/nullplatform/agent/README.md b/nullplatform/agent/README.md index bcebe07b..51e6a722 100644 --- a/nullplatform/agent/README.md +++ b/nullplatform/agent/README.md @@ -197,7 +197,7 @@ resource "example_resource" "this" { | [service\_template](#input\_service\_template) | Specifies the name or reference of the scope service template to be used for deployment. Required when extra\_envs.INGRESS\_TYPE is 'istio' — the k8s scope's default template is AWS ALB Ingress and won't route traffic correctly through Istio, so it must be pointed at an Istio-compatible template instead. | `string` | `""` | no | | [tags\_selectors](#input\_tags\_selectors) | Map of tags used to select and filter channels and agents | `map(string)` | n/a | yes | | [use\_account\_slug](#input\_use\_account\_slug) | Flag to determine whether to use the account slug in resource naming | `string` | `""` | no | -| [worker](#input\_worker) | Extra worker-orchestration config, merged on top of the module's own computed
worker block: backend ("kubernetes" by default), allowedRegistries
(["public.ecr.aws/nullplatform/*"] by default, so the platform's own scope
images keep working), and a patch for the worker container (2Gi memory
limit, the deploy/DNS env vars below, and a serviceAccountName that always
mirrors service\_account\_name). allowedRegistries and patches set here are
concatenated with (not replacing) the module defaults — add your own
registries or an extra patch rather than having to repeat the defaults;
set backend here to override it outright. Anything else — security, idleTTL
(reap idle workers), the legacy defaults/rules/pins — passes through as-is.
See the nullplatform-agent chart values (>= 2.37.0) for the full shape.
null = nothing extra.

Example:
worker = {
allowedRegistries = ["123456789012.dkr.ecr.us-east-1.amazonaws.com/your-org/*"]
patches = [{ target = { package = "my-pkg" }, merge = { spec = { serviceAccountName = "np-agent-sa" } } }]
idleTTL = "30m"
} | `any` | `null` | no | +| [worker](#input\_worker) | Extra worker-orchestration config, merged on top of the module's own computed
worker block: backend ("kubernetes" by default), allowedRegistries
(["public.ecr.aws/nullplatform/*"] by default, so the platform's own scope
images keep working), idleTTL ("30m" by default, so worker Deployments
left behind by an old/removed package revision get reaped instead of
accumulating forever), and a patch for the worker container (2Gi memory
limit, the deploy/DNS env vars below, and a serviceAccountName that always
mirrors service\_account\_name). allowedRegistries and patches set here are
concatenated with (not replacing) the module defaults — add your own
registries or an extra patch rather than having to repeat the defaults;
set backend or idleTTL here to override them outright (e.g. idleTTL = ""
to disable the reaper, matching this module's pre-idleTTL-default
behavior). Anything else — security, the legacy defaults/rules/pins —
passes through as-is. See the nullplatform-agent chart values (>= 2.37.0)
for the full shape. null = nothing extra beyond the defaults above.

Example:
worker = {
allowedRegistries = ["123456789012.dkr.ecr.us-east-1.amazonaws.com/your-org/*"]
patches = [{ target = { package = "my-pkg" }, merge = { spec = { serviceAccountName = "np-agent-sa" } } }]
idleTTL = "1h"
} | `any` | `null` | no | | [worker\_memory\_limit](#input\_worker\_memory\_limit) | Memory limit for a worker-orchestrated package's pod (packages in var.worker\_orchestrated\_packages). The chart's own default is small enough to OOM mid-tofu-apply for packages that run real IaC tooling. | `string` | `"2Gi"` | no | | [worker\_orchestrated\_packages](#input\_worker\_orchestrated\_packages) | Package slugs whose worker-orchestrator (package-exec) pods should run
under var.service\_account\_name (the same IRSA identity as the agent
itself) and var.worker\_memory\_limit, via a per-package worker-container
patch. Add a package's slug here whenever its worker needs to assume an
AWS role, or needs more memory than the chart's own default (e.g. to run
tofu/terraform); a worker for a package not listed here falls back to the
namespace's default ServiceAccount and the chart's own memory default.

This is separate from the "containers" scope's own k8s-deployment env
vars (DNS\_TYPE, DOMAIN, etc.), which remain specific to that package
regardless of what's listed here. | `list(string)` | 
[
  "containers"
]
| no | diff --git a/nullplatform/agent/locals.tf b/nullplatform/agent/locals.tf index e7bde312..18a7b3bc 100644 --- a/nullplatform/agent/locals.tf +++ b/nullplatform/agent/locals.tf @@ -127,6 +127,13 @@ locals { backend = "kubernetes" allowedRegistries = ["public.ecr.aws/nullplatform/*"] patches = concat(local.worker_common_patches, [local.worker_container_patch]) + # Reap worker-orchestrated pods (and their Deployments) after 30m with no + # activity. Previously unset (NP_WORKER_IDLE_TTL empty), which disables + # the reaper entirely — stale workers from old package revisions or + # removed packages accumulate forever instead of being cleaned up. + # Override per-install via var.worker.idleTTL (see its docs for the + # shape) if a longer/shorter window is needed. + idleTTL = "30m" } worker_final = merge( diff --git a/nullplatform/agent/tests/agent_values.tftest.hcl b/nullplatform/agent/tests/agent_values.tftest.hcl index beeb54dc..027aba93 100644 --- a/nullplatform/agent/tests/agent_values.tftest.hcl +++ b/nullplatform/agent/tests/agent_values.tftest.hcl @@ -233,6 +233,32 @@ run "worker_defaults" { condition = strcontains(helm_release.agent.values[0], "\"serviceAccountName\": \"nullplatform-agent\"") error_message = "the worker's serviceAccountName must default to service_account_name's default (nullplatform-agent)" } + + assert { + condition = strcontains(helm_release.agent.values[0], "\"idleTTL\": \"30m\"") + error_message = "idleTTL must default to 30m so worker Deployments from an old/removed package revision get reaped instead of accumulating forever" + } +} + +# idleTTL is a plain override (like backend), not additive (like +# allowedRegistries/patches) — var.worker's value must win outright, +# including the empty string, which disables the reaper. +run "worker_idle_ttl_override_wins_outright" { + command = plan + + variables { + worker = { + idleTTL = "" + } + } + + assert { + condition = ( + strcontains(helm_release.agent.values[0], "\"idleTTL\": \"\"") && + !strcontains(helm_release.agent.values[0], "\"idleTTL\": \"30m\"") + ) + error_message = "var.worker.idleTTL must override the module default outright, including disabling it with an empty string" + } } # var.worker stays available as an extra/override layer on top of the computed diff --git a/nullplatform/agent/variables.tf b/nullplatform/agent/variables.tf index b0ee81af..aa0146bd 100644 --- a/nullplatform/agent/variables.tf +++ b/nullplatform/agent/variables.tf @@ -108,21 +108,24 @@ variable "worker" { Extra worker-orchestration config, merged on top of the module's own computed worker block: backend ("kubernetes" by default), allowedRegistries (["public.ecr.aws/nullplatform/*"] by default, so the platform's own scope - images keep working), and a patch for the worker container (2Gi memory + images keep working), idleTTL ("30m" by default, so worker Deployments + left behind by an old/removed package revision get reaped instead of + accumulating forever), and a patch for the worker container (2Gi memory limit, the deploy/DNS env vars below, and a serviceAccountName that always mirrors service_account_name). allowedRegistries and patches set here are concatenated with (not replacing) the module defaults — add your own registries or an extra patch rather than having to repeat the defaults; - set backend here to override it outright. Anything else — security, idleTTL - (reap idle workers), the legacy defaults/rules/pins — passes through as-is. - See the nullplatform-agent chart values (>= 2.37.0) for the full shape. - null = nothing extra. + set backend or idleTTL here to override them outright (e.g. idleTTL = "" + to disable the reaper, matching this module's pre-idleTTL-default + behavior). Anything else — security, the legacy defaults/rules/pins — + passes through as-is. See the nullplatform-agent chart values (>= 2.37.0) + for the full shape. null = nothing extra beyond the defaults above. Example: worker = { allowedRegistries = ["123456789012.dkr.ecr.us-east-1.amazonaws.com/your-org/*"] patches = [{ target = { package = "my-pkg" }, merge = { spec = { serviceAccountName = "np-agent-sa" } } }] - idleTTL = "30m" + idleTTL = "1h" } EOT type = any