Skip to content

Commit 8ee61c7

Browse files
committed
[azure] implement excess IP release for IPAM nodes
Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com>
1 parent c798567 commit 8ee61c7

4 files changed

Lines changed: 543 additions & 4 deletions

File tree

pkg/azure/api/mock/mock.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,23 @@ func (a *API) AssignPrivateIpAddressesVMSS(ctx context.Context, vmName, vmssName
276276
return nil
277277
}
278278

279+
// AllocateSubnetIP marks ip as allocated in the named subnet's allocator. It is
280+
// a test helper so callers can observe the IP being freed back when it is later
281+
// released via Unassign*.
282+
func (a *API) AllocateSubnetIP(subnetID, ip string) error {
283+
a.mutex.Lock()
284+
defer a.mutex.Unlock()
285+
s, ok := a.subnets[subnetID]
286+
if !ok {
287+
return fmt.Errorf("subnet %s does not exist", subnetID)
288+
}
289+
addr, err := netip.ParseAddr(ip)
290+
if err != nil {
291+
return err
292+
}
293+
return s.allocator.Allocate(addr)
294+
}
295+
279296
// SetPrimaryIPs marks the given identifiers (IPs for the VM path or IPConfig
280297
// names for the VMSS path) as primary on the named interface. Calls to
281298
// UnassignPrivateIpAddresses* that intersect this set return

pkg/azure/ipam/instances.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"log/slog"
1010
"maps"
1111
"net/netip"
12+
"slices"
1213

1314
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v9"
1415
"k8s.io/apimachinery/pkg/util/sets"
@@ -225,6 +226,44 @@ func (m *InstancesManager) InstanceSync(ctx context.Context, instanceID string)
225226
return m.resyncInstance(ctx, instanceID)
226227
}
227228

229+
// RemoveIPsFromInterface removes the given IPs from the cached interface so the
230+
// IP pool reflects a release before the next full resync. It mirrors the AWS
231+
// RemoveIPsFromENI helper. Unknown instances/interfaces are logged and ignored;
232+
// the next resync reconciles the cache with Azure regardless.
233+
func (m *InstancesManager) RemoveIPsFromInterface(instanceID, interfaceID string, ips []string) {
234+
m.mutex.Lock()
235+
defer m.mutex.Unlock()
236+
237+
iface, ok := m.instances.GetInterface(instanceID, interfaceID)
238+
if !ok {
239+
// A concurrent resync may legitimately have dropped the interface
240+
// between the successful release and this cache update; the next
241+
// resync reconciles the cache regardless.
242+
m.logger.Warn("Interface not found while removing released IPs from cache",
243+
logfields.InstanceID, instanceID,
244+
logfields.Interface, interfaceID,
245+
)
246+
return
247+
}
248+
249+
azIface, ok := iface.DeepCopyInterface().(*types.AzureInterface)
250+
if !ok {
251+
// A non-Azure interface on an Azure node is an invariant violation, not
252+
// a benign race — surface it at error level.
253+
m.logger.Error("Unexpected interface type while removing released IPs from cache",
254+
logfields.InstanceID, instanceID,
255+
logfields.Interface, interfaceID,
256+
)
257+
return
258+
}
259+
260+
release := sets.New[string](ips...)
261+
azIface.Addresses = slices.DeleteFunc(azIface.Addresses, func(a types.AzureAddress) bool {
262+
return release.Has(a.IP.String())
263+
})
264+
m.instances.Update(instanceID, azIface)
265+
}
266+
228267
// DeleteInstance delete instance from m.instances
229268
func (m *InstancesManager) DeleteInstance(instanceID string) {
230269
m.mutex.Lock()

pkg/azure/ipam/node.go

Lines changed: 163 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"fmt"
99
"log/slog"
1010
"net/netip"
11+
"slices"
12+
"strings"
1113

1214
"github.com/cilium/cilium/operator/pkg/ipam/nodemanager"
1315
"github.com/cilium/cilium/operator/pkg/ipam/stats"
@@ -58,9 +60,73 @@ func (n *Node) PopulateStatusFields(k8sObj *v2.CiliumNode) {
5860
})
5961
}
6062

