From 4d718574e9d67674535d13f69dfc7dc0f27b856c Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Thu, 8 Aug 2024 14:27:58 +0200 Subject: [PATCH] split process finalizers functions and add integration testcase for multiple gc Signed-off-by: Karol Szwaj --- internal/provider/kubernetes/controller.go | 69 ++++++++++--------- .../provider/kubernetes/kubernetes_test.go | 27 ++++++-- 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index ee9ed2604698..c2a061752393 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -173,6 +173,11 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, err } + if err := r.processEnvoyProxyFinalizers(ctx, managedGCs); err != nil { + r.log.Error(err, "failed to process Envoy Proxy finalizers") + return reconcile.Result{}, err + } + // The gatewayclass was already deleted/finalized and there are stale queue entries. if managedGCs == nil { r.resources.GatewayAPIResources.Delete(string(r.classController)) @@ -288,8 +293,8 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques return reconcile.Result{}, err } - if err := r.processFinalizers(ctx, managedGC, gwcResource); err != nil { - r.log.Error(err, "unable to handle finalizers") + if err := r.processGatewayClassFinalizers(ctx, managedGC, gwcResource); err != nil { + r.log.Error(err, "failed to process GatewayClass finalizers") return reconcile.Result{}, err } } @@ -769,8 +774,8 @@ func (r *gatewayAPIReconciler) findReferenceGrant(ctx context.Context, from, to return nil, nil } -// processFinalizers encapsulates logic for managing finalizers on GatewayClass and EnvoyProxy objects. -func (r *gatewayAPIReconciler) processFinalizers(ctx context.Context, managedGC *gwapiv1.GatewayClass, resourceTree *gatewayapi.Resources) error { +// processGatewayClassFinalizers encapsulates logic for managing finalizers on GatewayClass objects. +func (r *gatewayAPIReconciler) processGatewayClassFinalizers(ctx context.Context, managedGC *gwapiv1.GatewayClass, resourceTree *gatewayapi.Resources) error { // Add finalizer to GatewayClass if there are gateways. if len(resourceTree.Gateways) > 0 && managedGC.DeletionTimestamp.IsZero() { if err := r.addFinalizer(ctx, managedGC); err != nil { @@ -778,31 +783,40 @@ func (r *gatewayAPIReconciler) processFinalizers(ctx context.Context, managedGC } } - // Check previously stored referenced Envoy Proxy and remove finalizer if not referenced anymore by the current managed GatewayClass. - managedResources := r.resources.GetResourcesByGatewayClass(managedGC.Name) - if managedResources != nil { - refEnvoyProxy := managedResources.EnvoyProxyForGatewayClass - if refEnvoyProxy != nil && !classRefsEnvoyProxy(managedGC, refEnvoyProxy) { - if err := r.removeFinalizer(ctx, refEnvoyProxy); err != nil { - return fmt.Errorf("failed to remove finalizer from previous referenced envoy proxy %s: %w", refEnvoyProxy.Name, err) + // Remove finalizer from GatewayClass if there are no gateways. + if len(resourceTree.Gateways) == 0 { + if !managedGC.DeletionTimestamp.IsZero() { + if err := r.removeFinalizer(ctx, managedGC); err != nil { + return fmt.Errorf("failed to remove finalizer from gatewayclass %s: %w", managedGC.Name, err) } } } - // Remove finalizer from GatewayClass and Envoy Proxy if there are no gateways. - if len(resourceTree.Gateways) == 0 { - if managedResources != nil { - refEnvoyProxy := managedResources.EnvoyProxyForGatewayClass - if refEnvoyProxy != nil && (!managedGC.DeletionTimestamp.IsZero() || !refEnvoyProxy.DeletionTimestamp.IsZero()) { - if err := r.removeFinalizer(ctx, refEnvoyProxy); err != nil { - return fmt.Errorf("failed to remove finalizer from previous referenced envoy proxy %s: %w", refEnvoyProxy.Name, err) + return nil +} + +// processEnvoyProxyFinalizers encapsulates logic for managing finalizers on EnvoyProxy objects. +func (r *gatewayAPIReconciler) processEnvoyProxyFinalizers(ctx context.Context, managedGCs []*gwapiv1.GatewayClass) error { + var epList egv1a1.EnvoyProxyList + if err := r.client.List(ctx, &epList, &client.ListOptions{Namespace: ""}); err != nil { + return err + } + for _, ep := range epList.Items { + ep := ep + isReferenced := false + for _, gc := range managedGCs { + if classRefsEnvoyProxy(gc, &ep) && gc.DeletionTimestamp.IsZero() { + if err := r.addFinalizer(ctx, &ep); err != nil { + return fmt.Errorf("failed to add finalizer to Envoy Proxy %s: %w", ep.Name, err) } + isReferenced = true + break } } - if !managedGC.DeletionTimestamp.IsZero() { - if err := r.removeFinalizer(ctx, managedGC); err != nil { - return fmt.Errorf("failed to remove finalizer from gatewayclass %s: %w", managedGC.Name, err) + if !isReferenced { + if err := r.removeFinalizer(ctx, &ep); err != nil { + return fmt.Errorf("failed to remove finalizer from Envoy Proxy %s: %w", ep.Name, err) } } } @@ -1660,8 +1674,8 @@ func (r *gatewayAPIReconciler) processGatewayClassParamsRef(ctx context.Context, ep := new(egv1a1.EnvoyProxy) if err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*gc.Spec.ParametersRef.Namespace), Name: gc.Spec.ParametersRef.Name}, ep); err != nil { - if kerrors.IsNotFound(err) { - return fmt.Errorf("envoyproxy referenced by gatewayclass is not found: %w", err) + if kerrors.IsNotFound(err) && !gc.DeletionTimestamp.IsZero() { + return nil } return fmt.Errorf("failed to find envoyproxy %s/%s: %w", r.namespace, gc.Spec.ParametersRef.Name, err) } @@ -1670,15 +1684,6 @@ func (r *gatewayAPIReconciler) processGatewayClassParamsRef(ctx context.Context, return err } - if classRefsEnvoyProxy(gc, ep) { - if err := r.addFinalizer(ctx, ep); err != nil { - return fmt.Errorf("failed to add finalizer to Envoy Proxy %s: %w", ep.Name, err) - } - if err := r.addFinalizer(ctx, gc); err != nil { - return fmt.Errorf("failed to add finalizer to gatewayclass %s: %w", gc.Name, err) - } - } - // Update the EnvoyProxyForGatewayClass reference to the current EnvoyProxy resourceTree.EnvoyProxyForGatewayClass = ep return nil diff --git a/internal/provider/kubernetes/kubernetes_test.go b/internal/provider/kubernetes/kubernetes_test.go index cbf7f0cccbbc..69fd3641a922 100644 --- a/internal/provider/kubernetes/kubernetes_test.go +++ b/internal/provider/kubernetes/kubernetes_test.go @@ -158,8 +158,6 @@ func testGatewayClassAcceptedStatus(ctx context.Context, t *testing.T, provider func testGatewayClassWithParamRef(ctx context.Context, t *testing.T, provider *Provider, resources *message.ProviderResources) { cli := provider.manager.GetClient() - - // Note: The namespace for the EnvoyProxy must match EG's configured namespace. testNs := config.DefaultNamespace epName := "test-envoy-proxy" @@ -179,6 +177,15 @@ func testGatewayClassWithParamRef(ctx context.Context, t *testing.T, provider *P } require.NoError(t, cli.Create(ctx, gc)) + gc2 := test.GetGatewayClass("gc-with-param-ref2", egv1a1.GatewayControllerName, nil) + gc2.Spec.ParametersRef = &gwapiv1.ParametersReference{ + Group: gwapiv1.Group(egv1a1.GroupVersion.Group), + Kind: egv1a1.KindEnvoyProxy, + Name: epName, + Namespace: gatewayapi.NamespacePtr(testNs), + } + require.NoError(t, cli.Create(ctx, gc2)) + defer func() { require.NoError(t, cli.Delete(ctx, gc)) }() @@ -322,21 +329,33 @@ func testGatewayClassWithParamRef(ctx context.Context, t *testing.T, provider *P return res != nil && len(res.Gateways) == 1 }, defaultWait, defaultTick) - // Ensure the gateway class has been finalized. + // Ensure the gateway class with gateways has been finalized. require.Eventually(t, func() bool { err := cli.Get(ctx, types.NamespacedName{Name: gc.Name}, gc) return err == nil && slices.Contains(gc.Finalizers, gatewayClassFinalizer) }, defaultWait, defaultTick) + // Ensure that gateway class without gateways has not been finalized. + require.Eventually(t, func() bool { + err := cli.Get(ctx, types.NamespacedName{Name: gc2.Name}, gc2) + return err == nil && !slices.Contains(gc2.Finalizers, gatewayClassFinalizer) + }, defaultWait, defaultTick) + // Ensure the envoyproxy has been finalized. require.Eventually(t, func() bool { err := cli.Get(ctx, types.NamespacedName{Name: ep.Name, Namespace: testNs}, ep) return err == nil && slices.Contains(ep.Finalizers, envoyProxyFinalizer) }, defaultWait, defaultTick) - //Ensure that envoyproxy finalizer will be removed when GatewayClass stops referencing it + //Ensure that envoyproxy finalizer will not be removed when 1 of 2 GatewayClasses stops referencing it gc.Spec.ParametersRef = nil require.NoError(t, cli.Update(ctx, gc)) + require.Eventually(t, func() bool { + err := cli.Get(ctx, types.NamespacedName{Name: ep.Name, Namespace: testNs}, ep) + return err == nil && slices.Contains(ep.Finalizers, envoyProxyFinalizer) + }, defaultWait, defaultTick) + + require.NoError(t, cli.Delete(ctx, gc2)) require.Eventually(t, func() bool { err := cli.Get(ctx, types.NamespacedName{Name: ep.Name, Namespace: testNs}, ep) return err == nil && !slices.Contains(ep.Finalizers, envoyProxyFinalizer)