Skip to content

Commit 5b95ab1

Browse files
committed
[ipam/azure] Derive AzureInterface VMSS/VM/RG values from the interface ID
Signed-off-by: Jared Ledvina <jared.ledvina@datadoghq.com>
1 parent 0a9b3d5 commit 5b95ab1

8 files changed

Lines changed: 120 additions & 55 deletions

File tree

pkg/azure/api/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ func parseInterface(iface *armnetwork.Interface, subnets ipamTypes.SubnetMap, us
329329
}
330330

331331
if iface.ID != nil {
332-
i.SetID(*iface.ID)
332+
i.ID = *iface.ID
333333
}
334334

335335
if iface.Name != nil {

pkg/azure/api/mock/mock_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestMock(t *testing.T) {
3535
ifaceID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
3636
instances = ipamTypes.NewInstanceMap()
3737
resource := &types.AzureInterface{Name: "eth0"}
38-
resource.SetID(ifaceID)
38+
resource.ID = ifaceID
3939
instances.Update("vm1", ipamTypes.InterfaceRevision{
4040
Resource: resource.DeepCopy(),
4141
})
@@ -69,7 +69,7 @@ func TestMock(t *testing.T) {
6969
vmIfaceID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Network/networkInterfaces/vm22-if"
7070
vmInstances := ipamTypes.NewInstanceMap()
7171
resource = &types.AzureInterface{Name: "eth0"}
72-
resource.SetID(vmIfaceID)
72+
resource.ID = vmIfaceID
7373
vmInstances.Update("vm2", ipamTypes.InterfaceRevision{
7474
Resource: resource.DeepCopy(),
7575
})

pkg/azure/ipam/instances_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func iteration1(t *testing.T, api *apimock.API, mngr *InstancesManager) {
8383
},
8484
State: types.StateSucceeded,
8585
}
86-
resource.SetID("intf-1")
86+
resource.ID = "intf-1"
8787
instances.Update("i-1", ipamTypes.InterfaceRevision{
8888
Resource: resource.DeepCopy(),
8989
})
@@ -99,7 +99,7 @@ func iteration1(t *testing.T, api *apimock.API, mngr *InstancesManager) {
9999
},
100100
State: types.StateSucceeded,
101101
}
102-
resource.SetID("intf-3")
102+
resource.ID = "intf-3"
103103
instances.Update("i-2", ipamTypes.InterfaceRevision{
104104
Resource: resource.DeepCopy(),
105105
})
@@ -124,7 +124,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
124124
},
125125
State: types.StateSucceeded,
126126
}
127-
resource.SetID("intf-1")
127+
resource.ID = "intf-1"
128128
instances.Update("i-1", ipamTypes.InterfaceRevision{
129129
Resource: resource.DeepCopy(),
130130
})
@@ -140,7 +140,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
140140
},
141141
State: types.StateSucceeded,
142142
}
143-
resource.SetID("intf-2")
143+
resource.ID = "intf-2"
144144
instances.Update("i-1", ipamTypes.InterfaceRevision{
145145
Resource: resource.DeepCopy(),
146146
})
@@ -156,7 +156,7 @@ func iteration2(t *testing.T, api *apimock.API, mngr *InstancesManager) {
156156
},
157157
State: types.StateSucceeded,
158158
}
159-
resource.SetID("intf-3")
159+
resource.ID = "intf-3"
160160
instances.Update("i-2", ipamTypes.InterfaceRevision{
161161
Resource: resource.DeepCopy(),
162162
})
@@ -223,7 +223,7 @@ func TestResyncInstancePreservesOtherNodesSubnets(t *testing.T) {
223223
},
224224
State: types.StateSucceeded,
225225
}
226-
iface1.SetID("intf-vm-1")
226+
iface1.ID = "intf-vm-1"
227227
instances.Update("vm-1", ipamTypes.InterfaceRevision{
228228
Resource: iface1.DeepCopy(),
229229
})
@@ -239,7 +239,7 @@ func TestResyncInstancePreservesOtherNodesSubnets(t *testing.T) {
239239
},
240240
State: types.StateSucceeded,
241241
}
242-
iface2.SetID("intf-vm-2")
242+
iface2.ID = "intf-vm-2"
243243
instances.Update("vm-2", ipamTypes.InterfaceRevision{
244244
Resource: iface2.DeepCopy(),
245245
})
@@ -292,7 +292,7 @@ func TestExtractSubnetIDs(t *testing.T) {
292292
},
293293
},
294294
}
295-
resource.SetID(interfaceID)
295+
resource.ID = interfaceID
296296

