[GCE cloud provider] external lb - move target pool operation into

its own function

pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go

@@ -63,8 +63,6 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 		portStr = append(portStr, fmt.Sprintf("%s/%d", p.Protocol, p.Port))
 	}
 
-	affinityType := apiService.Spec.SessionAffinity
-
 	serviceName := types.NamespacedName{Namespace: apiService.Namespace, Name: apiService.Name}
 	lbRefStr := fmt.Sprintf("%v(%v)", loadBalancerName, serviceName)
 	glog.V(2).Infof("ensureExternalLoadBalancer(%s, %v, %v, %v, %v, %v)", lbRefStr, gce.region, requestedIP, portStr, hostNames, apiService.Annotations)
@@ -192,7 +190,7 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 		}
 	}
 
-	tpExists, tpNeedsRecreation, err := gce.targetPoolNeedsRecreation(loadBalancerName, gce.region, affinityType)
+	tpExists, tpNeedsRecreation, err := gce.targetPoolNeedsRecreation(loadBalancerName, gce.region, apiService.Spec.SessionAffinity)
 	if err != nil {
 		return nil, err
 	}
@@ -251,48 +249,11 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 		}
 		glog.Infof("ensureExternalLoadBalancer(%s): Deleted forwarding rule.", lbRefStr)
 	}
