azure: derive AzureInterface VMSS/VM/RG values from the interface ID

The resource group, VMSS name and VM ID were cached in unexported json:"-"
fields, so an interface built from the Azure API never matched the copy
read back from the apiserver and the operator rewrote /status on every sync.

Derive them from ID on demand instead. SetID() then only assigns ID, so
remove it.

Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com>

Author: jaredledvina

pkg/azure/api/api.go

@@ -325,7 +325,7 @@ func parseInterface(logger *slog.Logger, iface *armnetwork.Interface, subnets ip
 	}
 
 	if iface.ID != nil {
-		i.SetID(*iface.ID)
+		i.ID = *iface.ID
 	}
 
 	if iface.Name != nil {

pkg/azure/api/api_test.go

@@ -12,8 +12,6 @@ import (
 	"github.com/cilium/hive/hivetest"
 	"github.com/stretchr/testify/require"
 
-	// Required so SetID() resolves the Azure resource-ID parser.
-	_ "github.com/cilium/cilium/pkg/azure/types/azureid"
 	iputil "github.com/cilium/cilium/pkg/ip"
 	ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
 )
@@ -156,6 +154,7 @@ func TestParseInterface(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			_, got := parseInterface(hivetest.Logger(t), tt.iface, tt.subnets, tt.usePrimary)
 			require.NotNil(t, got)
+			require.Equal(t, ifaceID, got.ID)
 			require.Equal(t, tt.expectedIP, got.IP)
 			require.Equal(t, tt.expectedSubnetID, got.Subnet.ID)
 			require.Equal(t, tt.expectedCIDR, got.Subnet.CIDR)

pkg/azure/api/mock/mock_test.go

@@ -11,8 +11,8 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/cilium/cilium/pkg/azure/types"
-	// Register the Azure resource-ID parser so AzureInterface.SetID() can
-	// populate VMSS/VM/RG fields used by AssignPrivateIpAddressesVMSS lookup.
+	// Register the Azure resource-ID parser so AzureInterface.GetVMID()
+	// resolves, which the AssignPrivateIpAddressesVMSS lookup compares against.
 	_ "github.com/cilium/cilium/pkg/azure/types/azureid"
 	ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
 )
@@ -36,7 +36,7 @@ func TestMock(t *testing.T) {
 	ifaceID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
 	instances = ipamTypes.NewInstanceMap()
 	resource := &types.AzureInterface{Name: "eth0"}
-	resource.SetID(ifaceID)
+	resource.ID = ifaceID
 	instances.Update("vm1", resource.DeepCopy())
 	api.UpdateInstances(instances)
 	nics, err = api.ListAllNetworkInterfaces(t.Context())
@@ -68,7 +68,7 @@ func TestMock(t *testing.T) {
 	vmIfaceID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Network/networkInterfaces/vm22-if"
 	vmInstances := ipamTypes.NewInstanceMap()
 	resource = &types.AzureInterface{Name: "eth0"}
-	resource.SetID(vmIfaceID)
+	resource.ID = vmIfaceID
 	vmInstances.Update("vm2", resource.DeepCopy())
 	require.NoError(t, err)
 	require.Equal(t, 1, vmInstances.NumInstances())

pkg/azure/ipam/instances.go

@@ -16,9 +16,8 @@ import (
 	"github.com/cilium/cilium/operator/pkg/ipam/nodemanager"
 	"github.com/cilium/cilium/pkg/azure/types"
 
-	// Register the Azure resource-ID parser. This is the canonical place
-	// for Azure-IPAM-enabled binaries to wire in pkg/azure/types' parser
-	// so AzureInterface.SetID() can populate the VMSS/VM/RG fields.
+	// Registers the Azure resource-ID parser used by AzureInterface's VMSS/VM
+	// getters.
 	_ "github.com/cilium/cilium/pkg/azure/types/azureid"
 	ipamTypes "github.com/cilium/cilium/pkg/ipam/types"
 	v2 "github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2"

pkg/azure/ipam/instances_test.go

@@ -79,7 +79,7 @@ func iteration1(t *testing.T, api *apimock.API, mngr *InstancesManager) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("intf-1")
+	resource.ID = "intf-1"
 	instances.Update("i-1", resource.DeepCopy())
 
 	resource = &types.AzureInterface{
@@ -93,7 +93,7 @@ func iteration1(t *testing.T, api *apimock.API, mngr *InstancesManager) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("intf-3")
+	resource.ID = "intf-3"
 	instances.Update("i-2", resource.DeepCopy())
 
 	api.UpdateInstances(instances)
@@ -117,7 +117,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("intf-1")
+	resource.ID = "intf-1"
 	instances.Update("i-1", resource.DeepCopy())
 
 	resource = &types.AzureInterface{
@@ -131,7 +131,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("intf-2")
+	resource.ID = "intf-2"
 	instances.Update("i-1", resource.DeepCopy())
 
 	resource = &types.AzureInterface{
@@ -145,7 +145,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("intf-3")
+	resource.ID = "intf-3"
 	instances.Update("i-2", resource.DeepCopy())
 
 	api.UpdateInstances(instances)
@@ -211,7 +211,7 @@ func TestResyncInstancePreservesOtherNodesSubnets(t *testing.T) {
 		},
 		State: types.StateSucceeded,
 	}
-	iface1.SetID("intf-vm-1")
+	iface1.ID = "intf-vm-1"
 	instances.Update("vm-1", iface1.DeepCopy())
 
 	iface2 := &types.AzureInterface{
@@ -225,7 +225,7 @@ func TestResyncInstancePreservesOtherNodesSubnets(t *testing.T) {
 		},
 		State: types.StateSucceeded,
 	}
-	iface2.SetID("intf-vm-2")
+	iface2.ID = "intf-vm-2"
 	instances.Update("vm-2", iface2.DeepCopy())
 
 	api.UpdateInstances(instances)
@@ -278,7 +278,7 @@ func TestExtractSubnetIDs(t *testing.T) {
 				},
 			},
 		}
-		resource.SetID(interfaceID)
+		resource.ID = interfaceID
 
 		instances.Update(instanceID, resource.DeepCopy())
 	}

pkg/azure/ipam/ipam_test.go

@@ -161,7 +161,7 @@ func TestIpamPreAllocate8(t *testing.T) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11")
+	resource.ID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
 	vm1ID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1"
 	m.Update(vm1ID, resource.DeepCopy())
 	api.UpdateInstances(m)
@@ -223,7 +223,7 @@ func TestIpamMinAllocate10(t *testing.T) {
 		},
 		State: types.StateSucceeded,
 	}
-	resource.SetID("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11")
+	resource.ID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
 	vm1ID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1"
 	m.Update(vm1ID, resource.DeepCopy())
 	api.UpdateInstances(m)
@@ -315,7 +315,7 @@ func TestIpamManyNodes(t *testing.T) {
 					},
 					State: types.StateSucceeded,
 				}
-				resource.SetID(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i))
+				resource.ID = fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i)
 				allInstances.Update(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d", i), resource.DeepCopy())
 			}
 
