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.
This commit is contained in:
wojtekt 2020-04-17 15:52:47 +02:00
parent 5655350b2b
commit 3758ab62d1
5 changed files with 40 additions and 28 deletions

View File

@ -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) {

View File

@ -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)

View File

@ -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
}

View File

@ -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
}

View File

@ -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)
}