Skip to content

Commit 4820b6c

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#90242 from wojtek-t/efficient_alias_ranges
Avoid unnecessary GCE API calls for IP-alias calls
2 parents d92fdeb + 3758ab6 commit 4820b6c

File tree

5 files changed

+40
-28
lines changed

5 files changed

+40
-28
lines changed

‎pkg/controller/nodeipam/ipam/adapter.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package ipam
2121
import (
2222
"context"
2323
"encoding/json"
24+
"fmt"
2425
"net"
2526

2627
"k8s.io/klog"
@@ -60,8 +61,12 @@ func newAdapter(k8s clientset.Interface, cloud *gce.Cloud) *adapter {
6061
return ret
6162
}
6263

63-
func (a *adapter) Alias(ctx context.Context, nodeName string) (*net.IPNet, error) {
64-
cidrs, err := a.cloud.AliasRanges(types.NodeName(nodeName))
64+
func (a *adapter) Alias(ctx context.Context, node *v1.Node) (*net.IPNet, error) {
65+
if node.Spec.ProviderID == "" {
66+
return nil, fmt.Errorf("node %s doesn't have providerID", node.Name)
67+
}
68+
69+
cidrs, err := a.cloud.AliasRangesByProviderID(node.Spec.ProviderID)
6570
if err != nil {
6671
return nil, err
6772
}
@@ -72,7 +77,7 @@ func (a *adapter) Alias(ctx context.Context, nodeName string) (*net.IPNet, error
7277
case 1:
7378
break
7479
default:
75-
klog.Warningf("Node %q has more than one alias assigned (%v), defaulting to the first", nodeName, cidrs)
80+
klog.Warningf("Node %q has more than one alias assigned (%v), defaulting to the first", node.Name, cidrs)
7681
}
7782

7883
_, cidrRange, err := net.ParseCIDR(cidrs[0])
@@ -83,8 +88,12 @@ func (a *adapter) Alias(ctx context.Context, nodeName string) (*net.IPNet, error
8388
return cidrRange, nil
8489
}
8590

86-
func (a *adapter) AddAlias(ctx context.Context, nodeName string, cidrRange *net.IPNet) error {
87-
return a.cloud.AddAliasToInstance(types.NodeName(nodeName), cidrRange)
91+
func (a *adapter) AddAlias(ctx context.Context, node *v1.Node, cidrRange *net.IPNet) error {
92+
if node.Spec.ProviderID == "" {
93+
return fmt.Errorf("node %s doesn't have providerID", node.Name)
94+
}
95+
96+
return a.cloud.AddAliasToInstanceByProviderID(node.Spec.ProviderID, cidrRange)
8897
}
8998

9099
func (a *adapter) Node(ctx context.Context, name string) (*v1.Node, error) {

‎pkg/controller/nodeipam/ipam/cloud_cidr_allocator.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,11 @@ func (ca *cloudCIDRAllocator) updateCIDRAllocation(nodeName string) error {
249249
klog.Errorf("Failed while getting node %v for updating Node.Spec.PodCIDR: %v", nodeName, err)
250250
return err
251251
}
252+
if node.Spec.ProviderID == "" {
253+
return fmt.Errorf("node %s doesn't have providerID", nodeName)
254+
}
252255

253-
cidrs, err := ca.cloud.AliasRanges(types.NodeName(nodeName))
256+
cidrs, err := ca.cloud.AliasRangesByProviderID(node.Spec.ProviderID)
254257
if err != nil {
255258
nodeutil.RecordNodeStatusChange(ca.recorder, node, "CIDRNotAvailable")
256259
return fmt.Errorf("failed to allocate cidr: %v", err)

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ const (
4343
// cloudAlias is the interface to the cloud platform APIs.
4444
type cloudAlias interface {
4545
// Alias returns the IP alias for the node.
46-
Alias(ctx context.Context, nodeName string) (*net.IPNet, error)
46+
Alias(ctx context.Context, node *v1.Node) (*net.IPNet, error)
4747
// AddAlias adds an alias to the node.
48-
AddAlias(ctx context.Context, nodeName string, cidrRange *net.IPNet) error
48+
AddAlias(ctx context.Context, node *v1.Node, cidrRange *net.IPNet) error
4949
}
5050

5151
// kubeAPI is the interface to the Kubernetes APIs.
@@ -204,7 +204,7 @@ func (op *updateOp) run(sync *NodeSync) error {
204204
op.node = node
205205
}
206206

207-
aliasRange, err := sync.cloudAlias.Alias(ctx, sync.nodeName)
207+
aliasRange, err := sync.cloudAlias.Alias(ctx, op.node)
208208
if err != nil {
209209
klog.Errorf("Error getting cloud alias for node %q: %v", sync.nodeName, err)
210210
return err
@@ -293,7 +293,7 @@ func (op *updateOp) updateAliasFromNode(ctx context.Context, sync *NodeSync, nod
293293
return err
294294
}
295295

296-
if err := sync.cloudAlias.AddAlias(ctx, node.Name, aliasRange); err != nil {
296+
if err := sync.cloudAlias.AddAlias(ctx, node, aliasRange); err != nil {
297297
klog.Errorf("Could not add alias %v for node %q: %v", aliasRange, node.Name, err)
298298
return err
299299
}
@@ -325,7 +325,7 @@ func (op *updateOp) allocateRange(ctx context.Context, sync *NodeSync, node *v1.
325325
// If addAlias returns a hard error, cidrRange will be leaked as there
326326
// is no durable record of the range. The missing space will be
327327
// recovered on the next restart of the controller.
328-
if err := sync.cloudAlias.AddAlias(ctx, node.Name, cidrRange); err != nil {
328+
if err := sync.cloudAlias.AddAlias(ctx, node, cidrRange); err != nil {
329329
klog.Errorf("Could not add alias %v for node %q: %v", cidrRange, node.Name, err)
330330
return err
331331
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ type fakeAPIs struct {
5858
results []error
5959
}
6060

61-
func (f *fakeAPIs) Alias(ctx context.Context, nodeName string) (*net.IPNet, error) {
62-
f.calls = append(f.calls, fmt.Sprintf("alias %v", nodeName))
61+
func (f *fakeAPIs) Alias(ctx context.Context, node *v1.Node) (*net.IPNet, error) {
62+
f.calls = append(f.calls, fmt.Sprintf("alias %v", node.Name))
6363
return f.aliasRange, f.aliasErr
6464
}
6565

66-
func (f *fakeAPIs) AddAlias(ctx context.Context, nodeName string, cidrRange *net.IPNet) error {
67-
f.calls = append(f.calls, fmt.Sprintf("addAlias %v %v", nodeName, cidrRange))
66+
func (f *fakeAPIs) AddAlias(ctx context.Context, node *v1.Node, cidrRange *net.IPNet) error {
67+
f.calls = append(f.calls, fmt.Sprintf("addAlias %v %v", node.Name, cidrRange))
6868
return f.addAliasErr
6969
}
7070

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

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -361,21 +361,20 @@ func (g *Cloud) CurrentNodeName(ctx context.Context, hostname string) (types.Nod
361361
return types.NodeName(hostname), nil
362362
}
363363

364-
// AliasRanges returns a list of CIDR ranges that are assigned to the
364+
// AliasRangesByProviderID returns a list of CIDR ranges that are assigned to the
365365
// `node` for allocation to pods. Returns a list of the form
366366
// "<ip>/<netmask>".
367-
func (g *Cloud) AliasRanges(nodeName types.NodeName) (cidrs []string, err error) {
367+
func (g *Cloud) AliasRangesByProviderID(providerID string) (cidrs []string, err error) {
368368
ctx, cancel := cloud.ContextWithCallTimeout()
369369
defer cancel()
370370

371-
var instance *gceInstance
372-
instance, err = g.getInstanceByName(mapNodeNameToInstanceName(nodeName))
371+
_, zone, name, err := splitProviderID(providerID)
373372
if err != nil {
374-
return
373+
return nil, err
375374
}
376375

377376
var res *computebeta.Instance
378-
res, err = g.c.BetaInstances().Get(ctx, meta.ZonalKey(instance.Name, lastComponent(instance.Zone)))
377+
res, err = g.c.BetaInstances().Get(ctx, meta.ZonalKey(canonicalizeInstanceName(name), zone))
379378
if err != nil {
380379
return
381380
}
@@ -388,28 +387,29 @@ func (g *Cloud) AliasRanges(nodeName types.NodeName) (cidrs []string, err error)
388387
return
389388
}
390389

391-
// AddAliasToInstance adds an alias to the given instance from the named
390+
// AddAliasToInstanceByProviderID adds an alias to the given instance from the named
392391
// secondary range.
393-
func (g *Cloud) AddAliasToInstance(nodeName types.NodeName, alias *net.IPNet) error {
392+
func (g *Cloud) AddAliasToInstanceByProviderID(providerID string, alias *net.IPNet) error {
394393
ctx, cancel := cloud.ContextWithCallTimeout()
395394
defer cancel()
396395

397-
v1instance, err := g.getInstanceByName(mapNodeNameToInstanceName(nodeName))
396+
_, zone, name, err := splitProviderID(providerID)
398397
if err != nil {
399398
return err
400399
}
401-
instance, err := g.c.BetaInstances().Get(ctx, meta.ZonalKey(v1instance.Name, lastComponent(v1instance.Zone)))
400+
401+
instance, err := g.c.BetaInstances().Get(ctx, meta.ZonalKey(canonicalizeInstanceName(name), zone))
402402
if err != nil {
403403
return err
404404
}
405405

406406
switch len(instance.NetworkInterfaces) {
407407
case 0:
408-
return fmt.Errorf("instance %q has no network interfaces", nodeName)
408+
return fmt.Errorf("instance %q has no network interfaces", providerID)
409409
case 1:
410410
default:
411411
klog.Warningf("Instance %q has more than one network interface, using only the first (%v)",
412-
nodeName, instance.NetworkInterfaces)
412+
providerID, instance.NetworkInterfaces)
413413
}
414414

415415
iface := &computebeta.NetworkInterface{}
@@ -420,7 +420,7 @@ func (g *Cloud) AddAliasToInstance(nodeName types.NodeName, alias *net.IPNet) er
420420
SubnetworkRangeName: g.secondaryRangeName,
421421
})
422422

423-
mc := newInstancesMetricContext("add_alias", v1instance.Zone)
423+
mc := newInstancesMetricContext("add_alias", zone)
424424
err = g.c.BetaInstances().UpdateNetworkInterface(ctx, meta.ZonalKey(instance.Name, lastComponent(instance.Zone)), iface.Name, iface)
425425
return mc.Observe(err)
426426
}

0 commit comments

Comments
 (0)