Merge pull request #355 from andreaskaris/network-name-overlappingipr…

…eservations

 Add overlapping ranges check to network_name feature

README.md

@@ -234,6 +234,24 @@ The overlapping ranges feature is enabled by default, and will not allow an IP a
 
 Please note: This feature is only implemented for the Kubernetes storage backend.
 
+### Network names
+
+By default, it is not possible to configure the same CIDR range twice and have whereabouts assign from the ranges
+independently. However, this is useful in multi-tenant situations where more than one group is responsible for
+selecting CIDR ranges.
+
+By using parameter `network_name` *(string)*, administrators can tell whereabouts to assign IP addresses for the same
+CIDR range multiple times.
+
+Parameter `enable_overlapping_ranges` (see above) is scoped per network name.
+
+```
+(...)
+    "network_name": "network-with-independent-allocation",
+    "enable_overlapping_ranges": true,
+(...)
+```
+
 ## Building
 
 Run the build command from the `./hack` directory:

e2e/e2e_test.go

@@ -43,11 +43,12 @@ func TestWhereaboutsE2E(t *testing.T) {
 var _ = Describe("Whereabouts functionality", func() {
 	Context("Test setup", func() {
 		const (
-			testNamespace   = "default"
-			ipv4TestRange   = "10.10.0.0/16"
-			testNetworkName = "wa-nad"
-			rsName          = "whereabouts-scale-test"
-			ipPoolCIDR      = "10.10.0.0/16"
+			testNamespace            = "default"
+			ipv4TestRange            = "10.10.0.0/16"
+			ipv4TestRangeOverlapping = "10.10.0.0/17"
+			testNetworkName          = "wa-nad"
+			rsName                   = "whereabouts-scale-test"
+			ipPoolCIDR               = "10.10.0.0/16"
 		)
 
 		var (
@@ -482,35 +483,130 @@ var _ = Describe("Whereabouts functionality", func() {
 			})
 		})
 
+		Context("OverlappingRangeIPReservation", func() {
+			const (
+				testNetwork2Name = "wa-nad-2"
+			)
+			var (
+				netAttachDef2 *nettypes.NetworkAttachmentDefinition
+				pod2          *core.Pod
+			)
+
+			for _, enableOverlappingRanges := range []bool{true, false} {
+				When(fmt.Sprintf("a second net-attach-definition with \"enable_overlapping_ranges\": %t is created",
+					enableOverlappingRanges), func() {
+					BeforeEach(func() {
+						netAttachDef2 = macvlanNetworkWithWhereaboutsIPAMNetwork(testNetwork2Name, testNamespace,
+							ipv4TestRangeOverlapping, []string{}, "", false)
+
+						By("creating a second NetworkAttachmentDefinition for whereabouts")
+						_, err := clientInfo.AddNetAttachDef(netAttachDef2)
+						Expect(err).NotTo(HaveOccurred())
+					})
+
+					AfterEach(func() {
+						Expect(clientInfo.DelNetAttachDef(netAttachDef2)).To(Succeed())
+					})
+
+					BeforeEach(func() {
+						const (
+							singlePodName  = "whereabouts-basic-test"
+							singlePod2Name = "whereabouts-basic-test-2"
+						)
+						var err error
+
+						By("creating a pod with whereabouts net-attach-def")
+						pod, err = clientInfo.ProvisionPod(
+							singlePodName,
+							testNamespace,
+							podTierLabel(singlePodName),
+							entities.PodNetworkSelectionElements(testNetworkName),
+						)
+						Expect(err).NotTo(HaveOccurred())
+
+						By("creating a second pod with the second whereabouts net-attach-def")
+						pod2, err = clientInfo.ProvisionPod(
+							singlePod2Name,
+							testNamespace,
+							podTierLabel(singlePodName),
+							entities.PodNetworkSelectionElements(testNetwork2Name),
+						)
+						Expect(err).NotTo(HaveOccurred())
+
+					})
+
+					AfterEach(func() {
+						By("deleting pod with whereabouts net-attach-def")
+						Expect(clientInfo.DeletePod(pod)).To(Succeed())
+						By("deleting the second pod with whereabouts net-attach-def")
+						Expect(clientInfo.DeletePod(pod2)).To(Succeed())
+					})
+
+					It("allocates the correct IP address to the second pod", func() {
+						By("checking pod IP is within whereabouts IPAM range")
+						secondaryIfaceIPs, err := retrievers.SecondaryIfaceIPValue(pod)
+						Expect(err).NotTo(HaveOccurred())
+						Expect(secondaryIfaceIPs).NotTo(BeEmpty())
+
+						By("checking pod 2 IP is within whereabouts IPAM range")
+						secondaryIfaceIPs2, err := retrievers.SecondaryIfaceIPValue(pod2)
+						Expect(err).NotTo(HaveOccurred())
+						Expect(secondaryIfaceIPs2).NotTo(BeEmpty())
+
+						if enableOverlappingRanges {
+							By("checking pod 2 IP is different from pod 1 IP")
+							Expect(secondaryIfaceIPs[0]).NotTo(Equal(secondaryIfaceIPs2[0]))
+						} else {
+							By("checking pod 2 IP equals pod 1 IP")
+							Expect(secondaryIfaceIPs[0]).To(Equal(secondaryIfaceIPs2[0]))
+						}
+					})
+				})
+			}
+		})
+
 		Context("Named ranges test", func() {
 			const (
+				namedNetworkName = "named-range"
 				testNetwork2Name = "wa-nad-2"
+				testNetwork3Name = "wa-nad-3"
 			)
 			var (
 				netAttachDef2 *nettypes.NetworkAttachmentDefinition
+				netAttachDef3 *nettypes.NetworkAttachmentDefinition
 				pod2          *core.Pod
+				pod3          *core.Pod
 			)
 
 			BeforeEach(func() {
 				var (
 					err error
 				)
 
-				netAttachDef2 = macvlanNetworkWithWhereaboutsIPAMNetwork(testNetwork2Name, testNamespace, ipv4TestRange, []string{}, testNetwork2Name, false)
+				netAttachDef2 = macvlanNetworkWithWhereaboutsIPAMNetwork(testNetwork2Name, testNamespace,
+					ipv4TestRange, []string{}, namedNetworkName, true)
+				netAttachDef3 = macvlanNetworkWithWhereaboutsIPAMNetwork(testNetwork3Name, testNamespace,
+					ipv4TestRangeOverlapping, []string{}, namedNetworkName, true)
 
 				By("creating a second NetworkAttachmentDefinition for whereabouts")
 				_, err = clientInfo.AddNetAttachDef(netAttachDef2)
 				Expect(err).NotTo(HaveOccurred())
+
+				By("creating a third NetworkAttachmentDefinition for whereabouts")
+				_, err = clientInfo.AddNetAttachDef(netAttachDef3)
+				Expect(err).NotTo(HaveOccurred())
 			})
 
 			AfterEach(func() {
 				Expect(clientInfo.DelNetAttachDef(netAttachDef2)).To(Succeed())
+				Expect(clientInfo.DelNetAttachDef(netAttachDef3)).To(Succeed())
 			})
 
 			BeforeEach(func() {
 				const (
 					singlePodName  = "whereabouts-basic-test"
 					singlePod2Name = "whereabouts-basic-test-2"
+					singlePod3Name = "whereabouts-basic-test-3"
 				)
 				var err error
 
@@ -531,27 +627,43 @@ var _ = Describe("Whereabouts functionality", func() {
 					entities.PodNetworkSelectionElements(testNetwork2Name),
 				)
 				Expect(err).NotTo(HaveOccurred())
+
+				By("creating a third pod with the third whereabouts net-attach-def")
+				pod3, err = clientInfo.ProvisionPod(
+					singlePod3Name,
+					testNamespace,
+					podTierLabel(singlePodName),
+					entities.PodNetworkSelectionElements(testNetwork3Name),
+				)
+				Expect(err).NotTo(HaveOccurred())
 			})
 
 			AfterEach(func() {
 				By("deleting pod with whereabouts net-attach-def")
 				Expect(clientInfo.DeletePod(pod)).To(Succeed())
 				By("deleting the second pod with whereabouts net-attach-def")
 				Expect(clientInfo.DeletePod(pod2)).To(Succeed())
+				By("deleting the third pod with whereabouts net-attach-def")
+				Expect(clientInfo.DeletePod(pod3)).To(Succeed())
 			})
 
-			It("allocates the same IP to the Pods as they are in differenct address collision domains", func() {
+			It("allocates the same IP to the Pods as they are in different address collision domains", func() {
 				By("checking pod IP is within whereabouts IPAM range")
 				secondaryIfaceIPs, err := retrievers.SecondaryIfaceIPValue(pod)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(secondaryIfaceIPs).NotTo(BeEmpty())
 
-				By("checking pod 2 IP is within whereabouts IPAM range")
+				By("checking pod 2 IP is within whereabouts IPAM range and has the same IP as pod 1")
 				secondaryIfaceIPs2, err := retrievers.SecondaryIfaceIPValue(pod2)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(secondaryIfaceIPs2).NotTo(BeEmpty())
-
 				Expect(secondaryIfaceIPs[0]).To(Equal(secondaryIfaceIPs2[0]))
+
+				By("checking pod 3 IP is within whereabouts IPAM range and has a different IP from pod 2")
+				secondaryIfaceIPs3, err := retrievers.SecondaryIfaceIPValue(pod3)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(secondaryIfaceIPs3).NotTo(BeEmpty())
+				Expect(secondaryIfaceIPs2[0]).NotTo(Equal(secondaryIfaceIPs3[0]))
 			})
 		})
 	})

pkg/storage/kubernetes/ipam.go

@@ -197,20 +197,16 @@ func (i *KubernetesIPAM) GetOverlappingRangeStore() (storage.OverlappingRangeSto
 	return &KubernetesOverlappingRangeStore{i.client, i.containerID, i.namespace}, nil
 }
 
-// IsAllocatedInOverlappingRange checks for IP addresses to see if they're allocated cluster wide, for overlapping ranges
-func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP) (bool, error) {
+// IsAllocatedInOverlappingRange checks for IP addresses to see if they're allocated cluster wide, for overlapping
+// ranges.
+func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP,
+	networkName string) (bool, error) {
+	normalizedIP := normalizeIP(ip, networkName)
 
-	// IPv6 doesn't make for valid CR names, so normalize it.
-	ipStr := fmt.Sprint(ip)
-	if ipStr[len(ipStr)-1] == ':' {
-		ipStr += "0"
-		logging.Debugf("modified: %s", ipStr)
-	}
-	normalizedip := strings.ReplaceAll(ipStr, ":", "-")
+	logging.Debugf("OverlappingRangewide allocation check; normalized IP: %q, IP: %q, networkName: %q",
+		normalizedIP, ip, networkName)
 
-	logging.Debugf("OverlappingRangewide allocation check for IP: %v", normalizedip)
-
-	_, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedip, metav1.GetOptions{})
+	_, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
 	if err != nil && errors.IsNotFound(err) {
 		// cluster ip reservation does not exist, this appears to be good news.
 		// logging.Debugf("IP %v is not reserved cluster wide, allowing.", ip)
@@ -220,22 +216,18 @@ func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx cont
 		return false, fmt.Errorf("k8s get OverlappingRangeIPReservation error: %s", err)
 	}
 
-	logging.Debugf("IP %v is reserved cluster wide.", ip)
+	logging.Debugf("Normalized IP is reserved; normalized IP: %q, IP: %q, networkName: %q",
+		normalizedIP, ip, networkName)
 	return true, nil
 }
 
 // UpdateOverlappingRangeAllocation updates clusterwide allocation for overlapping ranges.
-func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef string) error {
-	// Normalize the IP
-	ipStr := fmt.Sprint(ip)
-	if ipStr[len(ipStr)-1] == ':' {
-		ipStr += "0"
-		logging.Debugf("modified: %s", ipStr)
-	}
-	normalizedip := strings.ReplaceAll(ipStr, ":", "-")
+func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP,
+	containerID, podRef, networkName string) error {
+	normalizedIP := normalizeIP(ip, networkName)
 
 	clusteripres := &whereaboutsv1alpha1.OverlappingRangeIPReservation{
-		ObjectMeta: metav1.ObjectMeta{Name: normalizedip, Namespace: c.namespace},
+		ObjectMeta: metav1.ObjectMeta{Name: normalizedIP, Namespace: c.namespace},
 	}
 
 	var err error
@@ -266,6 +258,21 @@ func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx c
 	return nil
 }
 
+// normalizeIP normalizes the IP. This is important for IPv6 which doesn't make for valid CR names. It also allows us
+// to add the network-name when it's different from the unnamed network.
+func normalizeIP(ip net.IP, networkName string) string {
+	ipStr := fmt.Sprint(ip)
+	if ipStr[len(ipStr)-1] == ':' {
+		ipStr += "0"
+		logging.Debugf("modified: %s", ipStr)
+	}
+	normalizedIP := strings.ReplaceAll(ipStr, ":", "-")
+	if networkName != UnnamedNetwork {
+		normalizedIP = fmt.Sprintf("%s-%s", networkName, normalizedIP)
+	}
+	return normalizedIP
+}
+
 // KubernetesIPPool represents an IPPool resource and its parsed set of allocations
 type KubernetesIPPool struct {
 	client      wbclient.Interface
@@ -517,13 +524,14 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
 				// When it's allocated overlappingrange wide, we add it to a local reserved list
 				// And we try again.
 				if ipamConf.OverlappingRanges {
-					isallocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP)
+					isAllocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP,
+						ipamConf.NetworkName)
 					if err != nil {
 						logging.Errorf("Error checking overlappingrange allocation: %v", err)
 						return newips, err
 					}
 
-					if isallocated {
+					if isAllocated {
 						logging.Debugf("Continuing loop, IP is already allocated (possibly from another range): %v", newip)
 						// We create "dummy" records here for evaluation, but, we need to filter those out later.
 						overlappingrangeallocations = append(overlappingrangeallocations, whereaboutstypes.IPReservation{IP: newip.IP, IsAllocated: true})
@@ -566,7 +574,8 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
 		}
 
 		if ipamConf.OverlappingRanges {
-			err = overlappingrangestore.UpdateOverlappingRangeAllocation(requestCtx, mode, ipforoverlappingrangeupdate, containerID, podRef)
+			err = overlappingrangestore.UpdateOverlappingRangeAllocation(requestCtx, mode, ipforoverlappingrangeupdate,
+				containerID, podRef, ipamConf.NetworkName)
 			if err != nil {
 				logging.Errorf("Error performing UpdateOverlappingRangeAllocation: %v", err)
 				return newips, err

pkg/storage/storage.go

@@ -32,8 +32,9 @@ type Store interface {
 
 // OverlappingRangeStore is an interface for wrapping overlappingrange storage options
 type OverlappingRangeStore interface {
-	IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP) (bool, error)
-	UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef string) error
+	IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string) (bool, error)
+	UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef,
+		networkName string) error
 }
 
 type Temporary interface {