From 687e593601a66471643eed54456042bf9285b959 Mon Sep 17 00:00:00 2001 From: Zihong Zheng Date: Thu, 18 Jan 2018 15:06:51 -0800 Subject: [PATCH] [GCE cloud provider] external lb - move target pool operation into its own function --- .../gce/gce_loadbalancer_external.go | 98 ++++++++++--------- 1 file changed, 53 insertions(+), 45 deletions(-) diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go index 5ae3cc1a27..ceaaf05359 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go +++ b/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. - var hcNames []string - 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) + + if err := gce.ensureTargetPoolAndHealthCheck(tpExists, tpNeedsRecreation, apiService, loadBalancerName, clusterID, ipAddressToUse, hosts, hcToCreate, hcToDelete); err != nil { + return nil, err } - // 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, }