From 55c34cdab8cb4731605879ef3b52ba7cc2a48372 Mon Sep 17 00:00:00 2001 From: Daneyon Hansen Date: Thu, 3 Oct 2024 15:01:39 -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. Signed-off-by: Daneyon Hansen --- changelog/v1.18.0-beta26/issue_10099.yaml | 10 +++ projects/gateway2/controller/controller.go | 9 ++- .../deployer/istio_integration_suite.go | 6 +- ...minimal_default_gatewayparameters_suite.go | 2 +- .../kubernetes/e2e/features/deployer/suite.go | 68 ++++++++++++++++--- .../testdata/gateway-with-parameters.yaml | 1 - .../testdata/gatewayparameters-custom.yaml | 23 +++++++ .../kubernetes/e2e/features/deployer/types.go | 26 +++++-- 8 files changed, 122 insertions(+), 23 deletions(-) create mode 100644 changelog/v1.18.0-beta26/issue_10099.yaml create mode 100644 test/kubernetes/e2e/features/deployer/testdata/gatewayparameters-custom.yaml diff --git a/changelog/v1.18.0-beta26/issue_10099.yaml b/changelog/v1.18.0-beta26/issue_10099.yaml new file mode 100644 index 00000000000..b8c398dade4 --- /dev/null +++ b/changelog/v1.18.0-beta26/issue_10099.yaml @@ -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. diff --git a/projects/gateway2/controller/controller.go b/projects/gateway2/controller/controller.go index 1f368c866bd..8e961143f27 100644 --- a/projects/gateway2/controller/controller.go +++ b/projects/gateway2/controller/controller.go @@ -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() diff --git a/test/kubernetes/e2e/features/deployer/istio_integration_suite.go b/test/kubernetes/e2e/features/deployer/istio_integration_suite.go index 9d4ac893452..3884e013364 100644 --- a/test/kubernetes/e2e/features/deployer/istio_integration_suite.go +++ b/test/kubernetes/e2e/features/deployer/istio_integration_suite.go @@ -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}, } } diff --git a/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_suite.go b/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_suite.go index ec46425a891..82e5779a249 100644 --- a/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_suite.go +++ b/test/kubernetes/e2e/features/deployer/minimal_default_gatewayparameters_suite.go @@ -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}, } } diff --git a/test/kubernetes/e2e/features/deployer/suite.go b/test/kubernetes/e2e/features/deployer/suite.go index 967207d67c1..31ec2d380df 100644 --- a/test/kubernetes/e2e/features/deployer/suite.go +++ b/test/kubernetes/e2e/features/deployer/suite.go @@ -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" @@ -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}, } } @@ -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, @@ -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)) }) @@ -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) @@ -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)) { diff --git a/test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml b/test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml index a5f5e1e17c7..c5b80f9c104 100644 --- a/test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml +++ b/test/kubernetes/e2e/features/deployer/testdata/gateway-with-parameters.yaml @@ -56,4 +56,3 @@ spec: runAsUser: null runAsNonRoot: false allowPrivilegeEscalation: true - diff --git a/test/kubernetes/e2e/features/deployer/testdata/gatewayparameters-custom.yaml b/test/kubernetes/e2e/features/deployer/testdata/gatewayparameters-custom.yaml new file mode 100644 index 00000000000..d085fefe1d6 --- /dev/null +++ b/test/kubernetes/e2e/features/deployer/testdata/gatewayparameters-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/types.go b/test/kubernetes/e2e/features/deployer/types.go index e1f7d3402f1..ef8c57f6e1e 100644 --- a/test/kubernetes/e2e/features/deployer/types.go +++ b/test/kubernetes/e2e/features/deployer/types.go @@ -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{ @@ -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", + }, + } )