Skip to content

Commit

Permalink
Support dualstack for public IP in azure_loadbalancer.go
Browse files Browse the repository at this point in the history 
* Feature code for public IP
* Remove DualstackSupported var and refactor
* Add UT

Signed-off-by: Zhecheng Li <zhechengli@microsoft.com>
  • Loading branch information
@lzhecheng
lzhecheng committed Feb 28, 2023
1 parent e4a7465 commit 59a75fe
Show file tree
Hide file tree
Showing 9 changed files with 833 additions and 273 deletions.
3 changes: 0 additions & 3 deletions pkg/consts/consts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ var (

// load balancer
const (
// TODO: After dual-stack is supported, all references should be updated and this variable is not needed.
DualstackSupported = false

// PreConfiguredBackendPoolLoadBalancerTypesInternal means that the `internal` load balancers are pre-configured
PreConfiguredBackendPoolLoadBalancerTypesInternal = "internal"
// PreConfiguredBackendPoolLoadBalancerTypesExternal means that the `external` load balancers are pre-configured
Expand Down
155 changes: 115 additions & 40 deletions pkg/provider/azure_loadbalancer.go
View file

Large diffs are not rendered by default.

626 changes: 454 additions & 172 deletions pkg/provider/azure_loadbalancer_test.go
View file

Large diffs are not rendered by default.

11 changes: 8 additions & 3 deletions pkg/provider/azure_standard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,8 @@ func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protoc
ruleName := fmt.Sprintf("%s-%s-%d", prefix, protocol, port)
subnet := subnet(service)
if subnet == nil {
return getResourceByIPFamily(ruleName, isIPv6)
// TODO: Use getResourceByIPFamily()
return ruleName
}

// Load balancer rule name must be less or equal to 80 characters, so excluding the hyphen two segments cannot exceed 79
Expand All @@ -293,7 +294,8 @@ func (az *Cloud) getLoadBalancerRuleName(service *v1.Service, protocol v1.Protoc
subnetSegment = subnetSegment[:maxLength-len(ruleName)-1]
}

return getResourceByIPFamily(fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port), isIPv6)
// TODO: Use getResourceByIPFamily()
return fmt.Sprintf("%s-%s-%s-%d", prefix, subnetSegment, protocol, port)
}

func (az *Cloud) getloadbalancerHAmodeRuleName(service *v1.Service, isIPv6 bool) string {
Expand All @@ -308,7 +310,8 @@ func (az *Cloud) getSecurityRuleName(service *v1.Service, port v1.ServicePort, s
}
rulePrefix := az.getRulePrefix(service)
name := fmt.Sprintf("%s-%s-%d-%s", rulePrefix, port.Protocol, port.Port, safePrefix)
return getResourceByIPFamily(name, isIPv6)
// TODO: Use getResourceByIPFamily
return name
}

// This returns a human-readable version of the Service used to tag some resources.
Expand All @@ -322,6 +325,7 @@ func (az *Cloud) getRulePrefix(service *v1.Service) string {
return az.GetLoadBalancerName(context.TODO(), "", service)
}

