Skip to content

Commit

Permalink
GCE: filter addresses by IP when listing
Browse files Browse the repository at this point in the history
Also move the function to gce_addresses.go so that metrics can be
recorded for the call.
  • Loading branch information
yujuhong committed Aug 10, 2017
1 parent b86dd9a commit 58ea4e4
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 31 deletions.
29 changes: 29 additions & 0 deletions pkg/cloudprovider/providers/gce/gce_addresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ limitations under the License.
package gce

import (
"fmt"
"time"

"github.com/golang/glog"
compute "google.golang.org/api/compute/v1"
)

Expand Down Expand Up @@ -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))
}
17 changes: 15 additions & 2 deletions pkg/cloudprovider/providers/gce/gce_addresses_fakes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("")
}
32 changes: 3 additions & 29 deletions pkg/cloudprovider/providers/gce/gce_loadbalancer_external.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions pkg/cloudprovider/providers/gce/gce_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}

0 comments on commit 58ea4e4

Please sign in to comment.