Avoid unnecessary GCE API calls for IP-alias calls

This is to avoid unnecessary GCE API calls done by getInstanceByName
helper, which is iterating over all zones to find in which zone the
VM exists.
ProviderID already contains all the information - it's in the form:
gce://<VM URL> (VM URL contains project, zone, VM name).

ProviderID is propagated by Kubelet on node registration and in case
of bugs backfilled by node-controller.

Author: wojtek-t

pkg/controller/nodeipam/ipam/adapter.go

@@ -21,6 +21,7 @@ package ipam
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net"
 
 	"k8s.io/klog"
@@ -60,8 +61,12 @@ func newAdapter(k8s clientset.Interface, cloud *gce.Cloud) *adapter {
 	return ret
 }
 
-func (a *adapter) Alias(ctx context.Context, nodeName string) (*net.IPNet, error) {
-	cidrs, err := a.cloud.AliasRanges(types.NodeName(nodeName))
+func (a *adapter) Alias(ctx context.Context, node *v1.Node) (*net.IPNet, error) {
+	if node.Spec.ProviderID == "" {
+		return nil, fmt.Errorf("node %s doesn't have providerID", node.Name)
+	}
+
+	cidrs, err := a.cloud.AliasRangesByProviderID(node.Spec.ProviderID)
 	if err != nil {
 		return nil, err
 	}
@@ -72,7 +77,7 @@ func (a *adapter) Alias(ctx context.Context, nodeName string) (*net.IPNet, error
 	case 1:
 		break
 	default:
-		klog.Warningf("Node %q has more than one alias assigned (%v), defaulting to the first", nodeName, cidrs)
+		klog.Warningf("Node %q has more than one alias assigned (%v), defaulting to the first", node.Name, cidrs)
 	}
 
 	_, cidrRange, err := net.ParseCIDR(cidrs[0])
@@ -83,8 +88,12 @@ func (a *adapter) Alias(ctx context.Context, nodeName string) (*net.IPNet, error
 	return cidrRange, nil
 }
 
-func (a *adapter) AddAlias(ctx context.Context, nodeName string, cidrRange *net.IPNet) error {
-	return a.cloud.AddAliasToInstance(types.NodeName(nodeName), cidrRange)
+func (a *adapter) AddAlias(ctx context.Context, node *v1.Node, cidrRange *net.IPNet) error {
+	if node.Spec.ProviderID == "" {
+		return fmt.Errorf("node %s doesn't have providerID", node.Name)
+	}
+
+	return a.cloud.AddAliasToInstanceByProviderID(node.Spec.ProviderID, cidrRange)
 }
 
 func (a *adapter) Node(ctx context.Context, name string) (*v1.Node, error) {

pkg/controller/nodeipam/ipam/cloud_cidr_allocator.go

@@ -249,8 +249,11 @@ func (ca *cloudCIDRAllocator) updateCIDRAllocation(nodeName string) error {
 		klog.Errorf("Failed while getting node %v for updating Node.Spec.PodCIDR: %v", nodeName, err)
 		return err
 	}
+	if node.Spec.ProviderID == "" {
+		return fmt.Errorf("node %s doesn't have providerID", nodeName)
+	}
 
-	cidrs, err := ca.cloud.AliasRanges(types.NodeName(nodeName))
+	cidrs, err := ca.cloud.AliasRangesByProviderID(node.Spec.ProviderID)
 	if err != nil {
 		nodeutil.RecordNodeStatusChange(ca.recorder, node, "CIDRNotAvailable")
 		return fmt.Errorf("failed to allocate cidr: %v", err)

pkg/controller/nodeipam/ipam/sync/sync.go

@@ -43,9 +43,9 @@ const (
 // cloudAlias is the interface to the cloud platform APIs.
 type cloudAlias interface {
 	// Alias returns the IP alias for the node.
-	Alias(ctx context.Context, nodeName string) (*net.IPNet, error)
+	Alias(ctx context.Context, node *v1.Node) (*net.IPNet, error)
 	// AddAlias adds an alias to the node.
-	AddAlias(ctx context.Context, nodeName string, cidrRange *net.IPNet) error
+	AddAlias(ctx context.Context, node *v1.Node, cidrRange *net.IPNet) error
 }
 
 // kubeAPI is the interface to the Kubernetes APIs.
@@ -204,7 +204,7 @@ func (op *updateOp) run(sync *NodeSync) error {
 		op.node = node
 	}
 
-	aliasRange, err := sync.cloudAlias.Alias(ctx, sync.nodeName)
+	aliasRange, err := sync.cloudAlias.Alias(ctx, op.node)
 	if err != nil {
 		klog.Errorf("Error getting cloud alias for node %q: %v", sync.nodeName, err)
 		return err
@@ -293,7 +293,7 @@ func (op *updateOp) updateAliasFromNode(ctx context.Context, sync *NodeSync, nod
 		return err
 	}
 
-	if err := sync.cloudAlias.AddAlias(ctx, node.Name, aliasRange); err != nil {
+	if err := sync.cloudAlias.AddAlias(ctx, node, aliasRange); err != nil {
 		klog.Errorf("Could not add alias %v for node %q: %v", aliasRange, node.Name, err)
 		return err
 	}
@@ -325,7 +325,7 @@ func (op *updateOp) allocateRange(ctx context.Context, sync *NodeSync, node *v1.
 	// If addAlias returns a hard error, cidrRange will be leaked as there
 	// is no durable record of the range. The missing space will be
 	// recovered on the next restart of the controller.
-	if err := sync.cloudAlias.AddAlias(ctx, node.Name, cidrRange); err != nil {
+	if err := sync.cloudAlias.AddAlias(ctx, node, cidrRange); err != nil {
 		klog.Errorf("Could not add alias %v for node %q: %v", cidrRange, node.Name, err)
 		return err
 	}

pkg/controller/nodeipam/ipam/sync/sync_test.go

@@ -58,13 +58,13 @@ type fakeAPIs struct {
 	results []error
 }
 
-func (f *fakeAPIs) Alias(ctx context.Context, nodeName string) (*net.IPNet, error) {
-	f.calls = append(f.calls, fmt.Sprintf("alias %v", nodeName))
+func (f *fakeAPIs) Alias(ctx context.Context, node *v1.Node) (*net.IPNet, error) {
+	f.calls = append(f.calls, fmt.Sprintf("alias %v", node.Name))
 	return f.aliasRange, f.aliasErr
 }
 
-func (f *fakeAPIs) AddAlias(ctx context.Context, nodeName string, cidrRange *net.IPNet) error {
-	f.calls = append(f.calls, fmt.Sprintf("addAlias %v %v", nodeName, cidrRange))
+func (f *fakeAPIs) AddAlias(ctx context.Context, node *v1.Node, cidrRange *net.IPNet) error {
+	f.calls = append(f.calls, fmt.Sprintf("addAlias %v %v", node.Name, cidrRange))
 	return f.addAliasErr
 }
 

staging/src/k8s.io/legacy-cloud-providers/gce/gce_instances.go

@@ -361,21 +361,20 @@ func (g *Cloud) CurrentNodeName(ctx context.Context, hostname string) (types.Nod
 	return types.NodeName(hostname), nil
 }
 
-// AliasRanges returns a list of CIDR ranges that are assigned to the
+// AliasRangesByProviderID returns a list of CIDR ranges that are assigned to the
 // `node` for allocation to pods. Returns a list of the form
 // "<ip>/<netmask>".
-func (g *Cloud) AliasRanges(nodeName types.NodeName) (cidrs []string, err error) {
+func (g *Cloud) AliasRangesByProviderID(providerID string) (cidrs []string, err error) {
 	ctx, cancel := cloud.ContextWithCallTimeout()
 	defer cancel()
 
-	var instance *gceInstance
-	instance, err = g.getInstanceByName(mapNodeNameToInstanceName(nodeName))
+	_, zone, name, err := splitProviderID(providerID)
 	if err != nil {
-		return
+		return nil, err
 	}
 
 	var res *computebeta.Instance
-	res, err = g.c.BetaInstances().Get(ctx, meta.ZonalKey(instance.Name, lastComponent(instance.Zone)))
+	res, err = g.c.BetaInstances().Get(ctx, meta.ZonalKey(canonicalizeInstanceName(name), zone))
 	if err != nil {
 		return
 	}
@@ -388,28 +387,29 @@ func (g *Cloud) AliasRanges(nodeName types.NodeName) (cidrs []string, err error)
 	return
 }
 
-// AddAliasToInstance adds an alias to the given instance from the named
+// AddAliasToInstanceByProviderID adds an alias to the given instance from the named
 // secondary range.
-func (g *Cloud) AddAliasToInstance(nodeName types.NodeName, alias *net.IPNet) error {
+func (g *Cloud) AddAliasToInstanceByProviderID(providerID string, alias *net.IPNet) error {
 	ctx, cancel := cloud.ContextWithCallTimeout()
 	defer cancel()
 
-	v1instance, err := g.getInstanceByName(mapNodeNameToInstanceName(nodeName))
+	_, zone, name, err := splitProviderID(providerID)
 	if err != nil {
 		return err
 	}
-	instance, err := g.c.BetaInstances().Get(ctx, meta.ZonalKey(v1instance.Name, lastComponent(v1instance.Zone)))
+
+	instance, err := g.c.BetaInstances().Get(ctx, meta.ZonalKey(canonicalizeInstanceName(name), zone))
 	if err != nil {
 		return err
 	}
 
 	switch len(instance.NetworkInterfaces) {
 	case 0:
-		return fmt.Errorf("instance %q has no network interfaces", nodeName)
+		return fmt.Errorf("instance %q has no network interfaces", providerID)
 	case 1:
 	default:
 		klog.Warningf("Instance %q has more than one network interface, using only the first (%v)",
-			nodeName, instance.NetworkInterfaces)
+			providerID, instance.NetworkInterfaces)
 	}
 
 	iface := &computebeta.NetworkInterface{}
@@ -420,7 +420,7 @@ func (g *Cloud) AddAliasToInstance(nodeName types.NodeName, alias *net.IPNet) er
 		SubnetworkRangeName: g.secondaryRangeName,
 	})
 
-	mc := newInstancesMetricContext("add_alias", v1instance.Zone)
+	mc := newInstancesMetricContext("add_alias", zone)
 	err = g.c.BetaInstances().UpdateNetworkInterface(ctx, meta.ZonalKey(instance.Name, lastComponent(instance.Zone)), iface.Name, iface)
 	return mc.Observe(err)
 }