*: Integrate the kgateway chart into the repository #10485

timflannagan · 2025-01-21T21:37:44Z

Description

This commit is a follow-up to #10470 and integrates the newly bootstrapped kgateway helm chart into the repository and replacing the legacy chart in CI. While this is a foundational step, further iterations and chart design adjustments will follow to refine and fully replace the legacy chart. The goal is to gradually transition to the new chart while deprecating the legacy implementation.

Additional changes include publishing this helm chart along with the #10441 stream of work. This can be done as an extension of the release GHA workflow once we pass the initial hurdle with integrated this chart with CI and getting green runs.

Related to #10484 which renamed the default container image registry from quay to ghcr.io. The new helm chart will have this as the default registry going forward.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

timflannagan · 2025-01-21T21:41:15Z

Publishing this branch after some offline conversation with a couple of people. This branch is highly WIP but it's failing conformance and folks with more context could help triage.

yuval-k

left some comments, over all looks good!

install/helm/kgateway/templates/deployment.yaml

install/helm/kgateway/templates/role.yaml

install/helm/kgateway/values.yaml

yuval-k · 2025-01-23T02:49:44Z

projects/gateway2/translator/gateway/testutils/inputs/directresponse/manifest.yaml

@@ -3,7 +3,7 @@ apiVersion: gateway.networking.k8s.io/v1
 metadata:
  name: example-gateway
 spec:
-  gatewayClassName: gloo-gateway
+  gatewayClassName: kgateway


i agree we need to perform this change but will also need to think about downstream effects. cc @lgadban

let's track gatewayClass discussion in this repo

stubbed out #10503

projects/gateway2/install.sh

projects/envoyinit/cmd/Dockerfile.envoyinit

install/helm/kgateway/values.yaml

timflannagan · 2025-01-23T17:35:16Z

Sorry for the force push. I was looking into chunking the WIP commit into several commits to cherry-pick into their own PRs if necessary.

.github/actions/kube-gateway-api-conformance-tests/action.yaml

timflannagan · 2025-01-23T18:41:09Z

install/helm/kgateway/Chart.yaml

-version: 0.1.0
+version: "v0.0.1"


Just a temporary version here, but we can figure out the right value in a follow-up or do it here.

timflannagan · 2025-01-23T18:41:45Z

install/helm/kgateway/templates/tests/test-connection.yaml

Removed for now. We can add helm tests once the new chart is hooked into the release workflow.

timflannagan · 2025-01-23T18:42:42Z

install/helm/kgateway/values.yaml

+    namespace: "kgateway-system"
+
+# Gateway proxy configuration
+gateway:


I held off on changing any of the gateway param related helm chart values and c/p from the legacy chart as the deployer unit tests validate the AI extension and SDS containers components and I didn't want to change too much all at once.

we can leave for now so it's not a massive PR but i think we need to revisit soon; if these are the values consumed by the default GatewayParams i would want to move them closer to the gatewayClass itself.
happy to discuss more here but seems like a good follow-up

Totally agree. I was going to handle re-designing this as a follow-up during this stream of release work. I just wanted to get a chart integrated with CI so publishing this chart is unblocked in the release workflow.

timflannagan · 2025-01-23T18:43:22Z

pkg/utils/kubeutils/names.go

-	GlooDeploymentName = "gloo"
-	GlooServiceName    = "gloo"
+	GlooDeploymentName = "kgateway"
+	GlooServiceName    = "kgateway"


Mentioned in the commit message, but this was needed to ensure the right xds_cluster socket address got populated in managed proxies.

timflannagan · 2025-01-23T18:43:40Z

projects/gateway2/generate.sh

@@ -35,8 +34,8 @@ go run k8s.io/code-generator/cmd/register-gen --output-file zz_generated.registe
 go run sigs.k8s.io/controller-tools/cmd/controller-gen crd:maxDescLen=0 object rbac:roleName=k8sgw-controller paths="${APIS_PKG}/api/${VERSION}" \
    output:crd:artifacts:config=${SCRIPT_ROOT}/../../install/helm/gloo/crds/ output:rbac:artifacts:config=${SCRIPT_ROOT}/../../install/helm/gloo/files/rbac

