readd assigned ip addresses to ipam when subnet has been changed (#3448)

* update check for ippool static ip address Signed-off-by: 马洪贞 <hzma@alauda.io> * readd assigned ip addresses to ipam when subnet has been changed Signed-off-by: 马洪贞 <hzma@alauda.io> * modify ut to adapt CIDRContainIP func Signed-off-by: 马洪贞 <hzma@alauda.io> --------- Signed-off-by: 马洪贞 <hzma@alauda.io>
kubeovn · Nov 21, 2023 · 72444c1 · 72444c1
1 parent 03ed9da
commit 72444c1
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 6 deletions.
diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go
@@ -232,9 +232,19 @@ func (ipam *IPAM) AddOrUpdateSubnet(name, cidrStr, gw string, excludeIps []strin
 					klog.Errorf("%s address %s not in subnet %s new cidr %s", podName, ip.String(), name, cidrStr)
 					delete(subnet.V4NicToIP, nicName)
 					delete(subnet.V4IPToPod, ip.String())
-					continue
+				} else {
+					// The already assigned addresses should be added in ipam again when subnet cidr changed
+					pool.V4Available.Remove(ip)
+					pool.V4Using.Add(ip)
+					subnet.V4Free.Remove(ip)
+					subnet.V4Available.Remove(ip)
+					subnet.V4Using.Add(ip)
 				}
 			}
+
+			for nicName, ip := range subnet.V4NicToIP {
+				klog.Infof("already assigned ip %s to nic %s in subnet %s", ip.String(), nicName, name)
+			}
 		}
 		if (protocol == kubeovnv1.ProtocolDual || protocol == kubeovnv1.ProtocolIPv6) &&
 			(subnet.V6CIDR.String() != v6cidrStr || subnet.V6Gw != v6Gw || !subnet.V6Reserved.Equal(v6Reserved)) {
@@ -275,9 +285,19 @@ func (ipam *IPAM) AddOrUpdateSubnet(name, cidrStr, gw string, excludeIps []strin
 					klog.Errorf("%s address %s not in subnet %s new cidr %s", podName, ip.String(), name, cidrStr)
 					delete(subnet.V6NicToIP, nicName)
 					delete(subnet.V6IPToPod, ip.String())
-					continue
+				} else {
+					// The already assigned addresses should be added in ipam again when subnet cidr changed
+					pool.V6Available.Remove(ip)
+					pool.V6Using.Add(ip)
+					subnet.V6Free.Remove(ip)
+					subnet.V6Available.Remove(ip)
+					subnet.V6Using.Add(ip)
 				}
 			}
+
+			for nicName, ip := range subnet.V6NicToIP {
+				klog.Infof("already assigned ip %s to nic %s in subnet %s", ip.String(), nicName, name)
+			}
 		}
 
 		for nicName, mac := range subnet.NicToMac {

diff --git a/pkg/util/net.go b/pkg/util/net.go
@@ -130,6 +130,7 @@ func CIDRContainIP(cidrStr, ipStr string) bool {
 			return false
 		}
 
+		found := false
 		for _, ip := range ips {
 			if CheckProtocol(cidr) != CheckProtocol(ip) {
 				continue
@@ -139,10 +140,16 @@ func CIDRContainIP(cidrStr, ipStr string) bool {
 				return false
 			}
 
-			if !cidrNet.Contains(ipAddr) {
-				return false
+			// After ns supports multiple subnets, the ippool static addresses can be allocated in any subnets, such as "ovn.kubernetes.io/ip_pool: 11.16.10.14,12.26.11.21"
+			found = false
+			if cidrNet.Contains(ipAddr) {
+				found = true
+				break
 			}
 		}
+		if !found {
+			return false
+		}
 	}
 	// v4 and v6 address should be both matched for dualstack check
 	return true

diff --git a/pkg/util/net_test.go b/pkg/util/net_test.go
@@ -1013,10 +1013,12 @@ func Test_CIDRContainIP(t *testing.T) {
 			true,
 		},
 		{
+			// After ns supports multiple subnets, the ippool static addresses can be allocated in any subnets, such as "ovn.kubernetes.io/ip_pool: 11.16.10.14,12.26.11.21"
+			// so if anyone ip is included in cidr, return true
 			"ipv4 family which CIDR does't contain ip",
 			"192.168.230.0/24",
 			"192.168.231.10,192.168.230.11",
-			false,
+			true,
 		},
 		{
 			"ipv6 family",
@@ -1037,10 +1039,12 @@ func Test_CIDRContainIP(t *testing.T) {
 			true,
 		},
 		{
+			// After ns supports multiple subnets, the ippool static addresses can be allocated in any subnets, such as "ovn.kubernetes.io/ip_pool: 11.16.10.14,12.26.11.21"
+			// so if anyone ip is included in cidr, return true
 			"dual which CIDR does't contain ip",
 			"192.168.230.0/24,fc00::0af4:00/112",
 			"fc00::0af4:10,fd00::0af4:11,192.168.230.10,192.168.230.11",
-			false,
+			true,
 		},
 		{
 			"different family",
@@ -1064,6 +1068,7 @@ func Test_CIDRContainIP(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
+			t.Logf("test case %v", tt.desc)
 			got := CIDRContainIP(tt.cidrs, tt.ips)
 			require.Equal(t, got, tt.want)
 		})