diff --git a/Documentation/cmdref/cilium-operator-azure.md b/Documentation/cmdref/cilium-operator-azure.md index 5b764d70dd0b5..e6b7a4ed4991f 100644 --- a/Documentation/cmdref/cilium-operator-azure.md +++ b/Documentation/cmdref/cilium-operator-azure.md @@ -12,6 +12,7 @@ cilium-operator-azure [flags] ``` --auto-create-cilium-pod-ip-pools map Automatically create CiliumPodIPPool resources on startup. Specify pools in the form of =ipv4-cidrs:,[...];ipv4-mask-size: (multiple pools can also be passed by repeating the CLI flag) + --azure-release-excess-ips Enable releasing excess free IP addresses from Azure NICs. --azure-resource-group string Resource group to use for Azure IPAM --azure-subscription-id string Subscription ID to access Azure API --azure-use-primary-address Use Azure IP address from interface's primary IPConfigurations @@ -65,6 +66,7 @@ cilium-operator-azure [flags] --enable-policy-secrets-sync Enables fan-in TLS secrets sync from multiple namespaces to singular namespace (specified by policy-secrets-namespace flag) --enable-ztunnel Use zTunnel as Cilium's encryption infrastructure --enforce-ingress-https Enforces https for host having matching TLS host in Ingress. Incoming traffic to http listener will return 308 http error code with respective location in header. (default true) + --excess-ip-release-delay int Number of seconds operator would wait before it releases an IP previously marked as excess (default 180) --gateway-api-hostnetwork-enabled Exposes Gateway listeners on the host network. --gateway-api-hostnetwork-nodelabelselector string Label selector that matches the nodes where the gateway listeners should be exposed. It's a list of comma-separated key-value label pairs. e.g. 'kubernetes.io/os=linux,kubernetes.io/hostname=kind-worker' --gateway-api-secrets-namespace string Namespace having tls secrets used by CEC for Gateway API (default "cilium-secrets") diff --git a/Documentation/cmdref/cilium-operator.md b/Documentation/cmdref/cilium-operator.md index a582a59c607ae..197517eaddb28 100644 --- a/Documentation/cmdref/cilium-operator.md +++ b/Documentation/cmdref/cilium-operator.md @@ -17,6 +17,7 @@ cilium-operator [flags] --aws-max-results-per-call int32 Maximum results per AWS API call for DescribeNetworkInterfaces and DescribeSecurityGroups. Set to 0 to let AWS determine optimal page size (default). If set to 0 and AWS returns OperationNotPermitted errors, automatically switches to 1000 for all future requests --aws-release-excess-ips Enable releasing excess free IP addresses from AWS ENI. --aws-use-primary-address Allows for using primary address of the ENI for allocations on the node + --azure-release-excess-ips Enable releasing excess free IP addresses from Azure NICs. --azure-resource-group string Resource group to use for Azure IPAM --azure-subscription-id string Subscription ID to access Azure API --azure-use-primary-address Use Azure IP address from interface's primary IPConfigurations diff --git a/operator/cmd/provider_azure_flags.go b/operator/cmd/provider_azure_flags.go index a9a66e4e0fc66..7afd7be3a606f 100644 --- a/operator/cmd/provider_azure_flags.go +++ b/operator/cmd/provider_azure_flags.go @@ -34,5 +34,11 @@ func (hook *azureFlagsHooks) RegisterProviderFlag(cmd *cobra.Command, vp *viper. flags.Bool(operatorOption.AzureUsePrimaryAddress, false, "Use Azure IP address from interface's primary IPConfigurations") option.BindEnvWithLegacyEnvFallback(vp, operatorOption.AzureUsePrimaryAddress, "AZURE_USE_PRIMARY_ADDRESS") + flags.Bool(operatorOption.AzureReleaseExcessIPs, false, "Enable releasing excess free IP addresses from Azure NICs.") + option.BindEnvWithLegacyEnvFallback(vp, operatorOption.AzureReleaseExcessIPs, "AZURE_RELEASE_EXCESS_IPS") + + flags.Int(operatorOption.ExcessIPReleaseDelay, 180, "Number of seconds operator would wait before it releases an IP previously marked as excess") + option.BindEnv(vp, operatorOption.ExcessIPReleaseDelay) + vp.BindPFlags(flags) } diff --git a/operator/option/config.go b/operator/option/config.go index 9929b382b1da1..ad6989672dea0 100644 --- a/operator/option/config.go +++ b/operator/option/config.go @@ -148,6 +148,11 @@ const ( // primary IPConfiguration AzureUsePrimaryAddress = "azure-use-primary-address" + // AzureReleaseExcessIPs allows releasing excess free IP addresses from Azure NICs. + // Enabling this option reduces waste of IP addresses but may increase + // the number of API calls to Azure. + AzureReleaseExcessIPs = "azure-release-excess-ips" + // LeaderElectionLeaseDuration is the duration that non-leader candidates will wait to // force acquire leadership LeaderElectionLeaseDuration = "leader-election-lease-duration" @@ -349,6 +354,9 @@ type OperatorConfig struct { // primary IPConfiguration AzureUsePrimaryAddress bool + // AzureReleaseExcessIPs allows releasing excess free IP addresses from Azure NICs + AzureReleaseExcessIPs bool + // AlibabaCloud options // AlibabaCloudVPCID allow user to specific vpc @@ -466,6 +474,7 @@ func (c *OperatorConfig) Populate(logger *slog.Logger, vp *viper.Viper) { c.AzureResourceGroup = vp.GetString(AzureResourceGroup) c.AzureUsePrimaryAddress = vp.GetBool(AzureUsePrimaryAddress) c.AzureUserAssignedIdentityID = vp.GetString(AzureUserAssignedIdentityID) + c.AzureReleaseExcessIPs = vp.GetBool(AzureReleaseExcessIPs) // AlibabaCloud options diff --git a/pkg/azure/api/api.go b/pkg/azure/api/api.go index 5d408f5048c35..1ddbdab5074b5 100644 --- a/pkg/azure/api/api.go +++ b/pkg/azure/api/api.go @@ -342,6 +342,12 @@ func parseInterface(iface *armnetwork.Interface, subnets ipamTypes.SubnetMap, us IP: *ip.Properties.PrivateIPAddress, State: strings.ToLower(string(*ip.Properties.ProvisioningState)), } + if ip.Name != nil { + addr.Name = *ip.Name + } + if ip.Properties.Primary != nil { + addr.Primary = *ip.Properties.Primary + } if ip.Properties.Subnet != nil { addr.Subnet = *ip.Properties.Subnet.ID @@ -653,6 +659,151 @@ func (c *Client) AssignPrivateIpAddressesVM(ctx context.Context, subnetID, inter return nil } +// UnassignPrivateIpAddressesVM removes the given private IPs from an interface attached to a standalone instance +func (c *Client) UnassignPrivateIpAddressesVM(ctx context.Context, interfaceName string, addresses []string) error { + if len(addresses) == 0 { + return nil + } + + c.limiter.Limit(ctx, interfacesGet) + sinceStart := spanstat.Start() + + iface, err := c.interfaces.Get(ctx, c.resourceGroup, interfaceName, nil) + + c.metricsAPI.ObserveAPICall(interfacesGet, deriveStatus(err), sinceStart.Seconds()) + if err != nil { + return fmt.Errorf("failed to get standalone instance's interface %s: %w", interfaceName, err) + } + + releaseSet := make(map[string]struct{}, len(addresses)) + for _, ip := range addresses { + releaseSet[ip] = struct{}{} + } + + kept := iface.Properties.IPConfigurations[:0] + for _, ipConfig := range iface.Properties.IPConfigurations { + if ipConfig.Properties != nil && ipConfig.Properties.Primary != nil && *ipConfig.Properties.Primary { + kept = append(kept, ipConfig) + continue + } + if ipConfig.Properties == nil || ipConfig.Properties.PrivateIPAddress == nil { + kept = append(kept, ipConfig) + continue + } + if _, drop := releaseSet[*ipConfig.Properties.PrivateIPAddress]; drop { + continue + } + kept = append(kept, ipConfig) + } + iface.Properties.IPConfigurations = kept + + c.limiter.Limit(ctx, interfacesCreateOrUpdate) + sinceStart = spanstat.Start() + + poller, err := c.interfaces.BeginCreateOrUpdate(ctx, c.resourceGroup, interfaceName, iface.Interface, nil) + + defer func() { + c.metricsAPI.ObserveAPICall(interfacesCreateOrUpdate, deriveStatus(err), sinceStart.Seconds()) + }() + if err != nil { + return fmt.Errorf("unable to update interface %s: %w", interfaceName, err) + } + + if _, err := poller.PollUntilDone(ctx, nil); err != nil { + return fmt.Errorf("error while waiting for interface CreateOrUpdate to complete for %s: %w", interfaceName, err) + } + + return nil +} + +// UnassignPrivateIpAddressesVMSS removes the IPConfigurations with the given names from +// an interface attached to a VMSS instance. The caller is responsible for translating +// IPs to IPConfiguration names — typically using the cached AzureInterface populated by +// the InstancesManager — which avoids an additional Azure API call to fetch the NIC. +func (c *Client) UnassignPrivateIpAddressesVMSS(ctx context.Context, instanceID, vmssName, interfaceName string, ipConfigNames []string) error { + if len(ipConfigNames) == 0 { + return nil + } + + dropNames := make(map[string]struct{}, len(ipConfigNames)) + for _, name := range ipConfigNames { + dropNames[name] = struct{}{} + } + + vmssGetOptions := &armcompute.VirtualMachineScaleSetVMsClientGetOptions{ + Expand: to.Ptr(armcompute.InstanceViewTypesInstanceView), + } + + c.limiter.Limit(ctx, virtualMachineScaleSetVMsGet) + sinceStart := spanstat.Start() + + result, err := c.virtualMachineScaleSetVMs.Get(ctx, c.resourceGroup, vmssName, instanceID, vmssGetOptions) + + c.metricsAPI.ObserveAPICall(virtualMachineScaleSetVMsGet, deriveStatus(err), sinceStart.Seconds()) + if err != nil { + return fmt.Errorf("failed to get VM %s from VMSS %s: %w", instanceID, vmssName, err) + } + + var netIfConfig *armcompute.VirtualMachineScaleSetNetworkConfiguration + if result.Properties.NetworkProfileConfiguration != nil { + for _, networkInterfaceConfiguration := range result.Properties.NetworkProfileConfiguration.NetworkInterfaceConfigurations { + if networkInterfaceConfiguration.Name != nil && *networkInterfaceConfiguration.Name == interfaceName { + netIfConfig = networkInterfaceConfiguration + break + } + } + } + + if netIfConfig == nil { + return fmt.Errorf("interface %s does not exist in VM %s", interfaceName, instanceID) + } + + kept := netIfConfig.Properties.IPConfigurations[:0] + for _, ipConfig := range netIfConfig.Properties.IPConfigurations { + if ipConfig.Properties != nil && ipConfig.Properties.Primary != nil && *ipConfig.Properties.Primary { + kept = append(kept, ipConfig) + continue + } + if ipConfig.Name == nil { + kept = append(kept, ipConfig) + continue + } + if _, drop := dropNames[*ipConfig.Name]; drop { + continue + } + kept = append(kept, ipConfig) + } + netIfConfig.Properties.IPConfigurations = kept + + // Unset imageReference, because if this contains a reference to an image from the + // Azure Compute Gallery, including this reference in an update to the VMSS instance + // will cause a permissions error, because the reference includes an Azure-managed + // subscription ID. + // Removing the image reference indicates to the API that we don't want to change it. + // See https://github.com/Azure/AKS/issues/1819. + if result.Properties.StorageProfile != nil { + result.Properties.StorageProfile.ImageReference = nil + } + + c.limiter.Limit(ctx, virtualMachineScaleSetVMsUpdate) + sinceStart = spanstat.Start() + + poller, err := c.virtualMachineScaleSetVMs.BeginUpdate(ctx, c.resourceGroup, vmssName, instanceID, result.VirtualMachineScaleSetVM, nil) + + defer func() { + c.metricsAPI.ObserveAPICall(virtualMachineScaleSetVMsUpdate, deriveStatus(err), sinceStart.Seconds()) + }() + if err != nil { + return fmt.Errorf("unable to update virtualMachineScaleSetVMs: %w", err) + } + + if _, err := poller.PollUntilDone(ctx, nil); err != nil { + return fmt.Errorf("error while waiting for virtualMachineScaleSetVMs Update to complete: %w", err) + } + + return nil +} + // AssignPublicIPAddressesVMSS assigns a public IP to a VMSS instance. // The public IP is allocated from a Public IP Prefix matching publicIpTags func (c *Client) AssignPublicIPAddressesVMSS(ctx context.Context, instanceID, vmssName string, publicIpTags ipamTypes.Tags) (string, error) { diff --git a/pkg/azure/api/mock/mock.go b/pkg/azure/api/mock/mock.go index ce9092fcab2d6..2ee87482b3365 100644 --- a/pkg/azure/api/mock/mock.go +++ b/pkg/azure/api/mock/mock.go @@ -27,6 +27,7 @@ const ( GetInstances GetVpcsAndSubnets AssignPrivateIpAddressesVMSS + UnassignPrivateIpAddressesVMSS MaxOperation ) @@ -234,6 +235,7 @@ func (a *API) AssignPrivateIpAddressesVMSS(ctx context.Context, vmName, vmssName IP: ip.String(), Subnet: subnetID, State: types.StateSucceeded, + Name: "Cilium-mock-" + ip.String(), }) } @@ -253,6 +255,73 @@ func (a *API) AssignPrivateIpAddressesVMSS(ctx context.Context, vmName, vmssName return nil } +func (a *API) UnassignPrivateIpAddressesVM(ctx context.Context, interfaceName string, addresses []string) error { + return nil +} + +func (a *API) UnassignPrivateIpAddressesVMSS(ctx context.Context, vmName, vmssName, interfaceName string, ipConfigNames []string) error { + a.rateLimit() + a.delaySim.Delay(UnassignPrivateIpAddressesVMSS) + + a.mutex.Lock() + defer a.mutex.Unlock() + + if err, ok := a.errors[UnassignPrivateIpAddressesVMSS]; ok { + return err + } + + if len(ipConfigNames) == 0 { + return nil + } + + dropNames := make(map[string]struct{}, len(ipConfigNames)) + for _, name := range ipConfigNames { + dropNames[name] = struct{}{} + } + + foundInterface := false + instances := a.instances.DeepCopy() + err := instances.ForeachInterface("", func(id, _ string, iface ipamTypes.InterfaceRevision) error { + intf, ok := iface.Resource.(*types.AzureInterface) + if !ok { + return fmt.Errorf("invalid interface object") + } + + if intf.Name != interfaceName || intf.GetVMID() != vmName { + return nil + } + + kept := intf.Addresses[:0] + for _, addr := range intf.Addresses { + if _, drop := dropNames[addr.Name]; drop { + if s, ok := a.subnets[addr.Subnet]; ok { + _, ipNet, err := net.ParseCIDR(addr.IP + "/32") + if err == nil { + s.allocator.Release(ipNet.IP) + } + } + continue + } + kept = append(kept, addr) + } + intf.Addresses = kept + + foundInterface = true + return nil + }) + if err != nil { + return err + } + + a.updateInstancesLocked(instances) + + if !foundInterface { + return fmt.Errorf("interface %s not found", interfaceName) + } + + return nil +} + func (a *API) AssignPublicIPAddressesVMSS(ctx context.Context, instanceID, vmssName string, publicIpTags ipamTypes.Tags) (string, error) { a.rateLimit() return "mock-public-ip-prefix-id", nil diff --git a/pkg/azure/ipam/instances.go b/pkg/azure/ipam/instances.go index 69cf2ca873567..d43dd0ae9ce29 100644 --- a/pkg/azure/ipam/instances.go +++ b/pkg/azure/ipam/instances.go @@ -22,6 +22,8 @@ type AzureAPI interface { GetVpcsAndSubnets(ctx context.Context) (ipamTypes.VirtualNetworkMap, ipamTypes.SubnetMap, error) AssignPrivateIpAddressesVM(ctx context.Context, subnetID, interfaceName string, addresses int) error AssignPrivateIpAddressesVMSS(ctx context.Context, instanceID, vmssName, subnetID, interfaceName string, addresses int) error + UnassignPrivateIpAddressesVM(ctx context.Context, interfaceName string, addresses []string) error + UnassignPrivateIpAddressesVMSS(ctx context.Context, instanceID, vmssName, interfaceName string, ipConfigNames []string) error AssignPublicIPAddressesVM(ctx context.Context, instanceID string, publicIpTags ipamTypes.Tags) (string, error) AssignPublicIPAddressesVMSS(ctx context.Context, instanceID, vmssName string, publicIpTags ipamTypes.Tags) (string, error) } diff --git a/pkg/azure/ipam/node.go b/pkg/azure/ipam/node.go index 73f59fe18a708..f5e51744dbbec 100644 --- a/pkg/azure/ipam/node.go +++ b/pkg/azure/ipam/node.go @@ -59,7 +59,46 @@ func (n *Node) PopulateStatusFields(k8sObj *v2.CiliumNode) { // PrepareIPRelease prepares the release of IPs func (n *Node) PrepareIPRelease(excessIPs int, scopedLog *slog.Logger) *ipam.ReleaseAction { - return &ipam.ReleaseAction{} + r := &ipam.ReleaseAction{} + requiredIfaceName := n.k8sObj.Spec.Azure.InterfaceName + usedIPs := n.k8sObj.Status.IPAM.Used + + n.manager.mutex.RLock() + defer n.manager.mutex.RUnlock() + + n.manager.instances.ForeachInterface(n.node.InstanceID(), func(_, _ string, obj ipamTypes.InterfaceRevision) error { + iface, ok := obj.Resource.(*types.AzureInterface) + if !ok { + return nil + } + if requiredIfaceName != "" && iface.Name != requiredIfaceName { + return nil + } + + var free []string + var poolID ipamTypes.PoolID + for _, addr := range iface.Addresses { + if addr.Primary { + continue + } + if _, used := usedIPs[addr.IP]; used { + continue + } + free = append(free, addr.IP) + if poolID == "" { + poolID = ipamTypes.PoolID(addr.Subnet) + } + } + + maxRelease := min(len(free), excessIPs) + if maxRelease > len(r.IPsToRelease) { + r.InterfaceID = iface.ID + r.PoolID = poolID + r.IPsToRelease = free[:maxRelease] + } + return nil + }) + return r } // ReleaseIPPrefixes is a no-op on Azure since Azure ENIs don't @@ -71,7 +110,60 @@ func (n *Node) ReleaseIPPrefixes(ctx context.Context, r *ipam.ReleaseAction) err // ReleaseIPs performs the IP release operation func (n *Node) ReleaseIPs(ctx context.Context, r *ipam.ReleaseAction) error { - return fmt.Errorf("not implemented") + if len(r.IPsToRelease) == 0 { + return nil + } + + var iface *types.AzureInterface + n.manager.mutex.RLock() + n.manager.instances.ForeachInterface(n.node.InstanceID(), func(_, interfaceID string, obj ipamTypes.InterfaceRevision) error { + if interfaceID == r.InterfaceID { + iface, _ = obj.Resource.(*types.AzureInterface) + } + return nil + }) + n.manager.mutex.RUnlock() + + if iface == nil { + return fmt.Errorf("interface %s not found for instance %s", r.InterfaceID, n.node.InstanceID()) + } + + if iface.GetVMScaleSetName() == "" { + return n.manager.api.UnassignPrivateIpAddressesVM(ctx, iface.Name, r.IPsToRelease) + } + + // VMSS path: the desired-state model addresses IP configurations by name, not IP. + // Translate using the cached AzureInterface so we can skip an extra Azure API call. + // Require a complete translation: a partial unassign would silently desync the + // framework's bookkeeping (it marks every IP in r.IPsToRelease as released after + // this call returns nil). + wantNames := make(map[string]string, len(r.IPsToRelease)) + for _, ip := range r.IPsToRelease { + wantNames[ip] = "" + } + for _, addr := range iface.Addresses { + if _, want := wantNames[addr.IP]; !want { + continue + } + if addr.Name == "" { + continue + } + wantNames[addr.IP] = addr.Name + } + ipConfigNames := make([]string, 0, len(r.IPsToRelease)) + var missing []string + for ip, name := range wantNames { + if name == "" { + missing = append(missing, ip) + continue + } + ipConfigNames = append(ipConfigNames, name) + } + if len(missing) > 0 { + return fmt.Errorf("no cached IPConfiguration name for IPs %v on interface %s; will retry next cycle", missing, iface.Name) + } + + return n.manager.api.UnassignPrivateIpAddressesVMSS(ctx, iface.GetVMID(), iface.GetVMScaleSetName(), iface.Name, ipConfigNames) } // PrepareIPAllocation returns the number of IPs that can be allocated/created. diff --git a/pkg/azure/ipam/node_test.go b/pkg/azure/ipam/node_test.go index da0318d7bd793..61ca289e9123c 100644 --- a/pkg/azure/ipam/node_test.go +++ b/pkg/azure/ipam/node_test.go @@ -4,14 +4,185 @@ package ipam import ( + "net/netip" "testing" + "github.com/cilium/hive/hivetest" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apimock "github.com/cilium/cilium/pkg/azure/api/mock" "github.com/cilium/cilium/pkg/azure/types" + "github.com/cilium/cilium/pkg/ipam" + ipamTypes "github.com/cilium/cilium/pkg/ipam/types" + v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2" +) + +const ( + testVMSSInstanceID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1" + testVMSSIfaceID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/eth0" + testVMInstanceID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachines/vm1" + testVMIfaceID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Network/networkInterfaces/eth0" ) func TestGetMaximumAllocatableIPv4(t *testing.T) { n := &Node{} require.Equal(t, types.InterfaceAddressLimit, n.GetMaximumAllocatableIPv4()) } + +type fakeNodeActions string + +func (f fakeNodeActions) InstanceID() string { return string(f) } + +func newAzureInterface(t *testing.T, id, name string, ips []string) *types.AzureInterface { + t.Helper() + iface := &types.AzureInterface{ + Name: name, + State: types.StateSucceeded, + } + for _, ip := range ips { + iface.Addresses = append(iface.Addresses, types.AzureAddress{ + IP: ip, + Subnet: "subnet-1", + State: types.StateSucceeded, + Name: "Cilium-test-" + ip, + }) + } + iface.SetID(id) + return iface +} + +func newReleaseTestNode(t *testing.T, instanceID string, iface *types.AzureInterface, requiredIfaceName string, used map[string]struct{}) (*Node, *apimock.API) { + t.Helper() + + api := apimock.NewAPI([]*ipamTypes.Subnet{ + {ID: "subnet-1", CIDR: netip.MustParsePrefix("1.1.0.0/16"), VirtualNetworkID: "vpc-1"}, + }, nil) + instances := NewInstancesManager(hivetest.Logger(t), api) + + m := ipamTypes.NewInstanceMap() + m.Update(instanceID, ipamTypes.InterfaceRevision{Resource: iface.DeepCopy()}) + api.UpdateInstances(m) + instances.mutex.Lock() + instances.instances = m + instances.mutex.Unlock() + + usedAlloc := ipamTypes.AllocationMap{} + for ip := range used { + usedAlloc[ip] = ipamTypes.AllocationIP{} + } + + n := &Node{ + k8sObj: &v2.CiliumNode{ + ObjectMeta: metav1.ObjectMeta{Name: "node1"}, + Spec: v2.NodeSpec{ + InstanceID: instanceID, + Azure: types.AzureSpec{InterfaceName: requiredIfaceName}, + }, + Status: v2.NodeStatus{ + IPAM: ipamTypes.IPAMStatus{Used: usedAlloc}, + }, + }, + node: fakeNodeActions(instanceID), + manager: instances, + } + return n, api +} + +func TestPrepareIPRelease_SelectsFreeIPs(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4"}) + used := map[string]struct{}{"1.1.1.1": {}, "1.1.1.2": {}} + + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "", used) + r := n.PrepareIPRelease(2, hivetest.Logger(t)) + + require.NotNil(t, r) + require.Equal(t, testVMSSIfaceID, r.InterfaceID) + require.Equal(t, ipamTypes.PoolID("subnet-1"), r.PoolID) + require.ElementsMatch(t, []string{"1.1.1.3", "1.1.1.4"}, r.IPsToRelease) +} + +func TestPrepareIPRelease_RespectsExcessLimit(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1", "1.1.1.2", "1.1.1.3"}) + + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "", nil) + r := n.PrepareIPRelease(1, hivetest.Logger(t)) + + require.Len(t, r.IPsToRelease, 1) +} + +func TestPrepareIPRelease_EmptyWhenAllUsed(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1", "1.1.1.2"}) + used := map[string]struct{}{"1.1.1.1": {}, "1.1.1.2": {}} + + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "", used) + r := n.PrepareIPRelease(2, hivetest.Logger(t)) + + require.Empty(t, r.IPsToRelease) +} + +func TestPrepareIPRelease_RequiredIfaceName(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1", "1.1.1.2"}) + + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "eth1", nil) + r := n.PrepareIPRelease(2, hivetest.Logger(t)) + + require.Empty(t, r.IPsToRelease) +} + +func TestReleaseIPs_NoOpWhenEmpty(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1"}) + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "", nil) + + require.NoError(t, n.ReleaseIPs(t.Context(), &ipam.ReleaseAction{})) +} + +func TestReleaseIPs_VMSSPath(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1", "1.1.1.2", "1.1.1.3"}) + + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "", nil) + require.Equal(t, "vmss11", iface.GetVMScaleSetName()) + + err := n.ReleaseIPs(t.Context(), &ipam.ReleaseAction{ + InterfaceID: testVMSSIfaceID, + PoolID: "subnet-1", + IPsToRelease: []string{"1.1.1.2", "1.1.1.3"}, + }) + require.NoError(t, err) + + got, err := n.manager.api.GetInstance(t.Context(), nil, testVMSSInstanceID) + require.NoError(t, err) + for _, ifaceObj := range got.Interfaces { + az := ifaceObj.Resource.(*types.AzureInterface) + ips := make([]string, 0, len(az.Addresses)) + for _, a := range az.Addresses { + ips = append(ips, a.IP) + } + require.ElementsMatch(t, []string{"1.1.1.1"}, ips) + } +} + +func TestReleaseIPs_VMPath(t *testing.T) { + iface := newAzureInterface(t, testVMIfaceID, "eth0", []string{"1.1.1.1", "1.1.1.2"}) + + n, _ := newReleaseTestNode(t, testVMInstanceID, iface, "", nil) + require.Empty(t, iface.GetVMScaleSetName()) + + err := n.ReleaseIPs(t.Context(), &ipam.ReleaseAction{ + InterfaceID: testVMIfaceID, + PoolID: "subnet-1", + IPsToRelease: []string{"1.1.1.2"}, + }) + require.NoError(t, err) +} + +func TestReleaseIPs_InterfaceNotFound(t *testing.T) { + iface := newAzureInterface(t, testVMSSIfaceID, "eth0", []string{"1.1.1.1"}) + n, _ := newReleaseTestNode(t, testVMSSInstanceID, iface, "", nil) + + err := n.ReleaseIPs(t.Context(), &ipam.ReleaseAction{ + InterfaceID: "missing", + IPsToRelease: []string{"1.1.1.1"}, + }) + require.Error(t, err) +} diff --git a/pkg/azure/types/types.go b/pkg/azure/types/types.go index cb5d7b8ebc242..55adbe097be52 100644 --- a/pkg/azure/types/types.go +++ b/pkg/azure/types/types.go @@ -69,6 +69,16 @@ type AzureAddress struct { // State is the provisioning state of the address State string `json:"state,omitempty"` + + // Name is the name of the IPConfiguration on the underlying Azure NIC. + // Used to identify the configuration when releasing IPs from a VMSS NIC, + // where the desired-state model addresses configurations by name rather + // than IP. + Name string `json:"name,omitempty"` + + // Primary indicates whether this is the primary IP configuration on the + // NIC. Primary IPs are never selected for release. + Primary bool `json:"primary,omitempty"` } // AzureInterface represents an Azure Interface diff --git a/pkg/azure/types/zz_generated.deepequal.go b/pkg/azure/types/zz_generated.deepequal.go index 8761f19495136..d8a838b4656ca 100644 --- a/pkg/azure/types/zz_generated.deepequal.go +++ b/pkg/azure/types/zz_generated.deepequal.go @@ -24,6 +24,12 @@ func (in *AzureAddress) DeepEqual(other *AzureAddress) bool { if in.State != other.State { return false } + if in.Name != other.Name { + return false + } + if in.Primary != other.Primary { + return false + } return true } diff --git a/pkg/ipam/allocator/azure/azure.go b/pkg/ipam/allocator/azure/azure.go index d5a30cc58b5c3..5ae4d6c4a6014 100644 --- a/pkg/ipam/allocator/azure/azure.go +++ b/pkg/ipam/allocator/azure/azure.go @@ -83,7 +83,7 @@ func (a *AllocatorAzure) Start(ctx context.Context, getterUpdater ipam.CiliumNod return nil, fmt.Errorf("unable to create Azure client: %w", err) } instances := azureIPAM.NewInstancesManager(a.rootLogger, azureClient) - nodeManager, err := ipam.NewNodeManager(a.logger, instances, getterUpdater, iMetrics, operatorOption.Config.ParallelAllocWorkers, false, false) + nodeManager, err := ipam.NewNodeManager(a.logger, instances, getterUpdater, iMetrics, operatorOption.Config.ParallelAllocWorkers, operatorOption.Config.AzureReleaseExcessIPs, false) if err != nil { return nil, fmt.Errorf("unable to initialize Azure node manager: %w", err) } diff --git a/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnodes.yaml b/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnodes.yaml index 580220758abca..304fe464d2abd 100644 --- a/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnodes.yaml +++ b/pkg/k8s/apis/cilium.io/client/crds/v2/ciliumnodes.yaml @@ -587,6 +587,18 @@ spec: ip: description: IP is the ip address of the address type: string + name: + description: |- + Name is the name of the IPConfiguration on the underlying Azure NIC. + Used to identify the configuration when releasing IPs from a VMSS NIC, + where the desired-state model addresses configurations by name rather + than IP. + type: string + primary: + description: |- + Primary indicates whether this is the primary IP configuration on the + NIC. Primary IPs are never selected for release. + type: boolean state: description: State is the provisioning state of the address diff --git a/pkg/k8s/apis/cilium.io/register.go b/pkg/k8s/apis/cilium.io/register.go index e9765e7db4779..726a02471b0c5 100644 --- a/pkg/k8s/apis/cilium.io/register.go +++ b/pkg/k8s/apis/cilium.io/register.go @@ -15,5 +15,5 @@ const ( // // Maintainers: Run ./Documentation/check-crd-compat-table.sh for each release // Developers: Bump patch for each change in the CRD schema. - CustomResourceDefinitionSchemaVersion = "1.32.6" + CustomResourceDefinitionSchemaVersion = "1.32.7" )