e2e: Use ovnk allocator and reserve IPs

This change replace the customa llocator with the ovnk allocator and
also reserver the cluster IPs so test can just ask for another IP
without the problem of cluster ip collision.

Signed-off-by: Enrique Llorente <[email protected]>

Author: qinqon

test/e2e/ipalloc/ipalloc.go

@@ -3,19 +3,20 @@ package ipalloc
 import (
 	"context"
 	"fmt"
+	"net"
+	"sync"
+
+	ipallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/client-go/kubernetes/typed/core/v1"
-	"net"
-	"sync"
 )
 
 // primaryIPAllocator attempts to allocate an IP in the same subnet as a nodes primary network
 type primaryIPAllocator struct {
 	mu         *sync.Mutex
-	v4         *ipAllocator
-	v6         *ipAllocator
+	v4         *ipallocator.Range
+	v6         *ipallocator.Range
 	nodeClient v1.NodeInterface
 }
 
@@ -47,91 +48,37 @@ func newPrimaryIPAllocator(nodeClient v1.NodeInterface) (*primaryIPAllocator, er
 	if len(nodes.Items) == 0 {
 		return ipa, fmt.Errorf("expected at least one node but found zero")
 	}
-	// FIXME: the approach taken here to find the first node IP+mask and then to increment the second last octet wont work in
-	// all scenarios (node with /24). We should generate an EgressIP compatible with a Node providers primary network and then take care its unique globally.
 
-	// The approach here is to grab initial starting IP from first node found, increment the second last octet.
-	// Approach taken here won't work for Nodes handed /24 subnets.
-	nodePrimaryIPs, err := util.ParseNodePrimaryIfAddr(&nodes.Items[0])
-	if err != nil {
-		return ipa, fmt.Errorf("failed to parse node primary interface address from Node object: %v", err)
-	}
-	if nodePrimaryIPs.V4.IP != nil {
-		// should be ok with /16 and /64 node primary provider subnets
-		// TODO; fixme; what about /24 subnet Nodes like GCP
-		nodePrimaryIPs.V4.IP[len(nodePrimaryIPs.V4.IP)-2]++
-		ipa.v4 = newIPAllocator(&net.IPNet{IP: nodePrimaryIPs.V4.IP, Mask: nodePrimaryIPs.V4.Net.Mask})
-	}
-	if nodePrimaryIPs.V6.IP != nil {
-		nodePrimaryIPs.V6.IP[len(nodePrimaryIPs.V6.IP)-2]++
-		ipa.v6 = newIPAllocator(&net.IPNet{IP: nodePrimaryIPs.V6.IP, Mask: nodePrimaryIPs.V6.Net.Mask})
-	}
-	// verify the new starting base IP is within all Nodes subnets
-	if nodePrimaryIPs.V4.IP != nil {
-		ipNets, err := getNodePrimaryProviderIPs(nodes.Items, false)
-		if err != nil {
-			return ipa, err
-		}
-		nextIP, err := ipa.v4.AllocateNextIP()
-		if err != nil {
-			return ipa, err
-		}
-		if !isIPWithinAllSubnets(ipNets, nextIP) {
-			return ipa, fmt.Errorf("IP %s is not within all Node subnets", nextIP)
-		}
-	}
-	if nodePrimaryIPs.V6.IP != nil {
-		ipNets, err := getNodePrimaryProviderIPs(nodes.Items, true)
-		if err != nil {
-			return ipa, err
-		}
-		nextIP, err := ipa.v6.AllocateNextIP()
-		if err != nil {
-			return ipa, err
-		}
-		if !isIPWithinAllSubnets(ipNets, nextIP) {
-			return ipa, fmt.Errorf("IP %s is not within all Node subnets", nextIP)
-		}
-	}
-
-	return ipa, nil
-}
-
-func getNodePrimaryProviderIPs(nodes []corev1.Node, isIPv6 bool) ([]*net.IPNet, error) {
-	ipNets := make([]*net.IPNet, 0, len(nodes))
-	for _, node := range nodes {
+	for _, node := range nodes.Items {
 		nodePrimaryIPs, err := util.ParseNodePrimaryIfAddr(&node)
 		if err != nil {
-			return nil, fmt.Errorf("failed to parse node primary interface address from Node %s object: %v", node.Name, err)
+			return ipa, fmt.Errorf("failed to parse node primary interface address from Node %s object: %v", node.Name, err)
+		}
+		if nodePrimaryIPs.V4.IP != nil {
+			if ipa.v4 == nil {
+				ipa.v4, err = ipallocator.NewCIDRRange(nodePrimaryIPs.V4.Net)
+				if err != nil {
+					return ipa, fmt.Errorf("failed to create new CIDR range for IPv4: %v", err)
+				}
+			}
+			if err := ipa.v4.Allocate(nodePrimaryIPs.V4.IP); err != nil {
+				return ipa, fmt.Errorf("failed to allocate IPv4 %s: %v", nodePrimaryIPs.V4.IP, err)
+			}
+		}
+		if nodePrimaryIPs.V6.IP != nil {
+			if ipa.v6 == nil {
+				ipa.v6, err = ipallocator.NewCIDRRange(nodePrimaryIPs.V6.Net)
+				if err != nil {
+					return ipa, fmt.Errorf("failed to create new CIDR range for IPv6: %v", err)
+				}
+			}
+			if err := ipa.v6.Allocate(nodePrimaryIPs.V6.IP); err != nil {
+				return ipa, fmt.Errorf("failed to allocate IPv6 %s: %v", nodePrimaryIPs.V6.IP, err)
+			}
 		}
-		var mask net.IPMask
-		var ip net.IP
 
-		if isIPv6 {
-			ip = nodePrimaryIPs.V6.IP
-			mask = nodePrimaryIPs.V6.Net.Mask
-		} else {
-			ip = nodePrimaryIPs.V4.IP
-			mask = nodePrimaryIPs.V4.Net.Mask
-		}
-		if len(ip) == 0 || len(mask) == 0 {
-			return nil, fmt.Errorf("failed to find Node %s primary Node IP and/or mask", node.Name)
-		}
-		ipNets = append(ipNets, &net.IPNet{IP: ip, Mask: mask})
 	}
-	return ipNets, nil
-}
-
-func isIPWithinAllSubnets(ipNets []*net.IPNet, ip net.IP) bool {
-	if len(ipNets) == 0 {
-		return false
-	}
-	for _, ipNet := range ipNets {
-		if !ipNet.Contains(ip) {
-			return false
-		}
-	}
-	return true
+	return ipa, nil
 }
 
 func (pia *primaryIPAllocator) IncrementAndGetNextV4(times int) (net.IP, error) {
@@ -148,12 +95,9 @@ func (pia *primaryIPAllocator) AllocateNextV4() (net.IP, error) {
 	if pia.v4 == nil {
 		return nil, fmt.Errorf("IPv4 is not enable ")
 	}
-	if pia.v4.net == nil {
-		return nil, fmt.Errorf("IPv4 is not enabled but Allocation request was called")
-	}
 	pia.mu.Lock()
 	defer pia.mu.Unlock()
-	return allocateIP(pia.nodeClient, pia.v4.AllocateNextIP)
+	return pia.v4.AllocateNext()
 }
 
 func (pia *primaryIPAllocator) IncrementAndGetNextV6(times int) (net.IP, error) {
@@ -170,51 +114,7 @@ func (pia primaryIPAllocator) AllocateNextV6() (net.IP, error) {
 	if pia.v6 == nil {
 		return nil, fmt.Errorf("IPv6 is not enabled but Allocation request was called")
 	}
-	if pia.v6.net == nil {
-		return nil, fmt.Errorf("ipv6 network is not set")
-	}
 	pia.mu.Lock()
 	defer pia.mu.Unlock()
-	return allocateIP(pia.nodeClient, pia.v6.AllocateNextIP)
-}
-
-type allocNextFn func() (net.IP, error)
-
-func allocateIP(nodeClient v1.NodeInterface, allocateFn allocNextFn) (net.IP, error) {
-	nodeList, err := nodeClient.List(context.TODO(), metav1.ListOptions{})
-	if err != nil {
-		return nil, fmt.Errorf("failed to list nodes: %v", err)
-	}
-	for {
-		nextIP, err := allocateFn()
-		if err != nil {
-			return nil, fmt.Errorf("failed to allocated next IP address: %v", err)
-		}
-		firstOctet := nextIP[len(nextIP)-1]
-		// skip 0 and 1
-		if firstOctet == 0 || firstOctet == 1 {
-			continue
-		}
-		isConflict, err := isConflictWithExistingHostIPs(nodeList.Items, nextIP)
-		if err != nil {
-			return nil, fmt.Errorf("failed to determine if IP conflicts with existing IPs: %v", err)
-		}
-		if !isConflict {
-			return nextIP, nil
-		}
-	}
-}
-
-func isConflictWithExistingHostIPs(nodes []corev1.Node, ip net.IP) (bool, error) {
-	ipStr := ip.String()
-	for _, node := range nodes {
-		nodeIPsSet, err := util.ParseNodeHostCIDRsDropNetMask(&node)
-		if err != nil {
-			return false, fmt.Errorf("failed to parse node %s primary annotation info: %v", node.Name, err)
-		}
-		if nodeIPsSet.Has(ipStr) {
-			return true, nil
-		}
-	}
-	return false, nil
+	return pia.v6.AllocateNext()
 }

test/e2e/ipalloc/primaryipalloc.go

@@ -15,48 +15,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
-	utilsnet "k8s.io/utils/net"
 )
 
 func TestUtilSuite(t *testing.T) {
 	gomega.RegisterFailHandler(ginkgo.Fail)
 	ginkgo.RunSpecs(t, "node ip alloc suite")
 }
 
-func TestAllocateNext(t *testing.T) {
-	tests := []struct {
-		desc   string
-		input  *net.IPNet
-		output []net.IP
-	}{
-		{
-			desc:   "increments IPv4 address",
-			input:  mustParseCIDRIncIP("192.168.1.5/16"), // mask /24 would fail
-			output: []net.IP{net.ParseIP("192.168.1.6"), net.ParseIP("192.168.1.7"), net.ParseIP("192.168.1.8")},
-		},
-		{
-			desc:   "increments IPv6 address",
-			input:  mustParseCIDRIncIP("fc00:f853:ccd:e793::6/64"),
-			output: []net.IP{net.ParseIP("fc00:f853:ccd:e793::7"), net.ParseIP("fc00:f853:ccd:e793::8"), net.ParseIP("fc00:f853:ccd:e793::9")},
-		},
-	}
-
-	for i, tc := range tests {
-		t.Run(fmt.Sprintf("%d:%s", i, tc.desc), func(t *testing.T) {
-			nodeIPAlloc := newIPAllocator(tc.input)
-			for _, expectedIP := range tc.output {
-				allocatedIP, err := nodeIPAlloc.AllocateNextIP()
-				if err != nil {
-					t.Errorf("failed to allocated next IP: %v", err)
-				}
-				if !allocatedIP.Equal(expectedIP) {
-					t.Errorf("Expected IP %q, but got %q", expectedIP.String(), allocatedIP.String())
-				}
-			}
-		})
-	}
-}
-
 // mustParseCIDRIncIP parses the IP and CIDR. It adds the IP to the returned IPNet.
 func mustParseCIDRIncIP(cidr string) *net.IPNet {
 	ip, ipNet, err := net.ParseCIDR(cidr)
@@ -78,20 +43,19 @@ type node struct {
 }
 
 func TestIPAlloc(t *testing.T) {
+	g := gomega.NewWithT(t)
+
 	tests := []struct {
-		desc                     string
-		existingPrimaryNodeIPs   []node
-		expectedFromAllocateNext []string
+		desc                   string
+		existingPrimaryNodeIPs []node
 	}{
 		{
-			desc:                     "IPv4",
-			existingPrimaryNodeIPs:   []node{{v4: network{ip: "192.168.1.1", mask: "16"}}, {v4: network{ip: "192.168.1.2", mask: "16"}}},
-			expectedFromAllocateNext: []string{"192.168.2.3", "192.168.2.4"},
+			desc:                   "IPv4",
+			existingPrimaryNodeIPs: []node{{v4: network{ip: "192.168.1.1", mask: "16"}}, {v4: network{ip: "192.168.1.2", mask: "16"}}},
 		},
 		{
-			desc:                     "IPv6",
-			existingPrimaryNodeIPs:   []node{{v4: network{ip: "fc00:f853:ccd:e793::5", mask: "64"}}, {v4: network{ip: "fc00:f853:ccd:e793::6", mask: "64"}}},
-			expectedFromAllocateNext: []string{"fc00:f853:ccd:e793::8", "fc00:f853:ccd:e793::9"},
+			desc:                   "IPv6",
+			existingPrimaryNodeIPs: []node{{v6: network{ip: "fc00:f853:ccd:e793::5", mask: "64"}}, {v6: network{ip: "fc00:f853:ccd:e793::6", mask: "64"}}},
 		},
 	}
 
@@ -103,23 +67,33 @@ func TestIPAlloc(t *testing.T) {
 				t.Errorf(err.Error())
 				return
 			}
-			for _, expectedIPStr := range tc.expectedFromAllocateNext {
-				expectedIP := net.ParseIP(expectedIPStr)
-				var nextIP net.IP
-				var err error
-				if utilsnet.IsIPv6(expectedIP) {
-					nextIP, err = pipa.AllocateNextV6()
-				} else {
-					nextIP, err = pipa.AllocateNextV4()
-				}
-				if err != nil || nextIP == nil {
-					t.Errorf("failed to allocated next IPv4 or IPv6 address. err %v", err)
-					return
+			existingIPv4IPs := []string{}
+			existingIPv6IPs := []string{}
+			allocatedIPv4IPs := []string{}
+			allocatedIPv6IPs := []string{}
+			for _, existingPrimaryNodeIP := range tc.existingPrimaryNodeIPs {
+				if existingPrimaryNodeIP.v4.ip != "" {
+					existingIPv4IPs = append(existingIPv4IPs, existingPrimaryNodeIP.v4.ip)
+					nextIPv4, err := pipa.AllocateNextV4()
+					g.Expect(err).ToNot(gomega.HaveOccurred(), "should succeed in allocating the next IPv4 address")
+					g.Expect(nextIPv4).ToNot(gomega.BeNil(), "should allocate next IPv4 address")
+					allocatedIPv4IPs = append(allocatedIPv4IPs, nextIPv4.String())
 				}
-				if !nextIP.Equal(expectedIP) {
-					t.Errorf("expected IP %q, but found %q", expectedIP, nextIP)
+
+				if existingPrimaryNodeIP.v6.ip != "" {
+					existingIPv6IPs = append(existingIPv6IPs, existingPrimaryNodeIP.v6.ip)
+					nextIPv6, err := pipa.AllocateNextV6()
+					g.Expect(err).ToNot(gomega.HaveOccurred(), "should succeed in allocating the next IPv6 address")
+					g.Expect(nextIPv6).ToNot(gomega.BeNil(), "should allocate next IPv6 address")
+					allocatedIPv6IPs = append(allocatedIPv6IPs, nextIPv6.String())
 				}
 			}
+			if len(existingIPv4IPs) > 0 {
+				g.Expect(allocatedIPv4IPs).NotTo(gomega.ContainElements(existingIPv4IPs))
+			}
+			if len(existingIPv6IPs) > 0 {
+				g.Expect(allocatedIPv6IPs).NotTo(gomega.ContainElements(existingIPv6IPs))
+			}
 		})
 	}