From 28e26b7223501a63f5914d1f2714e4ef78a69770 Mon Sep 17 00:00:00 2001 From: Chun-Hung Tseng Date: Mon, 11 Nov 2024 10:09:52 +0100 Subject: [PATCH] Fix restoration hash recovery flow --- internal/controller/prefixclaim_controller.go | 64 +++++++++++++++---- pkg/netbox/api/prefix_claim.go | 4 +- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/internal/controller/prefixclaim_controller.go b/internal/controller/prefixclaim_controller.go index 7450c76..3ad48f6 100644 --- a/internal/controller/prefixclaim_controller.go +++ b/internal/controller/prefixclaim_controller.go @@ -81,24 +81,60 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) if prefixClaim.Spec.ParentPrefix != "" { prefixClaim.Status.ParentPrefix = prefixClaim.Spec.ParentPrefix } else if len(prefixClaim.Spec.ParentPrefixSelector) > 0 { - // The main idea is that we select one of the available parent prefixes as the ParentPrefix for all subsequent computation - // The existing algorithm for prefix allocation within a ParentPrefix remains unchanged - - // fetch available prefixes from netbox - parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixByParentPrefixSelector(&prefixClaim.Spec) - if err != nil || len(parentPrefixCandidates) == 0 { - errorMsg := fmt.Sprintf("no parent prefix can be obtained with the query conditions set in ParentPrefixSelector, err = %v, number of candidates = %v", err, len(parentPrefixCandidates)) - if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedFalse, corev1.EventTypeWarning, errorMsg); err != nil { - return ctrl.Result{}, err - } + // we first check if a prefix can be restored from the netbox - // we requeue as this might be a temporary prefix exhausation + // since the parent prefix is part of the restoration hash computation + // we need take all parent prefix candidates into consideration when recovering for it + allParentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixByParentPrefixSelector(&prefixClaim.Spec, true) + if err != nil { + // we requeue as this might be a temporary query error return ctrl.Result{Requeue: true}, nil } - // TODO(henrybear327): use best-fit algorithm to pick a parent prefix - parentPrefixCandidate := parentPrefixCandidates[0] - prefixClaim.Status.ParentPrefix = parentPrefixCandidate.Prefix + // query one by one to see if the restoration hash exists + // TODO(henrybear327): throw an warning if the prefix count is more than 10 (or a threshold) since it's going to be a lot of API calls + var canBeRestored *models.Prefix + for _, parentPrefix := range allParentPrefixCandidates { + prefixClaim.Status.ParentPrefix = parentPrefix.Prefix + + h := generatePrefixRestorationHash(prefixClaim) + canBeRestored, err = r.NetboxClient.RestoreExistingPrefixByHash(h) + if err != nil { + // we can't ignore failures as we need to make sure that all potential parent prefixes are tried, so we requeue + return ctrl.Result{Requeue: true}, nil + } + if canBeRestored != nil { + // restoration hash matched + break + } + + prefixClaim.Status.ParentPrefix = "" + } + + if canBeRestored != nil { + // yes, so we need to use the previously allocated parent prefix by setting prefixClaim.Status.ParentPrefix (which is already done) + } else { + // no, so we need to select one parent prefix from prefix candidates + + // The main idea is that we select one of the available parent prefixes as the ParentPrefix for all subsequent computation + // The existing algorithm for prefix allocation within a ParentPrefix remains unchanged + + // fetch available prefixes from netbox + parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixByParentPrefixSelector(&prefixClaim.Spec, false) + if err != nil || len(parentPrefixCandidates) == 0 { + errorMsg := fmt.Sprintf("no parent prefix can be obtained with the query conditions set in ParentPrefixSelector, err = %v, number of candidates = %v", err, len(parentPrefixCandidates)) + if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedFalse, corev1.EventTypeWarning, errorMsg); err != nil { + return ctrl.Result{}, err + } + + // we requeue as this might be a temporary prefix exhausation + return ctrl.Result{Requeue: true}, nil + } + + // TODO(henrybear327): use best-fit algorithm to pick a parent prefix + parentPrefixCandidate := parentPrefixCandidates[0] + prefixClaim.Status.ParentPrefix = parentPrefixCandidate.Prefix + } } else { // this case should not be triggered anymore, as we have validation rules put in place on the CR if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedFalse, corev1.EventTypeWarning, "either ParentPrefixSelector or ParentPrefix needs to be set"); err != nil { diff --git a/pkg/netbox/api/prefix_claim.go b/pkg/netbox/api/prefix_claim.go index c7f1b29..a9c8694 100644 --- a/pkg/netbox/api/prefix_claim.go +++ b/pkg/netbox/api/prefix_claim.go @@ -93,7 +93,7 @@ func validatePrefixLengthOrError(prefixClaim *models.PrefixClaim, prefixFamily i return nil } -func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec *netboxv1.PrefixClaimSpec) ([]*models.Prefix, error) { +func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec *netboxv1.PrefixClaimSpec, returnAll bool) ([]*models.Prefix, error) { fieldEntries := make(map[string]string) if prefixClaimSpec.Tenant != "" { details, err := r.GetTenantDetails(prefixClaimSpec.Tenant) @@ -136,7 +136,7 @@ func (r *NetboxClient) GetAvailablePrefixByParentPrefixSelector(prefixClaimSpec prefixes := make([]*models.Prefix, 0) for _, prefix := range list.Payload.Results { if prefix.Prefix != nil { - if r.isParentPrefixCandidate(prefixClaimSpec, *prefix.Prefix) { + if returnAll || r.isParentPrefixCandidate(prefixClaimSpec, *prefix.Prefix) { prefixes = append(prefixes, &models.Prefix{ Prefix: *prefix.Prefix, })