Merge pull request #52751 from MrHohn/e2e-service-cleanup-fix

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

Fix GCE LB resource cleanup for service e2e tests.

**What this PR does / why we need it**: Fix GCE LB resource cleanup logic.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #52347

**Special notes for your reviewer**:
/assign @shyamjvs @nicksardo 

**Release note**:

```release-note
NONE
```
pull/6/head
Kubernetes Submit Queue 2017-09-24 05:21:16 -07:00 committed by GitHub
commit 8c29b6540b
3 changed files with 20 additions and 18 deletions

View File

@ -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)
@ -761,7 +761,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

View File

@ -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)
}

View File

@ -4802,7 +4802,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
}
@ -4815,7 +4815,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)
@ -4824,11 +4829,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)