Repository: kubernetes-sigs/cloud-provider-azure
Pull request #512 (2021-02-17 10:41:50)

Made load balancer and public ip extended location aware.

**What type of PR is this?**
/kind feature


**What this PR does / why we need it**:
Adds extended location to the cloud config. 
If extended location is provided and a service is created, the corresponding slb and public ip are deployed with extended location.


**Special notes for your reviewer**:


**Release note**:
```
Adds extended location to the cloud config. 
If extended location is provided and a service is created, the corresponding slb and public ip are deployed with extended location.
```


k8s-ci-robot: Hi @MirzaSikander. Thanks for your PR.

I'm waiting for a [kubernetes-sigs](https://github.com/orgs/kubernetes-sigs/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://git.k8s.io/community/community-membership.md#member) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/cloud-provider-azure/pull/512#" title="Author self-approved">MirzaSikander</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **nilo19** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @nilo19` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes-sigs%2Fcloud-provider-azure).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes-sigs/cloud-provider-azure/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["nilo19"]} -->
nilo19: /ok-to-test
k8s-ci-robot: @MirzaSikander: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-cloud-provider-azure-unit | 3435c069c48aec00ba0dbd123234150bf430ea45 | [link](https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cloud-provider-azure/512/pull-cloud-provider-azure-unit/1362665581286789120) | `/test pull-cloud-provider-azure-unit`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=cloud-provider-azure&pr=512). [Your PR dashboard](https://prow.k8s.io/pr?query=is%3Apr%20state%3Aopen%20author%3AMirzaSikander). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes-sigs/cloud-provider-azure/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->

Patch:
diff --git a/pkg/provider/azure.go b/pkg/provider/azure.go
index 8e593d0f7..1aff1ffae 100644
--- a/pkg/provider/azure.go
+++ b/pkg/provider/azure.go
@@ -133,6 +133,12 @@ type Config struct {
 	ResourceGroup string `json:"resourceGroup,omitempty" yaml:"resourceGroup,omitempty"`
 	// The location of the resource group that the cluster is deployed in
 	Location string `json:"location,omitempty" yaml:"location,omitempty"`
+	// The name of site where the cluster will be deployed to that is more granular than the region specified by the "location" field.
+	// Currently only public ip and load balancer support this.
+	ExtendedLocationName string `json:"extendedLocationName,omitempty" yaml:"extendedLocationName,omitempty"`
+	// The type of site that is being targeted.
+	// Currently only public ip and load balancer support this.
+	ExtendedLocationType string `json:"extendedLocationType,omitempty" yaml:"extendedLocationType,omitempty"`
 	// The name of the VNet that the cluster is deployed in
 	VnetName string `json:"vnetName,omitempty" yaml:"vnetName,omitempty"`
 	// The name of the resource group that the Vnet is deployed in
@@ -236,6 +242,11 @@ type Config struct {
 	Tags string `json:"tags,omitempty" yaml:"tags,omitempty"`
 }
 
+// HasExtendedLocation returns true if extendedlocation prop are specified.
+func (config *Config) HasExtendedLocation() bool {
+	return config.ExtendedLocationName != "" && config.ExtendedLocationType != ""
+}
+
 var (
 	_ cloudprovider.Interface    = (*Cloud)(nil)
 	_ cloudprovider.Instances    = (*Cloud)(nil)
diff --git a/pkg/provider/azure_fakes.go b/pkg/provider/azure_fakes.go
index 614ffbf3c..7b6058500 100644
--- a/pkg/provider/azure_fakes.go
+++ b/pkg/provider/azure_fakes.go
@@ -94,3 +94,11 @@ func GetTestCloud(ctrl *gomock.Controller) (az *Cloud) {
 
 	return az
 }
+
+// GetTestCloudWithExtendedLocation returns a fake azure cloud for unit tests in Azure related CSI drivers with extended location.
+func GetTestCloudWithExtendedLocation(ctrl *gomock.Controller) (az *Cloud) {
+	az = GetTestCloud(ctrl)
+	az.Config.ExtendedLocationName = "microsoftlosangeles1"
+	az.Config.ExtendedLocationType = "EdgeZone"
+	return az
+}
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
index 17882b6c5..6ee0fc347 100644
--- a/pkg/provider/azure_loadbalancer.go
+++ b/pkg/provider/azure_loadbalancer.go
@@ -550,6 +550,12 @@ func (az *Cloud) getServiceLoadBalancer(service *v1.Service, clusterName string,
 				Name: network.LoadBalancerSkuNameStandard,
 			}
 		}
+		if az.HasExtendedLocation() {
+			defaultLB.ExtendedLocation = &network.ExtendedLocation{
+				Name: &az.ExtendedLocationName,
+				Type: &az.ExtendedLocationType,
+			}
+		}
 	}
 
 	return defaultLB, nil, false, nil
@@ -593,6 +599,12 @@ func (az *Cloud) selectLoadBalancer(clusterName string, service *v1.Service, exi
 				Sku:                          &network.LoadBalancerSku{Name: loadBalancerSKU},
 				LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{},
 			}
+			if az.HasExtendedLocation() {
+				selectedLB.ExtendedLocation = &network.ExtendedLocation{
+					Name: &az.ExtendedLocationName,
+					Type: &az.ExtendedLocationType,
+				}
+			}
 
 			return selectedLB, false, nil
 		}
@@ -823,6 +835,12 @@ func (az *Cloud) ensurePublicIPExists(service *v1.Service, pipName string, domai
 		}
 		pip.Name = to.StringPtr(pipName)
 		pip.Location = to.StringPtr(az.Location)