61-
// PrepareIPRelease prepares the release of IPs
63+
// PrepareIPRelease selects up to excessIPs free IP addresses on a single
64+
// interface attached to this node and returns them as a ReleaseAction. An IP is
65+
// considered "free" if it is in Succeeded state, IPv4, not the NIC's primary
66+
// IPConfiguration, and not present in CiliumNode.Status.IPAM.Used.
67+
//
68+
// Interfaces are iterated in sorted ID order and the one with the most
69+
// releasable IPs is chosen (mirroring the AWS path). Deterministic selection
70+
// matters: if the chosen interface flapped between runs, the node manager would
71+
// repeatedly reset the per-IP release-delay timer and never release anything.
72+
//
73+
// Primary IPConfigurations are excluded because Azure ARM rejects an update
74+
// that drops the primary, failing the whole batch atomically — so a primary in
75+
// the release set would jam every secondary selected alongside it. The
76+
// SDK-level Unassign* methods keep a pre-flight guard as defense-in-depth.
6277
func (n *Node) PrepareIPRelease(excessIPs int, scopedLog *slog.Logger) *nodemanager.ReleaseAction {
63-
return &nodemanager.ReleaseAction{}
78+
r := &nodemanager.ReleaseAction{}
79+
requiredIfaceName := n.k8sObj.Spec.Azure.InterfaceName
80+
usedIPs := n.k8sObj.Status.IPAM.Used
81+
82+
n.manager.mutex.RLock()
83+
defer n.manager.mutex.RUnlock()
84+
85+
var ifaces []*types.AzureInterface
86+
err := n.manager.instances.ForeachInterface(n.node.InstanceID(),
87+
func(_, _ string, ifaceObj ipamTypes.Interface) error {
88+
iface, ok := ifaceObj.(*types.AzureInterface)
89+
if !ok {
90+
return fmt.Errorf("invalid interface object")
91+
}
92+
if requiredIfaceName != "" && iface.Name != requiredIfaceName {
93+
return nil
94+
}
95+
ifaces = append(ifaces, iface)
96+
return nil
97+
})
98+
if err != nil {
99+
scopedLog.Warn(
100+
"Unable to enumerate interfaces while preparing IP release",
101+
logfields.InstanceID, n.node.InstanceID(),
102+
logfields.Error, err,
103+
)
104+
return r
105+
}
106+
slices.SortFunc(ifaces, func(a, b *types.AzureInterface) int {
107+
return strings.Compare(a.ID, b.ID)
108+
})
109+
110+
for _, iface := range ifaces {
111+
free := freeIPsOnInterface(iface, usedIPs)
112+
if len(free) == 0 {
113+
continue
114+
}
115+
maxRelease := min(len(free), excessIPs)
116+
// Select the interface with the most addresses available for release.
117+
if r.IPsToRelease == nil || maxRelease > len(r.IPsToRelease) {
118+
r.InterfaceID = iface.ID
119+
r.PoolID = ipamTypes.PoolID(iface.Subnet.ID)
120+
r.IPsToRelease = free[:maxRelease]
121+
scopedLog.Debug(
122+
"Interface has unused IPs that can be released",
123+
logfields.ID, iface.ID,
124+
logfields.ExcessIPs, excessIPs,
125+
logfields.IPAddrs, r.IPsToRelease,
126+
)
127+
}
128+
}
129+
return r
64130
}
65131

66132
// ReleaseIPPrefixes is a no-op on Azure since Azure ENIs don't
@@ -70,9 +136,102 @@ func (n *Node) ReleaseIPPrefixes(ctx context.Context, r *nodemanager.ReleaseActi
70136
return nil
71137
}
72138

