feat(chart): support multiple agent fleets in one release

[joryirving]

What

Model the foreman chart's agents as a map (agents: <key>: {...}) that renders one Deployment + ServiceAccount + ClusterRole + ClusterRoleBinding + gate-cache PVC per entry, suffixed with the agent key. Operator / webhook / CRDs / coder RBAC stay singleton, so multiple heterogeneous agent fleets coexist in one chart release instead of needing a second release (which would collide on the chart's cluster-scoped singletons).

Why

Running a second differently-configured agent today means a second HelmRelease of the same chart, which fights the primary release for ownership of the CRDs, the webhook, the operator, and the fixed-name coder SA. The companion CRD-toggle (#998) made that workaround work; this issue makes it unnecessary by collapsing "another agent with different values" into the same chart, one release. Concrete use cases in the issue: a fork-pinned dogfood agent with its own PAT/role/commit-identity, per-language or per-model coder pools, GPU vs remote-model pools with distinct nodeLabels.

Fixes #994

How

• Backward compatibility: the legacy agent: block continues to render the exact resources it always has (Deployment / SA / ClusterRole / ClusterRoleBinding / PVC), with the exact same names (<fullname>-agent, <fullname>-gate-cache). Leaving agents unset keeps an in-place upgrade a no-op. The foreman.agents helper wraps the legacy block under a sentinel key (_implicit_) so a single template path renders both shapes; legacy users must not pass that key explicitly.
• Deep-merge defaults: each agents.<name> entry deep-merges over .Values.agent via mustMergeOverwrite, so users opting into the map only restate what differs from the legacy defaults (replicaCount, image, roles, nodeLabels, nodeSelector/affinity/tolerations, resources, gitRemoteURL, githubToken, commitAuthor*, gateCache, and a per-agent coder-git-secret pass-through via new agents.<name>.coderGitSecret / coderGitSecretKey fields that fall back to chart-level coder.gitCredentialsSecret).
• Per-agent naming: explicit entries render as <fullname>-<agentKey>-agent (Deployment / SA / ClusterRole / ClusterRoleBinding) and <fullname>-<agentKey>-gate-cache (PVC), truncated to 63 chars, so two fleets never collide.
• Singletons stay singleton: the operator Deployment, webhook Service / Secret / ValidatingWebhookConfiguration, all six foreman CRDs, and the coder SA / Role / RoleBinding still render once per release regardless of how many agents are listed.
• enabled: false skip: an agents.<name>.enabled: false entry (or omitting agents entirely) drops that agent's resources cleanly without affecting the others.
• Tests: added charts/foreman/tests/agents_test.yaml with 15 helm-unittest cases covering the legacy single-agent shape, multi-agent rendering, per-agent field propagation (gitRemoteURL, commit-author, coder-git-secret, githubToken), the enabled: false skip, and the singleton invariants. The foreman-chart CI job in .github/workflows/helm-chart.yml now installs the plugin and runs the suite; both charts/llmkube/tests/* and charts/foreman/tests/* are covered.

Verified locally: helm lint charts/foreman clean, helm template renders 21 resources at default (unchanged from before the patch) and 26 with the multi-agent fixture, helm unittest charts/foreman 15/15 pass, kubeconform 0 invalid on both renders, make fmt/vet/lint/test/lint-all all clean.

Assisted-by: LLM (Minimax-M3 via opencode, generated the chart template refactor, helm-unittest suite, and CI step; I designed the backward-compat path, verified every render, ran the gate locally, and own the review conversation).

Checklist

• Tests added/updated
• make test passes locally
• make lint passes locally
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance (if any) is disclosed above, per CONTRIBUTING.md
• Documentation updated (if user-facing change)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Defilan]

Nice work, Jory. Pulled it down and verified the important bits: default render is byte-identical to main, the deepCopy-per-iteration keeps the merge safe, per-agent RBAC binds to the right SA, and check-helm-rbac + helm unittest (15/15) are green.

One fix before merge. All three agent templates gate on {{- if $cfg.enabled }}, the raw entry evaluated before the deep-merge, so enabled is the only field that doesn't inherit from .Values.agent. An entry that omits it renders nothing, silently:

agents:
  gpu: {replicaCount: 2}   # no enabled -> zero resources, no error

That contradicts the "only restate what differs" model, and the fixture misses it because every entry sets enabled explicitly. Can you compute $agent before the gate and check $agent.enabled, and add a fixture case for the omitted-enabled path?

Minor, non-blocking: _implicit_ is an unguarded reserved key; the 63-char truncation can collide on long release-name + key combos; map fields like nodeLabels deep-merge so a per-agent entry can add but not remove defaults (worth a NOTES line).

Fix the enabled gate and I'm good to merge.

[joryirving]

Fixed in fb4da01. Moved the deep-merge ahead of the enabled gate in all three agent templates so the check evaluates the merged $agent.enabled, not the raw $cfg.enabled. Added an omitted-enabled fixture entry (replicaCount: 5, no enabled field) and two new test cases asserting it renders both a Deployment and a gate-cache PVC. Updated the SA/CR/CRB count to 9 and the PVC count to 2 to reflect the new 3-enabled-agent fixture.

Legacy render verified byte-identical to main (filtered for auto-generated cert material); helm lint clean; helm unittest charts/foreman 18/18 pass; make fmt/vet/lint/lint-all clean.

Two follow-ups you flagged as non-blocking — happy to address either or both in this PR or as separate issues, your call:

1. _implicit_ reserved key — currently documented in the helper comment but not in values.yaml. I can move the note to the agents: block so users see it without reading the helper source.
2. 63-char truncation collision on long <release-name>-<agentKey>-<role> combos — currently truncates silently with trunc 63 | trimSuffix "-", which can produce identical names from distinct inputs. A defensive guard (fail with a clear error when the post-truncation name is a duplicate, or warn via helm template --debug) would catch this at install time rather than at first apply.
3. Map fields (nodeLabels etc.) deep-merge so a per-agent entry can add but not remove defaults — worth a NOTES.txt line so users know to set the whole map rather than expecting partial removal.

Let me know which (if any) you'd like folded in here vs. split out.

[Defilan]

Thanks Jory, this nails it. Verified all three templates now gate on $agent.enabled after the merge, so an omitted enabled inherits the default and renders (I reproduced the previously-silent gpu case). Backward-compat is byte-stable (default still 21 resources, legacy names unchanged), the new omitted-enabled fixtures cover it, unittest is 18/18, and check-helm-rbac + lint are green. Nice bonus hardening the plugin install too. Approving.