diff --git a/pkg/ipam/crd.go b/pkg/ipam/crd.go index b6141165232fc..3686cce34b62f 100644 --- a/pkg/ipam/crd.go +++ b/pkg/ipam/crd.go @@ -13,6 +13,7 @@ import ( "reflect" "slices" "strconv" + "strings" "sync" "github.com/vishvananda/netlink" @@ -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" @@ -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 { @@ -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 @@ -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 diff --git a/pkg/ipam/crd_test.go b/pkg/ipam/crd_test.go index 8faaaad63b5f9..6442ca58ee7ba 100644 --- a/pkg/ipam/crd_test.go +++ b/pkg/ipam/crd_test.go @@ -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" @@ -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