ipam: Remove unused ForeachAddress abstraction

Following cilium#45985, the `Interface.ForeachAddress` method now only
has one caller: azure's `ResyncInterfacesAndIPs`, where it is
followed by a `ForeachInterface`. The addresses loop can be inlined
under that existing interfaces one, resulting in a slight optimization.

With that change, ForeachAddress is no longer used anywhere, meaning
all 3 cloud provider implementations can be removed, as well as the
wrapper `InstanceMap.ForeachAddress`.

Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>

Author: HadrienPatte

pkg/alibabacloud/eni/types/types.go

@@ -135,20 +135,6 @@ func (e *ENI) InterfaceID() string {
 	return e.NetworkInterfaceID
 }
 
-// ForeachAddress iterates over all addresses and calls fn
-func (e *ENI) ForeachAddress(id string, fn types.AddressIterator) error {
-	for _, address := range e.PrivateIPSets {
-		if address.Primary {
-			continue
-		}
-		if err := fn(id, e.NetworkInterfaceID, address.PrivateIpAddress, address); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // ENIStatus is the status of ENI addressing of the node
 type ENIStatus struct {
 	// ENIs is the list of ENIs on the node

pkg/aws/eni/types/types.go

@@ -178,17 +178,6 @@ func (e *ENI) InterfaceID() string {
 	return e.ID
 }
 
-// ForeachAddress iterates over all addresses and calls fn
-func (e *ENI) ForeachAddress(id string, fn types.AddressIterator) error {
-	for _, address := range e.Addresses {
-		if err := fn(id, e.ID, address, address); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // IsExcludedBySpec returns true if the ENI is excluded by the provided spec and
 // therefore should not be managed by Cilium.
 func (e *ENI) IsExcludedBySpec(spec ENISpec) bool {

pkg/azure/ipam/node.go

@@ -169,30 +169,6 @@ func (n *Node) ResyncInterfacesAndIPs(ctx context.Context, scopedLog *slog.Logge
 	n.manager.mutex.RLock()
 	defer n.manager.mutex.RUnlock()
 	usePrimary := n.manager.usePrimary
-	err = n.manager.instances.ForeachAddress(n.node.InstanceID(), func(instanceID, interfaceID, ip string, addressObj ipamTypes.Address) error {
-		address, ok := addressObj.(types.AzureAddress)
-		if !ok {
-			scopedLog.Warn(
-				"Not an Azure address object, ignoring IP",
-				logfields.IPAddr, ip,
-			)
-			return nil
-		}
-
-		if address.State == types.StateSucceeded {
-			available[address.IP] = ipamTypes.AllocationIP{Resource: interfaceID}
-		} else {
-			scopedLog.Warn(
-				"Ignoring potentially available IP due to non-successful state",
-				logfields.IPAddr, ip,
-				logfields.State, address.State,
-			)
-		}
-		return nil
-	})
-	if err != nil {
-		return nil, stats, err
-	}
 
 	// Azure caps both NICs and VMs at 256 addresses; start from that ceiling
 	// and decrement per NIC below for any primary slot we can't allocate.
@@ -204,6 +180,18 @@ func (n *Node) ResyncInterfacesAndIPs(ctx context.Context, scopedLog *slog.Logge
 			return fmt.Errorf("invalid interface object")
 		}
 
+		for _, address := range iface.Addresses {
+			if address.State == types.StateSucceeded {
+				available[address.IP] = ipamTypes.AllocationIP{Resource: interfaceID}
+			} else {
+				scopedLog.Warn(
+					"Ignoring potentially available IP due to non-successful state",
+					logfields.IPAddr, address.IP,
+					logfields.State, address.State,
+				)
+			}
+		}
+
 		// Cache the VMSS name from the first interface we see
 		if n.vmss == "" {
 			n.vmss = iface.GetVMScaleSetName()
@@ -215,8 +203,7 @@ func (n *Node) ResyncInterfacesAndIPs(ctx context.Context, scopedLog *slog.Logge
 			nodeCapacity--
 		}
 
-		_, available := isAvailableInterface(requiredIfaceName, iface, usePrimary, scopedLog)
-		if available {
+		if _, isAvailable := isAvailableInterface(requiredIfaceName, iface, usePrimary, scopedLog); isAvailable {
 			stats.RemainingAvailableInterfaceCount++
 		}
 		return nil

pkg/azure/types/types.go

@@ -186,14 +186,3 @@ func (a *AzureInterface) GetVMScaleSetName() string {
 func (a *AzureInterface) GetVMID() string {
 	return a.vmID
 }
-
-// ForeachAddress iterates over all addresses and calls fn.
-func (a *AzureInterface) ForeachAddress(id string, fn types.AddressIterator) error {
-	for _, address := range a.Addresses {
-		if err := fn(id, a.ID, address.IP, address); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}

pkg/azure/types/types_test.go

@@ -12,45 +12,8 @@ import (
 	// Register the Azure resource-ID parser so SetID()/extractIDs() populate
 	// the VMSS/VM/RG fields exercised by TestExtractIDs.
 	_ "github.com/cilium/cilium/pkg/azure/types/azureid"
-	"github.com/cilium/cilium/pkg/ipam/types"
 )
 
-func TestForeachAddresses(t *testing.T) {
-	m := types.NewInstanceMap()
-	m.Update("i-1", &azuretypes.AzureInterface{ID: "1", Addresses: []azuretypes.AzureAddress{
-		{IP: "1.1.1.1"},
-		{IP: "2.2.2.2"},
-	}})
-	m.Update("i-2", &azuretypes.AzureInterface{ID: "1", Addresses: []azuretypes.AzureAddress{
-		{IP: "3.3.3.3"},
-		{IP: "4.4.4.4"},
-	}})
-
-	// Iterate over all instances
-	addresses := 0
-	m.ForeachAddress("", func(instanceID, interfaceID, ip string, address types.Address) error {
-		addresses++
-		return nil
-	})
-	require.Equal(t, 4, addresses)
-
-	// Iterate over "i-1"
-	addresses = 0
-	m.ForeachAddress("i-1", func(instanceID, interfaceID, ip string, address types.Address) error {
-		addresses++
-		return nil
-	})
-	require.Equal(t, 2, addresses)
-
-	// Iterate over all interfaces
-	interfaces := 0
-	m.ForeachInterface("", func(instanceID, interfaceID string, interfaceObj types.Interface) error {
-		interfaces++
-		return nil
-	})
-	require.Equal(t, 2, interfaces)
-}
-
 func TestExtractIDs(t *testing.T) {
 	tests := []struct {
 		name             string

pkg/ipam/types/types.go

@@ -468,10 +468,6 @@ type Interface interface {
 	// InterfaceID must return the identifier of the interface
 	InterfaceID() string
 
-	// ForeachAddress must iterate over all addresses of the interface and
-	// call fn for each address
-	ForeachAddress(instanceID string, fn AddressIterator) error
-
 	// DeepCopyInterface returns a deep copy of the underlying interface type.
 	DeepCopyInterface() Interface
 }
@@ -549,49 +545,6 @@ func (m *InstanceMap) updateLocked(instanceID string, iface Interface) {
 	i.Interfaces[iface.InterfaceID()] = iface
 }
 
-type Address any
-
-// AddressIterator is the function called by the ForeachAddress iterator
-type AddressIterator func(instanceID, interfaceID, ip string, address Address) error
-
-func foreachAddress(instanceID string, instance *Instance, fn AddressIterator) error {
-	for _, iface := range instance.Interfaces {
-		if err := iface.ForeachAddress(instanceID, fn); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
-// ForeachAddress calls fn for each address on each interface attached to each
-// instance. If an instanceID is specified, the only the interfaces and
-// addresses of the specified instance are considered.
-//
-// The InstanceMap is read-locked throughout the iteration process, i.e., no
-// updates will occur. However, the address object given to the AddressIterator
-// will point to live data and must be deep copied if used outside of the
-// context of the iterator function.
-func (m *InstanceMap) ForeachAddress(instanceID string, fn AddressIterator) error {
-	m.mutex.RLock()
-	defer m.mutex.RUnlock()
-
-	if instanceID != "" {
-		if instance := m.data[instanceID]; instance != nil {
-			return foreachAddress(instanceID, instance, fn)
-		}
-		return fmt.Errorf("instance does not exist: %q", instanceID)
-	}
-
-	for instanceID, instance := range m.data {
-		if err := foreachAddress(instanceID, instance, fn); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // InterfaceIterator is the function called by the ForeachInterface iterator
 type InterfaceIterator func(instanceID, interfaceID string, iface Interface) error
 

pkg/ipam/types/types_test.go

@@ -42,60 +42,6 @@ func (m *mockInterface) InterfaceID() string {
 	return m.id
 }
 
-func (m *mockInterface) ForeachAddress(instanceID string, fn AddressIterator) error {
-	for _, ips := range m.pools {
-		for _, ip := range ips {
-			if err := fn(instanceID, m.id, ip.String(), ip); err != nil {
-				return err
-			}
-		}
-	}
-
-	return nil
-}
-
-func TestForeachAddresses(t *testing.T) {
-	m := NewInstanceMap()
-	m.Update("i-1", &mockInterface{
-		id: "intf0",
-		pools: map[string][]net.IP{
-			"s1": {net.ParseIP("1.1.1.1"), net.ParseIP("2.2.2.2")},
-		},
-	})
-	m.Update("i-2", &mockInterface{
-		id: "intf0",
-		pools: map[string][]net.IP{
-			"s1": {net.ParseIP("3.3.3.3"), net.ParseIP("4.4.4.4")},
-		},
-	})
-
-	// Iterate over all instances
-	addresses := 0
-	m.ForeachAddress("", func(instanceID, interfaceID, ip string, address Address) error {
-		_, ok := address.(net.IP)
-		require.True(t, ok)
-		addresses++
-		return nil
-	})
-	require.Equal(t, 4, addresses)
-
-	// Iterate over "i-1"
-	addresses = 0
-	m.ForeachAddress("i-1", func(instanceID, interfaceID, ip string, address Address) error {
-		addresses++
-		return nil
-	})
-	require.Equal(t, 2, addresses)
-
-	// Iterate over all interfaces
-	interfaces := 0
-	m.ForeachInterface("", func(instanceID, interfaceID string, iface Interface) error {
-		interfaces++
-		return nil
-	})
-	require.Equal(t, 2, interfaces)
-}
-
 func TestGetInterface(t *testing.T) {
 	m := NewInstanceMap()
 	rev := &mockInterface{