From 81a089249c01cf7709dcc778bba4ac25bb5a43fa Mon Sep 17 00:00:00 2001 From: Daneyon Hansen Date: Tue, 24 Sep 2024 15:45:48 -0700 Subject: [PATCH] [gateway2] Updates Gateway Controller Watch Predicate Previously, the controller would only watch Gateway objects for `generation` field changes which is not updated when annotations change. Since Gateway reconciliation should be triggered when the `gateway.gloo.solo.io/gateway-parameters-name` annotation is added, removed, or modified, the predicate was updated to check for changes in either the generation field or the annotations. Fixes #10099 Signed-off-by: Daneyon Hansen --- ...-gateway-controller-watch-annotations.yaml | 10 +++++ projects/gateway2/controller/controller.go | 44 +++++++++++++++++-- ...efault_gatewayparameters_deployer_suite.go | 11 +++-- .../kubernetes/e2e/features/deployer/suite.go | 26 ++++++++++- .../testdata/gateway-with-custom-params.yaml | 15 +++++++ .../testdata/gateway-with-default-params.yaml | 15 +++++++ .../testdata/gatewayparams-custom.yaml | 23 ++++++++++ ...meters.yaml => gatewayparams-default.yaml} | 17 ------- ...ameters.yaml => istio-gateway-params.yaml} | 0 .../kubernetes/e2e/features/deployer/types.go | 22 +++++++++- 10 files changed, 155 insertions(+), 28 deletions(-) create mode 100644 changelog/v1.18.0-beta23/gateway2-gateway-controller-watch-annotations.yaml create mode 100644 test/kubernetes/e2e/features/deployer/testdata/gateway-with-custom-params.yaml create mode 100644 test/kubernetes/e2e/features/deployer/testdata/gateway-with-default-params.yaml create mode 100644 test/kubernetes/e2e/features/deployer/testdata/gatewayparams-custom.yaml rename test/kubernetes/e2e/features/deployer/testdata/{gateway-with-parameters.yaml => gatewayparams-default.yaml} (61%) rename test/kubernetes/e2e/features/deployer/testdata/{istio-gateway-parameters.yaml => istio-gateway-params.yaml} (100%) diff --git a/changelog/v1.18.0-beta23/gateway2-gateway-controller-watch-annotations.yaml b/changelog/v1.18.0-beta23/gateway2-gateway-controller-watch-annotations.yaml new file mode 100644 index 00000000000..cc928cdf0a0 --- /dev/null +++ b/changelog/v1.18.0-beta23/gateway2-gateway-controller-watch-annotations.yaml @@ -0,0 +1,10 @@ +changelog: + - type: NON_USER_FACING + issueLink: https://github.com/solo-io/gloo/issues/10099 + resolvesIssue: false + description: >- + Previously, the controller would only watch Gateway objects for generation field + changes which is not updated when annotations change. Since Gateway reconciliation + should be triggered when the gateway.gloo.solo.io/gateway-parameters-name annotation + is added, removed, or modified, the predicate was updated to check for changes in + either the generation field or the annotations. diff --git a/projects/gateway2/controller/controller.go b/projects/gateway2/controller/controller.go index 1f368c866bd..8366c818efe 100644 --- a/projects/gateway2/controller/controller.go +++ b/projects/gateway2/controller/controller.go @@ -14,6 +14,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" @@ -176,15 +177,50 @@ func (c *controllerBuilder) watchGw(ctx context.Context) error { return err } - buildr := ctrl.NewControllerManagedBy(c.cfg.Mgr). - // Don't use WithEventFilter here as it also filters events for Owned objects. - For(&apiv1.Gateway{}, builder.WithPredicates(predicate.NewPredicateFuncs(func(object client.Object) bool { + pred := predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + // Check for generation change + if e.ObjectOld.GetGeneration() != e.ObjectNew.GetGeneration() { + return true + } + + // Check for annotation changes + oldAnnotations := e.ObjectOld.GetAnnotations() + newAnnotations := e.ObjectNew.GetAnnotations() + + // Quick check: if lengths are different, annotations have changed + if len(oldAnnotations) != len(newAnnotations) { + return true + } + + // Check if any key-value pair differs + for key, oldValue := range oldAnnotations { + if newValue, exists := newAnnotations[key]; !exists || newValue != oldValue { + return true + } + } + + // Check for any new keys that exist in the new annotations but not in the old + for key := range newAnnotations { + if _, exists := oldAnnotations[key]; !exists { + return true + } + } + + return false + }, + } + + buildr := ctrl.NewControllerManagedBy(c.cfg.Mgr).For(&apiv1.Gateway{}, builder.WithPredicates( + predicate.NewPredicateFuncs(func(object client.Object) bool { // we only care about Gateways that use our GatewayClass if gw, ok := object.(*apiv1.Gateway); ok { return c.cfg.GWClasses.Has(string(gw.Spec.GatewayClassName)) } return false - }), predicate.GenerationChangedPredicate{})) + }), + pred, + )) // watch for changes in GatewayParameters cli := c.cfg.Mgr.GetClient() diff --git a/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_deployer_suite.go b/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_deployer_suite.go index f8178cc742b..91a07b94e15 100644 --- a/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_deployer_suite.go +++ b/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_deployer_suite.go @@ -33,12 +33,17 @@ func NewMinimalDefaultGatewayParametersTestingSuite(ctx context.Context, testIns func (s *minimalDefaultGatewayParametersDeployerSuite) TestConfigureProxiesFromGatewayParameters() { s.T().Cleanup(func() { - err := s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwParametersManifestFile) + err := s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwWithParametersManifestFile) s.NoError(err, "can delete basic gateway manifest") - s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, gwParams, proxyService, proxyDeployment) + err = s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwParamsManifestFile) + s.NoError(err, "can delete manifest") + s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, gwParams) + s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, gtw, gwParams, proxyService, proxyDeployment) }) - err := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwParametersManifestFile) + err := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwParamsManifestFile) + s.Require().NoError(err, "can apply manifest") + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwWithParametersManifestFile) s.Require().NoError(err, "can apply basic gateway manifest") s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, gwParams, proxyService, proxyDeployment) diff --git a/test/kubernetes/e2e/features/deployer/suite.go b/test/kubernetes/e2e/features/deployer/suite.go index cbbf8913b87..8f460835a73 100644 --- a/test/kubernetes/e2e/features/deployer/suite.go +++ b/test/kubernetes/e2e/features/deployer/suite.go @@ -57,10 +57,18 @@ func (s *testingSuite) TestProvisionDeploymentAndService() { func (s *testingSuite) TestConfigureProxiesFromGatewayParameters() { s.T().Cleanup(func() { - err := s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwParametersManifestFile) + err := s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwWithParametersManifestFile) + s.NoError(err, "can delete manifest") + s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, gtw) + + err = s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwParamsManifestFile) s.NoError(err, "can delete manifest") s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, gwParams) + err = s.testInstallation.Actions.Kubectl().DeleteFile(s.ctx, gwParamsCustomManifestFile) + s.NoError(err, "can delete manifest") + s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, gwCustomParams) + err = s.testInstallation.Actions.Kubectl().DeleteFileSafe(s.ctx, deployerProvisionManifestFile) s.NoError(err, "can delete manifest") s.testInstallation.Assertions.EventuallyObjectsNotExist(s.ctx, proxyService, proxyDeployment) @@ -69,12 +77,26 @@ func (s *testingSuite) TestConfigureProxiesFromGatewayParameters() { err := s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, deployerProvisionManifestFile) s.Require().NoError(err, "can apply manifest") - err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwParametersManifestFile) + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwParamsManifestFile) + s.Require().NoError(err, "can apply manifest") + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwWithParametersManifestFile) s.Require().NoError(err, "can apply manifest") s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, proxyService, proxyDeployment) s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, gwParams) + s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, gtw) s.testInstallation.Assertions.EventuallyRunningReplicas(s.ctx, proxyDeployment.ObjectMeta, gomega.Equal(1)) + // Create a custom GatewayParameters + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwParamsCustomManifestFile) + s.Require().NoError(err, "can apply manifest") + // Update the Gateway to use the custom GatewayParameters + err = s.testInstallation.Actions.Kubectl().ApplyFile(s.ctx, gwWithCustomParametersManifestFile) + s.Require().NoError(err, "can apply manifest") + s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, proxyService, proxyDeployment) + s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, gwCustomParams) + s.testInstallation.Assertions.EventuallyObjectsExist(s.ctx, gtw) + s.testInstallation.Assertions.EventuallyRunningReplicas(s.ctx, proxyDeployment.ObjectMeta, gomega.Equal(2)) + // We assert that we can port-forward requests to the proxy deployment, and then execute requests against the server if s.testInstallation.RuntimeContext.RunSource == runtime.LocalDevelopment { // There are failures when opening port-forwards to the Envoy Admin API in CI diff --git a/test/kubernetes/e2e/features/deployer/testdata/gateway-with-custom-params.yaml b/test/kubernetes/e2e/features/deployer/testdata/gateway-with-custom-params.yaml new file mode 100644 index 00000000000..73ee13516d7 --- /dev/null +++ b/test/kubernetes/e2e/features/deployer/testdata/gateway-with-custom-params.yaml @@ -0,0 +1,15 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: gw + annotations: + gateway.gloo.solo.io/gateway-parameters-name: "gw-params-custom" +spec: + gatewayClassName: gloo-gateway + listeners: + - protocol: HTTP + port: 8080 + name: http + allowedRoutes: + namespaces: + from: All diff --git a/test/kubernetes/e2e/features/deployer/testdata/gateway-with-default-params.yaml b/test/kubernetes/e2e/features/deployer/testdata/gateway-with-default-params.yaml new file mode 100644 index 00000000000..607a0d998ef --- /dev/null +++ b/test/kubernetes/e2e/features/deployer/testdata/gateway-with-default-params.yaml @@ -0,0 +1,15 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: gw + annotations: + gateway.gloo.solo.io/gateway-parameters-name: "gw-params" +spec: + gatewayClassName: gloo-gateway + listeners: + - protocol: HTTP + port: 8080 + name: http + allowedRoutes: + namespaces: + from: All diff --git a/test/kubernetes/e2e/features/deployer/testdata/gatewayparams-custom.yaml b/test/kubernetes/e2e/features/deployer/testdata/gatewayparams-custom.yaml new file mode 100644 index 00000000000..d085fefe1d6 --- /dev/null +++ b/test/kubernetes/e2e/features/deployer/testdata/gatewayparams-custom.yaml @@ -0,0 +1,23 @@ +apiVersion: gateway.gloo.solo.io/v1alpha1 +kind: GatewayParameters +metadata: + name: gw-params-custom +spec: + kube: + deployment: + replicas: 2 + podTemplate: + extraLabels: + pod-label-key: pod-label-val + extraAnnotations: + pod-anno-key: pod-anno-val + envoyContainer: + bootstrap: + logLevel: debug + componentLogLevels: + upstream: debug + connection: trace + securityContext: + runAsUser: null + runAsNonRoot: false + allowPrivilegeEscalation: true diff --git a/test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml b/test/kubernetes/e2e/features/deployer/testdata/gatewayparams-default.yaml similarity index 61% rename from test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml rename to test/kubernetes/e2e/features/deployer/testdata/gatewayparams-default.yaml index 948e8b9dcf8..37d8efe884a 100644 --- a/test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml +++ b/test/kubernetes/e2e/features/deployer/testdata/gatewayparams-default.yaml @@ -1,19 +1,3 @@ -apiVersion: gateway.networking.k8s.io/v1 -kind: Gateway -metadata: - name: gw - annotations: - gateway.gloo.solo.io/gateway-parameters-name: "gw-params" -spec: - gatewayClassName: gloo-gateway - listeners: - - protocol: HTTP - port: 8080 - name: http - allowedRoutes: - namespaces: - from: All ---- apiVersion: gateway.gloo.solo.io/v1alpha1 kind: GatewayParameters metadata: @@ -37,4 +21,3 @@ spec: runAsUser: null runAsNonRoot: false allowPrivilegeEscalation: true - diff --git a/test/kubernetes/e2e/features/deployer/testdata/istio-gateway-parameters.yaml b/test/kubernetes/e2e/features/deployer/testdata/istio-gateway-params.yaml similarity index 100% rename from test/kubernetes/e2e/features/deployer/testdata/istio-gateway-parameters.yaml rename to test/kubernetes/e2e/features/deployer/testdata/istio-gateway-params.yaml diff --git a/test/kubernetes/e2e/features/deployer/types.go b/test/kubernetes/e2e/features/deployer/types.go index d5292205af2..90f54b151f5 100644 --- a/test/kubernetes/e2e/features/deployer/types.go +++ b/test/kubernetes/e2e/features/deployer/types.go @@ -7,15 +7,19 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apiv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/solo-io/gloo/projects/gateway2/api/v1alpha1" ) var ( - gwParametersManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-with-parameters.yaml") + gwWithParametersManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-with-default-params.yaml") + gwWithCustomParametersManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-with-custom-params.yaml") + gwParamsManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "gatewayparams-default.yaml") + gwParamsCustomManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "gatewayparams-custom.yaml") basicGatewayManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "basic-gateway.yaml") deployerProvisionManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "deployer-provision.yaml") - istioGatewayParametersManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "istio-gateway-parameters.yaml") + istioGatewayParametersManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "istio-gateway-params.yaml") selfManagedGatewayManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "self-managed-gateway.yaml") // When we apply the deployer-provision.yaml file, we expect resources to be created with this metadata @@ -26,10 +30,24 @@ var ( proxyDeployment = &appsv1.Deployment{ObjectMeta: glooProxyObjectMeta} proxyService = &corev1.Service{ObjectMeta: glooProxyObjectMeta} + gtw = &apiv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + Namespace: "default", + }, + } + gwParams = &v1alpha1.GatewayParameters{ ObjectMeta: metav1.ObjectMeta{ Name: "gw-params", Namespace: "default", }, } + + gwCustomParams = &v1alpha1.GatewayParameters{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw-params-custom", + Namespace: "default", + }, + } )