Pipe clusterID into gce_loadbalancer_external.go

pkg/cloudprovider/providers/gce/gce_loadbalancer.go

@@ -135,7 +135,7 @@ func (gce *GCECloud) EnsureLoadBalancer(clusterName string, svc *v1.Service, nod
 			case schemeInternal:
 				err = gce.ensureInternalLoadBalancerDeleted(clusterName, clusterID, svc)
 			default:
-				err = gce.ensureExternalLoadBalancerDeleted(clusterName, svc)
+				err = gce.ensureExternalLoadBalancerDeleted(clusterName, clusterID, svc)
 			}
 			glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v): done deleting existing %v loadbalancer. err: %v", clusterName, svc.Namespace, svc.Name, loadBalancerName, gce.region, existingScheme, err)
 			if err != nil {
@@ -149,7 +149,7 @@ func (gce *GCECloud) EnsureLoadBalancer(clusterName string, svc *v1.Service, nod
 	case schemeInternal:
 		status, err = gce.ensureInternalLoadBalancer(clusterName, clusterID, svc, existingFwdRule, nodes)
 	default:
-		status, err = gce.ensureExternalLoadBalancer(clusterName, svc, existingFwdRule, nodes)
+		status, err = gce.ensureExternalLoadBalancer(clusterName, clusterID, svc, existingFwdRule, nodes)
 	}
 	glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v): done ensuring loadbalancer. err: %v", clusterName, svc.Namespace, svc.Name, loadBalancerName, gce.region, err)
 	return status, err
@@ -191,7 +191,7 @@ func (gce *GCECloud) EnsureLoadBalancerDeleted(clusterName string, svc *v1.Servi
 	case schemeInternal:
 		err = gce.ensureInternalLoadBalancerDeleted(clusterName, clusterID, svc)
 	default:
-		err = gce.ensureExternalLoadBalancerDeleted(clusterName, svc)
+		err = gce.ensureExternalLoadBalancerDeleted(clusterName, clusterID, svc)
 	}
 	glog.V(4).Infof("EnsureLoadBalancerDeleted(%v, %v, %v, %v, %v): done deleting loadbalancer. err: %v", clusterName, svc.Namespace, svc.Name, loadBalancerName, gce.region, err)
 	return err

pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go