-	if tpExists && tpNeedsRecreation {
+
-		// Pass healthchecks to DeleteExternalTargetPoolAndChecks to cleanup health checks after cleaning up the target pool itself.
+	if err := gce.ensureTargetPoolAndHealthCheck(tpExists, tpNeedsRecreation, apiService, loadBalancerName, clusterID, ipAddressToUse, hosts, hcToCreate, hcToDelete); err != nil {
-		var hcNames []string
+		return nil, err
-		if hcToDelete != nil {
-			hcNames = append(hcNames, hcToDelete.Name)
-		}
-		if err := gce.DeleteExternalTargetPoolAndChecks(apiService, loadBalancerName, gce.region, clusterID, hcNames...); err != nil {
-			return nil, fmt.Errorf("failed to delete existing target pool for load balancer (%s) update: %v", lbRefStr, err)
-		}
-		glog.Infof("ensureExternalLoadBalancer(%s): Deleted target pool.", lbRefStr)
 	}
 
-	// Once we've deleted the resources (if necessary), build them back up (or for
-	// the first time if they're new).
-	if tpNeedsRecreation {
-		createInstances := hosts
-		if len(hosts) > maxTargetPoolCreateInstances {
-			createInstances = createInstances[:maxTargetPoolCreateInstances]
-		}
-		// Pass healthchecks to createTargetPool which needs them as health check links in the target pool
-		if err := gce.createTargetPool(apiService, loadBalancerName, serviceName.String(), ipAddressToUse, gce.region, clusterID, createInstances, affinityType, hcToCreate); err != nil {
-			return nil, fmt.Errorf("failed to create target pool for load balancer (%s): %v", lbRefStr, err)
-		}
-		if hcToCreate != nil {
-			glog.Infof("ensureExternalLoadBalancer(%s): Created health checks %v.", lbRefStr, hcToCreate.Name)
-		}
-		if len(hosts) <= maxTargetPoolCreateInstances {
-			glog.Infof("ensureExternalLoadBalancer(%s): Created target pool.", lbRefStr)
-		} else {
-			glog.Infof("ensureExternalLoadBalancer(%s): Created initial target pool (now updating the remaining %d hosts).", lbRefStr, len(hosts)-maxTargetPoolCreateInstances)
-			if err := gce.updateTargetPool(loadBalancerName, hosts); err != nil {
-				return nil, fmt.Errorf("failed to update target pool for load balancer (%s): %v", lbRefStr, err)
-			}
-			glog.Infof("ensureExternalLoadBalancer(%s): Updated target pool (with %d hosts).", lbRefStr, len(hosts)-maxTargetPoolCreateInstances)
-		}
-	} else if tpExists {
-		// Ensure hosts are updated even if there is no other changes required on target pool.
-		if err := gce.updateTargetPool(loadBalancerName, hosts); err != nil {
-			return nil, fmt.Errorf("failed to update target pool for load balancer (%s): %v", lbRefStr, err)
-		}
-		glog.Infof("ensureExternalLoadBalancer(%s): Updated target pool (with %d hosts).", lbRefStr, len(hosts))
-	}
 	if tpNeedsRecreation || fwdRuleNeedsUpdate {
 		glog.Infof("ensureExternalLoadBalancer(%s): Creating forwarding rule, IP %s (tier: %s).", lbRefStr, ipAddressToUse, netTier)
 		if err := createForwardingRule(gce, loadBalancerName, serviceName.String(), gce.region, ipAddressToUse, gce.targetPoolURL(loadBalancerName), ports, netTier); err != nil {
@@ -500,7 +461,54 @@ func verifyUserRequestedIP(s CloudAddressService, region, requestedIP, fwdRuleIP
 	return false, fmt.Errorf("requested ip %q is neither static nor assigned to the LB", requestedIP)
 }
 
-func (gce *GCECloud) createTargetPool(svc *v1.Service, name, serviceName, ipAddress, region, clusterID string, hosts []*gceInstance, affinityType v1.ServiceAffinity, hc *compute.HttpHealthCheck) error {
+func (gce *GCECloud) ensureTargetPoolAndHealthCheck(tpExists, tpNeedsRecreation bool, svc *v1.Service, loadBalancerName, clusterID, ipAddressToUse string, hosts []*gceInstance, hcToCreate, hcToDelete *compute.HttpHealthCheck) error {
+	serviceName := types.NamespacedName{Namespace: svc.Namespace, Name: svc.Name}
+	lbRefStr := fmt.Sprintf("%v(%v)", loadBalancerName, serviceName)
+
+	if tpExists && tpNeedsRecreation {
+		// Pass healthchecks to DeleteExternalTargetPoolAndChecks to cleanup health checks after cleaning up the target pool itself.
+		var hcNames []string
+		if hcToDelete != nil {
+			hcNames = append(hcNames, hcToDelete.Name)
+		}
+		if err := gce.DeleteExternalTargetPoolAndChecks(svc, loadBalancerName, gce.region, clusterID, hcNames...); err != nil {
+			return fmt.Errorf("failed to delete existing target pool for load balancer (%s) update: %v", lbRefStr, err)
+		}
+		glog.Infof("ensureTargetPoolAndHealthCheck(%s): Deleted target pool.", lbRefStr)
+	}
+	// Once we've deleted the resources (if necessary), build them back up (or for
+	// the first time if they're new).
+	if tpNeedsRecreation {
+		createInstances := hosts
+		if len(hosts) > maxTargetPoolCreateInstances {
+			createInstances = createInstances[:maxTargetPoolCreateInstances]
+		}
+		if err := gce.createTargetPoolAndHealthCheck(svc, loadBalancerName, serviceName.String(), ipAddressToUse, gce.region, clusterID, createInstances, hcToCreate); err != nil {
+			return fmt.Errorf("failed to create target pool for load balancer (%s): %v", lbRefStr, err)
+		}
+		if hcToCreate != nil {
+			glog.Infof("ensureTargetPoolAndHealthCheck(%s): Created health checks %v.", lbRefStr, hcToCreate.Name)
+		}
+		if len(hosts) <= maxTargetPoolCreateInstances {
+			glog.Infof("ensureTargetPoolAndHealthCheck(%s): Created target pool.", lbRefStr)
+		} else {
+			glog.Infof("ensureTargetPoolAndHealthCheck(%s): Created initial target pool (now updating the remaining %d hosts).", lbRefStr, len(hosts)-maxTargetPoolCreateInstances)
+			if err := gce.updateTargetPool(loadBalancerName, hosts); err != nil {
+				return fmt.Errorf("failed to update target pool for load balancer (%s): %v", lbRefStr, err)
+			}
+			glog.Infof("ensureTargetPoolAndHealthCheck(%s): Updated target pool (with %d hosts).", lbRefStr, len(hosts)-maxTargetPoolCreateInstances)
+		}
+	} else if tpExists {
+		// Ensure hosts are updated even if there is no other changes required on target pool.
+		if err := gce.updateTargetPool(loadBalancerName, hosts); err != nil {
+			return fmt.Errorf("failed to update target pool for load balancer (%s): %v", lbRefStr, err)
+		}
+		glog.Infof("ensureTargetPoolAndHealthCheck(%s): Updated target pool (with %d hosts).", lbRefStr, len(hosts))
+	}
+	return nil
+}
+
+func (gce *GCECloud) createTargetPoolAndHealthCheck(svc *v1.Service, name, serviceName, ipAddress, region, clusterID string, hosts []*gceInstance, 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.
@@ -534,7 +542,7 @@ func (gce *GCECloud) createTargetPool(svc *v1.Service, name, serviceName, ipAddr
 		Name:            name,
 		Description:     fmt.Sprintf(`{"kubernetes.io/service-name":"%s"}`, serviceName),
 		Instances:       instances,
-		SessionAffinity: translateAffinityType(affinityType),
+		SessionAffinity: translateAffinityType(svc.Spec.SessionAffinity),
 		HealthChecks:    hcLinks,
 	}