Merge pull request #46979 from shashidharatd/fed-cleanup-cp-resource

Automatic merge from submit-queue (batch tested with PRs 46979, 47078, 47138, 46916)

[federation][e2e] Fix cleanupServiceShardLoadBalancer

**What this PR does / why we need it**:
Fixes the issue mentioned in #46976

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #46976

**Special notes for your reviewer**:

```release-note
NONE
```

test/e2e/framework/service_util.go

@@ -1300,9 +1300,17 @@ func VerifyServeHostnameServiceDown(c clientset.Interface, host string, serviceI
 	return fmt.Errorf("waiting for service to be down timed out")
 }
 
-func CleanupServiceGCEResources(loadBalancerName string) {
+func CleanupServiceResources(loadBalancerName, zone string) {
+	if TestContext.Provider == "gce" || TestContext.Provider == "gke" {
+		CleanupServiceGCEResources(loadBalancerName, zone)
+	}
+
+	// TODO: we need to add this function with other cloud providers, if there is a need.
+}
+
+func CleanupServiceGCEResources(loadBalancerName, zone string) {
 	if pollErr := wait.Poll(5*time.Second, LoadBalancerCleanupTimeout, func() (bool, error) {
-		if err := CleanupGCEResources(loadBalancerName); err != nil {
+		if err := CleanupGCEResources(loadBalancerName, zone); err != nil {
 			Logf("Still waiting for glbc to cleanup: %v", err)
 			return false, nil
 		}

test/e2e/framework/util.go

@@ -5158,21 +5158,25 @@ func (p *E2ETestNodePreparer) CleanupNodes() error {
 // CleanupGCEResources cleans up GCE Service Type=LoadBalancer resources with
 // the given name. The name is usually the UUID of the Service prefixed with an
 // alpha-numeric character ('a') to work around cloudprovider rules.
-func CleanupGCEResources(loadBalancerName string) (retErr error) {
+func CleanupGCEResources(loadBalancerName, zone string) (retErr error) {
 	gceCloud, err := GetGCECloud()
 	if err != nil {
 		return err
 	}
+	region, err := gcecloud.GetGCERegion(zone)
+	if err != nil {
+		return fmt.Errorf("error parsing GCE/GKE region from zone %q: %v", zone, err)
+	}
 	if err := gceCloud.DeleteFirewall(loadBalancerName); err != nil &&
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = err
 	}
-	if err := gceCloud.DeleteRegionForwardingRule(loadBalancerName, gceCloud.Region()); err != nil &&
+	if err := gceCloud.DeleteRegionForwardingRule(loadBalancerName, region); err != nil &&
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = fmt.Errorf("%v\n%v", retErr, err)
 
 	}
-	if err := gceCloud.DeleteRegionAddress(loadBalancerName, gceCloud.Region()); err != nil &&
+	if err := gceCloud.DeleteRegionAddress(loadBalancerName, region); err != nil &&
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = fmt.Errorf("%v\n%v", retErr, err)
 	}
@@ -5185,7 +5189,7 @@ func CleanupGCEResources(loadBalancerName string) (retErr error) {
 	if hc != nil {
 		hcNames = append(hcNames, hc.Name)
 	}
-	if err := gceCloud.DeleteExternalTargetPoolAndChecks(loadBalancerName, gceCloud.Region(), hcNames...); err != nil &&
+	if err := gceCloud.DeleteExternalTargetPoolAndChecks(loadBalancerName, region, hcNames...); err != nil &&
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = fmt.Errorf("%v\n%v", retErr, err)
 	}

test/e2e/service.go

@@ -57,7 +57,7 @@ var _ = framework.KubeDescribe("Services", func() {
 		}
 		for _, lb := range serviceLBNames {
 			framework.Logf("cleaning gce resource for %s", lb)
-			framework.CleanupServiceGCEResources(lb)
+			framework.CleanupServiceGCEResources(lb, framework.TestContext.CloudConfig.Zone)
 		}
 		//reset serviceLBNames
 		serviceLBNames = []string{}
@@ -1266,7 +1266,7 @@ var _ = framework.KubeDescribe("ESIPP [Slow]", func() {
 		}
 		for _, lb := range serviceLBNames {
 			framework.Logf("cleaning gce resource for %s", lb)
-			framework.CleanupServiceGCEResources(lb)
+			framework.CleanupServiceGCEResources(lb, framework.TestContext.CloudConfig.Zone)
 		}
 		//reset serviceLBNames
 		serviceLBNames = []string{}

test/e2e_federation/BUILD

@@ -35,6 +35,7 @@ go_library(
         "//pkg/api/v1:go_default_library",
         "//pkg/apis/extensions/v1beta1:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
+        "//pkg/cloudprovider:go_default_library",
         "//test/e2e/chaosmonkey:go_default_library",
         "//test/e2e/common:go_default_library",
         "//test/e2e/framework:go_default_library",

test/e2e_federation/framework/cluster.go

@@ -18,6 +18,7 @@ package framework
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -231,3 +232,9 @@ func restConfigForCluster(clusterConf *clusterConfig) *restclient.Config {
 	framework.ExpectNoError(err, fmt.Sprintf("Error creating client for cluster %q: %+v", clusterConf.name, err))
 	return restConfig
 }
+
+func GetZoneFromClusterName(clusterName string) string {
+	// Ref: https://github.com/kubernetes/kubernetes/blob/master/cluster/kube-util.sh#L55
+	prefix := "federation-e2e-" + framework.TestContext.Provider + "-"
+	return strings.TrimPrefix(clusterName, prefix)
+}

test/e2e_federation/util.go

@@ -27,9 +27,9 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	federationapi "k8s.io/kubernetes/federation/apis/federation/v1beta1"
 	fedclientset "k8s.io/kubernetes/federation/client/clientset_generated/federation_clientset"
-	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/v1"
 	kubeclientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
+	"k8s.io/kubernetes/pkg/cloudprovider"
 	"k8s.io/kubernetes/test/e2e/common"
 	"k8s.io/kubernetes/test/e2e/framework"
 	fedframework "k8s.io/kubernetes/test/e2e_federation/framework"
@@ -243,16 +243,18 @@ func cleanupServiceShardsAndProviderResources(namespace string, service *v1.Serv
 			continue
 		}
 
+		if cSvc.Spec.Type == v1.ServiceTypeLoadBalancer {
+			// In federation tests, e2e zone names are used to derive federation member cluster names
+			zone := fedframework.GetZoneFromClusterName(name)
+			serviceLBName := cloudprovider.GetLoadBalancerName(cSvc)
+			framework.Logf("cleaning cloud provider resource for service %q in namespace %q, in cluster %q", service.Name, namespace, name)
+			framework.CleanupServiceResources(serviceLBName, zone)
+		}
+
 		err = cleanupServiceShard(c.Clientset, name, namespace, cSvc, fedframework.FederatedDefaultTestTimeout)
 		if err != nil {
 			framework.Logf("Failed to delete service %q in namespace %q, in cluster %q: %v", service.Name, namespace, name, err)
 		}
-		if service.Spec.Type == v1.ServiceTypeLoadBalancer {
-			err = cleanupServiceShardLoadBalancer(name, cSvc, fedframework.FederatedDefaultTestTimeout)
-			if err != nil {
-				framework.Logf("Failed to delete cloud provider resources for service %q in namespace %q, in cluster %q, err: %v", service.Name, namespace, name, err)
-			}
-		}
 	}
 }
 
@@ -270,35 +272,6 @@ func cleanupServiceShard(clientset *kubeclientset.Clientset, clusterName, namesp
 	return err
 }
 
-func cleanupServiceShardLoadBalancer(clusterName string, service *v1.Service, timeout time.Duration) error {
-	provider := framework.TestContext.CloudConfig.Provider
-	if provider == nil {
-		return fmt.Errorf("cloud provider undefined")
-	}
-
-	internalSvc := &v1.Service{}
-	err := api.Scheme.Convert(service, internalSvc, nil)
-	if err != nil {
-		return fmt.Errorf("failed to convert versioned service object to internal type: %v", err)
-	}
-
-	err = wait.PollImmediate(framework.Poll, timeout, func() (bool, error) {
-		lbi, supported := provider.LoadBalancer()
-		if !supported {
-			return false, fmt.Errorf("%q doesn't support load balancers", provider.ProviderName())
-		}
-		err := lbi.EnsureLoadBalancerDeleted(clusterName, internalSvc)
-		if err != nil {
-			// Deletion failed with an error, try again.
-			framework.Logf("Failed to delete cloud provider resources for service %q in namespace %q, in cluster %q: %v", service.Name, service.Namespace, clusterName, err)
-			return false, nil
-		}
-		By(fmt.Sprintf("Cloud provider resources for Service %q in namespace %q in cluster %q deleted", service.Name, service.Namespace, clusterName))
-		return true, nil
-	})
-	return err
-}
-
 func podExitCodeDetector(f *fedframework.Framework, name, namespace string, code int32) func() error {
 	// If we ever get any container logs, stash them here.
 	logs := ""