-go run sigs.k8s.io/controller-tools/cmd/controller-gen crd:maxDescLen=0 object rbac:roleName=k8sgw-controller paths="${APIS_PKG}/api/${VERSION}" \


A follow-up will rip out the legacy chart's RBAC generation.

install/helm/gloo/files/rbac/role.yaml

install/helm/kgateway/templates/deployment.yaml

lgadban · 2025-01-23T21:45:31Z

install/helm/kgateway/values.yaml

+    namespace: "kgateway-system"
+
+# Gateway proxy configuration
+gateway:


we can leave for now so it's not a massive PR but i think we need to revisit soon; if these are the values consumed by the default GatewayParams i would want to move them closer to the gatewayClass itself.
happy to discuss more here but seems like a good follow-up

install/helm/kgateway/templates/deployment.yaml

lgadban · 2025-01-23T21:47:58Z

projects/gateway2/install.sh

do we have an equivalent to this anywhere else?

Not at the moment, but this is a gap imo with how we're installing kgateway in CI anyways (i.e. there's no single make target that handles all of this). Ex, the conformance suite first calls setup-kind.sh, then helm installs the locally packaged chart, then runs the make conformance target. I'll tackle this as a follow-up -- there's a couple of open questions w.r.t. VERSION default that I'd like to discuss more.

Signed-off-by: timflannagan <[email protected]>

… for kgateway chart Signed-off-by: timflannagan <[email protected]>

This commit adds several kubebuilder RBAC markers for the corev1, appsv1, GW API, etc. resources that are needed by the slimmed down stack. Previously, we were generating RBAC for kgateway-specific APIs (e.g. DirectResponse, GatewayParameters, etc.). Signed-off-by: timflannagan <[email protected]>

The kgateway chart was initially bootstrapped via helm create in a previous main branch PR. This chart is not functional and contains some cruft that helm generates that isn't relevant to this project (e.g. ingress). This commit deletes several files, re-structures the values.yaml and templates/ directories. Previous commits generated the required RBAC files. This expands on that motion and introduces the GatewayClass and default GatewayParameter CR templates. Signed-off-by: timflannagan <[email protected]>

This script will be no longer relevant once the repository overhaul is complete and Makefile targets have been introduced to replicate any install paths. Signed-off-by: timflannagan <[email protected]>

…tewayClass name Signed-off-by: timflannagan <[email protected]>

This removes the Settings CR from the setup.yaml bootstrap configuration in the envtests. The kgateway chart does not carry this CR over to it's crds/ directory or have the RBAC permissions to watch, list, etc. the resource. Previously, we removed the legacy Settings logic in the gateway2 control plane in kgateway-dev#10458, so this resource is no longer necessary in the setup.yaml and envtest fails trying to deploy it with the new chart. Signed-off-by: timflannagan <[email protected]>

…ebuilder Signed-off-by: timflannagan <[email protected]>

This updates the constant variables in the kubeutils package to use the "kgateway" container name. Without this, the conformance suite will fail as the xds_cluster socket address is pointing to a kube svc DNS name that does not exist (e.g. gloo.kgateway-system.svc). Signed-off-by: timflannagan <[email protected]>

…nce fails Signed-off-by: timflannagan <[email protected]>

Signed-off-by: timflannagan <[email protected]>

… chart Signed-off-by: timflannagan <[email protected]>

This was a temporary hack needed until kgateway-dev#10499 landed in main. Signed-off-by: timflannagan <[email protected]>

timflannagan · 2025-01-24T16:05:20Z

(Accidentally ran rebase --onto on the wrong branch lol).

lgadban

LGTM!

timflannagan force-pushed the helm/integrate-kgateway-chart branch 7 times, most recently from 2832a75 to de864ab Compare January 22, 2025 22:42

yuval-k reviewed Jan 23, 2025

View reviewed changes

timflannagan force-pushed the helm/integrate-kgateway-chart branch from de864ab to 8e245da Compare January 23, 2025 17:32

timflannagan changed the title ~~WIP: Integrate the kgateway chart into the repository~~ *: Integrate the kgateway chart into the repository Jan 23, 2025