// TODO: UT
func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service, isIPv6 bool) string {
pipName := fmt.Sprintf("%s-%s", clusterName, az.GetLoadBalancerName(context.TODO(), clusterName, service))
pipName = getResourceByIPFamily(pipName, isIPv6)
Expand All @@ -335,6 +339,7 @@ func (az *Cloud) getPublicIPName(clusterName string, service *v1.Service, isIPv6
return pipName
}

// TODO: UT
func (az *Cloud) serviceOwnsRule(service *v1.Service, rule string) bool {
prefix := az.getRulePrefix(service)
return strings.HasPrefix(strings.ToUpper(rule), strings.ToUpper(prefix))
Expand Down
120 changes: 82 additions & 38 deletions pkg/provider/azure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,43 @@ func setMockEnv(az *Cloud, ctrl *gomock.Controller, expectedInterfaces []network
}

func setMockPublicIPs(az *Cloud, ctrl *gomock.Controller, serviceCount int) {
v4Enabled, v6Enabled := true, true

mockPIPsClient := mockpublicipclient.NewMockInterface(ctrl)

expectedPIPs := []network.PublicIPAddress{}
if v4Enabled {
expectedPIP := setMockPublicIP(az, mockPIPsClient, serviceCount, false)
expectedPIPs = append(expectedPIPs, expectedPIP)
}
if v6Enabled {
expectedPIP := setMockPublicIP(az, mockPIPsClient, serviceCount, true)
expectedPIPs = append(expectedPIPs, expectedPIP)
}

az.PublicIPAddressesClient = mockPIPsClient
mockPIPsClient.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockPIPsClient.EXPECT().List(gomock.Any(), gomock.Not(az.ResourceGroup)).Return(nil, nil).AnyTimes()
mockPIPsClient.EXPECT().Get(gomock.Any(), gomock.Not(az.ResourceGroup), gomock.Any(), gomock.Any()).Return(network.PublicIPAddress{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()
}

func setMockPublicIP(az *Cloud, mockPIPsClient *mockpublicipclient.MockInterface, serviceCount int, isIPv6 bool) network.PublicIPAddress {
suffix := v4Suffix
ipVer := network.IPv4
ipAddr := "1.2.3.4"
if isIPv6 {
suffix = v6Suffix
ipVer = network.IPv6
ipAddr = "fd00::eef0"
}

expectedPIP := network.PublicIPAddress{
Name: pointer.String("testCluster-aservicea"),
Location: &az.Location,
PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
PublicIPAllocationMethod: network.Static,
PublicIPAddressVersion: network.IPv4,
IPAddress: pointer.String("1.2.3.4"),
PublicIPAddressVersion: ipVer,
IPAddress: pointer.String(ipAddr),
},
Tags: map[string]*string{
consts.ServiceTagKey: pointer.String("default/servicea"),
Expand All @@ -183,28 +213,24 @@ func setMockPublicIPs(az *Cloud, ctrl *gomock.Controller, serviceCount int) {
ID: pointer.String("testCluster-aservice1"),
}

mockPIPsClient := mockpublicipclient.NewMockInterface(ctrl)
az.PublicIPAddressesClient = mockPIPsClient
mockPIPsClient.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
mockPIPsClient.EXPECT().List(gomock.Any(), gomock.Not(az.ResourceGroup)).Return(nil, nil).AnyTimes()
mockPIPsClient.EXPECT().Get(gomock.Any(), gomock.Not(az.ResourceGroup), gomock.Any(), gomock.Any()).Return(network.PublicIPAddress{}, &retry.Error{HTTPStatusCode: http.StatusNotFound, RawError: cloudprovider.InstanceNotFound}).AnyTimes()

a := 'a'
var expectedPIPs []network.PublicIPAddress
for i := 1; i <= serviceCount; i++ {
expectedPIP.Name = pointer.String(fmt.Sprintf("testCluster-aservice%d", i))
expectedPIP.Name = pointer.String(fmt.Sprintf("testCluster-aservice%d-%s", i, suffix))
expectedPIP.Tags[consts.ServiceTagKey] = pointer.String(fmt.Sprintf("default/service%d", i))
mockPIPsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%d", i), gomock.Any()).Return(expectedPIP, nil).AnyTimes()
mockPIPsClient.EXPECT().Delete(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%d", i)).Return(nil).AnyTimes()
mockPIPsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%d-%s", i, suffix), gomock.Any()).Return(expectedPIP, nil).AnyTimes()
mockPIPsClient.EXPECT().Delete(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%d-%s", i, suffix)).Return(nil).AnyTimes()
expectedPIPs = append(expectedPIPs, expectedPIP)
expectedPIP.Name = pointer.String(fmt.Sprintf("testCluster-aservice%c", a))
expectedPIP.Name = pointer.String(fmt.Sprintf("testCluster-aservice%c-%s", a, suffix))
expectedPIP.Tags[consts.ServiceTagKey] = pointer.String(fmt.Sprintf("default/service%c", a))
mockPIPsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%c", a), gomock.Any()).Return(expectedPIP, nil).AnyTimes()
mockPIPsClient.EXPECT().Delete(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%c", a)).Return(nil).AnyTimes()
mockPIPsClient.EXPECT().Get(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%c-%s", a, suffix), gomock.Any()).Return(expectedPIP, nil).AnyTimes()
mockPIPsClient.EXPECT().Delete(gomock.Any(), az.ResourceGroup, fmt.Sprintf("testCluster-aservice%c-%s", a, suffix)).Return(nil).AnyTimes()
expectedPIPs = append(expectedPIPs, expectedPIP)
a++
}
mockPIPsClient.EXPECT().List(gomock.Any(), az.ResourceGroup).Return(expectedPIPs, nil).AnyTimes()

return expectedPIP
}

func setMockSecurityGroup(az *Cloud, ctrl *gomock.Controller, sgs ...*network.SecurityGroup) {
Expand Down Expand Up @@ -1441,103 +1467,115 @@ func TestReconcileSecurityGroupEtagMismatch(t *testing.T) {
assert.Equal(t, expectedError.Error(), err)
}

func TestReconcilePublicIPWithNewService(t *testing.T) {
func TestReconcilePublicIPsWithNewService(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

az := GetTestCloud(ctrl)
svc := getTestService("servicea", v1.ProtocolTCP, nil, false, 80, 443)
makeTestServiceDualStack(&svc)

setMockPublicIPs(az, ctrl, 1)

pip, err := az.reconcilePublicIP(testClusterName, &svc, "", true /* wantLb*/)
pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svc, true)
validatePublicIPs(t, pips, &svc, true)

pip2, err := az.reconcilePublicIP(testClusterName, &svc, "", true /* wantLb */)
pips2, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb */)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip2, &svc, true)
if pip.Name != pip2.Name ||
pip.PublicIPAddressPropertiesFormat.IPAddress != pip2.PublicIPAddressPropertiesFormat.IPAddress {
t.Errorf("We should get the exact same public ip resource after a second reconcile")
validatePublicIPs(t, pips2, &svc, true)

pipsNames, pips2Names := []string{}, []string{}
pipsAddrs, pips2Addrs := []string{}, []string{}
for _, pip := range pips {
pipsNames = append(pipsNames, pointer.StringDeref(pip.Name, ""))
pipsAddrs = append(pipsAddrs, pointer.StringDeref(pip.PublicIPAddressPropertiesFormat.IPAddress, ""))
}
for _, pip := range pips2 {
pips2Names = append(pips2Names, pointer.StringDeref(pip.Name, ""))
pips2Addrs = append(pips2Addrs, pointer.StringDeref(pip.PublicIPAddressPropertiesFormat.IPAddress, ""))
}
assert.Truef(t, compareStrings(pipsNames, pips2Names) && compareStrings(pipsAddrs, pips2Addrs),
"We should get the exact same public ip resource after a second reconcile")
}

func TestReconcilePublicIPRemoveService(t *testing.T) {
func TestReconcilePublicIPsRemoveService(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

az := GetTestCloud(ctrl)
svc := getTestService("servicea", v1.ProtocolTCP, nil, false, 80, 443)
makeTestServiceDualStack(&svc)

setMockPublicIPs(az, ctrl, 1)

pip, err := az.reconcilePublicIP(testClusterName, &svc, "", true /* wantLb*/)
pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}

validatePublicIP(t, pip, &svc, true)
validatePublicIPs(t, pips, &svc, true)

// Remove the service
pip, err = az.reconcilePublicIP(testClusterName, &svc, "", false /* wantLb */)
pips, err = az.reconcilePublicIPs(testClusterName, &svc, "", false /* wantLb */)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svc, false)

validatePublicIPs(t, pips, &svc, false)
}

func TestReconcilePublicIPWithInternalService(t *testing.T) {
func TestReconcilePublicIPsWithInternalService(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

az := GetTestCloud(ctrl)
svc := getInternalTestService("servicea", 80, 443)
makeTestServiceDualStack(&svc)

setMockPublicIPs(az, ctrl, 1)

pip, err := az.reconcilePublicIP(testClusterName, &svc, "", true /* wantLb*/)
pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}

validatePublicIP(t, pip, &svc, true)
validatePublicIPs(t, pips, &svc, true)
}

func TestReconcilePublicIPWithExternalAndInternalSwitch(t *testing.T) {
func TestReconcilePublicIPsWithExternalAndInternalSwitch(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

az := GetTestCloud(ctrl)
svc := getInternalTestService("servicea", 80, 443)
makeTestServiceDualStack(&svc)

setMockPublicIPs(az, ctrl, 1)

pip, err := az.reconcilePublicIP(testClusterName, &svc, "", true /* wantLb*/)
pips, err := az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svc, true)
validatePublicIPs(t, pips, &svc, true)

// Update to external service
svcUpdated := getTestService("servicea", v1.ProtocolTCP, nil, false, 80)
pip, err = az.reconcilePublicIP(testClusterName, &svcUpdated, "", true /* wantLb*/)
pips, err = az.reconcilePublicIPs(testClusterName, &svcUpdated, "", true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svcUpdated, true)
validatePublicIPs(t, pips, &svcUpdated, true)

// Update to internal service again
pip, err = az.reconcilePublicIP(testClusterName, &svc, "", true /* wantLb*/)
pips, err = az.reconcilePublicIPs(testClusterName, &svc, "", true /* wantLb*/)
if err != nil {
t.Errorf("Unexpected error: %q", err)
}
validatePublicIP(t, pip, &svc, true)
validatePublicIPs(t, pips, &svc, true)
}

const networkInterfacesIDTemplate = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/networkInterfaces/%s"
Expand Down Expand Up @@ -1915,6 +1953,12 @@ func describeFIPs(frontendIPs []network.FrontendIPConfiguration) string {
return description
}

func validatePublicIPs(t *testing.T, pips []*network.PublicIPAddress, service *v1.Service, wantLb bool) {
for _, pip := range pips {
validatePublicIP(t, pip, service, wantLb)
}
}

func validatePublicIP(t *testing.T, publicIP *network.PublicIPAddress, service *v1.Service, wantLb bool) {
isInternal := requiresInternalLoadBalancer(service)
if isInternal || !wantLb {
Expand Down
34 changes: 22 additions & 12 deletions pkg/provider/azure_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,8 @@ func getServicePIPName(service *v1.Service, isIPv6 bool) string {
return ""
}

if consts.DualstackSupported {
if name, ok := service.Annotations[consts.ServiceAnnotationPIPNameDualStack[isIPv6]]; ok && name != "" {
return name
}
if name, ok := service.Annotations[consts.ServiceAnnotationPIPNameDualStack[isIPv6]]; ok && name != "" {
return name
}
return service.Annotations[consts.ServiceAnnotationPIPName]
}
Expand All @@ -424,25 +422,30 @@ func getServicePIPPrefixID(service *v1.Service, isIPv6 bool) string {
return ""
}

if consts.DualstackSupported {
if name, ok := service.Annotations[consts.ServiceAnnotationPIPPrefixIDDualStack[isIPv6]]; ok && name != "" {
return name
}
if name, ok := service.Annotations[consts.ServiceAnnotationPIPPrefixIDDualStack[isIPv6]]; ok && name != "" {
return name
}
return service.Annotations[consts.ServiceAnnotationPIPPrefixID]
}

func getResourceByIPFamily(resource string, isIPv6 bool) string {
if !consts.DualstackSupported {
return resource
}

if isIPv6 {
return fmt.Sprintf("%s-%s", resource, v6Suffix)
}
return fmt.Sprintf("%s-%s", resource, v4Suffix)
}

// getResourceNoIPFamily returns resource name with no IP family suffix and if it is IPv6.
func getResourceNoIPFamily(resource string) (string, bool) {
if strings.HasSuffix(resource, v4Suffix) {
return resource[:len(resource)-len(v4Suffix)-1], false
}
if strings.HasSuffix(resource, v6Suffix) {
return resource[:len(resource)-len(v6Suffix)-1], true
}
return "", false
}

// isFIPIPv6 checks if the frontend IP configuration is of IPv6.
func (az *Cloud) isFIPIPv6(fip *network.FrontendIPConfiguration, pips *[]network.PublicIPAddress, isInternal bool) (isIPv6 bool, err error) {
if err := az.safeListPIP(az.ResourceGroup, pips); err != nil {
Expand Down Expand Up @@ -490,3 +493,10 @@ func getResourceIDPrefix(id string) string {
}
return id[:idx]
}

// matchResourceName checks if 2 resources are the same regardless of IP family suffix.
func matchResourceName(resource, resourceWithIPFamily string) bool {
return resource == resourceWithIPFamily ||
fmt.Sprintf("%s-%s", resource, v4Suffix) == resourceWithIPFamily ||
fmt.Sprintf("%s-%s", resource, v6Suffix) == resourceWithIPFamily
}

0 comments on commit 59a75fe

Please sign in to comment.