diff --git a/pkg/cloudprovider/providers/gce/gce_addresses.go b/pkg/cloudprovider/providers/gce/gce_addresses.go index 3592fcf3b6c..0a7b0d19974 100644 --- a/pkg/cloudprovider/providers/gce/gce_addresses.go +++ b/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)) +} diff --git a/pkg/cloudprovider/providers/gce/gce_addresses_fakes.go b/pkg/cloudprovider/providers/gce/gce_addresses_fakes.go index bafb3e9ab0a..156aaeb6d23 100644 --- a/pkg/cloudprovider/providers/gce/gce_addresses_fakes.go +++ b/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("") +} diff --git a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go b/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go index e3e9b80212f..f8d3ff4603b 100644 --- a/pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go +++ b/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. diff --git a/pkg/cloudprovider/providers/gce/gce_util.go b/pkg/cloudprovider/providers/gce/gce_util.go index bcbd523c889..a28fb0d6255 100644 --- a/pkg/cloudprovider/providers/gce/gce_util.go +++ b/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} +}