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 27, 2023
1 parent e4a7465 commit 1d6a93d
Show file tree
Hide file tree
Showing 9 changed files with 758 additions and 260 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.

604 changes: 431 additions & 173 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
66 changes: 42 additions & 24 deletions pkg/provider/azure_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1441,103 +1441,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, *pip.Name)
pipsAddrs = append(pipsAddrs, pointer.StringDeref(pip.PublicIPAddressPropertiesFormat.IPAddress, ""))
}
for _, pip := range pips2 {
pips2Names = append(pips2Names, *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 +1927,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
33 changes: 21 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,29 @@ 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)
}

func getResourceNoIPFamily(resource string) string {
if strings.HasSuffix(resource, v4Suffix) {
return resource[:len(resource)-len(v4Suffix)-1]
}
if strings.HasSuffix(resource, v6Suffix) {
return resource[:len(resource)-len(v6Suffix)-1]
}
return ""
}

// 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 +492,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 1d6a93d

Please sign in to comment.