diff --git a/pkg/azure/ipam/node.go b/pkg/azure/ipam/node.go index 73f59fe18a708..db0881147e538 100644 --- a/pkg/azure/ipam/node.go +++ b/pkg/azure/ipam/node.go @@ -44,17 +44,21 @@ func (n *Node) UpdatedNode(obj *v2.CiliumNode) { // PopulateStatusFields fills in the status field of the CiliumNode custom // resource with Azure specific information func (n *Node) PopulateStatusFields(k8sObj *v2.CiliumNode) { - k8sObj.Status.Azure.Interfaces = []types.AzureInterface{} + interfaces := []types.AzureInterface{} n.manager.mutex.RLock() defer n.manager.mutex.RUnlock() n.manager.instances.ForeachInterface(n.node.InstanceID(), func(instanceID, interfaceID string, interfaceObj ipamTypes.InterfaceRevision) error { iface, ok := interfaceObj.Resource.(*types.AzureInterface) if ok { - k8sObj.Status.Azure.Interfaces = append(k8sObj.Status.Azure.Interfaces, *(iface.DeepCopy())) + interfaces = append(interfaces, *(iface.DeepCopy())) } return nil }) + + // ForeachInterface iterates a Go map, so order is randomized per call; + // SetInterfaces sorts by ID for a stable slice. + k8sObj.Status.Azure.SetInterfaces(interfaces) } // PrepareIPRelease prepares the release of IPs diff --git a/pkg/azure/ipam/node_test.go b/pkg/azure/ipam/node_test.go index da0318d7bd793..0253c7934b3ee 100644 --- a/pkg/azure/ipam/node_test.go +++ b/pkg/azure/ipam/node_test.go @@ -6,12 +6,133 @@ package ipam import ( "testing" + "github.com/cilium/hive/hivetest" "github.com/stretchr/testify/require" + apimock "github.com/cilium/cilium/pkg/azure/api/mock" "github.com/cilium/cilium/pkg/azure/types" + ipamTypes "github.com/cilium/cilium/pkg/ipam/types" + v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2" ) func TestGetMaximumAllocatableIPv4(t *testing.T) { n := &Node{} require.Equal(t, types.InterfaceAddressLimit, n.GetMaximumAllocatableIPv4()) } + +// TestPopulateStatusFieldsDeterministicOrder ensures that repeated calls to +// PopulateStatusFields produce a byte-for-byte identical (DeepEqual) +// Status.Azure.Interfaces slice, sorted by interface ID, regardless of the +// randomized map-iteration order of ForeachInterface. Without this, the +// operator's DeepEqual-based write-skip gate cannot suppress no-op +// CiliumNode /status writes, reintroducing the per-tick spurious write bug +// this test guards against. +func TestPopulateStatusFieldsDeterministicOrder(t *testing.T) { + api := apimock.NewAPI(nil, nil) + require.NotNil(t, api) + + mngr := NewInstancesManager(hivetest.Logger(t), api) + require.NotNil(t, mngr) + + instances := ipamTypes.NewInstanceMap() + for _, id := range []string{"intf-c", "intf-a", "intf-b"} { + iface := &types.AzureInterface{ + SecurityGroup: "sg-" + id, + } + iface.SetID(id) + instances.Update("i-1", ipamTypes.InterfaceRevision{ + Resource: iface.DeepCopy(), + }) + } + api.UpdateInstances(instances) + require.False(t, mngr.Resync(t.Context()).IsZero()) + + node := &Node{ + node: fakeNodeActions{instanceID: "i-1"}, + manager: mngr, + } + + var results [][]types.AzureInterface + for i := 0; i < 10; i++ { + k8sObj := &v2.CiliumNode{} + node.PopulateStatusFields(k8sObj) + results = append(results, k8sObj.Status.Azure.Interfaces) + } + + for i, ifaces := range results { + require.Len(t, ifaces, 3, "iteration %d", i) + require.True(t, ifaces[0].ID < ifaces[1].ID && ifaces[1].ID < ifaces[2].ID, + "iteration %d: interfaces not sorted by ID: %v", i, []string{ifaces[0].ID, ifaces[1].ID, ifaces[2].ID}) + if i > 0 { + require.Equal(t, results[0], ifaces, "iteration %d produced a different order than iteration 0", i) + } + } +} + +// TestPopulateStatusFieldsDeepEqualAcrossShuffledOrders ties the ordering +// fix directly to the actual write-skip gate it exists to fix: the +// operator's origNode.Status.DeepEqual(&node.Status) call in +// operator/cmd/cilium_node.go. TestPopulateStatusFieldsDeterministicOrder +// only checks slice order via reflect-based require.Equal on hand-built +// interfaces with empty vmssName/vmID/resourceGroup; this test instead uses +// real VMSS-style resource IDs (so those unexported fields are actually +// populated by SetID/extractIDs, not empty) and asserts NodeStatus.DeepEqual +// -- the real consumer -- reports no difference across repeated, +// independently-randomized calls to PopulateStatusFields. +func TestPopulateStatusFieldsDeepEqualAcrossShuffledOrders(t *testing.T) { + api := apimock.NewAPI(nil, nil) + require.NotNil(t, api) + + mngr := NewInstancesManager(hivetest.Logger(t), api) + require.NotNil(t, mngr) + + instances := ipamTypes.NewInstanceMap() + resourceIDs := []string{ + "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss1/virtualMachines/0/networkInterfaces/intf-a", + "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss1/virtualMachines/1/networkInterfaces/intf-b", + "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss1/virtualMachines/2/networkInterfaces/intf-c", + } + for _, id := range resourceIDs { + iface := &types.AzureInterface{CIDR: "10.0.0.0/24"} + iface.SetID(id) + instances.Update("i-1", ipamTypes.InterfaceRevision{ + Resource: iface.DeepCopy(), + }) + } + api.UpdateInstances(instances) + require.False(t, mngr.Resync(t.Context()).IsZero()) + + node := &Node{ + node: fakeNodeActions{instanceID: "i-1"}, + manager: mngr, + } + + var reference *v2.CiliumNode + for i := 0; i < 20; i++ { + k8sObj := &v2.CiliumNode{} + node.PopulateStatusFields(k8sObj) + require.Len(t, k8sObj.Status.Azure.Interfaces, len(resourceIDs), "iteration %d", i) + for _, iface := range k8sObj.Status.Azure.Interfaces { + // Confirm the unexported, json:"-" fields are actually + // populated (non-empty) for this test, unlike + // TestPopulateStatusFieldsDeterministicOrder's bare interfaces. + require.NotEmpty(t, iface.GetVMScaleSetName(), "iteration %d", i) + require.NotEmpty(t, iface.GetVMID(), "iteration %d", i) + require.NotEmpty(t, iface.GetResourceGroup(), "iteration %d", i) + } + if reference == nil { + reference = k8sObj + continue + } + require.True(t, reference.Status.DeepEqual(&k8sObj.Status), + "iteration %d: NodeStatus.DeepEqual reported a difference despite identical underlying Azure interfaces", i) + } +} + +type fakeNodeActions struct { + instanceID string +} + +func (f fakeNodeActions) InstanceID() string { + return f.instanceID +} diff --git a/pkg/azure/types/types.go b/pkg/azure/types/types.go index cb5d7b8ebc242..2ff7925e008ad 100644 --- a/pkg/azure/types/types.go +++ b/pkg/azure/types/types.go @@ -4,6 +4,7 @@ package types import ( + "slices" "strings" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" @@ -59,6 +60,23 @@ type AzureStatus struct { Interfaces []AzureInterface `json:"interfaces,omitempty"` } +// SetInterfaces replaces Interfaces with a copy of ifaces sorted +// deterministically by ID. +// +// Interfaces must be kept sorted by ID: the generated AzureStatus.DeepEqual +// compares the slice index-by-index, and the operator's status-update path +// relies on that DeepEqual to skip no-op CiliumNode /status writes. Any code +// that populates Interfaces from a non-deterministically ordered source (e.g. +// Go map iteration) MUST use this method rather than assigning the field +// directly, to preserve that invariant. +func (s *AzureStatus) SetInterfaces(ifaces []AzureInterface) { + sorted := slices.Clone(ifaces) + slices.SortFunc(sorted, func(a, b AzureInterface) int { + return strings.Compare(a.ID, b.ID) + }) + s.Interfaces = sorted +} + // AzureAddress is an IP address assigned to an AzureInterface type AzureAddress struct { // IP is the ip address of the address @@ -121,14 +139,19 @@ type AzureInterface struct { // +optional CIDR string `json:"cidr,omitempty"` - // vmssName is the name of the virtual machine scale set. This field is - // set by extractIDs() + // vmssName is set by extractIDs() and is never serialized (json:"-"), so it + // is excluded from DeepEqual to avoid spurious diffs against apiserver copies. + // +deepequal-gen=false vmssName string `json:"-"` // vmID is the ID of the virtual machine + // + // +deepequal-gen=false vmID string `json:"-"` // resourceGroup is the resource group the interface belongs to + // + // +deepequal-gen=false resourceGroup string `json:"-"` } diff --git a/pkg/azure/types/types_test.go b/pkg/azure/types/types_test.go index ba5af0d94ab6b..0ed8878740730 100644 --- a/pkg/azure/types/types_test.go +++ b/pkg/azure/types/types_test.go @@ -51,6 +51,52 @@ func TestForeachAddresses(t *testing.T) { require.Equal(t, 2, interfaces) } +// TestAzureInterfaceDeepEqualIgnoresUnexportedFields locks in that +// AzureInterface.DeepEqual ignores the unexported, non-serialized +// (json:"-") vmssName/vmID/resourceGroup fields, while still comparing all +// exported fields. This is the exact write-suppression behavior the +// +deepequal-gen=false markers on those fields exist to provide: a +// freshly-populated in-memory AzureInterface (with vmssName/vmID/ +// resourceGroup set by SetID) must compare equal to a copy that round- +// tripped through the apiserver (where those fields are always empty, +// since they are never serialized). +func TestAzureInterfaceDeepEqualIgnoresUnexportedFields(t *testing.T) { + resourceID := "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss1/virtualMachines/0/networkInterfaces/vmss1" + base := &AzureInterface{Name: "eth0", MAC: "aa:bb:cc:dd:ee:ff", CIDR: "10.0.0.0/24"} + base.SetID(resourceID) + require.NotEmpty(t, base.GetResourceGroup()) + require.NotEmpty(t, base.GetVMID()) + require.NotEmpty(t, base.GetVMScaleSetName()) + + // apiserverRoundTripped simulates the same interface as fetched back + // from the apiserver: identical exported fields (including ID, which + // IS serialized), but the unexported vmssName/vmID/resourceGroup are + // zero-valued because those json:"-" fields are never serialized. + apiserverRoundTripped := &AzureInterface{ID: resourceID, Name: "eth0", MAC: "aa:bb:cc:dd:ee:ff", CIDR: "10.0.0.0/24"} + + require.True(t, base.DeepEqual(apiserverRoundTripped), + "AzureInterface.DeepEqual must ignore vmssName/vmID/resourceGroup so identical exported fields compare equal") + require.True(t, apiserverRoundTripped.DeepEqual(base)) + + tests := []struct { + name string + mutate func(*AzureInterface) + }{ + {"ID differs", func(a *AzureInterface) { a.ID = "intf-2" }}, + {"Name differs", func(a *AzureInterface) { a.Name = "eth1" }}, + {"MAC differs", func(a *AzureInterface) { a.MAC = "ff:ee:dd:cc:bb:aa" }}, + {"CIDR differs", func(a *AzureInterface) { a.CIDR = "10.0.1.0/24" }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + other := apiserverRoundTripped.DeepCopy() + tt.mutate(other) + require.False(t, base.DeepEqual(other), + "AzureInterface.DeepEqual must still detect differences in exported field: %s", tt.name) + }) + } +} + func TestExtractIDs(t *testing.T) { tests := []struct { name string diff --git a/pkg/azure/types/zz_generated.deepequal.go b/pkg/azure/types/zz_generated.deepequal.go index 8761f19495136..bd907660c17e2 100644 --- a/pkg/azure/types/zz_generated.deepequal.go +++ b/pkg/azure/types/zz_generated.deepequal.go @@ -76,15 +76,6 @@ func (in *AzureInterface) DeepEqual(other *AzureInterface) bool { if in.CIDR != other.CIDR { return false } - if in.vmssName != other.vmssName { - return false - } - if in.vmID != other.vmID { - return false - } - if in.resourceGroup != other.resourceGroup { - return false - } return true } 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