From 89909017b4479261ab28044c4414a7874ee0453d Mon Sep 17 00:00:00 2001 From: Wantong Jiang Date: Fri, 14 Oct 2022 02:50:52 +0000 Subject: [PATCH] Check ip in subnet --- pkg/provider/azure_loadbalancer.go | 32 ++++++++++++++++++++++++- pkg/provider/azure_loadbalancer_test.go | 30 +++++++++++++++++------ 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go index 3b85668b43..10cce0089a 100644 --- a/pkg/provider/azure_loadbalancer.go +++ b/pkg/provider/azure_loadbalancer.go @@ -23,6 +23,7 @@ import ( "fmt" "math" "net" + "net/netip" "reflect" "sort" "strconv" @@ -1873,7 +1874,7 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv if loadBalancerIP != "" { configProperties.PrivateIPAllocationMethod = network.IPAllocationMethodStatic configProperties.PrivateIPAddress = &loadBalancerIP - } else if status != nil && len(status.Ingress) > 0 { + } else if status != nil && len(status.Ingress) > 0 && ipInSubnet(status.Ingress[0].IP, &subnet) { klog.V(4).Infof("reconcileFrontendIPConfigs for service (%s): keep the original private IP %s", serviceName, status.Ingress[0].IP) configProperties.PrivateIPAllocationMethod = network.IPAllocationMethodStatic configProperties.PrivateIPAddress = to.StringPtr(status.Ingress[0].IP) @@ -3288,6 +3289,35 @@ func subnet(service *v1.Service) *string { return nil } +func ipInSubnet(ip string, subnet *network.Subnet) bool { + if subnet == nil || subnet.SubnetPropertiesFormat == nil { + return false + } + netIP, err := netip.ParseAddr(ip) + if err != nil { + klog.Errorf("ipInSubnet: failed to parse ip %s: %v", netIP, err) + return false + } + cidrs := make([]string, 0) + if subnet.AddressPrefix != nil { + cidrs = append(cidrs, *subnet.AddressPrefix) + } + if subnet.AddressPrefixes != nil { + cidrs = append(cidrs, *subnet.AddressPrefixes...) + } + for _, cidr := range cidrs { + network, err := netip.ParsePrefix(cidr) + if err != nil { + klog.Errorf("ipInSubnet: failed to parse ip cidr %s: %v", cidr, err) + continue + } + if network.Contains(netIP) { + return true + } + } + return false +} + // getServiceLoadBalancerMode parses the mode value. // if the value is __auto__ it returns isAuto = TRUE. // if anything else it returns the unique VM set names after trimming spaces. diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go index 997dca3da0..48b08e8fd2 100644 --- a/pkg/provider/azure_loadbalancer_test.go +++ b/pkg/provider/azure_loadbalancer_test.go @@ -5199,7 +5199,7 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { regionZonesMap map[string][]string expectedZones *[]string expectedDirty bool - expectedIP string + expectedIP *string expectedErr error }{ { @@ -5273,14 +5273,25 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { expectedDirty: true, }, { - description: "reconcileFrontendIPConfigs should reuse the existing private IP for internal services", + description: "reconcileFrontendIPConfigs should reuse the existing private IP for internal services when subnet does not change", service: getInternalTestService("test", 80), status: &v1.LoadBalancerStatus{ Ingress: []v1.LoadBalancerIngress{ {IP: "1.2.3.4"}, }, }, - expectedIP: "1.2.3.4", + expectedIP: to.StringPtr("1.2.3.4"), + expectedDirty: true, + }, + { + description: "reconcileFrontendIPConfigs should not reuse the existing private IP for internal services when subnet changes", + service: getInternalTestService("test", 80), + status: &v1.LoadBalancerStatus{ + Ingress: []v1.LoadBalancerIngress{ + {IP: "1.2.3.6"}, + }, + }, + expectedIP: to.StringPtr(""), expectedDirty: true, }, } { @@ -5298,7 +5309,8 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { mockPIPClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(nil).MaxTimes(1) subnetClient := cloud.SubnetsClient.(*mocksubnetclient.MockInterface) - subnetClient.EXPECT().Get(gomock.Any(), "rg", "vnet", "subnet", gomock.Any()).Return(network.Subnet{}, nil).MaxTimes(1) + subnetClient.EXPECT().Get(gomock.Any(), "rg", "vnet", "subnet", gomock.Any()).Return( + network.Subnet{SubnetPropertiesFormat: &network.SubnetPropertiesFormat{AddressPrefix: to.StringPtr("1.2.3.4/31")}}, nil).MaxTimes(1) zoneClient := mockzoneclient.NewMockInterface(ctrl) zoneClient.EXPECT().GetZones(gomock.Any(), gomock.Any()).Return(map[string][]string{}, tc.getZoneError).MaxTimes(1) @@ -5319,9 +5331,13 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) { } } - if tc.expectedIP != "" { - assert.Equal(t, network.IPAllocationMethodStatic, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) - assert.Equal(t, tc.expectedIP, to.String((*lb.FrontendIPConfigurations)[0].PrivateIPAddress)) + if tc.expectedIP != nil { + assert.Equal(t, *tc.expectedIP, to.String((*lb.FrontendIPConfigurations)[0].PrivateIPAddress)) + if *tc.expectedIP != "" { + assert.Equal(t, network.IPAllocationMethodStatic, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) + } else { + assert.Equal(t, network.IPAllocationMethodDynamic, (*lb.FrontendIPConfigurations)[0].PrivateIPAllocationMethod) + } } }) }