From 605b106d2d0356918d3b2887c27e1ad85da9531d Mon Sep 17 00:00:00 2001 From: shashidharatd Date: Wed, 7 Jun 2017 07:55:12 +0530 Subject: [PATCH 1/3] Made CleanupGCEResources explicitly take zone parameter --- test/e2e/framework/service_util.go | 12 ++++++++++-- test/e2e/framework/util.go | 12 ++++++++---- test/e2e/service.go | 4 ++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/test/e2e/framework/service_util.go b/test/e2e/framework/service_util.go index 13813f55ef3..08aa96bb072 100644 --- a/test/e2e/framework/service_util.go +++ b/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 } diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 37cf069d130..7b57ce35398 100644 --- a/test/e2e/framework/util.go +++ b/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) } diff --git a/test/e2e/service.go b/test/e2e/service.go index d78dbc6c515..6588f1c0892 100644 --- a/test/e2e/service.go +++ b/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{} From f81c1e6702882299aa0984926890177036feb74e Mon Sep 17 00:00:00 2001 From: shashidharatd Date: Mon, 5 Jun 2017 22:38:34 +0530 Subject: [PATCH 2/3] Fix cleanupServiceShardLoadBalancer --- test/e2e_federation/framework/cluster.go | 7 ++++ test/e2e_federation/util.go | 45 +++++------------------- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/test/e2e_federation/framework/cluster.go b/test/e2e_federation/framework/cluster.go index c93af924bb3..012dfb52af6 100644 --- a/test/e2e_federation/framework/cluster.go +++ b/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) +} diff --git a/test/e2e_federation/util.go b/test/e2e_federation/util.go index cdc65d9ce5d..7641ee2138c 100644 --- a/test/e2e_federation/util.go +++ b/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 := "" From 25dfe33bdba4aa3952f946a52035055afc8db7c7 Mon Sep 17 00:00:00 2001 From: shashidharatd Date: Wed, 7 Jun 2017 11:07:32 +0530 Subject: [PATCH 3/3] Auto generated file --- test/e2e_federation/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e_federation/BUILD b/test/e2e_federation/BUILD index 0e885aa27f8..5891e427f3c 100644 --- a/test/e2e_federation/BUILD +++ b/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",