Split helm UX into dedicated charts for Envoy based kgateway and agentgateway #13062

josh-pritchard · 2025-12-05T04:40:08Z

Description

Still WIP and depends/includes changes from #13018 but wanted to run CI and get up for review.

Change Type

/kind breaking_change

Changelog

Split helm UX into dedicated charts for Envoy based kgateway and agentgateway

Additional Notes

Fast Follow #13093
Resolves #13024

josh-pritchard · 2025-12-05T04:45:55Z

/retest

Copilot

Pull request overview

This PR splits the monolithic kgateway Helm chart into dedicated charts for Envoy-based kgateway and agentgateway deployments. This architectural change enables users to install either implementation independently, improving deployment flexibility and reducing coupling between the two gateway types.

Key changes include:

Introduction of new agentgateway and agentgateway-crds Helm charts with dedicated templates and values
Creation of AgentgatewayParameters CRD for agentgateway-specific configuration
Routing logic in test infrastructure and deployer to support both chart types
Removal of agentgateway-specific configuration from the kgateway chart

Reviewed changes

Copilot reviewed 86 out of 87 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`install/helm/agentgateway/*`	New Helm chart for agentgateway control plane deployment
`install/helm/agentgateway-crds/*`	New CRD chart for agentgateway resources
`install/helm/kgateway/*`	Removed agentgateway-specific configuration and RBAC rules
`pkg/kgateway/deployer/agentgateway_parameters.go`	New parameter handler for AgentgatewayParameters CRD
`pkg/kgateway/deployer/gateway_parameters.go`	Updated routing logic to distinguish between kgateway and agentgateway
`pkg/deployer/strategicpatch/*`	New strategic merge patch implementation for overlay support
`test/e2e/*`	Test infrastructure updates to support multiple chart types
`test/deployer/testdata/*`	Extensive test data for agentgateway scenarios
`pkg/utils/helmutils/constants.go`	Added constants for agentgateway chart names and URIs
`api/v1alpha1/agentgateway/*`	New AgentgatewayParameters type definitions and generated code

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

npolshakova · 2025-12-05T15:51:09Z

install/helm/kgateway/templates/role.yaml

  - list
  - patch
  - watch
- apiGroups:


So are we going to keep something like #13045 to handle upgrades? Or what's the upgrade path for the new helm chart split?

We could I need to try it out, there certainly is not a smooth upgrade path and I would argue that is fine becasue the Group name changed too. Maybe we should include the old group name in here for now

npolshakova · 2025-12-05T15:53:16Z

pkg/agentgateway/testutils/util.go

-		DirectResponses:      krttest.GetMockCollection[*v1alpha1.DirectResponse](mock),
-		GatewayExtensions:    krttest.GetMockCollection[*v1alpha1.GatewayExtension](mock),
 		ControllerName:       wellknown.DefaultAgwControllerName,
 		SystemNamespace:      "kgateway-system",


Will the system namespace still be kgateway-system for both charts?

its not hard coded anywhere then tests so I think this is up to documentation

npolshakova · 2025-12-05T15:57:18Z

install/helm/kgateway/values.yaml

  # -- Enable Inference Extension. If enabled, agentgateway.enabled should also be set to true. Enabling inference extension without agentgateway is deprecated in v2.1 and will not be supported in v2.2.
  enabled: false

 # -- List of namespace selectors (OR'ed): each entry can use 'matchLabels' or 'matchExpressions' (AND'ed within each entry if used together). Kgateway includes the selected namespaces in config discovery. For more information, see the docs https://kgateway.dev/docs/latest/install/advanced/#namespace-discovery.


How are we handling inference extensions in kgateway? Should we also remove

kgateway/install/helm/kgateway/values.yaml

Line 97 in ce60e64

inferenceExtension:

yeah it should be removed, I removed it from the new agentgateway one as its not relevant, I thought we already deprecated it in the main install

Signed-off-by: Joshua Pritchard <[email protected]>

lgadban · 2025-12-10T15:58:10Z

