Skip to content

feat: add --default-agent-pod-labels flag for global agent pod labels#1534

Merged
EItanya merged 8 commits intomainfrom
eitanya/default-pod-labels
Mar 24, 2026
Merged

feat: add --default-agent-pod-labels flag for global agent pod labels#1534
EItanya merged 8 commits intomainfrom
eitanya/default-pod-labels

Conversation

@EItanya
Copy link
Copy Markdown
Contributor

@EItanya EItanya commented Mar 23, 2026

Summary

  • Adds a new --default-agent-pod-labels CLI flag / DEFAULT_AGENT_POD_LABELS env var that applies a set of labels to all agent pod templates created by kagent
  • Labels are specified as comma-separated key=value pairs (e.g. team=platform,environment=production)
  • Fully wired through Helm via controller.agentDeployment.podLabels map in values.yaml
  • Label precedence: built-in defaults → global default labels → per-agent spec.deployment.labels

Test plan

  • Unit tests for ParseLabels (8 cases covering valid input, edge cases, and errors)
  • Unit test for DEFAULT_AGENT_POD_LABELS env var loading
  • Existing translator golden tests pass
  • 4 new Helm unit tests (labels set, empty, not configured, single label)
  • Deployed to Kind cluster with podLabels: {team: platform, environment: production} — verified all 11 agent pod templates contain both labels
  • kubectl get pods -l team=platform returns all agent pods

Comment left by Claude on behalf of @EItanya

🤖 Generated with Claude Code

Add a runtime flag/env var (DEFAULT_AGENT_POD_LABELS) that allows
setting labels on all agent pod templates created by kagent. Labels
are specified as comma-separated key=value pairs and can be configured
via CLI flag, environment variable, or Helm values.

Label precedence (lowest to highest):
1. Built-in defaults (app.kubernetes.io/managed-by, etc.)
2. Global DefaultAgentPodLabels (from this flag)
3. Per-agent spec.deployment.labels

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Copilot AI review requested due to automatic review settings March 23, 2026 10:03
@EItanya EItanya requested a review from peterj as a code owner March 23, 2026 10:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for globally configuring default Kubernetes labels on all agent pod templates, wired end-to-end from Helm values → controller env var → controller flag/env parsing → translator label merge logic.

Changes:

  • Introduces --default-agent-pod-labels / DEFAULT_AGENT_POD_LABELS and parses key=value label pairs in the controller.
  • Applies global default labels during agent deployment translation with precedence: built-ins → global defaults → per-agent labels.
  • Adds Helm values + template logic + chart unit tests to emit DEFAULT_AGENT_POD_LABELS when configured.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
helm/kagent/values.yaml Adds controller.agentDeployment.podLabels values entry and docs.
helm/kagent/tests/controller-deployment_test.yaml Adds Helm unit tests for DEFAULT_AGENT_POD_LABELS rendering behavior.
helm/kagent/templates/controller-configmap.yaml Renders DEFAULT_AGENT_POD_LABELS from podLabels.
go/core/pkg/app/app.go Adds flag/env plumbing and ParseLabels, applies parsed labels to translator defaults at startup.
go/core/pkg/app/app_test.go Adds unit tests for ParseLabels and env var loading of DEFAULT_AGENT_POD_LABELS.
go/core/internal/controller/translator/agent/deployments.go Merges global default labels into computed default labels before per-agent overrides.
go/core/internal/controller/translator/agent/adk_api_translator.go Introduces DefaultAgentPodLabels global for translator configuration.
Makefile Removes the top-level export behavior for .env overrides.
Comments suppressed due to low confidence (1)

helm/kagent/templates/controller-configmap.yaml:72

  • printf "%s=%s" $k $v will produce broken output if a label value is not a string (common when users set numeric values without quotes in values.yaml, e.g. build: 123). Use string conversion (e.g. toString $v or %v) to ensure all YAML scalar types render correctly as strings.
  {{- end }}
  {{- end }}


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +63 to +70
{{- if and .Values.controller.agentDeployment .Values.controller.agentDeployment.podLabels }}
{{- $pairs := list }}
{{- range $k, $v := .Values.controller.agentDeployment.podLabels }}
{{- $pairs = append $pairs (printf "%s=%s" $k $v) }}
{{- end }}
{{- if $pairs }}
DEFAULT_AGENT_POD_LABELS: {{ join "," $pairs | quote }}
{{- end }}
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Rendering DEFAULT_AGENT_POD_LABELS by ranging over a map produces nondeterministic key order in Go templates. That can cause flaky Helm unit tests and also non-reproducible chart renders (spurious diffs/rollouts) when podLabels has multiple entries. Consider sorting keys (e.g., iterate over keys ... | sortAlpha and build pairs in that order) so the joined string is stable.

