Merge pull request #1070 from nilo19/bug/nil-zone

fix: use zones in the pre-existing frontend IP configurations for int…

pkg/azureclients/interfaceclient/interface.go

@@ -26,7 +26,7 @@ import (
 
 const (
 	// APIVersion is the API version for network.
-	APIVersion = "2020-08-01"
+	APIVersion = "2021-02-01"
 	// AzureStackCloudAPIVersion is the API version for Azure Stack
 	AzureStackCloudAPIVersion = "2018-11-01"
 

pkg/azureclients/loadbalancerclient/interface.go

@@ -26,7 +26,7 @@ import (
 
 const (
 	// APIVersion is the API version for network.
-	APIVersion = "2020-08-01"
+	APIVersion = "2021-02-01"
 	// AzureStackCloudAPIVersion is the API version for Azure Stack
 	AzureStackCloudAPIVersion = "2018-11-01"
 	// AzureStackCloudName is the cloud name of Azure Stack

pkg/azureclients/publicipclient/interface.go

@@ -26,7 +26,7 @@ import (
 
 const (
 	// APIVersion is the API version for network.
-	APIVersion = "2020-08-01"
+	APIVersion = "2021-02-01"
 	// AzureStackCloudAPIVersion is the API version for Azure Stack
 	AzureStackCloudAPIVersion = "2018-11-01"
 

pkg/azureclients/routeclient/interface.go

@@ -26,7 +26,7 @@ import (
 
 const (
 	// APIVersion is the API version for network.
-	APIVersion = "2020-08-01"
+	APIVersion = "2021-02-01"
 	// AzureStackCloudAPIVersion is the API version for Azure Stack
 	AzureStackCloudAPIVersion = "2018-11-01"
 	// AzureStackCloudName is the cloud name of Azure Stack

pkg/azureclients/routetableclient/interface.go

@@ -26,7 +26,7 @@ import (
 
 const (
 	// APIVersion is the API version for network.
-	APIVersion = "2020-08-01"
+	APIVersion = "2021-02-01"
 	// AzureStackCloudAPIVersion is the API version for Azure Stack
 	AzureStackCloudAPIVersion = "2018-11-01"
 	// AzureStackCloudName is the cloud name of Azure Stack

pkg/azureclients/securitygroupclient/interface.go

@@ -26,7 +26,7 @@ import (
 
 const (
 	// APIVersion is the API version for network.
-	APIVersion = "2020-08-01"
+	APIVersion = "2021-02-01"
 	// AzureStackCloudAPIVersion is the API version for Azure Stack
 	AzureStackCloudAPIVersion = "2018-11-01"
 	// AzureStackCloudName is the cloud name of Azure Stack

pkg/provider/azure_loadbalancer.go

@@ -1734,16 +1734,21 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv
 			}
 		}
 	} else {
+		var (
+			previousZone *[]string
+			isFipChanged bool
+		)
 		for i := len(newConfigs) - 1; i >= 0; i-- {
 			config := newConfigs[i]
-			isFipChanged, err := az.isFrontendIPChanged(clusterName, config, service, defaultLBFrontendIPConfigName)
+			isFipChanged, err = az.isFrontendIPChanged(clusterName, config, service, defaultLBFrontendIPConfigName)
 			if err != nil {
 				return nil, false, err
 			}
 			if isFipChanged {
 				klog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - dropping", serviceName, wantLb, *config.Name)
 				newConfigs = append(newConfigs[:i], newConfigs[i+1:]...)
 				dirtyConfigs = true
+				previousZone = config.Zones
 			}
 		}
 
@@ -1810,14 +1815,11 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv
 				FrontendIPConfigurationPropertiesFormat: fipConfigurationProperties,
 			}
 
-			// only add zone information for new internal frontend IP configurations for standard load balancer not deployed to an edge zone.
-			location := az.Location
-			zones, err := az.getRegionZonesBackoff(location)
-			if err != nil {
-				return nil, false, err
-			}
-			if isInternal && az.useStandardLoadBalancer() && len(zones) > 0 && !az.HasExtendedLocation() {
-				newConfig.Zones = &zones
+			if isInternal {
+				if err := az.getFrontendZones(&newConfig, previousZone, isFipChanged, serviceName, defaultLBFrontendIPConfigName); err != nil {
+					klog.Errorf("reconcileLoadBalancer for service (%s)(%t): failed to getFrontendZones: %s", serviceName, wantLb, err.Error())
+					return nil, false, err
+				}
 			}
 			newConfigs = append(newConfigs, newConfig)
 			klog.V(2).Infof("reconcileLoadBalancer for service (%s)(%t): lb frontendconfig(%s) - adding", serviceName, wantLb, defaultLBFrontendIPConfigName)
@@ -1832,6 +1834,33 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv
 	return ownedFIPConfig, dirtyConfigs, err
 }
 
+func (az *Cloud) getFrontendZones(
+	fipConfig *network.FrontendIPConfiguration,
+	previousZone *[]string,
+	isFipChanged bool,
+	serviceName, defaultLBFrontendIPConfigName string) error {
+	if !isFipChanged { // fetch zone information from API for new frontends
+		// only add zone information for new internal frontend IP configurations for standard load balancer not deployed to an edge zone.
+		location := az.Location
+		zones, err := az.getRegionZonesBackoff(location)
+		if err != nil {
+			return err
+		}
+		if az.useStandardLoadBalancer() && len(zones) > 0 && !az.HasExtendedLocation() {
+			fipConfig.Zones = &zones
+		}
+	} else {
+		if previousZone == nil { // keep the existing zone information for existing frontends
+			klog.V(2).Infof("getFrontendZones for service (%s): lb frontendconfig(%s): setting zone to nil", serviceName, defaultLBFrontendIPConfigName)
+		} else {
+			zoneStr := strings.Join(*previousZone, ",")
+			klog.V(2).Infof("getFrontendZones for service (%s): lb frontendconfig(%s): setting zone to %s", serviceName, defaultLBFrontendIPConfigName, zoneStr)
+		}
+		fipConfig.Zones = previousZone
+	}
+	return nil
+}
+
 // checkLoadBalancerResourcesConflicts checks if the service is consuming
 // ports which conflict with the existing loadBalancer resources,
 // including inbound NAT rule, inbound NAT pools and loadBalancing rules

pkg/provider/azure_loadbalancer_test.go

@@ -34,13 +34,16 @@ import (
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/loadbalancerclient/mockloadbalancerclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/publicipclient/mockpublicipclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/securitygroupclient/mocksecuritygroupclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssclient/mockvmssclient"
+	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/zoneclient/mockzoneclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 	"sigs.k8s.io/cloud-provider-azure/pkg/retry"
 )
@@ -4356,9 +4359,11 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) {
 		existingFrontendIPConfigs []network.FrontendIPConfiguration
 		existingPIP               network.PublicIPAddress
 		getPIPError               *retry.Error
+		getZoneError              *retry.Error
 		regionZonesMap            map[string][]string
 		expectedZones             *[]string
 		expectedDirty             bool
+		expectedErr               error
 	}{
 		{
 			description:               "reconcileFrontendIPConfigs should reconcile the zones for the new fip config",
@@ -4379,6 +4384,48 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) {
 			expectedZones:             &[]string{"1", "2", "3"},
 			expectedDirty:             true,
 		},
+		{
+			description:  "reconcileFrontendIPConfigs should report an error if failed to get zones",
+			service:      getInternalTestService("test", 80),
+			getZoneError: retry.NewError(false, errors.New("get zone failed")),
+			expectedErr:  errors.New("get zone failed"),
+		},
+		{
+			description: "reconcileFrontendIPConfigs should use the nil zones of the existing frontend",
+			service: getTestServiceWithAnnotation("test", map[string]string{
+				consts.ServiceAnnotationLoadBalancerInternalSubnet: "subnet",
+				consts.ServiceAnnotationLoadBalancerInternal:       consts.TrueAnnotationValue}, 80),
+			existingFrontendIPConfigs: []network.FrontendIPConfiguration{
+				{
+					Name: to.StringPtr("atest1"),
+					FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{
+						Subnet: &network.Subnet{
+							Name: to.StringPtr("subnet-1"),
+						},
+					},
+				},
+			},
+			expectedDirty: true,
+		},
+		{
+			description: "reconcileFrontendIPConfigs should use the non-nil zones of the existing frontend",
+			service: getTestServiceWithAnnotation("test", map[string]string{
+				consts.ServiceAnnotationLoadBalancerInternalSubnet: "subnet",
+				consts.ServiceAnnotationLoadBalancerInternal:       consts.TrueAnnotationValue}, 80),
+			existingFrontendIPConfigs: []network.FrontendIPConfiguration{
+				{
+					Name: to.StringPtr("atest1"),
+					FrontendIPConfigurationPropertiesFormat: &network.FrontendIPConfigurationPropertiesFormat{
+						Subnet: &network.Subnet{
+							Name: to.StringPtr("subnet-1"),
+						},
+					},
+					Zones: &[]string{"1"},
+				},
+			},
+			expectedZones: &[]string{"1"},
+			expectedDirty: true,
+		},
 	} {
 		t.Run(tc.description, func(t *testing.T) {
 			cloud := GetTestCloud(ctrl)
@@ -4396,9 +4443,17 @@ func TestReconcileZonesForFrontendIPConfigs(t *testing.T) {
 			subnetClient := cloud.SubnetsClient.(*mocksubnetclient.MockInterface)
 			subnetClient.EXPECT().Get(gomock.Any(), "rg", "vnet", "subnet", gomock.Any()).Return(network.Subnet{}, nil).MaxTimes(1)
 
+			zoneClient := mockzoneclient.NewMockInterface(ctrl)
+			zoneClient.EXPECT().GetZones(gomock.Any(), gomock.Any()).Return(map[string][]string{}, tc.getZoneError).MaxTimes(1)
+			cloud.ZoneClient = zoneClient
+
 			defaultLBFrontendIPConfigName := cloud.getDefaultFrontendIPConfigName(&tc.service)
 			_, dirty, err := cloud.reconcileFrontendIPConfigs("testCluster", &tc.service, &lb, true, defaultLBFrontendIPConfigName)
-			assert.NoError(t, err)
+			if tc.expectedErr == nil {
+				assert.NoError(t, err)
+			} else {
+				assert.Contains(t, err.Error(), tc.expectedErr.Error())
+			}
 			assert.Equal(t, tc.expectedDirty, dirty)
 
 			for _, fip := range *lb.FrontendIPConfigurations {

pkg/provider/azure_test.go

@@ -1616,8 +1616,14 @@ func getTestService(identifier string, proto v1.Protocol, annotations map[string
 }
 
 func getInternalTestService(identifier string, requestedPorts ...int32) v1.Service {
+	return getTestServiceWithAnnotation(identifier, map[string]string{consts.ServiceAnnotationLoadBalancerInternal: consts.TrueAnnotationValue}, requestedPorts...)
+}
+
+func getTestServiceWithAnnotation(identifier string, annotations map[string]string, requestedPorts ...int32) v1.Service {
 	svc := getTestService(identifier, v1.ProtocolTCP, nil, false, requestedPorts...)
-	svc.Annotations[consts.ServiceAnnotationLoadBalancerInternal] = consts.TrueAnnotationValue
+	for k, v := range annotations {
+		svc.Annotations[k] = v
+	}
 	return svc
 }
 

tests/e2e/network/ensureloadbalancer.go

@@ -23,7 +23,7 @@ import (
 	"strings"
 	"time"
 
-	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
 
 	v1 "k8s.io/api/core/v1"
@@ -572,7 +572,7 @@ func defaultPublicIPAddress(ipName string) aznetwork.PublicIPAddress {
 			Name: skuName,
 		},
 		PublicIPAddressPropertiesFormat: &aznetwork.PublicIPAddressPropertiesFormat{
-			PublicIPAllocationMethod: aznetwork.Static,
+			PublicIPAllocationMethod: aznetwork.IPAllocationMethodStatic,
 		},
 	}
 }

tests/e2e/network/network_security_group.go

@@ -23,7 +23,7 @@ import (
 	"strings"
 	"time"
 
-	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
 
 	v1 "k8s.io/api/core/v1"

tests/e2e/network/node.go

@@ -24,7 +24,7 @@ import (
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-07-01/compute"
-	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 

tests/e2e/network/service_annotations.go

@@ -25,7 +25,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
 
 	appsv1 "k8s.io/api/apps/v1"

tests/e2e/utils/azure_test_client.go

@@ -25,7 +25,7 @@ import (
 	azauth "github.com/Azure/azure-sdk-for-go/services/authorization/mgmt/2015-07-01/authorization"
 	azcompute "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-07-01/compute"
 	acr "github.com/Azure/azure-sdk-for-go/services/containerregistry/mgmt/2019-05-01/containerregistry"
-	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	azresources "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-05-01/resources"
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/Azure/go-autorest/autorest/azure"

tests/e2e/utils/network_interface_utils.go

@@ -23,7 +23,7 @@ import (
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-07-01/compute"
-	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 )
 
 var (

tests/e2e/utils/network_utils.go

@@ -22,7 +22,7 @@ import (
 	"net"
 	"strings"
 
-	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
 
 	v1 "k8s.io/api/core/v1"

tests/e2e/utils/route_table_utils.go

@@ -20,7 +20,7 @@ import (
 	"context"
 	"fmt"
 
-	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 	"github.com/Azure/go-autorest/autorest/to"
 
 	providerazure "sigs.k8s.io/cloud-provider-azure/pkg/provider"

tests/e2e/utils/service_utils.go

@@ -26,7 +26,7 @@ import (
 
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 
-	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
+	aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network"
 
 	v1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"