replace iptree on the servicecidr controller

pkg/api/servicecidr/servicecidr.go

@@ -26,6 +26,46 @@ import (
 	networkinglisters "k8s.io/client-go/listers/networking/v1alpha1"
 )
 
+// OverlapsPrefix return the list of ServiceCIDR that overlaps with the prefix passed as argument
+func OverlapsPrefix(serviceCIDRLister networkinglisters.ServiceCIDRLister, prefix netip.Prefix) []*networkingv1alpha1.ServiceCIDR {
+	result := []*networkingv1alpha1.ServiceCIDR{}
+	serviceCIDRList, err := serviceCIDRLister.List(labels.Everything())
+	if err != nil {
+		return result
+	}
+
+	for _, serviceCIDR := range serviceCIDRList {
+		for _, cidr := range serviceCIDR.Spec.CIDRs {
+			if p, err := netip.ParsePrefix(cidr); err == nil { // it can not fail since is already validated
+				if p.Overlaps(prefix) {
+					result = append(result, serviceCIDR)
+				}
+			}
+		}
+	}
+	return result
+}
+
+// ContainsPrefix return the list of ServiceCIDR that contains the prefix passed as argument
+func ContainsPrefix(serviceCIDRLister networkinglisters.ServiceCIDRLister, prefix netip.Prefix) []*networkingv1alpha1.ServiceCIDR {
+	result := []*networkingv1alpha1.ServiceCIDR{}
+	serviceCIDRList, err := serviceCIDRLister.List(labels.Everything())
+	if err != nil {
+		return result
+	}
+
+	for _, serviceCIDR := range serviceCIDRList {
+		for _, cidr := range serviceCIDR.Spec.CIDRs {
+			if p, err := netip.ParsePrefix(cidr); err == nil { // it can not fail since is already validated
+				if p.Overlaps(prefix) && p.Bits() <= prefix.Bits() {
+					result = append(result, serviceCIDR)
+				}
+			}
+		}
+	}
+	return result
+}
+
 // ContainsIP return the list of ServiceCIDR that contains the IP address passed as argument
 func ContainsIP(serviceCIDRLister networkinglisters.ServiceCIDRLister, ip net.IP) []*networkingv1alpha1.ServiceCIDR {
 	address := IPToAddr(ip)

pkg/api/servicecidr/servicecidr_test.go

@@ -43,6 +43,372 @@ func newServiceCIDR(name, primary, secondary string) *networkingv1alpha1.Service
 	return serviceCIDR
 }
 
