[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 <[email protected]>
solo-io · Sep 30, 2024 · f3d37ef · f3d37ef
1 parent 31b7218
commit f3d37ef
Show file tree

Hide file tree

Showing 12 changed files with 151 additions and 28 deletions.
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
@@ -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
@@ -10,6 +10,7 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -44,6 +45,7 @@ const (
 type GatewayConfig struct {
 	Mgr            manager.Manager
 	GWClasses      sets.Set[string]
+	Gateways       map[types.NamespacedName]apiv1.Gateway
 	Dev            bool
 	ControllerName string
 	AutoProvision  bool
@@ -177,14 +179,35 @@ func (c *controllerBuilder) watchGw(ctx context.Context) error {
 	}
 
 	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 {
+		For(&apiv1.Gateway{}).
+		WithEventFilter(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))
+				gwNN := types.NamespacedName{Namespace: gw.Namespace, Name: gw.Name}
+				if c.cfg.GWClasses.Has(string(gw.Spec.GatewayClassName)) {
+					c.cfg.Gateways[gwNN] = *gw
+					return true
+				} else {
+					delete(c.cfg.Gateways, gwNN)
+					return false
+				}
+			}
+			// Check if this object is owned by a managed Gateway
+			ownerReferences := object.GetOwnerReferences()
+			for _, ownerRef := range ownerReferences {
+				if ownerRef.Kind == wellknown.GatewayKind && ownerRef.APIVersion == apiv1.GroupVersion.String() {
+					ownerNN := types.NamespacedName{Namespace: object.GetNamespace(), Name: ownerRef.Name}
+					if _, exists := c.cfg.Gateways[ownerNN]; exists {
+						return true
+					}
+				}
 			}
 			return false
-		}), predicate.GenerationChangedPredicate{}))
+		})).
+		WithEventFilter(predicate.Or(
+			predicate.AnnotationChangedPredicate{},
+			predicate.GenerationChangedPredicate{},
+		))
 
 	// watch for changes in GatewayParameters
 	cli := c.cfg.Mgr.GetClient()
@@ -226,6 +249,7 @@ func (c *controllerBuilder) watchGw(ctx context.Context) error {
 	}
 
 	gwReconciler := &gatewayReconciler{
+		gateways:      c.cfg.Gateways,
 		cli:           c.cfg.Mgr.GetClient(),
 		scheme:        c.cfg.Mgr.GetScheme(),
 		autoProvision: c.cfg.AutoProvision,

diff --git a/projects/gateway2/controller/gw_controller.go b/projects/gateway2/controller/gw_controller.go
@@ -8,6 +8,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -21,6 +22,7 @@ const (
 type gatewayReconciler struct {
 	cli           client.Client
 	autoProvision bool
+	gateways      map[types.NamespacedName]api.Gateway
 
 	scheme   *runtime.Scheme
 	deployer *deployer.Deployer

diff --git a/projects/gateway2/controller/start.go b/projects/gateway2/controller/start.go
@@ -8,11 +8,13 @@ import (
 
 	"github.com/solo-io/gloo/pkg/schemes"
 
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 
 	gatewayv1 "github.com/solo-io/gloo/projects/gateway/pkg/api/v1"
 	"github.com/solo-io/gloo/projects/gateway2/extensions"
@@ -137,6 +139,7 @@ func Start(ctx context.Context, cfg StartConfig) error {
 	gwCfg := GatewayConfig{
 		Mgr:            mgr,
 		GWClasses:      sets.New(append(cfg.Opts.ExtraGatewayClasses, wellknown.GatewayClassName)...),
+		Gateways:       make(map[types.NamespacedName]gwapiv1.Gateway),
 		ControllerName: wellknown.GatewayControllerName,
 		AutoProvision:  AutoProvision,
 		ControlPlane:   cfg.Opts.ControlPlane,

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
@@ -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
@@ -58,10 +58,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, proxyServiceAccount, proxyDeployment)
@@ -70,10 +78,13 @@ 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, proxyServiceAccount, 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))
 
 	// check that the labels and annotations got passed through from GatewayParameters to the ServiceAccount
@@ -87,6 +98,20 @@ func (s *testingSuite) TestConfigureProxiesFromGatewayParameters() {
 	s.testInstallation.Assertions.Gomega.Expect(sa.GetAnnotations()).To(
 		gomega.HaveKeyWithValue("sa-anno-key", "sa-anno-val"))
 
+	// 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")
+
+	// Assert that the managed objects exist with the expected configuration.
+	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
@@ -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
@@ -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
@@ -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/...yer/testdata/gateway-with-parameters.yaml → ...loyer/testdata/gatewayparams-default.yaml b/...yer/testdata/gateway-with-parameters.yaml → ...loyer/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:
@@ -42,4 +26,3 @@ spec:
         runAsUser: null
         runAsNonRoot: false
         allowPrivilegeEscalation: true
-
diff --git a/...er/testdata/istio-gateway-parameters.yaml → ...ployer/testdata/istio-gateway-params.yaml b/...er/testdata/istio-gateway-parameters.yaml → ...ployer/testdata/istio-gateway-params.yaml
diff --git 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
@@ -27,10 +31,24 @@ var (
 	proxyService        = &corev1.Service{ObjectMeta: glooProxyObjectMeta}
 	proxyServiceAccount = &corev1.ServiceAccount{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",
+		},
+	}
 )