Merge pull request #48849 from nicksardo/gce-panic-fix

Kubernetes Submit Queue · web-flow · commit 74f19437742a · 2017-07-12T23:59:03.000-07:00
Automatic merge from submit-queue (batch tested with PRs 48555, 48849) GCE: Fix panic when service loadbalancer has static IP address Fixes #48848 ```release-note Fix service controller crash loop when Service with GCP LoadBalancer uses static IP (#48848, @nicksardo) ```
diff --git a/pkg/cloudprovider/providers/gce/gce_addresses.go b/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.
diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go
@@ -921,14 +921,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
 }
diff --git a/test/e2e/framework/ingress_utils.go b/test/e2e/framework/ingress_utils.go
@@ -715,17 +715,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

Original file line number	Diff line number	Diff line change
`@@ -921,14 +921,18 @@ func (gce *GCECloud) ensureStaticIP(name, serviceName, region, existingIP string`
`921`	`921`	`addressObj.Address = existingIP`
`922`	`922`	`}`
`923`	`923`
`924`		`- address, err := gce.ReserveRegionAddress(addressObj, region)`
`925`		`- if err != nil {`
	`924`	`+ if err = gce.ReserveRegionAddress(addressObj, region); err != nil {`
`926`	`925`	`if !isHTTPErrorCode(err, http.StatusConflict) {`
`927`	`926`	`return "", false, fmt.Errorf("error creating gce static IP address: %v", err)`
`928`	`927`	`}`
`929`	`928`	`// StatusConflict == the IP exists already.`
`930`	`929`	`existed = true`
`931`	`930`	`}`
`932`	`931`
`933`		`- return address.Address, existed, nil`
	`932`	`+ addr, err := gce.GetRegionAddress(name, region)`
	`933`	`+ if err != nil {`
	`934`	`+ return "", false, fmt.Errorf("error getting static IP address: %v", err)`
	`935`	`+ }`
	`936`	`+`
	`937`	`+ return addr.Address, existed, nil`
`934`	`938`	`}`