73-
// ReleaseIPs performs the IP release operation
139+
// ReleaseIPs performs the IP release operation. For VM (non-VMSS) interfaces the
140+
// network model carries the IP on each IPConfiguration, so IPs are passed to
141+
// the SDK directly. For VMSS, the compute model only exposes IPConfiguration
142+
// names, so IPs are translated to names via the in-memory mapping populated by
143+
// parseInterface. On success the released IPs are removed from the cached
144+
// interface so the pool reflects the release before the next full resync.
74145
func (n *Node) ReleaseIPs(ctx context.Context, r *nodemanager.ReleaseAction) error {
75-
return fmt.Errorf("not implemented")
146+
if len(r.IPsToRelease) == 0 {
147+
return nil
148+
}
149+
150+
iface, err := n.findInterface(r.InterfaceID)
151+
if err != nil {
152+
return err
153+
}
154+
155+
if iface.GetVMScaleSetName() == "" {
156+
if err := n.manager.api.UnassignPrivateIpAddressesVM(ctx, iface.Name, r.IPsToRelease); err != nil {
157+
return err
158+
}
159+
} else {
160+
names, missing := ipsToConfigNames(iface, r.IPsToRelease)
161+
if len(missing) > 0 {
162+
return fmt.Errorf("interface %s: missing IPConfiguration name mapping for IPs %v (cache out of sync, will retry after next resync)", iface.Name, missing)
163+
}
164+
if err := n.manager.api.UnassignPrivateIpAddressesVMSS(ctx, iface.GetVMID(), iface.GetVMScaleSetName(), iface.Name, names); err != nil {
165+
return err
166+
}
167+
}
168+
169+
n.manager.RemoveIPsFromInterface(n.node.InstanceID(), r.InterfaceID, r.IPsToRelease)
170+
return nil
171+
}
172+
173+
// freeIPsOnInterface returns the releasable IPv4 addresses on iface as sorted
174+
// strings: Succeeded state, not the NIC's primary IPConfiguration, and absent
175+
// from used. The result is sorted so truncation to excessIPs is deterministic.
176+
func freeIPsOnInterface(iface *types.AzureInterface, used ipamTypes.AllocationMap) []string {
177+
free := make([]string, 0, len(iface.Addresses))
178+
for _, a := range iface.Addresses {
179+
if a.State != types.StateSucceeded {
180+
continue
181+
}
182+
// The release handshake is driven by IPv4 excess; never release an
183+
// IPv6 address through this path.
184+
if !a.IP.Addr.Is4() {
185+
continue
186+
}
187+
// Never release the primary IPConfiguration (see PrepareIPRelease).
188+
if a.IP == iface.IP {
189+
continue
190+
}
191+
ip := a.IP.String()
192+
if _, inUse := used[ip]; inUse {
193+
continue
194+
}
195+
free = append(free, ip)
196+
}
197+
slices.Sort(free)
198+
return free
199+
}
200+
201+
// findInterface returns a copy of the AzureInterface with the given ID on this
202+
// node, or an error if not found. A copy is returned so callers can read it
203+
// without holding the manager mutex.
204+
func (n *Node) findInterface(interfaceID string) (*types.AzureInterface, error) {
205+
n.manager.mutex.RLock()
206+
defer n.manager.mutex.RUnlock()
207+
iface, ok := n.manager.instances.GetInterface(n.node.InstanceID(), interfaceID)
208+
if !ok {
209+
return nil, fmt.Errorf("interface %s not found on instance %s", interfaceID, n.node.InstanceID())
210+
}
211+
azIface, ok := iface.DeepCopyInterface().(*types.AzureInterface)
212+
if !ok {
213+
return nil, fmt.Errorf("interface %s on instance %s has unexpected type", interfaceID, n.node.InstanceID())
214+
}
215+
return azIface, nil
216+
}
217+
218+
// ipsToConfigNames maps each IP in ips to its IPConfiguration name on iface.
219+
// IPs without a known mapping are returned in missing.
220+
func ipsToConfigNames(iface *types.AzureInterface, ips []string) (names, missing []string) {
221+
byIP := make(map[string]string, len(iface.Addresses))
222+
for _, a := range iface.Addresses {
223+
if name := a.IPConfigName(); name != "" {
224+
byIP[a.IP.String()] = name
225+
}
226+
}
227+
for _, ip := range ips {
228+
if name, ok := byIP[ip]; ok {
229+
names = append(names, name)
230+
} else {
231+
missing = append(missing, ip)
232+
}
233+
}
234+
return
76235
}
77236

78237
// PrepareIPAllocation returns the number of IPs that can be allocated/created.

0 commit comments

Comments
 (0)