Merge pull request #48855 from wojtek-t/automated-cherry-pick-of-#488…

…49-upstream-release-1.7

Automatic merge from submit-queue

Automated cherry pick of #48849 upstream release 1.7

Cherry pick of #48524 and #48849 on release-1.7.

#48849 : GCE: Fix panic when service loadbalancer has static IP address

pkg/cloudprovider/providers/gce/gce_addresses.go

@@ -33,18 +33,13 @@ func newAddressMetricContext(request, region string) *metricContext {
 // Caller is allocated a random IP if they do not specify an ipAddress. If an
 // ipAddress is specified, it must belong to the current project, eg: an
 // ephemeral IP associated with a global forwarding rule.
-func (gce *GCECloud) ReserveGlobalAddress(addr *compute.Address) (*compute.Address, error) {
+func (gce *GCECloud) ReserveGlobalAddress(addr *compute.Address) error {
 	mc := newAddressMetricContext("reserve", "")
 	op, err := gce.service.GlobalAddresses.Insert(gce.projectID, addr).Do()
 	if err != nil {
-		return nil, mc.Observe(err)
-	}
-
-	if err := gce.waitForGlobalOp(op, mc); err != nil {
-		return nil, err
+		return mc.Observe(err)
 	}
-
-	return gce.GetGlobalAddress(addr.Name)
+	return gce.waitForGlobalOp(op, mc)
 }
 
 // DeleteGlobalAddress deletes a global address by name.
@@ -65,17 +60,13 @@ func (gce *GCECloud) GetGlobalAddress(name string) (*compute.Address, error) {
 }
 
 // ReserveRegionAddress creates a region address
-func (gce *GCECloud) ReserveRegionAddress(addr *compute.Address, region string) (*compute.Address, error) {
+func (gce *GCECloud) ReserveRegionAddress(addr *compute.Address, region string) error {
 	mc := newAddressMetricContext("reserve", region)
 	op, err := gce.service.Addresses.Insert(gce.projectID, region, addr).Do()
 	if err != nil {
-		return nil, mc.Observe(err)
-	}
-	if err := gce.waitForRegionOp(op, region, mc); err != nil {
-		return nil, err
+		return mc.Observe(err)
 	}
-
-	return gce.GetRegionAddress(addr.Name, region)
+	return gce.waitForRegionOp(op, region, mc)
 }
 
 // DeleteRegionAddress deletes a region address by name.

pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go

@@ -939,14 +939,18 @@ func (gce *GCECloud) ensureStaticIP(name, serviceName, region, existingIP string
 		addressObj.Address = existingIP
 	}
 
-	address, err := gce.ReserveRegionAddress(addressObj, region)
-	if err != nil {
+	if err = gce.ReserveRegionAddress(addressObj, region); err != nil {
 		if !isHTTPErrorCode(err, http.StatusConflict) {
 			return "", false, fmt.Errorf("error creating gce static IP address: %v", err)
 		}
 		// StatusConflict == the IP exists already.
 		existed = true
 	}
 
-	return address.Address, existed, nil
+	addr, err := gce.GetRegionAddress(name, region)
+	if err != nil {
+		return "", false, fmt.Errorf("error getting static IP address: %v", err)
+	}
+
+	return addr.Address, existed, nil
 }

test/e2e/framework/ingress_utils.go

@@ -718,17 +718,22 @@ func (cont *GCEIngressController) Init() {
 func (cont *GCEIngressController) CreateStaticIP(name string) string {
 	gceCloud := cont.Cloud.Provider.(*gcecloud.GCECloud)
 	addr := &compute.Address{Name: name}
-	ip, err := gceCloud.ReserveGlobalAddress(addr)
-	if err != nil {
+	if err := gceCloud.ReserveGlobalAddress(addr); err != nil {
 		if delErr := gceCloud.DeleteGlobalAddress(name); delErr != nil {
 			if cont.isHTTPErrorCode(delErr, http.StatusNotFound) {
 				Logf("Static ip with name %v was not allocated, nothing to delete", name)
 			} else {
 				Logf("Failed to delete static ip %v: %v", name, delErr)
 			}
 		}
-		Failf("Failed to allocated static ip %v: %v", name, err)
+		Failf("Failed to allocate static ip %v: %v", name, err)
 	}
+
+	ip, err := gceCloud.GetGlobalAddress(name)
+	if err != nil {
+		Failf("Failed to get newly created static ip %v: %v", name, err)
+	}
+
 	cont.staticIPName = ip.Name
 	Logf("Reserved static ip %v: %v", cont.staticIPName, ip.Address)
 	return ip.Address