From a25cef5a265a5d9d9cf32c26f14edeed6d206049 Mon Sep 17 00:00:00 2001 From: Travis Raines <571832+rainest@users.noreply.github.com> Date: Thu, 26 Oct 2023 19:59:02 -0700 Subject: [PATCH] chore: update router default (#4934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: change default router * chore: make manifests * chore: keep postgres variants on trad router * chore: make manifests * chore: update router matrix * chore: remove wip garbage * chore: changelog update * chore: add tracking issue * chore: regenerate * tests: fix TestKongRouterFlavorCompatibility e2e test and make it test all Kong Gateway flavors --------- Co-authored-by: Patryk Małek --- .github/workflows/_integration_tests.yaml | 15 +++-- CHANGELOG.md | 4 ++ config/base/kong-ingress-dbless.yaml | 2 +- .../multi-gw/base/gateway_deployment.yaml | 2 +- config/variants/postgres/kustomization.yaml | 16 +++++ test/e2e/compatibilities_test.go | 53 +++++++++++------ test/e2e/helpers_test.go | 30 +++++++++- test/e2e/kustomizations_test.go | 59 +++++++++++++++++++ .../all-in-one-dbless-k4k8s-enterprise.yaml | 2 +- .../all-in-one-dbless-konnect-enterprise.yaml | 2 +- .../manifests/all-in-one-dbless-konnect.yaml | 2 +- test/e2e/manifests/all-in-one-dbless.yaml | 2 +- .../all-in-one-postgres-enterprise.yaml | 4 +- test/e2e/manifests/all-in-one-postgres.yaml | 4 +- test/e2e/utils_test.go | 10 +++- 15 files changed, 170 insertions(+), 37 deletions(-) diff --git a/.github/workflows/_integration_tests.yaml b/.github/workflows/_integration_tests.yaml index d9d2c98747..793e483771 100644 --- a/.github/workflows/_integration_tests.yaml +++ b/.github/workflows/_integration_tests.yaml @@ -62,32 +62,35 @@ jobs: needs: dependencies-versions env: KONG_CLUSTER_VERSION: ${{ needs.dependencies-versions.outputs.kind }} - TEST_KONG_ROUTER_FLAVOR: 'traditional' + TEST_KONG_ROUTER_FLAVOR: 'expressions' TEST_KONG_HELM_CHART_VERSION: ${{ needs.dependencies-versions.outputs.helm-kong }} strategy: fail-fast: false + # DB modes override to traditional or traditional_compatible only pending upstream gateway changes + # to expression mode when used with a database https://github.com/Kong/kubernetes-ingress-controller/issues/4966 matrix: include: - name: dbless test: dbless - name: postgres test: postgres + router-flavor: 'traditional_compatible' - name: enterprise-postgres test: enterprise.postgres enterprise: true + router-flavor: 'traditional_compatible' - name: enterprise-dbless test: enterprise.dbless enterprise: true - name: dbless-traditional-compatible test: dbless router-flavor: 'traditional_compatible' - - name: postgres-traditional-compatible + - name: postgres-traditional test: postgres - router-flavor: 'traditional_compatible' - - name: dbless-expression-router + router-flavor: 'traditional' + - name: dbless-traditional test: dbless - feature_gates: "GatewayAlpha=true" - router-flavor: "expressions" + router-flavor: "traditional" - name: dbless-gateway-alpha test: dbless feature_gates: "GatewayAlpha=true" diff --git a/CHANGELOG.md b/CHANGELOG.md index 87ff5d4cf1..4d6bd46bfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -126,6 +126,10 @@ Adding a new version? You'll need three changes: still supported but is now deprecated. A script to generate commands to update Secrets is available at https://github.com/Kong/kubernetes-ingress-controller/issues/2502#issuecomment-1758213596 [#4825](https://github.com/Kong/kubernetes-ingress-controller/pull/4825) +- The `expressions` router is now the default for DB-less mode. This is not + expected to functionally affect routing, but may affect performance for + some configurations. + [#4934](https://github.com/Kong/kubernetes-ingress-controller/pull/4934) ### Fixed diff --git a/config/base/kong-ingress-dbless.yaml b/config/base/kong-ingress-dbless.yaml index 0c07cb03ad..eb3debd8a4 100644 --- a/config/base/kong-ingress-dbless.yaml +++ b/config/base/kong-ingress-dbless.yaml @@ -72,7 +72,7 @@ spec: value: /dev/stderr # router mode in 3.0.0. use `traditional` here for full compatibility. - name: KONG_ROUTER_FLAVOR - value: traditional + value: expressions lifecycle: preStop: exec: diff --git a/config/variants/multi-gw/base/gateway_deployment.yaml b/config/variants/multi-gw/base/gateway_deployment.yaml index cc1e9c6fdf..160ab5e9a0 100644 --- a/config/variants/multi-gw/base/gateway_deployment.yaml +++ b/config/variants/multi-gw/base/gateway_deployment.yaml @@ -72,7 +72,7 @@ spec: value: /dev/stderr # router mode in 3.0.0. use `traditional` here for full compatibility. - name: KONG_ROUTER_FLAVOR - value: traditional + value: expressions lifecycle: preStop: exec: diff --git a/config/variants/postgres/kustomization.yaml b/config/variants/postgres/kustomization.yaml index e32f1a1c27..339f0cce35 100644 --- a/config/variants/postgres/kustomization.yaml +++ b/config/variants/postgres/kustomization.yaml @@ -5,3 +5,19 @@ resources: components: - ./wait/ - ../../image/oss/ + +patches: +- patch: |- + apiVersion: apps/v1 + kind: Deployment + metadata: + name: ingress-kong + namespace: kong + spec: + template: + spec: + containers: + - name: proxy + env: + - name: KONG_ROUTER_FLAVOR + value: traditional_compatible diff --git a/test/e2e/compatibilities_test.go b/test/e2e/compatibilities_test.go index a5a49fa8d9..5e729582d4 100644 --- a/test/e2e/compatibilities_test.go +++ b/test/e2e/compatibilities_test.go @@ -15,29 +15,38 @@ import ( k8stypes "k8s.io/apimachinery/pkg/types" ) -// TestKongRouterCompatibility verifies that KIC behaves consistently with Kong routers -// `traditional` and `traditional_compatible`. +// TestKongRouterCompatibility verifies that KIC behaves consistently with all +// Kong routers: +// - `expressions` +// - `traditional` +// - and `traditional_compatible`. func TestKongRouterFlavorCompatibility(t *testing.T) { t.Parallel() + ctx, env := setupE2ETest(t) cluster := env.Cluster() - t.Log("deploying kong components with traditional Kong router") - deployments := ManifestDeploy{Path: dblessPath}.Run(ctx, t, env) - proxyDeploymentNN := deployments.ProxyNN - ensureGatewayDeployedWithRouterFlavor(ctx, t, env, proxyDeploymentNN, "traditional") - - t.Log("running ingress tests to verify that KIC with traditonal Kong router works") - deployIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends) - verifyIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends) - - setGatewayRouterFlavor(ctx, t, cluster, proxyDeploymentNN, "traditional_compatible") - - t.Log("waiting for Kong with traditional_compatible router to start") - ensureGatewayDeployedWithRouterFlavor(ctx, t, env, proxyDeploymentNN, "traditional_compatible") + routerFlavors := []string{"expressions", "traditional_compatible", "traditional"} + for _, rf := range routerFlavors { + rf := rf + t.Run(rf, func(t *testing.T) { + deploy := ManifestDeploy{ + Path: dblessPath, + Patches: []ManifestPatch{ + patchKongRouterFlavorFn(rf), + }, + } + deployments := deploy.Run(ctx, t, env) + t.Cleanup(func() { deploy.Delete(ctx, t, env) }) + proxyDeploymentNN := deployments.ProxyNN - t.Log("running ingress tests to verify that KIC with traditonal_compatible Kong router works") - verifyIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends) + setGatewayRouterFlavor(ctx, t, cluster, proxyDeploymentNN, rf) + t.Logf("waiting for Kong with %s router to start", rf) + ensureGatewayDeployedWithRouterFlavor(ctx, t, env, proxyDeploymentNN, rf) + t.Logf("running ingress tests to verify that KIC with %s Kong router works", rf) + verifyIngressWithEchoBackends(ctx, t, env, numberOfEchoBackends) + }) + } } func setGatewayRouterFlavor( @@ -47,6 +56,8 @@ func setGatewayRouterFlavor( proxyDeploymentNN k8stypes.NamespacedName, flavor string, ) { + t.Helper() + // Since we cannot replace env vars in kustomize, here we update the deployment to set KONG_ROUTER_FLAVOR to traditional_compatible. t.Log("update deployment to modify Kong's router to traditional_compatible") deployments := cluster.Client().AppsV1().Deployments(proxyDeploymentNN.Namespace) @@ -70,6 +81,8 @@ func ensureGatewayDeployedWithRouterFlavor( proxyDeploymentNN k8stypes.NamespacedName, expectedFlavor string, ) { + t.Helper() + labelsForDeployment := metav1.ListOptions{ LabelSelector: fmt.Sprintf("app=%s", proxyDeploymentNN.Name), } @@ -89,8 +102,10 @@ func ensureGatewayDeployedWithRouterFlavor( allPodsMatch = false continue } - if getEnvValueInContainer(proxyContainer, "KONG_ROUTER_FLAVOR") != expectedFlavor { - t.Logf("KONG_ROUTER_FLAVOR is not set to expected value for Pod %s", pod.Name) + if v := getEnvValueInContainer(proxyContainer, "KONG_ROUTER_FLAVOR"); v != expectedFlavor { + t.Logf("KONG_ROUTER_FLAVOR is not set to expected value for Pod %s, actual: %s, expected: %s", + pod.Name, v, expectedFlavor, + ) allPodsMatch = false } } diff --git a/test/e2e/helpers_test.go b/test/e2e/helpers_test.go index f7a72d31cf..52f27808d2 100644 --- a/test/e2e/helpers_test.go +++ b/test/e2e/helpers_test.go @@ -232,6 +232,8 @@ func createGKEBuilder(t *testing.T) (*environments.Builder, error) { return environments.NewBuilder().WithClusterBuilder(clusterBuilder), nil } +type ManifestPatch func(io.Reader) (io.Reader, error) + type ManifestDeploy struct { // Path is the path to the manifest to deploy. Path string @@ -242,6 +244,9 @@ type ManifestDeploy struct { // AdditionalSecrets is a list of additional secrets to create before deploying the manifest. AdditionalSecrets []*corev1.Secret + + // Patches contain additionall patches that will be applied before deploying the manifest. + Patches []ManifestPatch } func (d ManifestDeploy) Run(ctx context.Context, t *testing.T, env environments.Environment) Deployments { @@ -268,7 +273,7 @@ func (d ManifestDeploy) Run(ctx context.Context, t *testing.T, env environments. } t.Logf("deploying %s manifest to the cluster", d.Path) - manifest := getTestManifest(t, d.Path, d.SkipTestPatches) + manifest := getTestManifest(t, d.Path, d.SkipTestPatches, d.Patches...) kubeconfigFilename := getTemporaryKubeconfig(t, env) cmd := exec.CommandContext(ctx, "kubectl", "--kubeconfig", kubeconfigFilename, "apply", "-f", "-") cmd.Stdin = manifest @@ -284,6 +289,29 @@ func (d ManifestDeploy) Run(ctx context.Context, t *testing.T, env environments. return deployments } +func (d ManifestDeploy) Delete(ctx context.Context, t *testing.T, env environments.Environment) { + t.Helper() + + t.Log("waiting for testing environment to be ready") + envReadyCtx, envReadyCancel := context.WithTimeout(ctx, testenv.EnvironmentReadyTimeout()) + defer envReadyCancel() + require.NoError(t, <-env.WaitForReady(envReadyCtx)) + + t.Logf("deleting any supplemental secrets (found: %d)", len(d.AdditionalSecrets)) + for _, secret := range d.AdditionalSecrets { + err := env.Cluster().Client().CoreV1().Secrets(namespace).Delete(ctx, secret.Name, metav1.DeleteOptions{}) + require.NoError(t, err) + } + + t.Logf("deleting %s manifest from the cluster", d.Path) + manifest := getTestManifest(t, d.Path, d.SkipTestPatches) + kubeconfigFilename := getTemporaryKubeconfig(t, env) + cmd := exec.CommandContext(ctx, "kubectl", "--kubeconfig", kubeconfigFilename, "delete", "-f", "-") + cmd.Stdin = manifest + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) +} + func waitForDeploymentRollout(ctx context.Context, t *testing.T, env environments.Environment, namespace, name string) { require.Eventuallyf(t, func() bool { deployment, err := env.Cluster().Client().AppsV1().Deployments(namespace).Get(ctx, name, metav1.GetOptions{}) diff --git a/test/e2e/kustomizations_test.go b/test/e2e/kustomizations_test.go index 7923b2253c..f68c9a4ae6 100644 --- a/test/e2e/kustomizations_test.go +++ b/test/e2e/kustomizations_test.go @@ -34,6 +34,26 @@ const ( - op: replace path: /spec/template/spec/containers/0/livenessProbe/failureThreshold value: %[3]d` + + kongRouterFlavorPatch = `- op: add + path: /spec/template/spec/containers/0/env/- + value: + name: KONG_ROUTER_FLAVOR + value: "%s"` + + kongRouterFlavorPatchDelete = `apiVersion: apps/v1 +kind: Deployment +metadata: + name: proxy-kong + namespace: kong +spec: + template: + spec: + containers: + - name: proxy + env: + - name: KONG_ROUTER_FLAVOR + $patch: delete` ) // patchControllerImage replaces the kong/kubernetes-ingress-controller image with the provided image and tag, @@ -96,6 +116,45 @@ func patchControllerStartTimeout(baseManifestReader io.Reader, tries int, delay return kubectl.GetKustomizedManifest(kustomization, baseManifestReader) } +func patchKongRouterFlavorFn(flavor string) func(io.Reader) (io.Reader, error) { + kustomization := types.Kustomization{ + Patches: []types.Patch{ + { + Target: &types.Selector{ + ResId: resid.ResId{ + Gvk: resid.Gvk{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Name: "proxy-kong", + Namespace: "kong", + }, + }, + Patch: kongRouterFlavorPatchDelete, + }, + { + Patch: fmt.Sprintf(kongRouterFlavorPatch, flavor), + Target: &types.Selector{ + ResId: resid.ResId{ + Gvk: resid.Gvk{ + Group: "apps", + Version: "v1", + Kind: "Deployment", + }, + Name: "proxy-kong", + Namespace: "kong", + }, + }, + }, + }, + } + + return func(baseManifestReader io.Reader) (io.Reader, error) { + return kubectl.GetKustomizedManifest(kustomization, baseManifestReader) + } +} + // patchLivenessProbes patches the given deployment's liveness probe, replacing the initial delay, period, and failure // threshold. func patchLivenessProbes(baseManifestReader io.Reader, deployment k8stypes.NamespacedName, failure int, initial, period time.Duration) (io.Reader, error) { diff --git a/test/e2e/manifests/all-in-one-dbless-k4k8s-enterprise.yaml b/test/e2e/manifests/all-in-one-dbless-k4k8s-enterprise.yaml index cbdf59718d..1e3963c03c 100644 --- a/test/e2e/manifests/all-in-one-dbless-k4k8s-enterprise.yaml +++ b/test/e2e/manifests/all-in-one-dbless-k4k8s-enterprise.yaml @@ -2584,7 +2584,7 @@ spec: - name: KONG_PROXY_ERROR_LOG value: /dev/stderr - name: KONG_ROUTER_FLAVOR - value: traditional + value: expressions image: kong/kong-gateway:3.4 lifecycle: preStop: diff --git a/test/e2e/manifests/all-in-one-dbless-konnect-enterprise.yaml b/test/e2e/manifests/all-in-one-dbless-konnect-enterprise.yaml index 475d34960b..7229486e7e 100644 --- a/test/e2e/manifests/all-in-one-dbless-konnect-enterprise.yaml +++ b/test/e2e/manifests/all-in-one-dbless-konnect-enterprise.yaml @@ -2594,7 +2594,7 @@ spec: - name: KONG_PROXY_ERROR_LOG value: /dev/stderr - name: KONG_ROUTER_FLAVOR - value: traditional + value: expressions image: kong/kong-gateway:3.4 lifecycle: preStop: diff --git a/test/e2e/manifests/all-in-one-dbless-konnect.yaml b/test/e2e/manifests/all-in-one-dbless-konnect.yaml index de2f716c23..703f5ba88e 100644 --- a/test/e2e/manifests/all-in-one-dbless-konnect.yaml +++ b/test/e2e/manifests/all-in-one-dbless-konnect.yaml @@ -2596,7 +2596,7 @@ spec: - name: KONG_PROXY_ERROR_LOG value: /dev/stderr - name: KONG_ROUTER_FLAVOR - value: traditional + value: expressions image: kong:3.4 lifecycle: preStop: diff --git a/test/e2e/manifests/all-in-one-dbless.yaml b/test/e2e/manifests/all-in-one-dbless.yaml index 6f3dbd20ca..07eff47f36 100644 --- a/test/e2e/manifests/all-in-one-dbless.yaml +++ b/test/e2e/manifests/all-in-one-dbless.yaml @@ -2581,7 +2581,7 @@ spec: - name: KONG_PROXY_ERROR_LOG value: /dev/stderr - name: KONG_ROUTER_FLAVOR - value: traditional + value: expressions image: kong:3.4 lifecycle: preStop: diff --git a/test/e2e/manifests/all-in-one-postgres-enterprise.yaml b/test/e2e/manifests/all-in-one-postgres-enterprise.yaml index 435dc2e888..2872440bcf 100644 --- a/test/e2e/manifests/all-in-one-postgres-enterprise.yaml +++ b/test/e2e/manifests/all-in-one-postgres-enterprise.yaml @@ -2509,6 +2509,8 @@ spec: value: '{"cookie_secure":false,"storage":"kong","cookie_name":"admin_session","cookie_lifetime":31557600,"cookie_samesite":"off","secret":"please-change-me"}' - name: KONG_ADMIN_LISTEN value: 0.0.0.0:8001, 0.0.0.0:8444 ssl + - name: KONG_ROUTER_FLAVOR + value: traditional_compatible - name: KONG_DATABASE value: postgres - name: KONG_PG_HOST @@ -2532,8 +2534,6 @@ spec: value: /dev/stderr - name: KONG_PROXY_ERROR_LOG value: /dev/stderr - - name: KONG_ROUTER_FLAVOR - value: traditional image: kong/kong-gateway:3.4 lifecycle: preStop: diff --git a/test/e2e/manifests/all-in-one-postgres.yaml b/test/e2e/manifests/all-in-one-postgres.yaml index fc667cae7e..f94027efeb 100644 --- a/test/e2e/manifests/all-in-one-postgres.yaml +++ b/test/e2e/manifests/all-in-one-postgres.yaml @@ -2462,6 +2462,8 @@ spec: automountServiceAccountToken: false containers: - env: + - name: KONG_ROUTER_FLAVOR + value: traditional_compatible - name: KONG_DATABASE value: postgres - name: KONG_PG_HOST @@ -2487,8 +2489,6 @@ spec: value: /dev/stderr - name: KONG_PROXY_ERROR_LOG value: /dev/stderr - - name: KONG_ROUTER_FLAVOR - value: traditional image: kong:3.4 lifecycle: preStop: diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 989136fc41..3c4e8d6482 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -109,8 +109,9 @@ func exposeAdminAPI(ctx context.Context, t *testing.T, env environments.Environm // getTestManifest gets a manifest io.Reader, applying optional patches to the base manifest provided. // In case of any failure while patching, the base manifest is returned. // If skipTestPatches is true, no patches are applied (useful when untouched manifest is needed, e.g. in upgrade tests). -func getTestManifest(t *testing.T, baseManifestPath string, skipTestPatches bool) io.Reader { +func getTestManifest(t *testing.T, baseManifestPath string, skipTestPatches bool, testPatches ...ManifestPatch) io.Reader { t.Helper() + t.Logf("getting test manifest from %v", baseManifestPath) var ( manifestsReader io.Reader @@ -155,6 +156,13 @@ func getTestManifest(t *testing.T, baseManifestPath string, skipTestPatches bool } + for _, patch := range testPatches { + manifestsReader, err = patch(manifestsReader) + if err != nil { + t.Logf("failed patching manifest, using default manifest %v, err: %v", baseManifestPath, err) + } + } + return manifestsReader }