@@ -42,7 +42,7 @@ import (
 // Due to an interesting series of design decisions, this handles both creating
 // new load balancers and updating existing load balancers, recognizing when
 // each is needed.
-func (gce *GCECloud) ensureExternalLoadBalancer(clusterName string, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
+func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, apiService *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
 	if len(nodes) == 0 {
 		return nil, fmt.Errorf("Cannot EnsureLoadBalancer() with no hosts")
 	}
@@ -222,10 +222,6 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName string, apiService *
 		glog.Infof("Target pool %v for Service %v/%v doesn't exist", loadBalancerName, apiService.Namespace, apiService.Name)
 	}
 
-	clusterID, err := gce.ClusterID.GetID()
-	if err != nil {
-		return nil, fmt.Errorf("error getting cluster ID %s: %v", loadBalancerName, err)
-	}
 	// Check which health check needs to create and which health check needs to delete.
 	// Health check management is coupled with target pool operation to prevent leaking.
 	var hcToCreate, hcToDelete *compute.HttpHealthCheck
@@ -283,7 +279,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName string, apiService *
 		if hcToDelete != nil {
 			hcNames = append(hcNames, hcToDelete.Name)
 		}
-		if err := gce.DeleteExternalTargetPoolAndChecks(loadBalancerName, gce.region, hcNames...); err != nil {
+		if err := gce.DeleteExternalTargetPoolAndChecks(loadBalancerName, gce.region, clusterID, hcNames...); err != nil {
 			return nil, fmt.Errorf("failed to delete existing target pool %s for load balancer update: %v", loadBalancerName, err)
 		}
 		glog.Infof("EnsureLoadBalancer(%v(%v)): deleted target pool", loadBalancerName, serviceName)
@@ -297,7 +293,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName string, apiService *
 			createInstances = createInstances[:maxTargetPoolCreateInstances]
 		}
 		// Pass healthchecks to createTargetPool which needs them as health check links in the target pool
-		if err := gce.createTargetPool(loadBalancerName, serviceName.String(), ipAddress, gce.region, createInstances, affinityType, hcToCreate); err != nil {
+		if err := gce.createTargetPool(loadBalancerName, serviceName.String(), ipAddress, gce.region, clusterID, createInstances, affinityType, hcToCreate); err != nil {
 			return nil, fmt.Errorf("failed to create target pool %s: %v", loadBalancerName, err)
 		}
 		if hcToCreate != nil {
@@ -358,7 +354,7 @@ func (gce *GCECloud) updateExternalLoadBalancer(clusterName string, service *v1.
 }
 
 // ensureExternalLoadBalancerDeleted is the external implementation of LoadBalancer.EnsureLoadBalancerDeleted
-func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName string, service *v1.Service) error {
+func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName, clusterID string, service *v1.Service) error {
 	loadBalancerName := cloudprovider.GetLoadBalancerName(service)
 
 	var hcNames []string
@@ -370,10 +366,6 @@ func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName string, servi
 		}
 		hcNames = append(hcNames, hcToDelete.Name)
 	} else {
-		clusterID, err := gce.ClusterID.GetID()
-		if err != nil {
-			return fmt.Errorf("error getting cluster ID %s: %v", loadBalancerName, err)
-		}
 		// EnsureLoadBalancerDeleted() could be triggered by changing service from
 		// LoadBalancer type to others. In this case we have no idea whether it was
 		// using local traffic health check or nodes health check. Attempt to delete
@@ -394,7 +386,7 @@ func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName string, servi
 			if err := ignoreNotFound(gce.DeleteRegionForwardingRule(loadBalancerName, gce.region)); err != nil {
 				return err
 			}
-			if err := gce.DeleteExternalTargetPoolAndChecks(loadBalancerName, gce.region, hcNames...); err != nil {
+			if err := gce.DeleteExternalTargetPoolAndChecks(loadBalancerName, gce.region, clusterID, hcNames...); err != nil {
 				return err
 			}
 			return nil
@@ -406,7 +398,7 @@ func (gce *GCECloud) ensureExternalLoadBalancerDeleted(clusterName string, servi
 	return nil
 }
 
-func (gce *GCECloud) DeleteExternalTargetPoolAndChecks(name, region string, hcNames ...string) error {
+func (gce *GCECloud) DeleteExternalTargetPoolAndChecks(name, region, clusterID string, hcNames ...string) error {
 	if err := gce.DeleteTargetPool(name, region); err != nil && isHTTPErrorCode(err, http.StatusNotFound) {
 		glog.Infof("Target pool %s already deleted. Continuing to delete other resources.", name)
 	} else if err != nil {
@@ -444,10 +436,6 @@ func (gce *GCECloud) DeleteExternalTargetPoolAndChecks(name, region string, hcNa
 				// We continue to delete the healthcheck firewall to prevent leaking.
 				glog.V(4).Infof("Health check %v is already deleted.", hcName)
 			}
-			clusterID, err := gce.ClusterID.GetID()
-			if err != nil {
-				return fmt.Errorf("error getting cluster ID: %v", err)
-			}
 			// If health check is deleted without error, it means no load-balancer is using it.
 			// So we should delete the health check firewall as well.
 			fwName := MakeHealthCheckFirewallName(clusterID, hcName, isNodesHealthCheck)
@@ -468,7 +456,7 @@ func (gce *GCECloud) DeleteExternalTargetPoolAndChecks(name, region string, hcNa
 	return nil
 }
 
-func (gce *GCECloud) createTargetPool(name, serviceName, ipAddress, region string, hosts []*gceInstance, affinityType v1.ServiceAffinity, hc *compute.HttpHealthCheck) error {
+func (gce *GCECloud) createTargetPool(name, serviceName, ipAddress, region, clusterID string, hosts []*gceInstance, affinityType v1.ServiceAffinity, hc *compute.HttpHealthCheck) error {
 	// health check management is coupled with targetPools to prevent leaks. A
 	// target pool is the only thing that requires a health check, so we delete
 	// associated checks on teardown, and ensure checks on setup.
@@ -482,7 +470,7 @@ func (gce *GCECloud) createTargetPool(name, serviceName, ipAddress, region strin
 			defer gce.sharedResourceLock.Unlock()
 		}
 		if !gce.OnXPN() {
-			if err := gce.ensureHttpHealthCheckFirewall(serviceName, ipAddress, region, hosts, hc.Name, int32(hc.Port), isNodesHealthCheck); err != nil {
+			if err := gce.ensureHttpHealthCheckFirewall(serviceName, ipAddress, region, clusterID, hosts, hc.Name, int32(hc.Port), isNodesHealthCheck); err != nil {
 				return err
 			}
 		}
@@ -776,12 +764,7 @@ func (gce *GCECloud) firewallNeedsUpdate(name, serviceName, region, ipAddress st
 	return true, false, nil
 }
 
-func (gce *GCECloud) ensureHttpHealthCheckFirewall(serviceName, ipAddress, region string, hosts []*gceInstance, hcName string, hcPort int32, isNodesHealthCheck bool) error {
-	clusterID, err := gce.ClusterID.GetID()
-	if err != nil {
-		return fmt.Errorf("error getting cluster ID: %v", err)
-	}
-
+func (gce *GCECloud) ensureHttpHealthCheckFirewall(serviceName, ipAddress, region, clusterID string, hosts []*gceInstance, hcName string, hcPort int32, isNodesHealthCheck bool) error {
 	// Prepare the firewall params for creating / checking.
 	desc := fmt.Sprintf(`{"kubernetes.io/cluster-id":"%s"}`, clusterID)
 	if !isNodesHealthCheck {