+func TestOverlapsPrefix(t *testing.T) {
+	tests := []struct {
+		name         string
+		serviceCIDRs []*networkingv1alpha1.ServiceCIDR
+		prefix       netip.Prefix
+		want         []string
+	}{
+		{
+			name: "only one ServiceCIDR and IPv4 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/26"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and same IPv4 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/24"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and larger IPv4 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/16"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and non contained IPv4 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("192.168.0.0/24"),
+			want:   []string{},
+		},
+		{
+			name: "only one ServiceCIDR and IPv6 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/112"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and same IPv6 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/96"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and IPv6 larger",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/64"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and IPv6 prefix out of range",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db2::/112"),
+			want:   []string{},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/24"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two overlapping ServiceCIDR and IPv4 prefix only contained in one",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/18"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 larger",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/8"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 prefix not contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("192.168.0.0/24"),
+			want:   []string{},
+		},
+		{
+			name: "two ServiceCIDR and IPv6 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/96"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and IPv6 prefix contained in one",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/72"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and aprefix larger",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/52"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and prefix out of range",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db2::/64"),
+			want:   []string{},
+		},
+		{
+			name: "multiple ServiceCIDR match with overlap contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("kubernetes2", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/28"),
+			want:   []string{"kubernetes", "kubernetes2", "secondary"},
+		},
+		{
+			name: "multiple ServiceCIDR match with overlap contains",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("kubernetes2", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/8"),
+			want:   []string{"kubernetes", "kubernetes2", "secondary"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
+			for _, serviceCIDR := range tt.serviceCIDRs {
+				err := indexer.Add(serviceCIDR)
+				if err != nil {
+					t.Fatalf("unexpected error %v", err)
+				}
+			}
+			lister := networkinglisters.NewServiceCIDRLister(indexer)
+			got := []string{}
+			for _, serviceCIDR := range OverlapsPrefix(lister, tt.prefix) {
+				got = append(got, serviceCIDR.Name)
+			}
+			// sort slices to make the order predictable and avoid flakiness
+			sort.Strings(got)
+			sort.Strings(tt.want)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("OverlapsAddress() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestContainsPrefix(t *testing.T) {
+	tests := []struct {
+		name         string
+		serviceCIDRs []*networkingv1alpha1.ServiceCIDR
+		prefix       netip.Prefix
+		want         []string
+	}{
+		{
+			name: "only one ServiceCIDR and IPv4 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/26"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and same IPv4 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/24"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and larger IPv4 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/16"),
+			want:   []string{},
+		},
+		{
+			name: "only one ServiceCIDR and non containerd IPv4 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("192.168.0.0/24"),
+			want:   []string{},
+		},
+		{
+			name: "only one ServiceCIDR and IPv6 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/112"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and same IPv6 prefix",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/96"),
+			want:   []string{"kubernetes"},
+		},
+		{
+			name: "only one ServiceCIDR and IPv6 larger",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/64"),
+			want:   []string{},
+		},
+		{
+			name: "only one ServiceCIDR and IPv6 prefix out of range",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+			},
+			prefix: netip.MustParsePrefix("2001:db2::/112"),
+			want:   []string{},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/24"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 prefix only contained in one",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/18"),
+			want:   []string{"secondary"},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 larger",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/8"),
+			want:   []string{},
+		},
+		{
+			name: "two ServiceCIDR and IPv4 prefix not contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("192.168.0.0/24"),
+			want:   []string{},
+		},
+		{
+			name: "two ServiceCIDR and IPv6 prefix contained",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/96"),
+			want:   []string{"kubernetes", "secondary"},
+		},
+		{
+			name: "two ServiceCIDR and IPv6 prefix contained in one",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/72"),
+			want:   []string{"secondary"},
+		},
+		{
+			name: "two ServiceCIDR and aprefix larger",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db8::/52"),
+			want:   []string{},
+		},
+		{
+			name: "two ServiceCIDR and prefix out of range",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("2001:db2::/64"),
+			want:   []string{},
+		},
+		{
+			name: "multiple ServiceCIDR match with overlap",
+			serviceCIDRs: []*networkingv1alpha1.ServiceCIDR{
+				newServiceCIDR("kubernetes", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("kubernetes2", "10.0.0.0/24", "2001:db8::/96"),
+				newServiceCIDR("secondary", "10.0.0.0/16", "2001:db8::/64"),
+			},
+			prefix: netip.MustParsePrefix("10.0.0.0/28"),
+			want:   []string{"kubernetes", "kubernetes2", "secondary"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc})
+			for _, serviceCIDR := range tt.serviceCIDRs {
+				err := indexer.Add(serviceCIDR)
+				if err != nil {
+					t.Fatalf("unexpected error %v", err)
+				}
+			}
+			lister := networkinglisters.NewServiceCIDRLister(indexer)
+			got := []string{}
+			for _, serviceCIDR := range ContainsPrefix(lister, tt.prefix) {
+				got = append(got, serviceCIDR.Name)
+			}
+			// sort slices to make the order predictable and avoid flakiness
+			sort.Strings(got)
+			sort.Strings(tt.want)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ContainsAddress() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
 func TestContainsAddress(t *testing.T) {
 	tests := []struct {
 		name         string

pkg/controller/servicecidrs/servicecidrs_controller.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"encoding/json"
 	"net/netip"
-	"sync"
 	"time"
 
 	v1 "k8s.io/api/core/v1"
@@ -43,8 +42,8 @@ import (
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog/v2"
+	"k8s.io/kubernetes/pkg/api/servicecidr"
 	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
-	"k8s.io/kubernetes/pkg/util/iptree"
 	netutils "k8s.io/utils/net"
 )
 
@@ -80,7 +79,6 @@ func NewController(
 			workqueue.DefaultTypedControllerRateLimiter[string](),
 			workqueue.TypedRateLimitingQueueConfig[string]{Name: "ipaddresses"},
 		),
-		tree:             iptree.New[sets.Set[string]](),
 		workerLoopPeriod: time.Second,
 	}
 
@@ -122,10 +120,6 @@ type Controller struct {
 
 	// workerLoopPeriod is the time between worker runs. The workers process the queue of service and ipRange changes.
 	workerLoopPeriod time.Duration
-
-	// tree store the ServiceCIDRs names associated to each
-	muTree sync.Mutex
-	tree   *iptree.Tree[sets.Set[string]]
 }
 
 // Run will not return until stopCh is closed.
@@ -213,24 +207,17 @@ func (c *Controller) deleteIPAddress(obj interface{}) {
 // this is required because adding or removing a CIDR will require to recompute the
 // state of each ServiceCIDR to check if can be unblocked on deletion.
 func (c *Controller) overlappingServiceCIDRs(serviceCIDR *networkingapiv1alpha1.ServiceCIDR) []string {
-	c.muTree.Lock()
+	result := sets.New[string]()
-	defer c.muTree.Unlock()
-
-	serviceCIDRs := sets.New[string]()
 	for _, cidr := range serviceCIDR.Spec.CIDRs {
 		if prefix, err := netip.ParsePrefix(cidr); err == nil { // if is empty err will not be nil
-			c.tree.WalkPath(prefix, func(k netip.Prefix, v sets.Set[string]) bool {
+			serviceCIDRs := servicecidr.OverlapsPrefix(c.serviceCIDRLister, prefix)
-				serviceCIDRs.Insert(v.UnsortedList()...)
+			for _, v := range serviceCIDRs {
-				return false
+				result.Insert(v.Name)
-			})
+			}
-			c.tree.WalkPrefix(prefix, func(k netip.Prefix, v sets.Set[string]) bool {
-				serviceCIDRs.Insert(v.UnsortedList()...)
-				return false
-			})
 		}
 	}
 
-	return serviceCIDRs.UnsortedList()
+	return result.UnsortedList()
 }
 
 // containingServiceCIDRs, given an IPAddress return the ServiceCIDRs that contains the IP,
@@ -249,16 +236,13 @@ func (c *Controller) containingServiceCIDRs(ip *networkingapiv1alpha1.IPAddress)
 		return []string{}
 	}
 
-	c.muTree.Lock()
+	result := sets.New[string]()
-	defer c.muTree.Unlock()
+	serviceCIDRs := servicecidr.ContainsAddress(c.serviceCIDRLister, address)
-	serviceCIDRs := []string{}
+	for _, v := range serviceCIDRs {
-	// walk the tree to get all the ServiceCIDRs that contain this IP address
+		result.Insert(v.Name)
-	prefixes := c.tree.GetHostIPPrefixMatches(address)
-	for _, v := range prefixes {
-		serviceCIDRs = append(serviceCIDRs, v.UnsortedList()...)
 	}
 
-	return serviceCIDRs
+	return result.UnsortedList()
 }
 
 func (c *Controller) worker(ctx context.Context) {
@@ -290,38 +274,6 @@ func (c *Controller) processNext(ctx context.Context) bool {
 	return true
 }
 
-// syncCIDRs rebuilds the radix tree based from the informers cache
-func (c *Controller) syncCIDRs() error {
-	serviceCIDRList, err := c.serviceCIDRLister.List(labels.Everything())
-	if err != nil {
-		return err
-	}
-
-	// track the names of the different ServiceCIDRs, there
-	// can be multiple ServiceCIDRs sharing the same prefixes
-	// and this is important to determine if a ServiceCIDR can
-	// be deleted.
-	tree := iptree.New[sets.Set[string]]()
-	for _, serviceCIDR := range serviceCIDRList {
-		for _, cidr := range serviceCIDR.Spec.CIDRs {
-			if prefix, err := netip.ParsePrefix(cidr); err == nil { // if is empty err will not be nil
-				// if the prefix already exist append the new ServiceCIDR name
-				v, ok := tree.GetPrefix(prefix)
-				if !ok {
-					v = sets.Set[string]{}
-				}
-				v.Insert(serviceCIDR.Name)
-				tree.InsertPrefix(prefix, v)
-			}
-		}
-	}
-
-	c.muTree.Lock()
-	defer c.muTree.Unlock()
-	c.tree = tree
-	return nil
-}
-
 func (c *Controller) sync(ctx context.Context, key string) error {
 	logger := klog.FromContext(ctx)
 	startTime := time.Now()
@@ -329,13 +281,6 @@ func (c *Controller) sync(ctx context.Context, key string) error {
 		logger.V(4).Info("Finished syncing ServiceCIDR)", "ServiceCIDR", key, "elapsed", time.Since(startTime))
 	}()
 
-	// TODO(aojea) verify if this present a performance problem
-	// restore the radix tree from the current state
-	err := c.syncCIDRs()
-	if err != nil {
-		return err
-	}
-
 	logger.V(4).Info("syncing ServiceCIDR", "ServiceCIDR", key)
 	cidr, err := c.serviceCIDRLister.Get(key)
 	if err != nil {
@@ -406,23 +351,16 @@ func (c *Controller) sync(ctx context.Context, key string) error {
 
 // canDeleteCIDR checks that the ServiceCIDR can be safely deleted and not leave orphan IPAddresses
 func (c *Controller) canDeleteCIDR(ctx context.Context, serviceCIDR *networkingapiv1alpha1.ServiceCIDR) (bool, error) {
-	// TODO(aojea) Revisit the lock usage and if we need to keep it only for the tree operations
-	// to avoid holding it during the whole operation.
-	c.muTree.Lock()
-	defer c.muTree.Unlock()
 	logger := klog.FromContext(ctx)
 	// Check if there is a subnet that already contains the ServiceCIDR that is going to be deleted.
 	hasParent := true
 	for _, cidr := range serviceCIDR.Spec.CIDRs {
-		// Walk the tree to find if there is a larger subnet that contains the existing one,
+		// Find if there is a larger subnet that contains the existing one,
 		// or there is another ServiceCIDR with the same subnet.
 		if prefix, err := netip.ParsePrefix(cidr); err == nil {
-			serviceCIDRs := sets.New[string]()
+			serviceCIDRs := servicecidr.ContainsPrefix(c.serviceCIDRLister, prefix)
-			c.tree.WalkPath(prefix, func(k netip.Prefix, v sets.Set[string]) bool {
+			if len(serviceCIDRs) == 0 ||
-				serviceCIDRs.Insert(v.UnsortedList()...)
+				len(serviceCIDRs) == 1 && serviceCIDRs[0].Name == serviceCIDR.Name {
-				return false
-			})
-			if serviceCIDRs.Len() == 1 && serviceCIDRs.Has(serviceCIDR.Name) {
 				hasParent = false
 			}
 		}
@@ -431,7 +369,7 @@ func (c *Controller) canDeleteCIDR(ctx context.Context, serviceCIDR *networkinga
 	// All the existing IP addresses will be contained on the parent ServiceCIDRs,
 	// it is safe to delete, remove the finalizer.
 	if hasParent {
-		logger.V(2).Info("Removing finalizer for ServiceCIDR", "ServiceCIDR", serviceCIDR.String())
+		logger.Info("Deleting ServiceCIDR contained in other ServiceCIDR", "ServiceCIDR", serviceCIDR.String())
 		return true, nil
 	}
 
@@ -458,22 +396,18 @@ func (c *Controller) canDeleteCIDR(ctx context.Context, serviceCIDR *networkinga
 				logger.Info("[SHOULD NOT HAPPEN] unexpected error parsing IPAddress", "IPAddress", ip.Name, "error", err)
 				continue
 			}
-			// walk the tree to find all ServiceCIDRs containing this IP
+			// find all ServiceCIDRs containing this IP
-			prefixes := c.tree.GetHostIPPrefixMatches(address)
+			serviceCIDRs := servicecidr.ContainsAddress(c.serviceCIDRLister, address)
-			if len(prefixes) != 1 {
+			if len(serviceCIDRs) == 1 && serviceCIDRs[0].Name == serviceCIDR.Name {
-				continue
+				logger.Info("Deleting ServiceCIDR blocked by IP address", "IPAddress", address.String())
-			}
+				return false, nil
-			for _, v := range prefixes {
-				if v.Len() == 1 && v.Has(serviceCIDR.Name) {
-					return false, nil
-				}
 			}
 		}
 	}
 
 	// There are no IPAddresses that depend on the existing ServiceCIDR, so
 	// it is safe to delete, remove finalizer.
-	logger.Info("ServiceCIDR no longer have orphan IPs", "ServiceCDIR", serviceCIDR.String())
+	logger.Info("Deleting ServiceCIDR no longer have orphan IPs", "ServiceCIDR", serviceCIDR.String())
 	return true, nil
 }
 

pkg/controller/servicecidrs/servicecidrs_controller_test.go

@@ -428,11 +428,6 @@ func TestController_canDeleteCIDR(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			tCtx := ktesting.Init(t)
 			_, controller := newController(tCtx, t, tc.cidrs, tc.ips)
-			err := controller.syncCIDRs()
-			if err != nil {
-				t.Fatal(err)
-			}
-
 			got, err := controller.canDeleteCIDR(tCtx, tc.cidrSynced)
 			if err != nil {
 				t.Fatal(err)
@@ -534,10 +529,6 @@ func TestController_ipToCidrs(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			tCtx := ktesting.Init(t)
 			_, controller := newController(tCtx, t, tt.cidrs, nil)
-			err := controller.syncCIDRs()
-			if err != nil {
-				t.Fatal(err)
-			}
 			if got := controller.containingServiceCIDRs(tt.ip); !cmp.Equal(got, tt.want, cmpopts.SortSlices(func(a, b string) bool { return a < b })) {
 				t.Errorf("Controller.ipToCidrs() = %v, want %v", got, tt.want)
 			}
@@ -591,10 +582,6 @@ func TestController_cidrToCidrs(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			tCtx := ktesting.Init(t)
 			_, controller := newController(tCtx, t, tt.cidrs, nil)
-			err := controller.syncCIDRs()
-			if err != nil {
-				t.Fatal(err)
-			}
 			if got := controller.overlappingServiceCIDRs(tt.cidr); !cmp.Equal(got, tt.want, cmpopts.SortSlices(func(a, b string) bool { return a < b })) {
 				t.Errorf("Controller.cidrToCidrs() = %v, want %v", got, tt.want)
 			}