Merge pull request #50341 from yujuhong/filter-ip-list

Automatic merge from submit-queue

GCE: filter IP by address when listing

pkg/cloudprovider/providers/gce/gce_addresses.go

@@ -17,8 +17,10 @@ limitations under the License.
 package gce
 
 import (
+	"fmt"
 	"time"
 
+	"github.com/golang/glog"
 	compute "google.golang.org/api/compute/v1"
 )
 
@@ -85,3 +87,30 @@ func (gce *GCECloud) GetRegionAddress(name, region string) (*compute.Address, er
 	v, err := gce.service.Addresses.Get(gce.projectID, region, name).Do()
 	return v, mc.Observe(err)
 }
+
+// GetRegionAddressByIP returns the regional address matching the given IP
+// address.
+func (gce *GCECloud) GetRegionAddressByIP(region, ipAddress string) (*compute.Address, error) {
+	mc := newAddressMetricContext("list", region)
+	addrs, err := gce.service.Addresses.List(gce.projectID, region).Filter("address eq " + ipAddress).Do()
+	// Record the metrics for the call.
+	mc.Observe(err)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(addrs.Items) > 1 {
+		// We don't expect more than one match.
+		addrsToPrint := []compute.Address{}
+		for _, addr := range addrs.Items {
+			addrsToPrint = append(addrsToPrint, *addr)
+		}
+		glog.Errorf("More than one addresses matching the IP %q: %+v", ipAddress, addrsToPrint)
+	}
+	for _, addr := range addrs.Items {
+		if addr.Address == ipAddress {
+			return addr, nil
+		}
+	}
+	return nil, makeGoogleAPINotFoundError(fmt.Sprintf("Address with IP %q was not found in region %q", ipAddress, region))
+}

pkg/cloudprovider/providers/gce/gce_addresses_fakes.go

@@ -66,12 +66,25 @@ func (cas *FakeCloudAddressService) ReserveRegionAddress(addr *compute.Address,
 
 func (cas *FakeCloudAddressService) GetRegionAddress(name, region string) (*compute.Address, error) {
 	if _, exists := cas.addrsByRegionAndName[region]; !exists {
-		return nil, &googleapi.Error{Code: http.StatusNotFound}
+		return nil, makeGoogleAPINotFoundError("")
 	}
 
 	if addr, exists := cas.addrsByRegionAndName[region][name]; !exists {
-		return nil, &googleapi.Error{Code: http.StatusNotFound}
+		return nil, makeGoogleAPINotFoundError("")
 	} else {
 		return addr, nil
 	}
 }
+
+func (cas *FakeCloudAddressService) GetRegionAddressByIP(region, ipAddress string) (*compute.Address, error) {
+	if _, exists := cas.addrsByRegionAndName[region]; !exists {
+		return nil, makeGoogleAPINotFoundError("")
+	}
+
+	for _, addr := range cas.addrsByRegionAndName[region] {
+		if addr.Address == ipAddress {
+			return addr, nil
+		}
+	}
+	return nil, makeGoogleAPINotFoundError("")
+}

pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go

@@ -127,14 +127,14 @@ func (gce *GCECloud) ensureExternalLoadBalancer(clusterName, clusterID string, a
 		// a different IP, it will be harmlessly abandoned because it was only an
 		// ephemeral IP (or it was a different static IP owned by the user, in which
 		// case we shouldn't delete it anyway).
-		if isStatic, err := gce.projectOwnsStaticIP(loadBalancerName, gce.region, loadBalancerIP); err != nil {
+		if existingAddress, err := gce.GetRegionAddressByIP(gce.region, loadBalancerIP); err != nil && !isNotFound(err) {
 			return nil, fmt.Errorf("failed to test if this GCE project owns the static IP %s: %v", loadBalancerIP, err)
-		} else if isStatic {
+		} else if err == nil {
 			// The requested IP is a static IP, owned and managed by the user.
 			isUserOwnedIP = true
 			isSafeToReleaseIP = false
 			ipAddress = loadBalancerIP
-			glog.V(4).Infof("EnsureLoadBalancer(%v(%v)): using user-provided static IP %s", loadBalancerName, serviceName, ipAddress)
+			glog.V(4).Infof("EnsureLoadBalancer(%v(%v)): using user-provided static IP %s (name: %s)", loadBalancerName, serviceName, ipAddress, existingAddress.Name)
 		} else if loadBalancerIP == fwdRuleIP {
 			// The requested IP is not a static IP, but is currently assigned
 			// to this forwarding rule, so we can keep it.
@@ -880,32 +880,6 @@ func (gce *GCECloud) firewallObject(name, region, desc string, sourceRanges nets
 	return firewall, nil
 }
 
-func (gce *GCECloud) projectOwnsStaticIP(name, region string, ipAddress string) (bool, error) {
-	pageToken := ""
-	page := 0
-	for ; page == 0 || (pageToken != "" && page < maxPages); page++ {
-		listCall := gce.service.Addresses.List(gce.projectID, region)
-		if pageToken != "" {
-			listCall = listCall.PageToken(pageToken)
-		}
-		addresses, err := listCall.Do()
-		if err != nil {
-			return false, fmt.Errorf("failed to list gce IP addresses: %v", err)
-		}
-		pageToken = addresses.NextPageToken
-		for _, addr := range addresses.Items {
-			if addr.Address == ipAddress {
-				// This project does own the address, so return success.
-				return true, nil
-			}
-		}
-	}
-	if page >= maxPages {
-		glog.Errorf("projectOwnsStaticIP exceeded maxPages=%d for Addresses.List; truncating.", maxPages)
-	}
-	return false, nil
-}
-
 func ensureStaticIP(s CloudAddressService, name, serviceName, region, existingIP string) (ipAddress string, existing bool, err error) {
 	// If the address doesn't exist, this will create it.
 	// If the existingIP exists but is ephemeral, this will promote it to static.

pkg/cloudprovider/providers/gce/gce_util.go

@@ -149,3 +149,7 @@ func ignoreNotFound(err error) error {
 func isNotFoundOrInUse(err error) bool {
 	return isNotFound(err) || isInUsedByError(err)
 }
+
+func makeGoogleAPINotFoundError(message string) error {
+	return &googleapi.Error{Code: http.StatusNotFound, Message: message}
+}