Skip to content

[ipam] Derive Azure native-routing CIDR from IPAM pool interfaces #673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jaredledvina wants to merge 1 commit into from
Closed

[ipam] Derive Azure native-routing CIDR from IPAM pool interfaces #673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 100 additions & 4 deletions pkg/ipam/crd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"reflect"
"slices"
"strconv"
"strings"
"sync"

"github.com/vishvananda/netlink"
Expand All @@ -23,6 +24,7 @@ import (
"k8s.io/client-go/tools/cache"

alibabaCloud "github.com/cilium/cilium/pkg/alibabacloud/utils"
azureTypes "github.com/cilium/cilium/pkg/azure/types"
"github.com/cilium/cilium/pkg/cidr"
"github.com/cilium/cilium/pkg/datapath/linux/sysctl"
"github.com/cilium/cilium/pkg/ip"
Expand Down Expand Up @@ -235,7 +237,54 @@ func newNodeStore(logger *slog.Logger, nodeName string, conf *option.DaemonConfi
return store
}

func deriveVpcCIDRs(node *ciliumv2.CiliumNode) (primaryCIDR *cidr.CIDR, secondaryCIDRs []*cidr.CIDR) {
// deriveAzureCIDRsFromPool returns the ID-sorted CIDRs of the Azure interfaces
// backing node's Spec.IPAM.Pool allocations, or nil if none resolve yet.
func deriveAzureCIDRsFromPool(logger *slog.Logger, node *ciliumv2.CiliumNode) []*cidr.CIDR {
if len(node.Spec.IPAM.Pool) == 0 {
return nil
}

poolResources := make(map[string]struct{}, len(node.Spec.IPAM.Pool))
for _, allocIP := range node.Spec.IPAM.Pool {
if allocIP.Resource != "" {
poolResources[allocIP.Resource] = struct{}{}
}
}
if len(poolResources) == 0 {
return nil
}

ifaces := make([]azureTypes.AzureInterface, 0, len(poolResources))
for _, azif := range node.Status.Azure.Interfaces {
if _, ok := poolResources[azif.ID]; !ok {
continue
}
ifaces = append(ifaces, azif)
}
slices.SortFunc(ifaces, func(a, b azureTypes.AzureInterface) int {
return strings.Compare(a.ID, b.ID)
})

cidrs := make([]*cidr.CIDR, 0, len(ifaces))
for _, azif := range ifaces {
c, err := cidr.ParseCIDR(azif.CIDR)
if err != nil {
if logger != nil {
logger.Warn(
"Unable to parse CIDR of Azure interface backing IPAM pool allocation, skipping",
logfields.Error, err,
logfields.CIDR, azif.CIDR,
logfields.Interface, azif.ID,
)
}
continue
}
cidrs = append(cidrs, c)
}
return cidrs
}

func deriveVpcCIDRs(logger *slog.Logger, node *ciliumv2.CiliumNode) (primaryCIDR *cidr.CIDR, secondaryCIDRs []*cidr.CIDR) {
// A node belongs to a single VPC so we can pick the first ENI
// in the list and derive the VPC CIDR from it.
for _, eni := range node.Status.ENI.ENIs {
Expand All @@ -251,8 +300,55 @@ func deriveVpcCIDRs(node *ciliumv2.CiliumNode) (primaryCIDR *cidr.CIDR, secondar
return
}
}
for _, azif := range node.Status.Azure.Interfaces {
c, err := cidr.ParseCIDR(azif.CIDR)
// Azure NICs can be on different subnets; each Spec.IPAM.Pool entry records
// (via Resource) its interface, so the backing CIDRs are the pool's
// interfaces' CIDRs, not an arbitrary single one.
if azureCIDRs := deriveAzureCIDRsFromPool(logger, node); len(azureCIDRs) > 0 {
primaryCIDR = azureCIDRs[0]
secondaryCIDRs = azureCIDRs[1:]
return
}
// No pool allocation yet: fall back to a deterministic choice -- the pinned
// Spec.Azure.InterfaceName if present and parseable, else the smallest-ID
// interface with a parseable CIDR.
requiredIfaceName := node.Spec.Azure.InterfaceName
var azureNamedIface, azureFallbackIface *azureTypes.AzureInterface
for i, azif := range node.Status.Azure.Interfaces {
if _, err := cidr.ParseCIDR(azif.CIDR); err != nil {
if logger != nil {
logger.Warn(
"Unable to parse Azure interface CIDR, skipping",
logfields.Error, err,
logfields.CIDR, azif.CIDR,
logfields.Interface, azif.ID,
)
}
continue
}
if requiredIfaceName != "" && azif.Name == requiredIfaceName {
azureNamedIface = &node.Status.Azure.Interfaces[i]
}
if azureFallbackIface == nil || azif.ID < azureFallbackIface.ID {
azureFallbackIface = &node.Status.Azure.Interfaces[i]
}
}
azurePrimaryIface := azureFallbackIface
if requiredIfaceName != "" {
if azureNamedIface != nil {
azurePrimaryIface = azureNamedIface
} else {
// Pinned interface absent/unparseable and no pool data; log it and
// use the deterministic fallback rather than disabling autodetection.
if logger != nil {
logger.Warn(
"Pinned Azure interface (Spec.Azure.InterfaceName) not found or has an unparseable CIDR; falling back to auto-selected primary VPC CIDR",
logfields.Interface, requiredIfaceName,
)
}
}
}
if azurePrimaryIface != nil {
c, err := cidr.ParseCIDR(azurePrimaryIface.CIDR)
if err == nil {
primaryCIDR = c
return
Expand All @@ -278,7 +374,7 @@ func deriveVpcCIDRs(node *ciliumv2.CiliumNode) (primaryCIDR *cidr.CIDR, secondar
}

func (n *nodeStore) autoDetectIPv4NativeRoutingCIDR(localNodeStore *node.LocalNodeStore) bool {
if primaryCIDR, secondaryCIDRs := deriveVpcCIDRs(n.ownNode); primaryCIDR != nil {
if primaryCIDR, secondaryCIDRs := deriveVpcCIDRs(n.logger, n.ownNode); primaryCIDR != nil {
allCIDRs := append([]*cidr.CIDR{primaryCIDR}, secondaryCIDRs...)
if nativeCIDR := n.conf.IPv4NativeRoutingCIDR; nativeCIDR != nil {
found := false
Expand Down
153 changes: 153 additions & 0 deletions pkg/ipam/crd_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

eniTypes "github.com/cilium/cilium/pkg/aws/eni/types"
azureTypes "github.com/cilium/cilium/pkg/azure/types"
"github.com/cilium/cilium/pkg/cidr"
fakeTypes "github.com/cilium/cilium/pkg/datapath/fake/types"
ipamOption "github.com/cilium/cilium/pkg/ipam/option"
ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
Expand Down Expand Up @@ -350,6 +351,158 @@ func TestAzureIPMasq(t *testing.T) {
ipMasqAgent.Stop()
}

// TestDeriveVpcCIDRsAzure covers the primary-CIDR selection criterion for
// Azure nodes: when Spec.Azure.InterfaceName pins the node to a specific
// interface, that interface's CIDR must be selected regardless of slice
// order; otherwise selection must be permutation-invariant (deterministic
// regardless of Status.Azure.Interfaces ordering), unparseable CIDRs must
// be skipped rather than aborting selection, and an empty/all-unparseable
// interface list must fall through cleanly without panicking.
func TestDeriveVpcCIDRsAzure(t *testing.T) {
logger := hivetest.Logger(t)

mkIfaces := func(order []int) []azureTypes.AzureInterface {
all := map[int]azureTypes.AzureInterface{
0: {ID: "intf-b", Name: "eth1", CIDR: "10.0.10.0/24"},
1: {ID: "intf-a", Name: "eth0", CIDR: "10.0.9.0/24"},
2: {ID: "intf-c", Name: "eth2", CIDR: "bogus-cidr"},
}
out := make([]azureTypes.AzureInterface, 0, len(order))
for _, i := range order {
out = append(out, all[i])
}
return out
}

t.Run("permutation invariant without configured interface name", func(t *testing.T) {
orderings := [][]int{{0, 1, 2}, {1, 0, 2}, {2, 0, 1}, {2, 1, 0}}
var want *cidr.CIDR
for i, order := range orderings {
cn := &ciliumv2.CiliumNode{}
cn.Status.Azure.Interfaces = mkIfaces(order)
got, secondary := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got, "ordering %v", order)
require.Empty(t, secondary)
if i == 0 {
want = got
} else {
require.Equal(t, want.String(), got.String(), "ordering %v selected a different primary CIDR than ordering %v", order, orderings[0])
}
}
// The deterministic winner must be one of the parseable CIDRs.
require.Contains(t, []string{"10.0.9.0/24", "10.0.10.0/24"}, want.String())
})

t.Run("configured interface name takes precedence over determinism criterion", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Spec.Azure.InterfaceName = "eth1"
cn.Status.Azure.Interfaces = mkIfaces([]int{1, 0, 2})
got, _ := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got)
require.Equal(t, "10.0.10.0/24", got.String())
})

t.Run("configured interface name not found falls back to deterministic selection", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Spec.Azure.InterfaceName = "eth99" // does not exist in mkIfaces
cn.Status.Azure.Interfaces = mkIfaces([]int{1, 0, 2})
got, secondary := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got)
require.Empty(t, secondary)
require.Equal(t, "10.0.9.0/24", got.String()) // intf-a, lexicographically smallest ID
})

t.Run("configured interface name matches but has unparseable CIDR falls back to deterministic selection", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Spec.Azure.InterfaceName = "eth2" // intf-c, CIDR is "bogus-cidr"
cn.Status.Azure.Interfaces = mkIfaces([]int{1, 0, 2})
got, secondary := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got)
require.Empty(t, secondary)
require.Equal(t, "10.0.9.0/24", got.String()) // intf-a, lexicographically smallest ID
})

t.Run("configured interface name set but no interfaces have a parseable CIDR returns nil", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Spec.Azure.InterfaceName = "eth1"
cn.Status.Azure.Interfaces = []azureTypes.AzureInterface{
{ID: "intf-1", Name: "eth1", CIDR: "not-a-cidr"},
}
got, secondary := deriveVpcCIDRs(logger, cn)
require.Nil(t, got)
require.Empty(t, secondary)
})

t.Run("all unparseable CIDRs falls through without panicking", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Status.Azure.Interfaces = []azureTypes.AzureInterface{
{ID: "intf-1", CIDR: "not-a-cidr"},
{ID: "intf-2", CIDR: ""},
}
got, secondary := deriveVpcCIDRs(logger, cn)
require.Nil(t, got)
require.Empty(t, secondary)
})

t.Run("empty interfaces slice falls through cleanly", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
got, secondary := deriveVpcCIDRs(logger, cn)
require.Nil(t, got)
require.Empty(t, secondary)
})

t.Run("pool spanning multiple interfaces returns all backing CIDRs, ignoring InterfaceName pin", func(t *testing.T) {
// intf-a and intf-c both back live pool allocations; intf-b does
// not and must not be selected even though it would otherwise win
// the lexicographic-ID tie-break, and even though InterfaceName is
// pinned to it.
cn := &ciliumv2.CiliumNode{}
cn.Spec.Azure.InterfaceName = "eth1" // intf-b, not in the pool
cn.Status.Azure.Interfaces = mkIfaces([]int{0, 1, 2}) // intf-b, intf-a, intf-c(bogus)
cn.Spec.IPAM.Pool = ipamTypes.AllocationMap{
"10.0.9.4": {Resource: "intf-a"},
"10.0.9.5": {Resource: "intf-a"},
"10.0.11.4": {Resource: "intf-d"},
}
cn.Status.Azure.Interfaces = append(cn.Status.Azure.Interfaces, azureTypes.AzureInterface{
ID: "intf-d", Name: "eth3", CIDR: "10.0.11.0/24",
})

got, secondary := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got)
require.Equal(t, "10.0.9.0/24", got.String(), "primary must be the lowest-ID interface actually backing the pool")
require.Len(t, secondary, 1)
require.Equal(t, "10.0.11.0/24", secondary[0].String())
})