timflannagan commented Jan 23, 2025

View reviewed changes

.github/actions/kube-gateway-api-conformance-tests/action.yaml Outdated Show resolved Hide resolved

timflannagan commented Jan 23, 2025

View reviewed changes

lgadban reviewed Jan 23, 2025

View reviewed changes

lgadban mentioned this pull request Jan 23, 2025

gatewayClass investigation #10503

Open

timflannagan added 10 commits January 24, 2025 16:04

Add Makefile target for the kgateway helm chart and deploy in CI

2818eea

Signed-off-by: timflannagan <[email protected]>

projects/gateway2: Fix broken output path in kubebuilder RBAC markers…

92202fa

… for kgateway chart Signed-off-by: timflannagan <[email protected]>

projects/gateway2: Remove the install.sh script

7605199

This script will be no longer relevant once the repository overhaul is complete and Makefile targets have been introduced to replicate any install paths. Signed-off-by: timflannagan <[email protected]>

*: Update constant variables, static testdata, etc. to reflect new Ga…

cb2e80e

…tewayClass name Signed-off-by: timflannagan <[email protected]>

install/helm: Mention kgateway RBAC template is auto-generated by kub…

0332fb8

…ebuilder Signed-off-by: timflannagan <[email protected]>

.github: Use the kgateway-system instead of gloo-system when conforma…

1111eee

…nce fails Signed-off-by: timflannagan <[email protected]>

timflannagan added 5 commits January 24, 2025 16:04

install: Default to ClusterIP for kgateway controller

f641576

Signed-off-by: timflannagan <[email protected]>

install: Remove SDS and AI sidecars, purge stale envvars, etc"

a137ab8

Signed-off-by: timflannagan <[email protected]>

install/helm: Default to ClusterIP for controller service

5b7d273

Signed-off-by: timflannagan <[email protected]>

.github: Remove --set global.image.invariant when installing kgateway…

9eeccd8

… chart Signed-off-by: timflannagan <[email protected]>

install: Remove hardcoded envoy init image in kgateway chart

0bf312e

This was a temporary hack needed until kgateway-dev#10499 landed in main. Signed-off-by: timflannagan <[email protected]>

timflannagan force-pushed the helm/integrate-kgateway-chart branch from 881b237 to 0bf312e Compare January 24, 2025 16:04

lgadban approved these changes Jan 24, 2025

View reviewed changes

lgadban added this pull request to the merge queue Jan 24, 2025

Merged via the queue into kgateway-dev:main with commit 00af1d6 Jan 24, 2025
6 checks passed

timflannagan deleted the helm/integrate-kgateway-chart branch January 24, 2025 16:28

This was referenced Jan 24, 2025

.github: Publish kgateway helm chart in the release workflow #10504

Merged

Remove controller-gen generation for legacy chart #10505

Merged

Integrate the kgateway chart into the repository #10423

Closed

*: Rename gloo container to kgateway #10532

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: Integrate the kgateway chart into the repository #10485

*: Integrate the kgateway chart into the repository #10485

timflannagan commented Jan 21, 2025

timflannagan commented Jan 21, 2025

yuval-k left a comment

yuval-k Jan 23, 2025

lgadban Jan 23, 2025

lgadban Jan 23, 2025

timflannagan commented Jan 23, 2025

timflannagan Jan 23, 2025

timflannagan Jan 23, 2025

timflannagan Jan 23, 2025

lgadban Jan 23, 2025

timflannagan Jan 23, 2025

timflannagan Jan 23, 2025

timflannagan Jan 23, 2025

lgadban Jan 23, 2025

lgadban Jan 23, 2025

timflannagan Jan 23, 2025

timflannagan commented Jan 24, 2025

lgadban left a comment

		version: 0.1.0
		version: "v0.0.1"

*: Integrate the kgateway chart into the repository #10485

*: Integrate the kgateway chart into the repository #10485

Conversation

timflannagan commented Jan 21, 2025

Description

Checklist:

timflannagan commented Jan 21, 2025

yuval-k left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timflannagan commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timflannagan commented Jan 24, 2025

lgadban left a comment

Choose a reason for hiding this comment