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.

Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
  • Loading branch information
danehans committed Oct 4, 2024
1 parent a1dc990 commit 55c34cd
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 23 deletions.
10 changes: 10 additions & 0 deletions changelog/v1.18.0-beta26/issue_10099.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/10099
resolvesIssue: true
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.
9 changes: 7 additions & 2 deletions projects/gateway2/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,17 @@ 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 {
// we only care about Gateways that use our GatewayClass
// 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{}))
}),
predicate.Or(
predicate.AnnotationChangedPredicate{},
predicate.GenerationChangedPredicate{},
),
))

// watch for changes in GatewayParameters
cli := c.cfg.Mgr.GetClient()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ func NewIstioIntegrationTestingSuite(ctx context.Context, testInst *e2e.TestInst

func (s *istioIntegrationDeployerSuite) SetupSuite() {
s.manifests = map[string][]string{
"TestConfigureIstioIntegrationFromGatewayParameters": {testdefaults.NginxPodManifest, istioGatewayParametersManifestFile},
"TestConfigureIstioIntegrationFromGatewayParameters": {testdefaults.NginxPodManifest, istioGatewayParameters},
}
s.manifestObjects = map[string][]client.Object{
testdefaults.NginxPodManifest: {testdefaults.NginxPod, testdefaults.NginxSvc},
istioGatewayParametersManifestFile: {proxyService, proxyServiceAccount, proxyDeployment, gwParams},
testdefaults.NginxPodManifest: {testdefaults.NginxPod, testdefaults.NginxSvc},
istioGatewayParameters: {proxyService, proxyServiceAccount, proxyDeployment, gwParamsDefault},
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (s *minimalDefaultGatewayParametersDeployerSuite) SetupSuite() {
}
s.manifestObjects = map[string][]client.Object{
testdefaults.NginxPodManifest: {testdefaults.NginxPod, testdefaults.NginxSvc},
gatewayWithParameters: {proxyService, proxyServiceAccount, proxyDeployment, gwParams},
gatewayWithParameters: {proxyService, proxyServiceAccount, proxyDeployment, gwParamsDefault},
}
}

Expand Down
68 changes: 57 additions & 11 deletions test/kubernetes/e2e/features/deployer/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/onsi/gomega/gstruct"
"github.com/solo-io/gloo/projects/gateway2/api/v1alpha1"
"github.com/solo-io/gloo/projects/gateway2/wellknown"
testdefaults "github.com/solo-io/gloo/test/kubernetes/e2e/defaults"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -53,17 +54,33 @@ func NewTestingSuite(ctx context.Context, testInst *e2e.TestInstallation) suite.

func (s *testingSuite) SetupSuite() {
s.manifests = map[string][]string{
"TestProvisionDeploymentAndService": {testdefaults.NginxPodManifest, gatewayWithoutParameters},
"TestConfigureProxiesFromGatewayParameters": {testdefaults.NginxPodManifest, gatewayWithParameters},
"TestProvisionResourcesUpdatedWithValidParameters": {testdefaults.NginxPodManifest, gatewayWithParameters},
"TestProvisionResourcesNotUpdatedWithInvalidParameters": {testdefaults.NginxPodManifest, gatewayWithParameters},
"TestSelfManagedGateway": {selfManagedGatewayManifestFile},
"TestProvisionDeploymentAndService": {
testdefaults.NginxPodManifest,
gatewayWithoutParameters,
},
"TestConfigureProxiesFromGatewayParameters": {
testdefaults.NginxPodManifest,
gatewayParametersCustom,
gatewayWithParameters,
},
"TestProvisionResourcesUpdatedWithValidParameters": {
testdefaults.NginxPodManifest,
gatewayWithParameters,
},
"TestProvisionResourcesNotUpdatedWithInvalidParameters": {
testdefaults.NginxPodManifest,
gatewayWithParameters,
},
"TestSelfManagedGateway": {
selfManagedGateway,
},
}
s.manifestObjects = map[string][]client.Object{
testdefaults.NginxPodManifest: {testdefaults.NginxPod, testdefaults.NginxSvc},
gatewayWithoutParameters: {proxyService, proxyServiceAccount, proxyDeployment},
gatewayWithParameters: {proxyService, proxyServiceAccount, proxyDeployment, gwParams},
selfManagedGatewayManifestFile: {gwParams},
testdefaults.NginxPodManifest: {testdefaults.NginxPod, testdefaults.NginxSvc},
gatewayWithoutParameters: {proxyService, proxyServiceAccount, proxyDeployment},
gatewayWithParameters: {proxyService, proxyServiceAccount, proxyDeployment, gwParamsDefault},
gatewayParametersCustom: {gwParamsCustom},
selfManagedGateway: {gwParamsDefault},
}
}

Expand Down Expand Up @@ -107,6 +124,17 @@ func (s *testingSuite) TestConfigureProxiesFromGatewayParameters() {
s.testInstallation.Assertions.Gomega.Expect(sa.GetAnnotations()).To(
gomega.HaveKeyWithValue("sa-anno-key", "sa-anno-val"))

// Update the Gateway to use the custom GatewayParameters
gwName := types.NamespacedName{Name: gw.Name, Namespace: gw.Namespace}
err = s.testInstallation.ClusterContext.Client.Get(s.ctx, gwName, gw)
s.Require().NoError(err)
s.patchGateway(gw.ObjectMeta, func(gw *gwv1.Gateway) {
gw.Annotations[wellknown.GatewayParametersAnnotationName] = gwParamsCustom.Name
})

// Assert that the expected custom configuration exists.
s.testInstallation.Assertions.EventuallyRunningReplicas(s.ctx, proxyDeployment.ObjectMeta, gomega.Equal(2))

s.testInstallation.Assertions.AssertEnvoyAdminApi(
s.ctx,
proxyDeployment.ObjectMeta,
Expand All @@ -119,7 +147,7 @@ func (s *testingSuite) TestProvisionResourcesUpdatedWithValidParameters() {
s.testInstallation.Assertions.EventuallyRunningReplicas(s.ctx, proxyDeployment.ObjectMeta, gomega.Equal(1))

// modify the number of replicas in the GatewayParameters
s.patchGatewayParameters(gwParams.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) {
s.patchGatewayParameters(gwParamsDefault.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) {
parameters.Spec.Kube.Deployment.Replicas = ptr.To(uint32(2))
})

Expand All @@ -137,7 +165,7 @@ func (s *testingSuite) TestProvisionResourcesNotUpdatedWithInvalidParameters() {
origPrivileged = gomega.BeNil()
)

s.patchGatewayParameters(gwParams.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) {
s.patchGatewayParameters(gwParamsDefault.ObjectMeta, func(parameters *v1alpha1.GatewayParameters) {
gomega.Expect(proxyDeployment.Spec.Template.Spec.Containers).To(gomega.HaveLen(1))
envoyContainer := proxyDeployment.Spec.Template.Spec.Containers[0]
gomega.Expect(envoyContainer.SecurityContext.AllowPrivilegeEscalation).To(origAllowPrivilegeEscalation)
Expand Down Expand Up @@ -192,6 +220,24 @@ func (s *testingSuite) TestSelfManagedGateway() {
s.testInstallation.Assertions.ConsistentlyObjectsNotExist(s.ctx, proxyService, proxyServiceAccount, proxyDeployment)
}

// patchGateway accepts a reference to an object, and a patch function. It then queries the object,
// performs the patch in memory, and writes the object back to the cluster.
func (s *testingSuite) patchGateway(objectMeta metav1.ObjectMeta, patchFn func(*gwv1.Gateway)) {
gw := new(gwv1.Gateway)
gwName := types.NamespacedName{
Namespace: objectMeta.GetNamespace(),
Name: objectMeta.GetName(),
}
err := s.testInstallation.ClusterContext.Client.Get(s.ctx, gwName, gw)
s.Assert().NoError(err, "can get the Gateway object")
updated := gw.DeepCopy()

patchFn(updated)

err = s.testInstallation.ClusterContext.Client.Patch(s.ctx, updated, client.MergeFrom(gw))
s.Assert().NoError(err, "can update the Gateway object")
}

// patchGatewayParameters accepts a reference to an object, and a patch function
// It then queries the object, performs the patch in memory, and writes the object back to the cluster
func (s *testingSuite) patchGatewayParameters(objectMeta metav1.ObjectMeta, patchFn func(*v1alpha1.GatewayParameters)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,3 @@ spec:
runAsUser: null
runAsNonRoot: false
allowPrivilegeEscalation: true

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
26 changes: 21 additions & 5 deletions test/kubernetes/e2e/features/deployer/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/solo-io/gloo/projects/gateway2/api/v1alpha1"
)

var (
gatewayWithoutParameters = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-without-parameters.yaml")
gatewayWithParameters = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-with-parameters.yaml")
istioGatewayParametersManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "istio-gateway-parameters.yaml")
selfManagedGatewayManifestFile = filepath.Join(util.MustGetThisDir(), "testdata", "self-managed-gateway.yaml")
gatewayWithoutParameters = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-without-parameters.yaml")
gatewayWithParameters = filepath.Join(util.MustGetThisDir(), "testdata", "gateway-with-parameters.yaml")
gatewayParametersCustom = filepath.Join(util.MustGetThisDir(), "testdata", "gatewayparameters-custom.yaml")
istioGatewayParameters = filepath.Join(util.MustGetThisDir(), "testdata", "istio-gateway-parameters.yaml")
selfManagedGateway = 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
glooProxyObjectMeta = metav1.ObjectMeta{
Expand All @@ -26,10 +28,24 @@ var (
proxyService = &corev1.Service{ObjectMeta: glooProxyObjectMeta}
proxyServiceAccount = &corev1.ServiceAccount{ObjectMeta: glooProxyObjectMeta}

gwParams = &v1alpha1.GatewayParameters{
gwParamsDefault = &v1alpha1.GatewayParameters{
ObjectMeta: metav1.ObjectMeta{
Name: "gw-params",
Namespace: "default",
},
}

gwParamsCustom = &v1alpha1.GatewayParameters{
ObjectMeta: metav1.ObjectMeta{
Name: "gw-params-custom",
Namespace: "default",
},
}

gw = &gwapiv1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Name: "gw",
Namespace: "default",
},
}
)

0 comments on commit 55c34cd

Please sign in to comment.