Copilot uses AI. Check for mistakes.
Comment on lines +151 to +153
- equal:
path: data.DEFAULT_AGENT_POD_LABELS
value: "env=prod,team=platform"
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

This assertion hard-codes an ordering for DEFAULT_AGENT_POD_LABELS ("env=prod,team=platform"), but the template currently builds the string by ranging over a map, which is not order-stable. Either make the template sort keys before joining, or relax the test to accept either ordering to avoid flakes.

Suggested change
- equal:
path: data.DEFAULT_AGENT_POD_LABELS
value: "env=prod,team=platform"
- matchRegex:
path: data.DEFAULT_AGENT_POD_LABELS
pattern: '^(env=prod,team=platform|team=platform,env=prod)$'

Copilot uses AI. Check for mistakes.
@@ -1,6 +1,5 @@
# Load local overrides (gitignored) — e.g. KAGENT_HELM_EXTRA_ARGS=-f helm/kagent/values.local.yaml
-include .env
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Removing the top-level export means variables loaded from .env (intended as local overrides) will no longer be exported to recipe environments or sub-make -C ... invocations. This can break expected override behavior (e.g., a .env-provided VERSION/registry not propagating to make -C go ...). If the goal is to reduce exports, consider exporting only the override vars that need to propagate rather than dropping export entirely.

Suggested change
-include .env
-include .env
# Export selected override variables so .env values propagate to recipes and sub-make invocations
export DOCKER_REGISTRY BASE_IMAGE_REGISTRY DOCKER_REPO HELM_REPO HELM_DIST_FOLDER VERSION

Copilot uses AI. Check for mistakes.
EItanya and others added 4 commits March 23, 2026 10:10
Implement MapValue as a flag.Value for map[string]string, eliminating
the need for a raw string variable and post-parse step. LoadFromEnv
now parses labels directly via the flag's Set() method.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
- Sort keys in configmap template for deterministic label ordering
- Use toString for label values to handle numeric YAML scalars
- Selectively export .env variables instead of dropping export entirely

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
input string
want map[string]string
wantErr bool
}{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing the test case where an env var contains an equals (such as: key=val=ue). This could occur with a base64 value, for example, so we should have a test case for it.

Comment on lines +338 to +339
target = map[string]string{"team": "platform"}
assert.Equal(t, "team=platform", mv.String())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth noting that iterating over a map in Go is random, so if there were multiple keys here, the output would be inconsistent. It's worth considering sorting before ranging over the keys in the implementation.

iplay88keys
iplay88keys previously approved these changes Mar 23, 2026
Copy link
Copy Markdown
Contributor

@iplay88keys iplay88keys left a comment

Choose a reason for hiding this comment

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

Looks good overall. Left a few small comments. Also curious if we want an e2e test for this behavior so there isn't a regression in the future.

- Sort keys in MapValue.String() for deterministic output
- Add test case for values containing '=' (e.g. base64 strings)
- Add multi-key assertion to TestMapValueString to verify sort order

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
@EItanya EItanya merged commit 83ae4ca into main Mar 24, 2026
22 checks passed
@EItanya EItanya deleted the eitanya/default-pod-labels branch March 24, 2026 08:04
jholt96 pushed a commit to jholt96/kagent that referenced this pull request Mar 24, 2026
…kagent-dev#1534)

## Summary
- Adds a new `--default-agent-pod-labels` CLI flag /
`DEFAULT_AGENT_POD_LABELS` env var that applies a set of labels to all
agent pod templates created by kagent
- Labels are specified as comma-separated `key=value` pairs (e.g.
`team=platform,environment=production`)
- Fully wired through Helm via `controller.agentDeployment.podLabels`
map in values.yaml
- Label precedence: built-in defaults → global default labels →
per-agent `spec.deployment.labels`

## Test plan
- [x] Unit tests for `ParseLabels` (8 cases covering valid input, edge
cases, and errors)
- [x] Unit test for `DEFAULT_AGENT_POD_LABELS` env var loading
- [x] Existing translator golden tests pass
- [x] 4 new Helm unit tests (labels set, empty, not configured, single
label)
- [x] Deployed to Kind cluster with `podLabels: {team: platform,
environment: production}` — verified all 11 agent pod templates contain
both labels
- [x] `kubectl get pods -l team=platform` returns all agent pods

Comment left by Claude on behalf of @EItanya

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants