Skip to content

Commit

Permalink
[gateway2] Updates Gateway Controller Watch Predicate
Browse files Browse the repository at this point in the history
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 <daneyon.hansen@solo.io>
  • Loading branch information
danehans committed Sep 25, 2024
1 parent 58c532b commit 81a0892
Show file tree
Hide file tree
Showing 10 changed files with 155 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -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.
44 changes: 40 additions & 4 deletions projects/gateway2/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
26 changes: 24 additions & 2 deletions test/kubernetes/e2e/features/deployer/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -37,4 +21,3 @@ spec:
runAsUser: null
runAsNonRoot: false
allowPrivilegeEscalation: true

22 changes: 20 additions & 2 deletions test/kubernetes/e2e/features/deployer/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
},
}
)

0 comments on commit 81a0892

Please sign in to comment.