Skip to content

helm: Allow configuring agent memory#1869

Open
marcofranssen wants to merge 1 commit into
kagent-dev:mainfrom
marcofranssen:helm-agent-memory
Open

helm: Allow configuring agent memory#1869
marcofranssen wants to merge 1 commit into
kagent-dev:mainfrom
marcofranssen:helm-agent-memory

Conversation

@marcofranssen
Copy link
Copy Markdown
Contributor

No description provided.

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 a configurable per-agent memory block (with enabled, ttlDays, and modelConfigRef) to every built-in agent subchart, exposes those same knobs at the top-level helm/kagent/values.yaml so users can override them, and introduces an .editorconfig for repo-wide formatting defaults.

Changes:

  • Add memory.enabled / memory.ttlDays / memory.modelConfigRef to each agent's values.yaml and render a corresponding memory: block in each agent's templates/agent.yaml when enabled.
  • Surface the same modelConfigRef and memory.* fields per agent in the top-level helm/kagent/values.yaml.
  • Add a root .editorconfig and minor whitespace/style cleanup.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

File Description
helm/kagent/values.yaml Adds modelConfigRef/memory blocks for each agent; contains a duplicated memory: key and an inconsistent ~ value under kgateway-agent.
helm/agents/{k8s,kgateway,istio,promql,observability,argo-rollouts,helm,cilium-policy,cilium-manager,cilium-debug}/values.yaml Introduce memory defaults (enabled: false, ttlDays: 15, modelConfigRef: "").
helm/agents/{...}/templates/agent.yaml Conditionally render memory.modelConfig and memory.ttlDays under spec.declarative when memory.enabled is true.
.editorconfig New repo-wide formatting rules (LF, UTF-8, 2-space indent, trim trailing whitespace).

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

Comment thread helm/kagent/values.yaml
Comment thread helm/kagent/values.yaml Outdated
@EItanya
Copy link
Copy Markdown
Contributor

EItanya commented May 14, 2026

@mesutoezdil here's another example.

#1849 (comment)

@marcofranssen
Copy link
Copy Markdown
Contributor Author

marcofranssen commented May 15, 2026

@mesutoezdil here's another example.

#1849 (comment)

#1849 (comment)

I think we are about to break helm best practices again if we go that route. Similar like I recently fixed to be able to pass through per agent settings in my earlier PR.

This is really the way to pass through values to subcharts.

One example of a chart that does this correctly is the spire helm chart(s). Please let's not break the pattern again.

global is another pattern that allows global defaults. Usually used for e.g. image registry. And usually that is used combined with post component ways of overriding.

Even though there is duplication like this it makes the helm charts usable as individual components. Which is actually great as it allows people to make their own composite charts for their own specific usecases.

@marcofranssen
Copy link
Copy Markdown
Contributor Author

Fixed the DCO just now.

@marcofranssen marcofranssen force-pushed the helm-agent-memory branch 4 times, most recently from 89c9946 to 5a7a65e Compare May 15, 2026 11:39
Comment thread helm/agents/argo-rollouts/templates/agent.yaml Outdated
@marcofranssen marcofranssen force-pushed the helm-agent-memory branch 2 times, most recently from 5a7a65e to 0ff1f9f Compare May 15, 2026 19:31
Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
Comment thread .editorconfig
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.

Can we please remove this from the PR. If we want an .editorconfig let's leave that in a separate PR where we can discuss it

Copy link
Copy Markdown
Contributor

@mesutoezdil mesutoezdil left a comment

Choose a reason for hiding this comment

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

2 issues need to be fixed.

required argument order is wrong in all 10 templates

The Helm required function signature is required "message" .value, but every template has the arguments swapped:

# current (broken)
modelConfig: {{ required .modelConfigRef "A compatible modelConfig must be provided when memory is enabled" }}

# should be
modelConfig: {{ required "A compatible modelConfig must be provided when memory is enabled" .modelConfigRef }}

With the arguments flipped, required treats the literal string as the value and .modelConfigRef as the error message.

An empty modelConfigRef will silently render the error string as the field value instead of failing at template render time.

This affects all 10 agent templates.

Trailing blank lines in argo-rollouts/templates/agent.yaml

The diff adds 6 empty lines at the end of the file.

Also agree with EItanya that .editorconfig belongs in a separate PR.

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.

5 participants