Merge pull request #2074 from k8s-infra-cherrypick-robot/cherry-pick-…

…2073-to-release-1.24 [release-1.24] Allow external service with floating ip disabled to use PLS
kubernetes-sigs · Jul 25, 2022 · c622e3b · c622e3b
2 parents c0d07b8 + ae3dc85
commit c622e3b
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 6 deletions.
diff --git a/pkg/provider/azure_privatelinkservice.go b/pkg/provider/azure_privatelinkservice.go
@@ -47,8 +47,8 @@ func (az *Cloud) reconcilePrivateLinkService(
 
 	if createPLS {
 		// Firstly, make sure it's internal service
-		if !requiresInternalLoadBalancer(service) {
-			return fmt.Errorf("reconcilePrivateLinkService for service(%s): service requiring private link service must be internal", serviceName)
+		if !requiresInternalLoadBalancer(service) && !consts.IsK8sServiceDisableLoadBalancerFloatingIP(service) {
+			return fmt.Errorf("reconcilePrivateLinkService for service(%s): service requiring private link service must be internal or disable floating ip", serviceName)
 		}
 
 		// Secondly, check if there is a private link service already created
@@ -518,8 +518,10 @@ func getPLSSubnetName(service *v1.Service) *string {
 		return &l
 	}
 
-	if l, found := service.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet]; found && strings.TrimSpace(l) != "" {
-		return &l
+	if requiresInternalLoadBalancer(service) {
+		if l, found := service.Annotations[consts.ServiceAnnotationLoadBalancerInternalSubnet]; found && strings.TrimSpace(l) != "" {
+			return &l
+		}
 	}
 
 	return nil

diff --git a/pkg/provider/azure_privatelinkservice_test.go b/pkg/provider/azure_privatelinkservice_test.go
@@ -56,13 +56,33 @@ func TestReconcilePrivateLinkService(t *testing.T) {
 			wantPLS: true,
 		},
 		{
-			desc: "reconcilePrivateLinkService should return error if service requires PLS but needs external LB",
+			desc: "reconcilePrivateLinkService should return error if service requires PLS but needs external LB and floating ip enabled",
 			annotations: map[string]string{
 				consts.ServiceAnnotationPLSCreation: "true",
 			},
 			wantPLS:       true,
 			expectedError: true,
 		},
+		{
+			desc: "reconcilePrivateLinkService should create a new PLS for external service with floating ip disabled",
+			annotations: map[string]string{
+				consts.ServiceAnnotationPLSCreation:                   "true",
+				consts.ServiceAnnotationDisableLoadBalancerFloatingIP: "true",
+			},
+			wantPLS:           true,
+			expectedSubnetGet: true,
+			existingSubnet: &network.Subnet{
+				Name: to.StringPtr("subnet"),
+				ID:   to.StringPtr("subnetID"),
+				SubnetPropertiesFormat: &network.SubnetPropertiesFormat{
+					PrivateLinkServiceNetworkPolicies: network.VirtualNetworkPrivateLinkServiceNetworkPoliciesDisabled,
+				},
+			},
+			expectedPLSList:   true,
+			existingPLSList:   []network.PrivateLinkService{},
+			expectedPLSCreate: true,
+			expectedPLS:       &network.PrivateLinkService{Name: to.StringPtr("pls-fipConfig")},
+		},
 		{
 			desc: "reconcilePrivateLinkService should create a new PLS if no existing PLS attached to the LB frontend",
 			annotations: map[string]string{
@@ -300,7 +320,10 @@ func TestReconcilePrivateLinkService(t *testing.T) {
 	for i, test := range testCases {
 		az := GetTestCloud(ctrl)
 		service := getTestServiceWithAnnotation("test", test.annotations, 80)
-		fipConfig := &network.FrontendIPConfiguration{ID: to.StringPtr("fipConfigID")}
+		fipConfig := &network.FrontendIPConfiguration{
+			Name: to.StringPtr("fipConfig"),
+			ID:   to.StringPtr("fipConfigID"),
+		}
 		clusterName := testClusterName
 
 		mockSubnetsClient := az.SubnetsClient.(*mocksubnetclient.MockInterface)
@@ -1329,6 +1352,7 @@ func TestGetPLSSubnetName(t *testing.T) {
 		{
 			desc: "Service with empty private link subnet specified but LB subnet specified should return LB subnet",
 			annotations: map[string]string{
+				consts.ServiceAnnotationLoadBalancerInternal:       "true",
 				consts.ServiceAnnotationPLSIpConfigurationSubnet:   "",
 				consts.ServiceAnnotationLoadBalancerInternalSubnet: "lb-subnet",
 			},
@@ -1337,13 +1361,15 @@ func TestGetPLSSubnetName(t *testing.T) {
 		{
 			desc: "Service with LB subnet specified should return it",
 			annotations: map[string]string{
+				consts.ServiceAnnotationLoadBalancerInternal:       "true",
 				consts.ServiceAnnotationLoadBalancerInternalSubnet: "lb-subnet",
 			},
 			expectedSubnet: to.StringPtr("lb-subnet"),
 		},
 		{
 			desc: "Service with both empty subnets specified should return nil",
 			annotations: map[string]string{
+				consts.ServiceAnnotationLoadBalancerInternal:       "true",
 				consts.ServiceAnnotationPLSIpConfigurationSubnet:   "",
 				consts.ServiceAnnotationLoadBalancerInternalSubnet: "",
 			},