297297
instances.Update(instanceID, ipamTypes.InterfaceRevision{
298298
Resource: resource.DeepCopy(),

pkg/azure/ipam/ipam_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func TestIpamPreAllocate8(t *testing.T) {
164164
},
165165
State: types.StateSucceeded,
166166
}
167-
resource.SetID("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11")
167+
resource.ID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
168168
vm1ID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1"
169169
m.Update(vm1ID, ipamTypes.InterfaceRevision{
170170
Resource: resource.DeepCopy(),
@@ -227,7 +227,7 @@ func TestIpamMinAllocate10(t *testing.T) {
227227
},
228228
State: types.StateSucceeded,
229229
}
230-
resource.SetID("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11")
230+
resource.ID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
231231
vm1ID := "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1"
232232
m.Update(vm1ID, ipamTypes.InterfaceRevision{
233233
Resource: resource.DeepCopy(),
@@ -320,7 +320,7 @@ func TestIpamManyNodes(t *testing.T) {
320320
},
321321
State: types.StateSucceeded,
322322
}
323-
resource.SetID(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i))
323+
resource.ID = fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i)
324324
allInstances.Update(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d", i), ipamTypes.InterfaceRevision{
325325
Resource: resource.DeepCopy(),
326326
})
@@ -397,7 +397,7 @@ func benchmarkAllocWorker(b *testing.B, workers int64, delay time.Duration, rate
397397
Addresses: []types.AzureAddress{},
398398
State: types.StateSucceeded,
399399
}
400-
resource.SetID(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i))
400+
resource.ID = fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d/networkInterfaces/vmss11", i)
401401
allInstances.Update(fmt.Sprintf("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm%d", i), ipamTypes.InterfaceRevision{
402402
Resource: resource.DeepCopy(),
403403
})

pkg/azure/ipam/node_stats_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestENIIPAMCapacityAccounting(t *testing.T) {
2929
},
3030
State: types.StateSucceeded,
3131
}
32-
resource.SetID("/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11")
32+
resource.ID = "/subscriptions/xxx/resourceGroups/g1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss11/virtualMachines/vm1/networkInterfaces/vmss11"
3333
m.Update("vm1", ipamTypes.InterfaceRevision{
3434
Resource: resource.DeepCopy(),
3535
})