+		if az.HasExtendedLocation() {
+			pip.ExtendedLocation = &network.ExtendedLocation{
+				Name: &az.ExtendedLocationName,
+				Type: &az.ExtendedLocationType,
+			}
+		}
 		pip.PublicIPAddressPropertiesFormat = &network.PublicIPAddressPropertiesFormat{
 			PublicIPAllocationMethod: network.Static,
 			IPTags:                   getServiceIPTagRequestForPublicIP(service).IPTags,


Test patch:
diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
index 04d098e71..9ab439b8b 100644
--- a/pkg/provider/azure_loadbalancer_test.go
+++ b/pkg/provider/azure_loadbalancer_test.go
@@ -1258,6 +1258,62 @@ func TestGetServiceLoadBalancer(t *testing.T) {
 	}
 }
 
+func TestGetServiceLoadBalancerWithExtendedLocation(t *testing.T) {
+	service := getTestService("service1", v1.ProtocolTCP, nil, false, 80)
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	az := GetTestCloudWithExtendedLocation(ctrl)
+	clusterResources, expectedInterfaces, expectedVirtualMachines := getClusterResources(az, 3, 3)
+	setMockEnv(az, ctrl, expectedInterfaces, expectedVirtualMachines, 1)
+
+	// Test with wantLB=false
+	expectedLB := &network.LoadBalancer{
+		Name:     to.StringPtr("testCluster"),
+		Location: to.StringPtr("westus"),
+		ExtendedLocation: &network.ExtendedLocation{
+			Name: to.StringPtr("microsoftlosangeles1"),
+			Type: to.StringPtr("EdgeZone"),
+		},
+		LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{},
+	}
+	mockLBsClient := mockloadbalancerclient.NewMockInterface(ctrl)
+	mockLBsClient.EXPECT().List(gomock.Any(), "rg").Return(nil, nil)
+	az.LoadBalancerClient = mockLBsClient
+
+	lb, status, exists, err := az.getServiceLoadBalancer(&service, testClusterName,
+		clusterResources.nodes, false)
+	assert.Equal(t, expectedLB, lb, "GetServiceLoadBalancer shall return a default LB with expected location.")
+	assert.Nil(t, status, "GetServiceLoadBalancer: Status should be nil for default LB.")
+	assert.Equal(t, false, exists, "GetServiceLoadBalancer: Default LB should not exist.")
+	assert.NoError(t, err, "GetServiceLoadBalancer: No error should be thrown when returning default LB.")
+
+	// Test with wantLB=true
+	expectedLB = &network.LoadBalancer{
+		Name:     to.StringPtr("testCluster"),
+		Location: to.StringPtr("westus"),
+		ExtendedLocation: &network.ExtendedLocation{
+			Name: to.StringPtr("microsoftlosangeles1"),
+			Type: to.StringPtr("EdgeZone"),
+		},
+		LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{},
+		Sku: &network.LoadBalancerSku{
+			Name: network.LoadBalancerSkuName("Basic"),
+			Tier: network.LoadBalancerSkuTier(""),
+		},
+	}
+	mockLBsClient = mockloadbalancerclient.NewMockInterface(ctrl)
+	mockLBsClient.EXPECT().List(gomock.Any(), "rg").Return(nil, nil)
+	az.LoadBalancerClient = mockLBsClient
+
+	lb, status, exists, err = az.getServiceLoadBalancer(&service, testClusterName,
+		clusterResources.nodes, true)
+	assert.Equal(t, expectedLB, lb, "GetServiceLoadBalancer shall return a new LB with expected location.")
+	assert.Nil(t, status, "GetServiceLoadBalancer: Status should be nil for new LB.")
+	assert.Equal(t, false, exists, "GetServiceLoadBalancer: LB should not exist before hand.")
+	assert.NoError(t, err, "GetServiceLoadBalancer: No error should be thrown when returning new LB.")
+}
+
 func TestIsFrontendIPChanged(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -3092,6 +3148,42 @@ func TestEnsurePublicIPExists(t *testing.T) {
 		}
 	}
 }
+func TestEnsurePublicIPExistsWithExtendedLocation(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	az := GetTestCloudWithExtendedLocation(ctrl)
+	service := getTestService("test1", v1.ProtocolTCP, nil, false, 80)
+
+	expectedPIP := &network.PublicIPAddress{
+		Name:     to.StringPtr("pip1"),
+		Location: &az.location,
+		ExtendedLocation: &network.ExtendedLocation{
+			Name: to.StringPtr("microsoftlosangeles1"),
+			Type: to.StringPtr("EdgeZone"),
+		},
+		PublicIPAddressPropertiesFormat: &network.PublicIPAddressPropertiesFormat{
+			PublicIPAllocationMethod: "Static",
+			PublicIPAddressVersion:   "IPv4",
+			ProvisioningState:        "",
+		},
+		Tags: map[string]*string{
+			"service":                 to.StringPtr("default/test1"),
+			"kubernetes-cluster-name": to.StringPtr(""),
+		},
+	}
+	mockPIPsClient := az.PublicIPAddressesClient.(*mockpublicipclient.MockInterface)
+	first := mockPIPsClient.EXPECT().Get(gomock.Any(), "rg", "pip1", gomock.Any()).Return(network.PublicIPAddress{}, &retry.Error{
+		HTTPStatusCode: 404,
+	})
+	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)
+	pip, err := az.ensurePublicIPExists(&service, "pip1", "", "", false, false)
+	assert.Equal(t, expectedPIP, 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.")
+}
 
 func TestShouldUpdateLoadBalancer(t *testing.T) {
 	ctrl := gomock.NewController(t)