Fix GCE LB resource cleanup for service e2e tests.

pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go

@@ -181,13 +181,13 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 		// without needing to be deleted and recreated.
 		if firewallExists {
 			glog.Infof("EnsureLoadBalancer(%v(%v)): updating firewall", loadBalancerName, serviceName)
-			if err := gce.updateFirewall(apiService, makeFirewallName(loadBalancerName), gce.region, desc, sourceRanges, ports, hosts); err != nil {
+			if err := gce.updateFirewall(apiService, MakeFirewallName(loadBalancerName), gce.region, desc, sourceRanges, ports, hosts); err != nil {
 				return nil, err
 			}
 			glog.Infof("EnsureLoadBalancer(%v(%v)): updated firewall", loadBalancerName, serviceName)
 		} else {
 			glog.Infof("EnsureLoadBalancer(%v(%v)): creating firewall", loadBalancerName, serviceName)
-			if err := gce.createFirewall(apiService, makeFirewallName(loadBalancerName), gce.region, desc, sourceRanges, ports, hosts); err != nil {
+			if err := gce.createFirewall(apiService, MakeFirewallName(loadBalancerName), gce.region, desc, sourceRanges, ports, hosts); err != nil {
 				return nil, err
 			}
 			glog.Infof("EnsureLoadBalancer(%v(%v)): created firewall", loadBalancerName, serviceName)
@@ -217,7 +217,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 			// target pool to use local traffic health check.
 			glog.V(2).Infof("Updating from nodes health checks to local traffic health checks for service %v LB %v", apiService.Name, loadBalancerName)
 			if supportsNodesHealthCheck {
-				hcToDelete = makeHttpHealthCheck(makeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort())
+				hcToDelete = makeHttpHealthCheck(MakeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort())
 			}
 			tpNeedsUpdate = true
 		}
@@ -233,7 +233,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 			tpNeedsUpdate = true
 		}
 		if supportsNodesHealthCheck {
-			hcToCreate = makeHttpHealthCheck(makeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort())
+			hcToCreate = makeHttpHealthCheck(MakeNodesHealthCheckName(clusterID), GetNodesHealthCheckPath(), GetNodesHealthCheckPort())
 		}
 	}
 	// Now we get to some slightly more interesting logic.
@@ -351,12 +351,12 @@ func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName, clusterID st
 		// using local traffic health check or nodes health check. Attempt to delete
 		// both to prevent leaking.
 		hcNames = append(hcNames, loadBalancerName)
-		hcNames = append(hcNames, makeNodesHealthCheckName(clusterID))
+		hcNames = append(hcNames, MakeNodesHealthCheckName(clusterID))
 	}
 
 	errs := utilerrors.AggregateGoroutines(
 		func() error {
-			fwName := makeFirewallName(loadBalancerName)
+			fwName := MakeFirewallName(loadBalancerName)
 			err := ignoreNotFound(gce.DeleteFirewall(fwName))
 			if isForbidden(err) && gce.OnXPN() {
 				glog.V(4).Infof("ensureExternalLoadBalancerDeleted(%v): do not have permission to delete firewall rule (on XPN). Raising event.", loadBalancerName)
@@ -760,7 +760,7 @@ func translateAffinityType(affinityType v1.ServiceAffinity) string {
 }
 
 func (gce *GCECloud) firewallNeedsUpdate(name, serviceName, region, ipAddress string, ports []v1.ServicePort, sourceRanges netsets.IPNet) (exists bool, needsUpdate bool, err error) {
-	fw, err := gce.service.Firewalls.Get(gce.NetworkProjectID(), makeFirewallName(name)).Do()
+	fw, err := gce.service.Firewalls.Get(gce.NetworkProjectID(), MakeFirewallName(name)).Do()
 	if err != nil {
 		if isHTTPErrorCode(err, http.StatusNotFound) {
 			return false, true, nil

pkg/cloudprovider/providers/gce/gce_loadbalancer_naming.go

@@ -89,9 +89,9 @@ func makeServiceDescription(serviceName string) string {
 	return fmt.Sprintf(`{"kubernetes.io/service-name":"%s"}`, serviceName)
 }
 
-// makeNodesHealthCheckName returns name of the health check resource used by
+// MakeNodesHealthCheckName returns name of the health check resource used by
 // the GCE load balancers (l4) for performing health checks on nodes.
-func makeNodesHealthCheckName(clusterID string) string {
+func MakeNodesHealthCheckName(clusterID string) string {
 	return fmt.Sprintf("k8s-%v-node", clusterID)
 }
 
@@ -103,12 +103,14 @@ func makeHealthCheckDescription(serviceName string) string {
 // balancers (l4) for performing health checks.
 func MakeHealthCheckFirewallName(clusterID, hcName string, isNodesHealthCheck bool) string {
 	if isNodesHealthCheck {
-		return makeNodesHealthCheckName(clusterID) + "-http-hc"
+		return MakeNodesHealthCheckName(clusterID) + "-http-hc"
 	}
 	return "k8s-" + hcName + "-http-hc"
 }
 
-func makeFirewallName(name string) string {
+// MakeFirewallName returns the firewall name used by the GCE load
+// balancers (l4) for serving traffic.
+func MakeFirewallName(name string) string {
 	return fmt.Sprintf("k8s-fw-%s", name)
 }
 

test/e2e/framework/util.go

@@ -4775,7 +4775,7 @@ func CleanupGCEResources(c clientset.Interface, loadBalancerName, zone string) (
 	if err != nil {
 		return fmt.Errorf("error parsing GCE/GKE region from zone %q: %v", zone, err)
 	}
-	if err := gceCloud.DeleteFirewall(loadBalancerName); err != nil &&
+	if err := gceCloud.DeleteFirewall(gcecloud.MakeFirewallName(loadBalancerName)); err != nil &&
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = err
 	}
@@ -4788,7 +4788,12 @@ func CleanupGCEResources(c clientset.Interface, loadBalancerName, zone string) (
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = fmt.Errorf("%v\n%v", retErr, err)
 	}
-	var hcNames []string
+	clusterID, err := GetClusterID(c)
+	if err != nil {
+		retErr = fmt.Errorf("%v\n%v", retErr, err)
+		return
+	}
+	hcNames := []string{gcecloud.MakeNodesHealthCheckName(clusterID)}
 	hc, getErr := gceCloud.GetHttpHealthCheck(loadBalancerName)
 	if getErr != nil && !IsGoogleAPIHTTPErrorCode(getErr, http.StatusNotFound) {
 		retErr = fmt.Errorf("%v\n%v", retErr, getErr)
@@ -4797,11 +4802,6 @@ func CleanupGCEResources(c clientset.Interface, loadBalancerName, zone string) (
 	if hc != nil {
 		hcNames = append(hcNames, hc.Name)
 	}
-	clusterID, err := GetClusterID(c)
-	if err != nil {
-		retErr = fmt.Errorf("%v\n%v", retErr, err)
-		return
-	}
 	if err := gceCloud.DeleteExternalTargetPoolAndChecks(nil, loadBalancerName, region, clusterID, hcNames...); err != nil &&
 		!IsGoogleAPIHTTPErrorCode(err, http.StatusNotFound) {
 		retErr = fmt.Errorf("%v\n%v", retErr, err)