Skip to content

Commit

Permalink
fix finalizer removal logic for ingresses
Browse files Browse the repository at this point in the history
  • Loading branch information
piyush-tiwari committed Jul 24, 2024
1 parent 6ed209e commit 4dbd95e
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 21 deletions.
14 changes: 13 additions & 1 deletion pkg/controllers/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,24 @@ func (c *Controller) sync(key string) error {

ingressClass, err := util.GetIngressClass(ingress, c.ingressClassLister)
if err != nil {
deleteFinalizerErr := c.deleteFinalizer(ingress)
if deleteFinalizerErr != nil {
return fmt.Errorf("got error %w while getting IngressClass for Ingress %s/%s. Unable to delete finalizer "+
"%s due to error: %s", err, ingress.Namespace, ingress.Name, util.IngressControllerFinalizer, deleteFinalizerErr.Error())
}

return err
}

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
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,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
}

Expand All @@ -345,7 +346,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) {
Expand Down
49 changes: 30 additions & 19 deletions pkg/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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{
Expand All @@ -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) {
Expand Down

0 comments on commit 4dbd95e

Please sign in to comment.