Skip to content

Commit 5540a63

Browse files
Aidan Obleyflawedmatrix
authored and
tylerschultz
committed
display count of address claims bound in condition
while there are unbound ip address claims the condition will show the count of bound to total claims. Signed-off-by: Aidan Obley <aobley@vmware.com> Co-authored-by: Edwin Xie <exie@vmware.com>
1 parent 77c7c6a commit 5540a63

File tree

2 files changed

+92
-38
lines changed

2 files changed

+92
-38
lines changed

‎pkg/services/govmomi/service.go‎

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,11 @@ func createIPAddressClaim(ctx *context.VMContext, ipAddrClaimName string, poolRe
334334
// expected to contain a valid IP, Prefix and Gateway.
335335
func (vms *VMService) reconcileIPAddresses(ctx *virtualMachineContext) (bool, error) {
336336
ctx.IPAMState = map[string]infrav1.NetworkDeviceSpec{}
337+
ipAddrNames, err := resolveIpAddressNamesForClaims(ctx)
338+
if err != nil {
339+
return false, err
340+
}
341+
337342
for devIdx, device := range ctx.VSphereVM.Spec.Network.Devices {
338343
var ipAddrs []string
339344
var gateway4 string
@@ -342,34 +347,13 @@ func (vms *VMService) reconcileIPAddresses(ctx *virtualMachineContext) (bool, er
342347
//TODO: Break this up into smaller functions
343348
for poolRefIdx := range device.AddressesFromPools {
344349
// check if claim exists
345-
ipAddrClaim := &ipamv1.IPAddressClaim{}
346-
ipAddrClaimName := IPAddressClaimName(ctx.VSphereVM.Name, devIdx, poolRefIdx)
347-
ipAddrClaimKey := apitypes.NamespacedName{
348-
Namespace: ctx.VSphereVM.Namespace,
349-
Name: ipAddrClaimName,
350-
}
351-
var err error
352-
ctx.Logger.V(5).Info("fetching IPAddressClaim", "name", ipAddrClaimKey.String())
353-
if err = ctx.Client.Get(ctx, ipAddrClaimKey, ipAddrClaim); err != nil && !apierrors.IsNotFound(err) {
354-
ctx.Logger.Error(err, "error fetching IPAddressClaim", "name", ipAddrClaimName)
355-
return false, err
356-
}
357-
358-
ipAddrName := ipAddrClaim.Status.AddressRef.Name
359-
ctx.Logger.V(5).Info("fetched IPAddressClaim", "name", ipAddrClaimName, "IPAddressClaim.Status.AddressRef.Name", ipAddrName)
360-
if ipAddrName == "" {
361-
ctx.Logger.V(5).Info("IPAddress name was empty on IPAddressClaim", "name", ipAddrClaimName, "IPAddressClaim.Status.AddressRef.Name", ipAddrName)
362-
msg := "Waiting for IPAddressClaim to have an IPAddress bound"
363-
markIPAddressClaimedConditionWaitingForClaimAddress(ctx.VSphereVM, msg)
364-
return false, errors.New(msg)
365-
}
366-
350+
ipAddrName := ipAddrNames[devIdx][poolRefIdx]
367351
ipAddr := &ipamv1.IPAddress{}
368352
ipAddrKey := apitypes.NamespacedName{
369353
Namespace: ctx.VSphereVM.Namespace,
370354
Name: ipAddrName,
371355
}
372-
if err = ctx.Client.Get(ctx, ipAddrKey, ipAddr); err != nil {
356+
if err := ctx.Client.Get(ctx, ipAddrKey, ipAddr); err != nil {
373357
return false, err
374358
}
375359

@@ -406,29 +390,29 @@ func (vms *VMService) reconcileIPAddresses(ctx *virtualMachineContext) (bool, er
406390
}
407391

408392
if gatewayAddr.Is4() {
409-
if device.Gateway4 != "" && device.Gateway4 != ipAddr.Spec.Gateway {
393+
if areGatewaysMismatched(device.Gateway4, ipAddr.Spec.Gateway) {
410394
msg := fmt.Sprintf("The IPv4 Gateway for IPAddress %s does not match the Gateway4 already configured on device (index %d)",
411395
ipAddrName,
412396
devIdx,
413397
)
414398
return markIPAddressClaimedConditionInvalidIPWithError(ctx.VSphereVM, msg)
415399
}
416-
if gateway4 != "" && gateway4 != ipAddr.Spec.Gateway {
400+
if areGatewaysMismatched(gateway4, ipAddr.Spec.Gateway) {
417401
msg := fmt.Sprintf("The IPv4 IPAddresses assigned to the same device (index %d) do not have the same gateway",
418402
devIdx,
419403
)
420404
return markIPAddressClaimedConditionInvalidIPWithError(ctx.VSphereVM, msg)
421405
}
422406
gateway4 = ipAddr.Spec.Gateway
423407
} else {
424-
if device.Gateway6 != "" && device.Gateway6 != ipAddr.Spec.Gateway {
408+
if areGatewaysMismatched(device.Gateway6, ipAddr.Spec.Gateway) {
425409
msg := fmt.Sprintf("The IPv6 Gateway for IPAddress %s does not match the Gateway6 already configured on device (index %d)",
426410
ipAddrName,
427411
devIdx,
428412
)
429413
return markIPAddressClaimedConditionInvalidIPWithError(ctx.VSphereVM, msg)
430414
}
431-
if gateway6 != "" && gateway6 != ipAddr.Spec.Gateway {
415+
if areGatewaysMismatched(gateway6, ipAddr.Spec.Gateway) {
432416
msg := fmt.Sprintf("The IPv6 IPAddresses assigned to the same device (index %d) do not have the same gateway",
433417
devIdx,
434418
)
@@ -452,6 +436,61 @@ func (vms *VMService) reconcileIPAddresses(ctx *virtualMachineContext) (bool, er
452436
return true, nil
453437
}
454438

439+
// resolveIpAddressNamesForClaims checks that all the IPAddressClaims have been
440+
// satisfied. If waiting on a claim to have an IPAddress, the status will refelct
441+
// how many are bound out of the total expected. A slice of slices matching the
442+
// structure of the device and AddressFromPools slices is returned so that
443+
// names don't need to be resolved more than once.
444+
func resolveIpAddressNamesForClaims(ctx *virtualMachineContext) ([][]string, error) {
445+
boundClaims := 0
446+
totalClaims := 0
447+
ipAddrNames := make([][]string, len(ctx.VSphereVM.Spec.Network.Devices))
448+
for devIdx, device := range ctx.VSphereVM.Spec.Network.Devices {
449+
ipAddrNames[devIdx] = make([]string, len(device.AddressesFromPools))
450+
for poolRefIdx := range device.AddressesFromPools {
451+
// check if claim exists
452+
ipAddrClaim := &ipamv1.IPAddressClaim{}
453+
ipAddrClaimName := IPAddressClaimName(ctx.VSphereVM.Name, devIdx, poolRefIdx)
454+
ipAddrClaimKey := apitypes.NamespacedName{
455+
Namespace: ctx.VSphereVM.Namespace,
456+
Name: ipAddrClaimName,
457+
}
458+
var err error
459+
ctx.Logger.V(5).Info("fetching IPAddressClaim", "name", ipAddrClaimKey.String())
460+
if err = ctx.Client.Get(ctx, ipAddrClaimKey, ipAddrClaim); err != nil && !apierrors.IsNotFound(err) {
461+
ctx.Logger.Error(err, "error fetching IPAddressClaim", "name", ipAddrClaimName)
462+
return nil, err
463+
}
464+
465+
ipAddrName := ipAddrClaim.Status.AddressRef.Name
466+
ctx.Logger.V(5).Info("fetched IPAddressClaim", "name", ipAddrClaimName, "IPAddressClaim.Status.AddressRef.Name", ipAddrName)
467+
if ipAddrName == "" {
468+
ctx.Logger.V(5).Info("IPAddress name was empty on IPAddressClaim", "name", ipAddrClaimName, "IPAddressClaim.Status.AddressRef.Name", ipAddrName)
469+
} else {
470+
boundClaims++
471+
ipAddrNames[devIdx][poolRefIdx] = ipAddrName
472+
}
473+
totalClaims++
474+
}
475+
}
476+
477+
if boundClaims < totalClaims {
478+
msg := fmt.Sprintf("Waiting for IPAddressClaim to have an IPAddress bound, %d out of %d bound", boundClaims, totalClaims)
479+
markIPAddressClaimedConditionWaitingForClaimAddress(ctx.VSphereVM, msg)
480+
return nil, errors.New(msg)
481+
}
482+
483+
return ipAddrNames, nil
484+
}
485+
486+
// areGatewaysMismatched checks that a gateway for a device is equal to an
487+
// IPAddresses gateway. We can assume that IPAddresses will always have
488+
// gateways so we do not need to check for empty string. It is possible to
489+
// configure a device and not a gateway, we don't want to fail in that case.
490+
func areGatewaysMismatched(deviceGateway, ipAddressGateway string) bool {
491+
return deviceGateway != "" && deviceGateway != ipAddressGateway
492+
}
493+
455494
func (vms *VMService) reconcileMetadata(ctx *virtualMachineContext) (bool, error) {
456495
existingMetadata, err := vms.getMetadata(ctx)
457496
if err != nil {

‎pkg/services/govmomi/service_test.go‎

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -299,53 +299,68 @@ func Test_reconcileIPAddresses_ShouldUpdateVMDevicesWithAddresses(t *testing.T)
299299

300300
// IP provider has not provided Addresses yet
301301
reconciled, err := vms.reconcileIPAddresses(ctx)
302-
g.Expect(err).To(MatchError("Waiting for IPAddressClaim to have an IPAddress bound"))
302+
g.Expect(err).To(MatchError("Waiting for IPAddressClaim to have an IPAddress bound, 0 out of 3 bound"))
303303
g.Expect(reconciled).To(BeFalse())
304304

305305
// Ensure that the VM has a IPAddressClaimed condition set to False
306306
// for the WaitingForIPAddress reason.
307307
claimedCondition := conditions.Get(ctx.VSphereVM, infrav1.IPAddressClaimedCondition)
308308
g.Expect(claimedCondition).NotTo(BeNil())
309309
g.Expect(claimedCondition.Reason).To(Equal(infrav1.WaitingForIPAddressReason))
310-
g.Expect(claimedCondition.Message).To(Equal("Waiting for IPAddressClaim to have an IPAddress bound"))
310+
g.Expect(claimedCondition.Message).To(Equal("Waiting for IPAddressClaim to have an IPAddress bound, 0 out of 3 bound"))
311311
g.Expect(claimedCondition.Status).To(Equal(corev1.ConditionFalse))
312312

313-
// Simulate IP provider reconciling claim
314-
ctx.Client.Create(ctx, address1)
315-
ctx.Client.Create(ctx, address2)
313+
// Simulate IP provider reconciling one claim
316314
ctx.Client.Create(ctx, address3)
317315

318316
ipAddrClaim := &ipamv1a1.IPAddressClaim{}
319317
ipAddrClaimKey := apitypes.NamespacedName{
320318
Namespace: ctx.VSphereVM.Namespace,
321-
Name: "vsphereVM1-0-0",
319+
Name: "vsphereVM1-0-2",
322320
}
323321
err = ctx.Client.Get(ctx, ipAddrClaimKey, ipAddrClaim)
324322
g.Expect(err).NotTo(HaveOccurred())
325323

326-
ipAddrClaim.Status.AddressRef.Name = "vsphereVM1-0-0-address0"
324+
ipAddrClaim.Status.AddressRef.Name = "vsphereVM1-0-2-address2"
327325

328326
ctx.Client.Update(ctx, ipAddrClaim)
329327

328+
// Only the last claim has been bound
329+
reconciled, err = vms.reconcileIPAddresses(ctx)
330+
g.Expect(err).To(MatchError("Waiting for IPAddressClaim to have an IPAddress bound, 1 out of 3 bound"))
331+
g.Expect(reconciled).To(BeFalse())
332+
333+
// Ensure that the VM has a IPAddressClaimed condition set to False
334+
// for the WaitingForIPAddress reason.
335+
claimedCondition = conditions.Get(ctx.VSphereVM, infrav1.IPAddressClaimedCondition)
336+
g.Expect(claimedCondition).NotTo(BeNil())
337+
g.Expect(claimedCondition.Reason).To(Equal(infrav1.WaitingForIPAddressReason))
338+
g.Expect(claimedCondition.Message).To(Equal("Waiting for IPAddressClaim to have an IPAddress bound, 1 out of 3 bound"))
339+
g.Expect(claimedCondition.Status).To(Equal(corev1.ConditionFalse))
340+
341+
// Simulate IP provider reconciling remaining claims
342+
ctx.Client.Create(ctx, address1)
343+
ctx.Client.Create(ctx, address2)
344+
330345
ipAddrClaimKey = apitypes.NamespacedName{
331346
Namespace: ctx.VSphereVM.Namespace,
332-
Name: "vsphereVM1-0-1",
347+
Name: "vsphereVM1-0-0",
333348
}
334349
err = ctx.Client.Get(ctx, ipAddrClaimKey, ipAddrClaim)
335350
g.Expect(err).NotTo(HaveOccurred())
336351

337-
ipAddrClaim.Status.AddressRef.Name = "vsphereVM1-0-1-address1"
352+
ipAddrClaim.Status.AddressRef.Name = "vsphereVM1-0-0-address0"
338353

339354
ctx.Client.Update(ctx, ipAddrClaim)
340355

341356
ipAddrClaimKey = apitypes.NamespacedName{
342357
Namespace: ctx.VSphereVM.Namespace,
343-
Name: "vsphereVM1-0-2",
358+
Name: "vsphereVM1-0-1",
344359
}
345360
err = ctx.Client.Get(ctx, ipAddrClaimKey, ipAddrClaim)
346361
g.Expect(err).NotTo(HaveOccurred())
347362

348-
ipAddrClaim.Status.AddressRef.Name = "vsphereVM1-0-2-address2"
363+
ipAddrClaim.Status.AddressRef.Name = "vsphereVM1-0-1-address1"
349364

350365
ctx.Client.Update(ctx, ipAddrClaim)
351366

0 commit comments

Comments
 (0)