t.Run("pool referencing an interface without a parseable CIDR is skipped but other pool interfaces are still returned", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Status.Azure.Interfaces = mkIfaces([]int{0, 1, 2}) // intf-b, intf-a, intf-c(bogus)
cn.Spec.IPAM.Pool = ipamTypes.AllocationMap{
"10.0.9.4": {Resource: "intf-a"},
"10.0.x.4": {Resource: "intf-c"}, // intf-c's CIDR is unparseable
}

got, secondary := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got)
require.Equal(t, "10.0.9.0/24", got.String())
require.Empty(t, secondary)
})

t.Run("pool referencing an interface not present in status falls through to auto-selected CIDR", func(t *testing.T) {
cn := &ciliumv2.CiliumNode{}
cn.Status.Azure.Interfaces = mkIfaces([]int{1, 0, 2})
cn.Spec.IPAM.Pool = ipamTypes.AllocationMap{
"10.0.99.4": {Resource: "intf-not-attached"},
}

got, secondary := deriveVpcCIDRs(logger, cn)
require.NotNil(t, got)
require.Empty(t, secondary)
require.Equal(t, "10.0.9.0/24", got.String(), "intf-a, lexicographically smallest ID fallback since the pool does not resolve to any attached interface")
})
}

func Test_validateENIConfig(t *testing.T) {
type args struct {
node *ciliumv2.CiliumNode
Expand Down
Loading