pkg/azure/types/types.go

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ type AzureAddress struct {
7373

7474
// AzureInterface represents an Azure Interface
7575
//
76+
// Every field must be exported and JSON-serialized; see
77+
// TestAzureStatusHasNoUnserializedState.
78+
//
7679
// +k8s:deepcopy-gen=true
7780
type AzureInterface struct {
7881
// ID is the identifier
@@ -120,44 +123,27 @@ type AzureInterface struct {
120123
//
121124
// +optional
122125
CIDR string `json:"cidr,omitempty"`
123-
124-
// vmssName is the name of the virtual machine scale set. This field is
125-
// set by extractIDs()
126-
vmssName string `json:"-"`
127-
128-
// vmID is the ID of the virtual machine
129-
vmID string `json:"-"`
130-
131-
// resourceGroup is the resource group the interface belongs to
132-
resourceGroup string `json:"-"`
133126
}
134127

135128
func (a *AzureInterface) DeepCopyInterface() types.Interface {
136129
return a.DeepCopy()
137130
}
138131

139-
// SetID sets the Azure interface ID, as well as extracting other fields from
140-
// the ID itself.
141-
func (a *AzureInterface) SetID(id string) {
142-
a.ID = id
143-
a.extractIDs()
144-
}
145-
146132
// InterfaceID returns the identifier of the interface
147133
func (a *AzureInterface) InterfaceID() string {
148134
return a.ID
149135
}
150136

151-
// extractIDs extracts resource group name, VMSS name, and VM ID from the network interface Azure resource ID
152-
func (a *AzureInterface) extractIDs() {
153-
resourceID, err := arm.ParseResourceID(a.ID)
137+
// parseAzureResourceID extracts the resource group name, VMSS name, and VM ID
138+
// from a network interface Azure resource ID. Missing or unparseable components
139+
// yield empty strings.
140+
func parseAzureResourceID(id string) (resourceGroup, vmssName, vmID string) {
141+
resourceID, err := arm.ParseResourceID(id)
154142
if err != nil {
155-
// If parsing fails, leave fields empty
156-
return
143+
return "", "", ""
157144
}
158145

159-
// Extract resource group name directly from the parsed ID
160-
a.resourceGroup = resourceID.ResourceGroupName
146+
resourceGroup = resourceID.ResourceGroupName
161147

162148
// For VMSS instances, walk up the parent chain to extract VMSS name and VM ID
163149
// Resource ID structure for VMSS VM interfaces:
@@ -173,30 +159,35 @@ func (a *AzureInterface) extractIDs() {
173159
lastType := resourceType.Types[len(resourceType.Types)-1]
174160

175161
if strings.EqualFold(lastType, resourceTypeVirtualMachines) {
176-
a.vmID = current.Name
162+
vmID = current.Name
177163
}
178164

179165
if strings.EqualFold(lastType, resourceTypeVirtualMachineScaleSets) {
180-
a.vmssName = current.Name
166+
vmssName = current.Name
181167
}
182168

183169
current = current.Parent
184170
}
171+
172+
return resourceGroup, vmssName, vmID
185173
}
186174

187175
// GetResourceGroup returns the resource group the interface belongs to
188176
func (a *AzureInterface) GetResourceGroup() string {
189-
return a.resourceGroup
177+
resourceGroup, _, _ := parseAzureResourceID(a.ID)
178+
return resourceGroup
190179
}
191180

192181
// GetVMScaleSetName returns the VM scale set name the interface belongs to
193182
func (a *AzureInterface) GetVMScaleSetName() string {
194-
return a.vmssName
183+
_, vmssName, _ := parseAzureResourceID(a.ID)
184+
return vmssName
195185
}
196186

197187
// GetVMID returns the VM ID the interface belongs to
198188
func (a *AzureInterface) GetVMID() string {
199-
return a.vmID
189+
_, _, vmID := parseAzureResourceID(a.ID)
190+
return vmID
200191
}
201192

202193
// ForeachAddress iterates over all addresses and calls fn

pkg/azure/types/types_test.go

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
package types
55

66
import (
7+
"encoding/json"
8+
"maps"
9+
"reflect"
10+
"slices"
711
"testing"
812

913
"github.com/stretchr/testify/require"
@@ -78,11 +82,90 @@ func TestExtractIDs(t *testing.T) {
7882
for _, tt := range tests {
7983
t.Run(tt.name, func(t *testing.T) {
8084
intf := AzureInterface{}
81-
intf.SetID(tt.resourceID)
85+
intf.ID = tt.resourceID
8286

8387
require.Equal(t, tt.expectedRG, intf.GetResourceGroup())
8488
require.Equal(t, tt.expectedVMID, intf.GetVMID())
8589
require.Equal(t, tt.expectedVMSSName, intf.GetVMScaleSetName())
8690
})
8791
}
8892
}
93+
94+
// State the status carries but does not serialize cannot survive the apiserver,
95+
// so the operator's freshly built status would never compare equal to the copy
96+
// it reads back and every IPAM sync would write /status.
97+
func TestAzureStatusHasNoUnserializedState(t *testing.T) {
98+
status := reflect.TypeFor[AzureStatus]()
99+
100+
var check func(ty reflect.Type)
101+
check = func(ty reflect.Type) {
102+
// Only the types declared here are walked.
103+
for ty.Kind() == reflect.Pointer || ty.Kind() == reflect.Slice || ty.Kind() == reflect.Array || ty.Kind() == reflect.Map {
104+
ty = ty.Elem()
105+
}
106+
if ty.Kind() != reflect.Struct || ty.PkgPath() != status.PkgPath() {
107+
return
108+
}
109+
for i := range ty.NumField() {
110+
field := ty.Field(i)
111+
require.True(t, field.IsExported(), "%s.%s is unexported", ty.Name(), field.Name)
112+
require.NotEqual(t, "-", field.Tag.Get("json"), "%s.%s is excluded from JSON", ty.Name(), field.Name)
113+
check(field.Type)
114+
}
115+
}
116+
check(status)
117+
}
118+
119+
// A round trip must not perturb the interface, or the operator's DeepEqual
120+
// write-skip gate breaks.
121+
func TestAzureInterfaceJSONRoundTrip(t *testing.T) {
122+
base := &AzureInterface{
123+
ID: "/subscriptions/xxx/resourceGroups/rg1/providers/Microsoft.Compute/virtualMachineScaleSets/vmss1/virtualMachines/0/networkInterfaces/vmss1",
124+
Name: "eth0",
125+
MAC: "aa:bb:cc:dd:ee:ff",
126+
State: StateSucceeded,
127+
SecurityGroup: "sg1",
128+
Addresses: []AzureAddress{
129+
{IP: "10.0.0.2", Subnet: "s-1", State: StateSucceeded},
130+
{IP: "10.0.0.3", Subnet: "s-1", State: StateSucceeded},
131+
},
132+
GatewayIP: "10.0.0.1",
133+
Gateway: "10.0.0.1",
134+
CIDR: "10.0.0.0/24",
135+
}
136+
137+
marshalled, err := json.Marshal(base)
138+
require.NoError(t, err)
139+
var roundTripped AzureInterface
140+
require.NoError(t, json.Unmarshal(marshalled, &roundTripped))
141+
142+
require.Equal(t, *base, roundTripped)
143+
require.True(t, base.DeepEqual(&roundTripped))
144+
145+
// A difference in any field, including the nested types, must be visible to
146+
// DeepEqual or the operator skips a /status write it owes.
147+
mutations := map[string]func(*AzureInterface){
148+
"ID": func(a *AzureInterface) { a.ID = "intf-2" },
149+
"Name": func(a *AzureInterface) { a.Name = "eth1" },
150+
"MAC": func(a *AzureInterface) { a.MAC = "ff:ee:dd:cc:bb:aa" },
151+
"State": func(a *AzureInterface) { a.State = "failed" },
152+
"SecurityGroup": func(a *AzureInterface) { a.SecurityGroup = "sg2" },
153+
"Addresses[].IP": func(a *AzureInterface) { a.Addresses[0].IP = "10.0.0.9" },
154+
"Addresses[].Subnet": func(a *AzureInterface) { a.Addresses[0].Subnet = "s-9" },
155+
"Addresses[].State": func(a *AzureInterface) { a.Addresses[0].State = "failed" },
156+
"GatewayIP": func(a *AzureInterface) { a.GatewayIP = "10.0.1.1" },
157+
"Gateway": func(a *AzureInterface) { a.Gateway = "10.0.1.1" },
158+
"CIDR": func(a *AzureInterface) { a.CIDR = "10.0.1.0/24" },
159+
160+
"Addresses removed": func(a *AzureInterface) { a.Addresses = nil },
161+
"Addresses reordered": func(a *AzureInterface) { a.Addresses[0], a.Addresses[1] = a.Addresses[1], a.Addresses[0] },
162+
}
163+
164+
for _, path := range slices.Sorted(maps.Keys(mutations)) {
165+
t.Run(path, func(t *testing.T) {
166+
other := roundTripped.DeepCopy()
167+
mutations[path](other)
168+
require.False(t, base.DeepEqual(other))
169+
})
170+
}
171+
}

pkg/azure/types/zz_generated.deepequal.go

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)