Release reserved GCE IP address after ensure completes.

Previous behavior was to release the ip only upon success, but it should be released
on failure as well. Otherwise, an IP address will be unnecessarily consumed due to an
in-error service.

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go

@@ -158,6 +158,12 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
 			return nil, err
 		}
 		klog.V(2).Infof("ensureInternalLoadBalancer(%v): reserved IP %q for the forwarding rule", loadBalancerName, ipToUse)
+		defer func() {
+			// Release the address if all resources were created successfully, or if we error out.
+			if err := addrMgr.ReleaseAddress(); err != nil {
+				klog.Errorf("ensureInternalLoadBalancer: failed to release address reservation, possibly causing an orphan: %v", err)
+			}
+		}()
 	}
 
 	// Ensure firewall rules if necessary
@@ -197,13 +203,6 @@ func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v
 		g.clearPreviousInternalResources(svc, loadBalancerName, existingBackendService, backendServiceName, hcName)
 	}
 
-	if addrMgr != nil {
-		// Now that the controller knows the forwarding rule exists, we can release the address.
-		if err := addrMgr.ReleaseAddress(); err != nil {
-			klog.Errorf("ensureInternalLoadBalancer: failed to release address reservation, possibly causing an orphan: %v", err)
-		}
-	}
-
 	// Get the most recent forwarding rule for the address.
 	updatedFwdRule, err := g.GetRegionForwardingRule(loadBalancerName, g.region)
 	if err != nil {

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go

@@ -950,6 +950,11 @@ func TestEnsureInternalLoadBalancerErrors(t *testing.T) {
 			)
 			assert.Error(t, err, "Should return an error when "+desc)
 			assert.Nil(t, status, "Should not return a status when "+desc)
+
+			// ensure that the temporarily reserved IP address is released upon sync errors
+			ip, err := gce.GetRegionAddress(gce.GetLoadBalancerName(context.TODO(), params.clusterName, params.service), gce.region)
+			require.Error(t, err)
+			assert.Nil(t, ip)
 		})
 	}
 }

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_utils_test.go

@@ -247,6 +247,11 @@ func assertInternalLbResources(t *testing.T, gce *Cloud, apiService *v1.Service,
 	assert.Equal(t, backendServiceLink, fwdRule.BackendService)
 	// if no Subnetwork specified, defaults to the GCE NetworkURL
 	assert.Equal(t, gce.NetworkURL(), fwdRule.Subnetwork)
+
+	// Check that the IP address has been released. IP is only reserved until ensure function exits.
+	ip, err := gce.GetRegionAddress(lbName, gce.region)
+	require.Error(t, err)
+	assert.Nil(t, ip)
 }
 
 func assertInternalLbResourcesDeleted(t *testing.T, gce *Cloud, apiService *v1.Service, vals TestClusterValues, firewallsDeleted bool) {
@@ -285,6 +290,11 @@ func assertInternalLbResourcesDeleted(t *testing.T, gce *Cloud, apiService *v1.S
 	healthcheck, err := gce.GetHealthCheck(hcName)
 	require.Error(t, err)
 	assert.Nil(t, healthcheck)
+
+	// Check that the IP address has been released
+	ip, err := gce.GetRegionAddress(lbName, gce.region)
+	require.Error(t, err)
+	assert.Nil(t, ip)
 }
 
 func checkEvent(t *testing.T, recorder *record.FakeRecorder, expected string, shouldMatch bool) bool {