azure: Remove unused GetVpcsAndSubnets function

The refactor in cilium#41555 to do targeted subnet discovery replaced all
usages of `GetVpcsAndSubnets` with `GetSubnetsByIDs`. That function is
now just dead code.

This PR removes the unused `GetVpcsAndSubnets` function along with
related supporting code: the `listAllVPCs` and `parseSubnet` functions
that were only called from `GetVpcsAndSubnets` and mocks for all those.

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

Author: HadrienPatte

pkg/azure/api/api.go

@@ -38,7 +38,6 @@ const (
 	virtualMachineScaleSetsList     = "VirtualMachineScaleSets.List"
 	virtualMachineScaleSetVMsGet    = "VirtualMachineScaleSetVMs.Get"
 	virtualMachineScaleSetVMsUpdate = "VirtualMachineScaleSetVMs.Update"
-	virtualNetworksListAll          = "VirtualNetworks.ListAll"
 	subnetsGet                      = "Subnets.Get"
 
 	interfacesListVirtualMachineScaleSetNetworkInterfaces   = "Interfaces.ListVirtualMachineScaleSetNetworkInterfaces"
@@ -52,7 +51,6 @@ type Client struct {
 	resourceGroup             string
 	interfaces                *armnetwork.InterfacesClient
 	publicIPPrefixes          *armnetwork.PublicIPPrefixesClient
-	virtualNetworks           *armnetwork.VirtualNetworksClient
 	virtualMachines           *armcompute.VirtualMachinesClient
 	subnets                   *armnetwork.SubnetsClient
 	virtualMachineScaleSetVMs *armcompute.VirtualMachineScaleSetVMsClient
@@ -132,11 +130,6 @@ func NewClient(logger *slog.Logger, cloudName, subscriptionID, resourceGroup, us
 		return nil, err
 	}
 
-	virtualNetworksClient, err := armnetwork.NewVirtualNetworksClient(subscriptionID, credential, armClientOptions)
-	if err != nil {
-		return nil, err
-	}
-
 	virtualMachinesClient, err := armcompute.NewVirtualMachinesClient(subscriptionID, credential, armClientOptions)
 	if err != nil {
 		return nil, err
@@ -168,7 +161,6 @@ func NewClient(logger *slog.Logger, cloudName, subscriptionID, resourceGroup, us
 		resourceGroup:             resourceGroup,
 		interfaces:                interfacesClient,
 		publicIPPrefixes:          publicIPPrefixesClient,
-		virtualNetworks:           virtualNetworksClient,
 		virtualMachines:           virtualMachinesClient,
 		subnets:                   subnetsClient,
 		virtualMachineScaleSetVMs: virtualMachineScaleSetVMsClient,
@@ -446,96 +438,13 @@ func (c *Client) ParseInterfacesIntoInstance(networkInterfaces []*armnetwork.Int
 	return &instance
 }
 
-// listAllVPCs lists all VPCs
-func (c *Client) listAllVPCs(ctx context.Context) (vpcs []*armnetwork.VirtualNetwork, err error) {
-	c.limiter.Limit(ctx, virtualNetworksListAll)
-	sinceStart := spanstat.Start()
-
-	// Note: lists all VPCs, not just those in c.resourcegroup
-	pager := c.virtualNetworks.NewListAllPager(nil)
-
-	defer func() {
-		c.metricsAPI.ObserveAPICall(virtualNetworksListAll, deriveStatus(err), sinceStart.Seconds())
-	}()
-
-	for pager.More() {
-		nextResult, err := pager.NextPage(ctx)
-		if err != nil {
-			return nil, err
-		}
-		vpcs = append(vpcs, nextResult.Value...)
-	}
-
-	return vpcs, nil
-}
-
-func parseSubnet(subnet *armnetwork.Subnet) (s *ipamTypes.Subnet) {
-	s = &ipamTypes.Subnet{ID: *subnet.ID}
-	if subnet.Name != nil {
-		s.Name = *subnet.Name
-	}
-
-	if subnet.Properties.AddressPrefix != nil {
-		cidr, err := netip.ParsePrefix(*subnet.Properties.AddressPrefix)
-		if err != nil {
-			return nil
-		}
-		s.CIDR = cidr
-		if subnet.Properties.IPConfigurations != nil {
-			s.AvailableAddresses = availableIPs(cidr) - len(subnet.Properties.IPConfigurations)
-		} else {
-			// Azure currently returns nil for subnet IPConfigs if the subnet has a large number of existing IPConfigs.
-			// API / SDK is supposed to return a IpConfigurationsNextLink which can be used to make an additional
-			// call to get all IPConfigs. This field however seems to be missing from the API spec.
-			// Since we cannot fall back to other subnets anyway, assume all IPs are available.
-			// TODO: Update this once azure-sdk-for-go supports ipConfigurationsNextLink
-			s.AvailableAddresses = availableIPs(cidr)
-		}
-	}
-
-	return
-}
-
 // availableIPs returns the number of IPs available in a CIDR
 func availableIPs(p netip.Prefix) int {
 	ones := p.Bits()
 	bits := p.Addr().BitLen()
 	return 1 << (bits - ones)
 }
 
-// GetVpcsAndSubnets retrieves and returns all Vpcs
-func (c *Client) GetVpcsAndSubnets(ctx context.Context) (ipamTypes.VirtualNetworkMap, ipamTypes.SubnetMap, error) {
-	vpcs := ipamTypes.VirtualNetworkMap{}
-	subnets := ipamTypes.SubnetMap{}
-
-	vpcList, err := c.listAllVPCs(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	for _, v := range vpcList {
-		if v.ID == nil {
-			continue
-		}
-
-		vpc := &ipamTypes.VirtualNetwork{ID: *v.ID}
-		vpcs[vpc.ID] = vpc
-
-		if v.Properties.Subnets != nil {
-			for _, subnet := range v.Properties.Subnets {
-				if subnet.ID == nil {
-					continue
-				}
-				if s := parseSubnet(subnet); s != nil {
-					subnets[*subnet.ID] = s
-				}
-			}
-		}
-	}
-
-	return vpcs, subnets, nil
-}
-
 // parseSubnetID extracts resource group, virtual network, and subnet names from an Azure subnet ID.
 // Expected format: /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/virtualNetworks/{vnetName}/subnets/{subnetName}
 // Uses arm.ParseResourceID from the Azure SDK for robust parsing.

pkg/azure/api/mock/mock.go

@@ -27,7 +27,6 @@ const (
 	AllOperations Operation = iota
 	ListVMNetworkInterfaces
 	ListAllNetworkInterfaces
-	GetVpcsAndSubnets
 	GetSubnetsByIDs
 	AssignPrivateIpAddressesVMSS
 	MaxOperation
@@ -42,27 +41,21 @@ type API struct {
 	mutex     lock.RWMutex
 	subnets   map[string]*subnet
 	instances *ipamTypes.InstanceMap
-	vnets     map[string]*ipamTypes.VirtualNetwork
 	errors    map[Operation]error
 	delaySim  *helpers.DelaySimulator
 	limiter   *rate.Limiter
 }
 
-func NewAPI(subnets []*ipamTypes.Subnet, vnets []*ipamTypes.VirtualNetwork) *API {
+func NewAPI(subnets []*ipamTypes.Subnet) *API {
 	api := &API{
 		instances: ipamTypes.NewInstanceMap(),
 		subnets:   map[string]*subnet{},
-		vnets:     map[string]*ipamTypes.VirtualNetwork{},
 		errors:    map[Operation]error{},
 		delaySim:  helpers.NewDelaySimulator(),
 	}
 
 	api.UpdateSubnets(subnets)
 
-	for _, v := range vnets {
-		api.vnets[v.ID] = v
-	}
-
 	return api
 }
 
@@ -129,33 +122,6 @@ func (a *API) rateLimit() {
 	}
 }
 
-func (a *API) GetVpcsAndSubnets(ctx context.Context) (ipamTypes.VirtualNetworkMap, ipamTypes.SubnetMap, error) {
-	a.rateLimit()
-	a.delaySim.Delay(GetVpcsAndSubnets)
-
-	a.mutex.RLock()
-	defer a.mutex.RUnlock()
-
-	if err, ok := a.errors[GetVpcsAndSubnets]; ok {
-		return nil, nil, err
-	}
-
-	vnets := ipamTypes.VirtualNetworkMap{}
-	subnets := ipamTypes.SubnetMap{}
-
-	for _, s := range a.subnets {
-		sd := s.subnet.DeepCopy()
-		sd.AvailableAddresses = s.allocator.Free()
-		subnets[sd.ID] = sd
-	}
-
-	for _, v := range a.vnets {
-		vnets[v.ID] = v.DeepCopy()
-	}
-
-	return vnets, subnets, nil
-}
-
 func (a *API) GetSubnetsByIDs(ctx context.Context, nodeSubnetIDs []string) (ipamTypes.SubnetMap, error) {
 	a.rateLimit()
 	a.delaySim.Delay(GetSubnetsByIDs)

pkg/azure/api/mock/mock_test.go

@@ -20,18 +20,16 @@ import (
 func TestMock(t *testing.T) {
 	cidr := netip.MustParsePrefix("10.0.0.0/16")
 	subnet := &ipamTypes.Subnet{ID: "s-1", CIDR: cidr, AvailableAddresses: 65534}
-	api := NewAPI([]*ipamTypes.Subnet{subnet}, []*ipamTypes.VirtualNetwork{{ID: "v-1"}})
+	api := NewAPI([]*ipamTypes.Subnet{subnet})
 	require.NotNil(t, api)
 
 	nics, err := api.ListAllNetworkInterfaces(t.Context())
 	require.NoError(t, err)
 	instances := api.ParseInterfacesIntoInstanceMap(nics, ipamTypes.SubnetMap{})
 	require.Equal(t, 0, instances.NumInstances())
 
-	vnets, subnets, err := api.GetVpcsAndSubnets(t.Context())
+	subnets, err := api.GetSubnetsByIDs(t.Context(), []string{"s-1"})
 	require.NoError(t, err)
-	require.Len(t, vnets, 1)
-	require.Equal(t, &ipamTypes.VirtualNetwork{ID: "v-1"}, vnets["v-1"])
 	require.Len(t, subnets, 1)
 	require.Equal(t, subnet, subnets["s-1"])
 
@@ -83,7 +81,7 @@ func TestMock(t *testing.T) {
 }
 
 func TestSetMockError(t *testing.T) {
-	api := NewAPI([]*ipamTypes.Subnet{}, []*ipamTypes.VirtualNetwork{})
+	api := NewAPI([]*ipamTypes.Subnet{})
 	require.NotNil(t, api)
 
 	mockError := errors.New("error")
@@ -92,8 +90,8 @@ func TestSetMockError(t *testing.T) {
 	_, err := api.ListAllNetworkInterfaces(t.Context())
 	require.ErrorIs(t, err, mockError)
 
-	api.SetMockError(GetVpcsAndSubnets, mockError)
-	_, _, err = api.GetVpcsAndSubnets(t.Context())
+	api.SetMockError(GetSubnetsByIDs, mockError)
+	_, err = api.GetSubnetsByIDs(t.Context(), nil)
 	require.ErrorIs(t, err, mockError)
 
 	api.SetMockError(AssignPrivateIpAddressesVMSS, mockError)
@@ -104,7 +102,7 @@ func TestSetMockError(t *testing.T) {
 func TestSetLimiter(t *testing.T) {
 	cidr := netip.MustParsePrefix("10.0.0.0/16")
 	subnet := &ipamTypes.Subnet{ID: "s-1", CIDR: cidr, AvailableAddresses: 100}
-	api := NewAPI([]*ipamTypes.Subnet{subnet}, []*ipamTypes.VirtualNetwork{{ID: "v-1"}})
+	api := NewAPI([]*ipamTypes.Subnet{subnet})
 	require.NotNil(t, api)
 
 	api.SetLimiter(10.0, 2)

pkg/azure/ipam/instances.go

@@ -28,7 +28,6 @@ import (
 
 // AzureAPI is the API surface used of the Azure API.
 type AzureAPI interface {
-	GetVpcsAndSubnets(ctx context.Context) (ipamTypes.VirtualNetworkMap, ipamTypes.SubnetMap, error)
 	GetSubnetsByIDs(ctx context.Context, nodeSubnetIDs []string) (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

pkg/azure/ipam/instances_test.go

@@ -62,11 +62,6 @@ var (
 			},
 		},
 	}
-
-	vnets = []*ipamTypes.VirtualNetwork{
-		{ID: "vpc-0"},
-		{ID: "vpc-1"},
-	}
 )
 
 func iteration1(t *testing.T, api *apimock.API, mngr *InstancesManager) {
@@ -158,7 +153,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
 }
 
 func TestSubnetDiscovery(t *testing.T) {
-	api := apimock.NewAPI(subnets, vnets)
+	api := apimock.NewAPI(subnets)
 	require.NotNil(t, api)
 
 	mngr := NewInstancesManager(hivetest.Logger(t), api, false)
@@ -193,7 +188,7 @@ func TestSubnetDiscovery(t *testing.T) {
 // the wrong subnet, which Azure rejects with
 // VMScaleSetIpConfigurationsOnSameNicCannotUseDifferentSubnets.
 func TestResyncInstancePreservesOtherNodesSubnets(t *testing.T) {
-	api := apimock.NewAPI(subnets2, vnets)
+	api := apimock.NewAPI(subnets2)
 	require.NotNil(t, api)
 
 	mngr := NewInstancesManager(hivetest.Logger(t), api, false)
@@ -250,7 +245,7 @@ func TestResyncInstancePreservesOtherNodesSubnets(t *testing.T) {
 }
 
 func TestExtractSubnetIDs(t *testing.T) {
-	api := apimock.NewAPI(subnets, vnets)
+	api := apimock.NewAPI(subnets)
 	require.NotNil(t, api)
 
 	mngr := NewInstancesManager(hivetest.Logger(t), api, false)

pkg/azure/ipam/ipam_test.go

@@ -37,10 +37,6 @@ var (
 		{ID: "s-2", CIDR: netip.MustParsePrefix("2.2.0.0/16"), VirtualNetworkID: "vpc-1"},
 		{ID: "s-3", CIDR: netip.MustParsePrefix("3.3.3.3/16"), VirtualNetworkID: "vpc-1"},
 	}
-
-	testVnet = &ipamTypes.VirtualNetwork{
-		ID: "vpc-1",
-	}
 )
 
 type k8sMock struct {
@@ -146,7 +142,7 @@ func TestIpamPreAllocate8(t *testing.T) {
 	minAllocate := 0
 	toUse := 7
 
-	api := apimock.NewAPI([]*ipamTypes.Subnet{testSubnet}, []*ipamTypes.VirtualNetwork{testVnet})
+	api := apimock.NewAPI([]*ipamTypes.Subnet{testSubnet})
 	instances := NewInstancesManager(hivetest.Logger(t), api, false)
 	require.NotNil(t, instances)
 
@@ -208,7 +204,7 @@ func TestIpamMinAllocate10(t *testing.T) {
 	minAllocate := 10
 	toUse := 7
 
-	api := apimock.NewAPI([]*ipamTypes.Subnet{testSubnet}, []*ipamTypes.VirtualNetwork{testVnet})
+	api := apimock.NewAPI([]*ipamTypes.Subnet{testSubnet})
 	instances := NewInstancesManager(hivetest.Logger(t), api, false)
 	require.NotNil(t, instances)
 
@@ -292,7 +288,7 @@ func TestIpamManyNodes(t *testing.T) {
 				numNodes    = 2
 				minAllocate = 1
 			)
-			api := apimock.NewAPI(testSubnets, []*ipamTypes.VirtualNetwork{testVnet})
+			api := apimock.NewAPI(testSubnets)
 			instances := NewInstancesManager(hivetest.Logger(t), api, false)
 			require.NotNil(t, instances)
 
@@ -364,7 +360,7 @@ func TestIpamManyNodes(t *testing.T) {
 }
 
 func benchmarkAllocWorker(b *testing.B, workers int64, delay time.Duration, rateLimit float64, burst int) {
-	api := apimock.NewAPI(testSubnets, []*ipamTypes.VirtualNetwork{testVnet})
+	api := apimock.NewAPI(testSubnets)
 	api.SetDelay(apimock.AllOperations, delay)
 	api.SetLimiter(rateLimit, burst)
 

pkg/azure/ipam/node_stats_test.go

@@ -25,7 +25,7 @@ func newCapacityTestNode(t *testing.T, ifaces []*types.AzureInterface, usePrimar
 		node: mockIPAMNode("vm1"),
 		manager: &InstancesManager{
 			instances:  m,
-			api:        apimock.NewAPI(nil, nil),
+			api:        apimock.NewAPI(nil),
 			usePrimary: usePrimary,
 		},
 		k8sObj: &v2.CiliumNode{