Merge pull request #709 from MirzaSikander/user/mirsik/cp-az-0.7

Not send availability zones as part of create for edge zones.
kubernetes-sigs · Jul 18, 2021 · 449c2cf · 449c2cf
2 parents 50e9f4f + efb2861
commit 449c2cf
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 13 deletions.
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
@@ -1027,6 +1027,7 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai
 		pip.Name = to.StringPtr(pipName)
 		pip.Location = to.StringPtr(az.Location)
 		if az.HasExtendedLocation() {
+			klog.V(2).Infof("Using extended location with name %s, and type %s for PIP", az.ExtendedLocationName, az.ExtendedLocationType)
 			pip.ExtendedLocation = &network.ExtendedLocation{
 				Name: &az.ExtendedLocationName,
 				Type: &az.ExtendedLocationType,
@@ -1049,13 +1050,16 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai
 				Name: network.PublicIPAddressSkuNameStandard,
 			}
 
-			// only add zone information for the new standard pips
-			zones, err := az.getRegionZonesBackoff(to.String(pip.Location))
-			if err != nil {
-				return nil, err
-			}
-			if len(zones) > 0 {
-				pip.Zones = &zones
+			// skip adding zone info since edge zones doesn't support multiple availability zones.
+			if !az.HasExtendedLocation() {
+				// only add zone information for the new standard pips
+				zones, err := az.getRegionZonesBackoff(to.String(pip.Location))
+				if err != nil {
+					return nil, err
+				}
+				if len(zones) > 0 {
+					pip.Zones = &zones
+				}
 			}
 		}
 		klog.V(2).Infof("ensurePublicIPExists for service(%s): pip(%s) - creating", serviceName, *pip.Name)
@@ -1789,13 +1793,13 @@ func (az *Cloud) reconcileFrontendIPConfigs(clusterName string, service *v1.Serv
 				FrontendIPConfigurationPropertiesFormat: fipConfigurationProperties,
 			}
 
-			// only add zone information for new internal frontend IP configurations
+			// 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 {
+			if isInternal && az.useStandardLoadBalancer() && len(zones) > 0 && !az.HasExtendedLocation() {
 				newConfig.Zones = &zones
 			}
 			newConfigs = append(newConfigs, newConfig)

diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
@@ -3404,14 +3404,17 @@ func TestEnsurePublicIPExistsWithExtendedLocation(t *testing.T) {
 	defer ctrl.Finish()
 
 	az := GetTestCloudWithExtendedLocation(ctrl)
+	az.LoadBalancerSku = consts.LoadBalancerSkuStandard
 	service := getTestService("test1", v1.ProtocolTCP, nil, false, 80)
 
+	exLocName := "microsoftlosangeles1"
+	exLocType := "EdgeZone"
 	expectedPIP := &network.PublicIPAddress{
 		Name:     to.StringPtr("pip1"),
 		Location: &az.location,
 		ExtendedLocation: &network.ExtendedLocation{
-			Name: to.StringPtr("microsoftlosangeles1"),
-			Type: to.StringPtr("EdgeZone"),
+			Name: to.StringPtr(exLocName),
+			Type: to.StringPtr(exLocType),
 		},
 		PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
 			PublicIPAllocationMethod: "Static",
@@ -3429,9 +3432,18 @@ func TestEnsurePublicIPExistsWithExtendedLocation(t *testing.T) {
 	})
 	mockPIPsClient.EXPECT().Get(gomock.Any(), "rg", "pip1", gomock.Any()).Return(*expectedPIP, nil).After(first)
 
-	mockPIPsClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", "pip1", gomock.Any()).Return(nil).Times(1)
+	mockPIPsClient.EXPECT().CreateOrUpdate(gomock.Any(), "rg", "pip1", gomock.Any()).
+		DoAndReturn(func(ctx context.Context, resourceGroupName string, publicIPAddressName string, publicIPAddressParameters network.PublicIPAddress) *retry.Error {
+			assert.NotNil(t, publicIPAddressParameters)
+			assert.NotNil(t, publicIPAddressParameters.ExtendedLocation)
+			assert.Equal(t, *publicIPAddressParameters.ExtendedLocation.Name, exLocName)
+			assert.Equal(t, *publicIPAddressParameters.ExtendedLocation.Type, exLocType)
+			// Edge zones don't support availability zones.
+			assert.Nil(t, publicIPAddressParameters.Zones)
+			return nil
+		}).Times(1)
 	pip, err := az.ensurePublicIPExists(&service, "pip1", "", "", false, false)
-	assert.Equal(t, expectedPIP, pip, "ensurePublicIPExists shall create a new pip"+
+	assert.NotNil(t, pip, "ensurePublicIPExists shall create a new pip"+
 		"with extendedLocation if there is no existed pip")
 	assert.Nil(t, err, "ensurePublicIPExists should create a new pip without errors.")
 }