From 18483f48e1f8696cc8b11ff7744c4aafef4f1424 Mon Sep 17 00:00:00 2001 From: David Boslee Date: Mon, 21 Aug 2023 16:07:55 -0600 Subject: [PATCH 1/3] Enqueue a single request for all resources Signed-off-by: David Boslee --- internal/provider/kubernetes/controller.go | 58 ++++++++++--------- .../provider/kubernetes/controller_test.go | 23 +++----- 2 files changed, 38 insertions(+), 43 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 9adae79407..f56a81ac1f 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -147,7 +147,7 @@ func newResourceMapping() *resourceMappings { } func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - r.log.WithName(request.Name).Info("reconciling object", "namespace", request.Namespace, "name", request.Name) + r.log.WithName(request.Name).Info("reconciling gateways", "namespace") var gatewayClasses gwapiv1b1.GatewayClassList if err := r.client.List(ctx, &gatewayClasses); err != nil { @@ -313,7 +313,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile. // Store will be required to trigger a cleanup of envoy infra resources. r.resources.GatewayAPIResources.Store(acceptedGC.Name, resourceTree) - r.log.WithName(request.Name).Info("reconciled gatewayAPI object successfully", "namespace", request.Namespace, "name", request.Name) + r.log.WithName(request.Name).Info("reconciled gateways successfully") return reconcile.Result{}, nil } @@ -1155,7 +1155,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Only enqueue GatewayClass objects that match this Envoy Gateway's controller name. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1b1.GatewayClass{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.hasMatchingController), ); err != nil { return err @@ -1164,8 +1164,9 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Only enqueue EnvoyProxy objects that match this Envoy Gateway's GatewayClass. if err := c.Watch( source.Kind(mgr.GetCache(), &egcfgv1a1.EnvoyProxy{}), - handler.EnqueueRequestsFromMapFunc(r.enqueueManagedClass), + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.ResourceVersionChangedPredicate{}, + predicate.NewPredicateFuncs(r.hasManagedClass), ); err != nil { return err } @@ -1173,7 +1174,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch Gateway CRUDs and reconcile affected GatewayClass. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1b1.Gateway{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.validateGatewayForReconcile), ); err != nil { return err @@ -1185,7 +1186,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch HTTPRoute CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1b1.HTTPRoute{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), ); err != nil { return err } @@ -1196,7 +1197,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch GRPCRoute CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1a2.GRPCRoute{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), ); err != nil { return err } @@ -1207,7 +1208,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch TLSRoute CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1a2.TLSRoute{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), ); err != nil { return err } @@ -1218,7 +1219,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch UDPRoute CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1a2.UDPRoute{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), ); err != nil { return err } @@ -1229,7 +1230,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch TCPRoute CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1a2.TCPRoute{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), ); err != nil { return err } @@ -1240,7 +1241,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch Service CRUDs and process affected *Route objects. if err := c.Watch( source.Kind(mgr.GetCache(), &corev1.Service{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.validateServiceForReconcile)); err != nil { return err } @@ -1248,7 +1249,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch EndpointSlice CRUDs and process affected *Route objects. if err := c.Watch( source.Kind(mgr.GetCache(), &discoveryv1.EndpointSlice{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.validateEndpointSliceForReconcile)); err != nil { return err } @@ -1258,7 +1259,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // resource address. if err := c.Watch( source.Kind(mgr.GetCache(), &corev1.Node{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.handleNode), ); err != nil { return err @@ -1267,7 +1268,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch Secret CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &corev1.Secret{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.validateSecretForReconcile), ); err != nil { return err @@ -1276,7 +1277,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch ReferenceGrant CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &gwapiv1a2.ReferenceGrant{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), ); err != nil { return err } @@ -1287,7 +1288,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch Deployment CRUDs and process affected Gateways. if err := c.Watch( source.Kind(mgr.GetCache(), &appsv1.Deployment{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.validateDeploymentForReconcile), ); err != nil { return err @@ -1296,7 +1297,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch AuthenticationFilter CRUDs and enqueue associated HTTPRoute objects. if err := c.Watch( source.Kind(mgr.GetCache(), &egv1a1.AuthenticationFilter{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.httpRoutesForAuthenticationFilter)); err != nil { return err } @@ -1304,7 +1305,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch RateLimitFilter CRUDs and enqueue associated HTTPRoute objects. if err := c.Watch( source.Kind(mgr.GetCache(), &egv1a1.RateLimitFilter{}), - &handler.EnqueueRequestForObject{}, + handler.EnqueueRequestsFromMapFunc(r.enqueueClass), predicate.NewPredicateFuncs(r.httpRoutesForRateLimitFilter)); err != nil { return err } @@ -1326,7 +1327,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M u := &unstructured.Unstructured{} u.SetGroupVersionKind(gvk) if err := c.Watch(source.Kind(mgr.GetCache(), u), - &handler.EnqueueRequestForObject{}); err != nil { + handler.EnqueueRequestsFromMapFunc(r.enqueueClass)); err != nil { return err } r.log.Info("Watching additional resource", "resource", gvk.String()) @@ -1334,7 +1335,13 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M return nil } -func (r *gatewayAPIReconciler) enqueueManagedClass(_ context.Context, obj client.Object) []reconcile.Request { +func (r *gatewayAPIReconciler) enqueueClass(_ context.Context, _ client.Object) []reconcile.Request { + return []reconcile.Request{{NamespacedName: types.NamespacedName{ + Name: string(r.classController), + }}} +} + +func (r *gatewayAPIReconciler) hasManagedClass(obj client.Object) bool { ep, ok := obj.(*egcfgv1a1.EnvoyProxy) if !ok { panic(fmt.Sprintf("unsupported object type %T", obj)) @@ -1344,14 +1351,14 @@ func (r *gatewayAPIReconciler) enqueueManagedClass(_ context.Context, obj client if ep.Namespace != r.namespace { r.log.Info("envoyproxy namespace does not match Envoy Gateway's namespace", "namespace", ep.Namespace, "name", ep.Name) - return []reconcile.Request{} + return false } gcList := new(gwapiv1b1.GatewayClassList) err := r.client.List(context.TODO(), gcList) if err != nil { r.log.Error(err, "failed to list gatewayclasses") - return []reconcile.Request{} + return false } for i := range gcList.Items { @@ -1360,14 +1367,11 @@ func (r *gatewayAPIReconciler) enqueueManagedClass(_ context.Context, obj client if r.hasMatchingController(&gc) && classAccepted(&gc) && classRefsEnvoyProxy(&gc, ep) { - req := reconcile.Request{ - NamespacedName: types.NamespacedName{Name: gc.Name}, - } - return []reconcile.Request{req} + return true } } - return []reconcile.Request{} + return false } // processParamsRef processes the parametersRef of the provided GatewayClass. diff --git a/internal/provider/kubernetes/controller_test.go b/internal/provider/kubernetes/controller_test.go index 840e80d5dc..03957a2699 100644 --- a/internal/provider/kubernetes/controller_test.go +++ b/internal/provider/kubernetes/controller_test.go @@ -14,7 +14,6 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" - "sigs.k8s.io/controller-runtime/pkg/reconcile" gwapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1" @@ -152,14 +151,14 @@ func TestRemoveGatewayClassFinalizer(t *testing.T) { } } -func TestEnqueueManagedClass(t *testing.T) { +func TestHasManagedClass(t *testing.T) { gcCtrlName := gwapiv1b1.GatewayController(egcfgv1a1.GatewayControllerName) testCases := []struct { name string ep client.Object classes []*gwapiv1b1.GatewayClass - expected []reconcile.Request + expected bool }{ { name: "no matching gatewayclasses", @@ -193,7 +192,7 @@ func TestEnqueueManagedClass(t *testing.T) { }, }, }, - expected: []reconcile.Request{}, + expected: false, }, { name: "match one gatewayclass", @@ -227,11 +226,7 @@ func TestEnqueueManagedClass(t *testing.T) { }, }, }, - expected: []reconcile.Request{ - { - NamespacedName: types.NamespacedName{Name: "test-gc"}, - }, - }, + expected: true, }, { name: "envoyproxy in different namespace as eg", @@ -249,7 +244,7 @@ func TestEnqueueManagedClass(t *testing.T) { Spec: gwapiv1b1.GatewayClassSpec{ControllerName: gcCtrlName}, }, }, - expected: []reconcile.Request{}, + expected: false, }, { name: "multiple gatewayclasses one with accepted status", @@ -305,11 +300,7 @@ func TestEnqueueManagedClass(t *testing.T) { }, }, }, - expected: []reconcile.Request{ - { - NamespacedName: types.NamespacedName{Name: "test-gc1"}, - }, - }, + expected: true, }, } @@ -339,7 +330,7 @@ func TestEnqueueManagedClass(t *testing.T) { Build() // Process the test case gatewayclasses. - results := r.enqueueManagedClass(context.TODO(), tc.ep) + results := r.hasManagedClass(tc.ep) require.Equal(t, tc.expected, results) }) } From 1c6e1e4a8709fbef1b109ecbd950f95f759bf983 Mon Sep 17 00:00:00 2001 From: David Boslee Date: Mon, 21 Aug 2023 16:53:08 -0600 Subject: [PATCH 2/3] Remove namespace from log Signed-off-by: David Boslee --- internal/provider/kubernetes/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index f56a81ac1f..0c1db832fe 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -147,7 +147,7 @@ func newResourceMapping() *resourceMappings { } func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - r.log.WithName(request.Name).Info("reconciling gateways", "namespace") + r.log.WithName(request.Name).Info("reconciling gateways") var gatewayClasses gwapiv1b1.GatewayClassList if err := r.client.List(ctx, &gatewayClasses); err != nil { From aaa25af72696e2ce09fdf0f4400946bc452a0502 Mon Sep 17 00:00:00 2001 From: David Boslee Date: Mon, 21 Aug 2023 17:31:18 -0600 Subject: [PATCH 3/3] Add comment and missing enqueue class Signed-off-by: David Boslee --- internal/provider/kubernetes/controller.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 0c1db832fe..0aa7d470f5 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -146,8 +146,11 @@ func newResourceMapping() *resourceMappings { } } -func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { - r.log.WithName(request.Name).Info("reconciling gateways") +// Reconcile handles reconciling all resources in a single call. Any resource event should enqueue the +// same reconcile.Request containing the gateway controller name. This allows multiple resource updates to +// be handled by a single call to Reconcile. The reconcile.Request DOES NOT map to a specific resource. +func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) { + r.log.Info("reconciling gateways") var gatewayClasses gwapiv1b1.GatewayClassList if err := r.client.List(ctx, &gatewayClasses); err != nil { @@ -313,7 +316,7 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, request reconcile. // Store will be required to trigger a cleanup of envoy infra resources. r.resources.GatewayAPIResources.Store(acceptedGC.Name, resourceTree) - r.log.WithName(request.Name).Info("reconciled gateways successfully") + r.log.Info("reconciled gateways successfully") return reconcile.Result{}, nil } @@ -1315,7 +1318,7 @@ func (r *gatewayAPIReconciler) watchResources(ctx context.Context, mgr manager.M // Watch EnvoyPatchPolicy CRUDs if err := c.Watch( source.Kind(mgr.GetCache(), &egv1a1.EnvoyPatchPolicy{}), - &handler.EnqueueRequestForObject{}); err != nil { + handler.EnqueueRequestsFromMapFunc(r.enqueueClass)); err != nil { return err } }