@@ -386,7 +386,7 @@ func benchmarkAllocWorker(b *testing.B, workers int64, delay time.Duration, rate
 			Addresses:     []types.AzureAddress{},
 			State:         types.StateSucceeded,
 		}
-		resource.SetID(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i))
+		resource.ID = fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i)
 		allInstances.Update(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d", i), resource.DeepCopy())
 	}
 

pkg/azure/ipam/node.go

@@ -156,10 +156,11 @@ func (n *Node) AllocateIPs(ctx context.Context, a *nodemanager.AllocationAction)
 		return fmt.Errorf("invalid interface object")
 	}
 
-	if iface.GetVMScaleSetName() == "" {
+	vmss := iface.GetVMScaleSetName()
+	if vmss == "" {
 		return n.manager.api.AssignPrivateIpAddressesVM(ctx, string(a.PoolID), iface.Name, a.IPv4.AvailableForAllocation)
 	} else {
-		return n.manager.api.AssignPrivateIpAddressesVMSS(ctx, iface.GetVMID(), iface.GetVMScaleSetName(), string(a.PoolID), iface.Name, a.IPv4.AvailableForAllocation)
+		return n.manager.api.AssignPrivateIpAddressesVMSS(ctx, iface.GetVMID(), vmss, string(a.PoolID), iface.Name, a.IPv4.AvailableForAllocation)
 	}
 }
 

pkg/azure/ipam/node_stats_test.go

@@ -54,7 +54,7 @@ func newCapacityTestInterface(name, id, primaryIP string, secondaryIPs ...string
 		Addresses:     addrs,
 		State:         types.StateSucceeded,
 	}
-	iface.SetID(id)
+	iface.ID = id
 	return iface
 }
 

pkg/azure/types/azureid/azureid.go

@@ -9,8 +9,8 @@
 // pkg/azure/types via CiliumNode's AzureSpec).
 //
 // On import, this package registers Parse with pkg/azure/types so that
-// AzureInterface.SetID() can extract the resource group, VMSS name, and VM ID
-// from a network interface resource ID.
+// AzureInterface's GetResourceGroup(), GetVMScaleSetName(), and GetVMID() can
+// derive their values from the network interface resource ID.
 package azureid
 
 import (

pkg/azure/types/extract_ids.go

@@ -8,7 +8,7 @@ package types
 // from pkg/azure/types/azureid (which uses the Azure SDK) so that pkg/azure/types
 // itself does not transitively pull the Azure SDK into every consumer of
 // CiliumNode (which embeds AzureSpec). Non-Azure binaries leave this stub in
-// place; they never call extractIDs() on real Azure resource IDs.
+// place.
 var resourceIDParser = func(_ string) (resourceGroup, vmssName, vmID string) {
 	return "", "", ""
 }