From f1af45e0949f75b94107109d14f55c04d162bea0 Mon Sep 17 00:00:00 2001 From: Piyush Tiwari Date: Wed, 24 Jul 2024 12:19:20 +0530 Subject: [PATCH] fix finalizer removal logic for ingresses --- pkg/controllers/ingress/ingress.go | 8 ++++- pkg/util/util.go | 5 ++- pkg/util/util_test.go | 49 ++++++++++++++++++------------ 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/pkg/controllers/ingress/ingress.go b/pkg/controllers/ingress/ingress.go index f397d8a6..d5823c42 100644 --- a/pkg/controllers/ingress/ingress.go +++ b/pkg/controllers/ingress/ingress.go @@ -201,7 +201,13 @@ func (c *Controller) sync(key string) error { if ingressClass == nil || ingressClass.Spec.Controller != c.controllerClass { klog.V(4).InfoS("skipping ingress class, not for this controller", "ingress", klog.KRef(namespace, name)) - // skipping since ingress class is not applicable to this controller + // skipping since ingress class is not applicable to this controller, we remove our finalizer if it exists + deleteFinalizerErr := c.deleteFinalizer(ingress) + if deleteFinalizerErr != nil { + klog.V(4).Infof("Found Ingress %s/%s with finalizer %s, but not managed by this controller. Unable to delete"+ + " finalizer due to error: %s", ingress.Namespace, ingress.Name, util.IngressControllerFinalizer, deleteFinalizerErr.Error()) + } + return nil } diff --git a/pkg/util/util.go b/pkg/util/util.go index 2d129fe3..96bfc291 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -321,6 +321,7 @@ func GetPodReadinessCondition(ingressName string, host string, path networkingv1 return corev1.PodConditionType(fmt.Sprintf("%s/%s", PodReadinessConditionPrefix, PathToRoutePolicyName(ingressName, host, path))) } +// GetIngressClass returns non-nil error only if it's unable to list IngressClass resources. Returns (nil, nil) if no matching IngressClass found. func GetIngressClass(ingress *networkingv1.Ingress, ingressClassLister networkinglisters.IngressClassLister) (*networkingv1.IngressClass, error) { icList, err := ingressClassLister.List(labels.Everything()) if err != nil { @@ -336,6 +337,7 @@ func GetIngressClass(ingress *networkingv1.Ingress, ingressClassLister networkin } } + klog.V(4).Infof("no IngressClassName set for %s, and no default IngressClass found", ingress.Name) return nil, nil } @@ -345,7 +347,8 @@ func GetIngressClass(ingress *networkingv1.Ingress, ingressClassLister networkin } } - return nil, errors.New("ingress class not found for ingress") + klog.V(4).Infof("for Ingress %s, set IngressClass %s not found", ingress.Name, *ingress.Spec.IngressClassName) + return nil, nil } func PathToServiceAndPort(ingressNamespace string, path networkingv1.HTTPIngressPath, serviceLister corelisters.ServiceLister) (string, int32, error) { diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 25db9ce2..aa65bd22 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -13,6 +13,7 @@ import ( "fmt" "k8s.io/apimachinery/pkg/util/sets" corelisters "k8s.io/client-go/listers/core/v1" + networkingListers "k8s.io/client-go/listers/networking/v1" "strings" "testing" @@ -463,27 +464,10 @@ func TestGetPodReadinessCondition(t *testing.T) { func TestGetIngressClass(t *testing.T) { RegisterTestingT(t) - client := fakeclientset.NewSimpleClientset() - - ingressClassList := getIngressClassList() - - client.NetworkingV1().(*fake2.FakeNetworkingV1). - PrependReactor("list", "ingressclasses", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { - return true, ingressClassList, nil - }) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - informerFactory := informers.NewSharedInformerFactory(client, 0) - ingressClassInformer := informerFactory.Networking().V1().IngressClasses() - ingressClassLister := ingressClassInformer.Lister() - - informerFactory.Start(ctx.Done()) - cache.WaitForCacheSync(ctx.Done(), ingressClassInformer.Informer().HasSynced) + ingressClassLister := getIngressClassLister(getIngressClassList()) ingressClassName := "ingress-class" - i := networkingv1.Ingress{ TypeMeta: metav1.TypeMeta{}, Spec: networkingv1.IngressSpec{ @@ -502,7 +486,34 @@ func TestGetIngressClass(t *testing.T) { i.Spec.IngressClassName = common.String("unknownIngress") ic, err = GetIngressClass(&i, ingressClassLister) - Expect(err).To(HaveOccurred()) + Expect(err).NotTo(HaveOccurred()) + Expect(ic).To(BeNil()) + + ingressClassLister = getIngressClassLister(&networkingv1.IngressClassList{Items: []networkingv1.IngressClass{}}) + + i.Spec.IngressClassName = nil + ic, err = GetIngressClass(&i, ingressClassLister) + Expect(err).ToNot(HaveOccurred()) + Expect(ic).To(BeNil()) +} + +func getIngressClassLister(ingressClassList *networkingv1.IngressClassList) networkingListers.IngressClassLister { + client := fakeclientset.NewSimpleClientset() + + client.NetworkingV1().(*fake2.FakeNetworkingV1). + PrependReactor("list", "ingressclasses", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + return true, ingressClassList, nil + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + informerFactory := informers.NewSharedInformerFactory(client, 0) + ingressClassInformer := informerFactory.Networking().V1().IngressClasses() + ingressClassLister := ingressClassInformer.Lister() + informerFactory.Start(ctx.Done()) + cache.WaitForCacheSync(ctx.Done(), ingressClassInformer.Informer().HasSynced) + return ingressClassLister } func TestGetHealthChecker(t *testing.T) {