pkg/pluginsdk/collections/collections.go

+	// Only create GatewayExtensions collection if Envoy is enabled
+	// This CRD is specific to Envoy and not used by agentgateway
+	var gwExts krt.Collection[ir.GatewayExtension]
+	if settings.EnableEnvoy {
+		gwExts = krtcollections.NewGatewayExtensionsCollection(ctx, client, krtOptions)
+	}


there are several other CRDs/collections that are specific to the Envoy translator,
why is this necessary for GwExt only?

Others are in the plugins so those get skipped at a high level but this is in common collections and it needed for agw though these are not leveraged, this should likely get moved from common collections but works for now

lgadban · 2025-12-10T15:59:24Z

pkg/schemes/extended_scheme.go

+		// Treat permission errors as "CRD doesn't exist" rather than failing
+		// This allows controllers to start even without RBAC for all API groups
+		// Kind of a hack because kgateway and agentgateway share controllers so some of these don't exist
+		if errors.IsNotFound(err) || errors.IsForbidden(err) || errors.IsUnauthorized(err) || discovery.IsGroupDiscoveryFailedError(err) || meta.IsNoMatchError(err) {


what is the functional impact here?

really just hides this error that was causing issues when it shouldn't because a collection was never initialized on these anyways, you would still get an error later if you did that. idk why we can't just add the scheme and not worry about if the CRD exists or not because I don't think it hurts anything. In other controllers we have we just do that but maybe I am missing some context.

puertomontt · 2025-12-10T16:17:44Z

test/e2e/features/leaderelection/suite.go

+		// kgateway chart: only Envoy enabled -> lease is "kgateway-envoy"
+		// agentgateway chart: only Agentgateway enabled -> lease is "kgateway-agentgateway"
+		// both controllers enabled (not typical in tests): lease is "kgateway"
+		leaseID := s.getLeaderElectionID()


isn't this test only testing the default installation

it is but we could add agentgateway and it would work now and it did need to get the envoy appended to it

I don't mind though because its the same code its testing either way

yuval-k · 2025-12-10T16:13:15Z

pkg/agentgateway/plugins/collection.go

-		Backends:             krt.NewInformer[*agentgateway.AgentgatewayBackend](commoncol.Client),
+	// Only watch agentgateway-specific CRDs if agentgateway is enabled
+	// This prevents permission errors when only Envoy is enabled
+	if commoncol.Settings.EnableAgentgateway {


if agent gateway is disabled, why do we need any of these collections initialized?
i.e. why not return nil early in the function?

Good callout, these low level gates are a source of some of the test failures, just changed this around

yuval-k · 2025-12-10T16:21:00Z

pkg/kgateway/extensions2/plugins/trafficpolicy/constructor.go

-	gatewayExtensions := krt.NewCollection(commoncol.GatewayExtensions, defaultExtBuilder)
+	var gatewayExtensions krt.Collection[TrafficPolicyGatewayExtensionIR]
+	if commoncol.GatewayExtensions != nil {
+		gatewayExtensions = krt.NewCollection(commoncol.GatewayExtensions, defaultExtBuilder)


should we error if GatewayExtensions is nil? i.e. can this happen normally?

good catch this is gated at a higher level and this won't run at all if Envoy is not enabled. I will remove this

yuval-k · 2025-12-10T16:22:15Z

pkg/kgateway/setup/setup.go

+	if s.globalSettings.EnableEnvoy && !s.globalSettings.EnableAgentgateway {
+		// Envoy-only controller (kgateway chart)
+		leaderElectionID = s.leaderElectionID + "-envoy"
+	} else if !s.globalSettings.EnableEnvoy && s.globalSettings.EnableAgentgateway {
+		// Agentgateway-only controller (agentgateway chart)
+		leaderElectionID = s.leaderElectionID + "-agentgateway"
+	}


controller names are different right? i.e. if they both write a status on the same httproute, will that work ok?

yeah it should be right because its its by parent https://gateway-api.sigs.k8s.io/api-types/httproute/#parents same as if was just a different GW API implementation

Signed-off-by: Joshua Pritchard <[email protected]>

npolshakova · 2025-12-10T20:25:31Z

test/e2e/testutils/assertions/kgateway.go

+func (p *Provider) getChartLabelSelector() string {
+	chartType := p.installContext.GetChartType()
+	if chartType == "agentgateway" {
+		return "app.kubernetes.io/name=agentgateway"


[nit] can we make consts for this?

npolshakova · 2025-12-10T20:27:01Z

pkg/utils/helmutils/constants.go

+	DefaultAgentGatewayChartUri    = "oci://ghcr.io/kgateway-dev/charts/agentgateway"
+	DefaultAgentGatewayCRDChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway-crds"


[nit] or Agw here

Suggested change

DefaultAgentGatewayChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway"

DefaultAgentGatewayCRDChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway-crds"

DefaultAgentgatewayChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway"

DefaultAgentgatewayCRDChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway-crds"

npolshakova · 2025-12-10T20:27:23Z

pkg/utils/helmutils/constants.go

+	AgentGatewayChartName    = "agentgateway"
+	AgentGatewayCRDChartName = "agentgateway-crds"


[nit]

Suggested change

AgentGatewayChartName = "agentgateway"

AgentGatewayCRDChartName = "agentgateway-crds"

AgentgatewayChartName = "agentgateway"

AgentgatewayCRDChartName = "agentgateway-crds"

Signed-off-by: Joshua Pritchard <[email protected]>

josh-pritchard · 2025-12-11T02:31:00Z

/re-test

josh-pritchard · 2025-12-11T02:31:21Z

/kick-ci

josh-pritchard · 2025-12-11T02:32:40Z

/retest

npolshakova · 2025-12-11T02:45:51Z

.github/workflows/e2e.yaml

        # December 4, 2025: ~9 minutes
        - cluster-name: 'api-validation' # TODO: rename to cluster-seven
          go-test-args: '-timeout=15m'
-          go-test-run-regex: '^TestAPIValidation|^TestKgateway$$/^OAuth$$|^TestCustomGWP$$'


Is TestCustomGWP envoy-dataplane specific now?

Uh no they both can be available in all the regular clusters, this didn't actually need to move I think I just left this accidentally when I was reshuffling

In general there is a bit of test cleanup to do here that we should followup on now with the chart split there were some workarounds I needed to apply and I don't love how the install is handled for the tests now in context of there being 2 charts, I think we want to change it around anyways to be able to re-use the same install between tests as well.

josh-pritchard · 2025-12-11T02:59:47Z

/merge

Copilot AI review requested due to automatic review settings December 5, 2025 04:40

Copilot started reviewing on behalf of josh-pritchard December 5, 2025 04:40 View session

gateway-bot added do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. kind/breaking_change labels Dec 5, 2025

Copilot finished reviewing on behalf of josh-pritchard December 5, 2025 04:41

gateway-bot added release-note and removed do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 5, 2025

josh-pritchard marked this pull request as draft December 5, 2025 04:44

gateway-bot added do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. and removed release-note labels Dec 5, 2025

Copilot AI reviewed Dec 5, 2025

View reviewed changes

gateway-bot added release-note and removed do-not-merge/release-note-invalid Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 5, 2025

npolshakova reviewed Dec 5, 2025

View reviewed changes

josh-pritchard force-pushed the josh/split-helm branch from ce60e64 to d307800 Compare December 9, 2025 19:53

josh-pritchard marked this pull request as ready for review December 9, 2025 19:54

josh-pritchard force-pushed the josh/split-helm branch 3 times, most recently from 697fcb7 to a5a50f0 Compare December 10, 2025 07:18

josh-pritchard added 8 commits December 10, 2025 10:26

First pass at split helm charts

e545c80

Signed-off-by: Joshua Pritchard <[email protected]>

add missing builds

5645e90

Signed-off-by: Joshua Pritchard <[email protected]>

test fix

c6effeb

Signed-off-by: Joshua Pritchard <[email protected]>

fix

7395efa

Signed-off-by: Joshua Pritchard <[email protected]>

fix tests

4382b4f

Signed-off-by: Joshua Pritchard <[email protected]>

test fixes

43bb87d

Signed-off-by: Joshua Pritchard <[email protected]>

fix test

a2ae155

Signed-off-by: Joshua Pritchard <[email protected]>

fix tests

efcb103

Signed-off-by: Joshua Pritchard <[email protected]>

josh-pritchard added 4 commits December 10, 2025 10:26

CI fixes

87e38b2

Signed-off-by: Joshua Pritchard <[email protected]>

Fix tests

e10b61b

Signed-off-by: Joshua Pritchard <[email protected]>

fix tests

4576b29

Signed-off-by: Joshua Pritchard <[email protected]>

fix CI

5f5033c

Signed-off-by: Joshua Pritchard <[email protected]>

josh-pritchard force-pushed the josh/split-helm branch from ae091ab to 5f5033c Compare December 10, 2025 15:27

yuval-k self-assigned this Dec 10, 2025

skip broken tests

aab792e

Signed-off-by: Joshua Pritchard <[email protected]>

lgadban reviewed Dec 10, 2025

View reviewed changes

josh-pritchard force-pushed the josh/split-helm branch from 276d478 to aab792e Compare December 10, 2025 16:01

chandler-solo mentioned this pull request Dec 10, 2025

HelmConfig and Deployer split for agentgateway and envoy #13068

Open

puertomontt reviewed Dec 10, 2025

View reviewed changes

yuval-k reviewed Dec 10, 2025

View reviewed changes

josh-pritchard added 2 commits December 10, 2025 13:23

fix tests

f1bab3a

Signed-off-by: Joshua Pritchard <[email protected]>

Test CI

338fcaa

Signed-off-by: Joshua Pritchard <[email protected]>

npolshakova reviewed Dec 10, 2025

View reviewed changes

josh-pritchard added 2 commits December 10, 2025 15:47

Fix tests! Definitely going to work this time!!!!!!

b2e14d5

Signed-off-by: Joshua Pritchard <[email protected]>

Some test fixes

2e89d43

Signed-off-by: Joshua Pritchard <[email protected]>

josh-pritchard force-pushed the josh/split-helm branch from 902a26e to 2e89d43 Compare December 10, 2025 22:19

Fix unit tests

27449e2

Signed-off-by: Joshua Pritchard <[email protected]>

josh-pritchard force-pushed the josh/split-helm branch from 8d4cb1e to 497837d Compare December 11, 2025 01:59

Fix unit tests

d21afc0

Signed-off-by: Joshua Pritchard <[email protected]>

josh-pritchard force-pushed the josh/split-helm branch from 497837d to d21afc0 Compare December 11, 2025 02:03

npolshakova reviewed Dec 11, 2025

View reviewed changes

npolshakova approved these changes Dec 11, 2025

View reviewed changes

		DefaultAgentGatewayChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway"
		DefaultAgentGatewayCRDChartUri = "oci://ghcr.io/kgateway-dev/charts/agentgateway-crds"

		AgentGatewayChartName = "agentgateway"
		AgentGatewayCRDChartName = "agentgateway-crds"

Split helm UX into dedicated charts for Envoy based kgateway and agentgateway #13062

Are you sure you want to change the base?

Split helm UX into dedicated charts for Envoy based kgateway and agentgateway #13062

Uh oh!

Conversation

josh-pritchard commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Change Type

Changelog

Additional Notes

Uh oh!

josh-pritchard commented Dec 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josh-pritchard Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josh-pritchard commented Dec 11, 2025

Uh oh!

josh-pritchard commented Dec 11, 2025

Uh oh!

josh-pritchard commented Dec 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josh-pritchard commented Dec 11, 2025

Uh oh!

Reviewers

josh-pritchard commented Dec 5, 2025 •

edited

Loading

josh-pritchard Dec 9, 2025 •

edited

Loading