Fix restoration hash recovery flow

netbox-community · Nov 11, 2024 · 28e26b7 · 28e26b7
1 parent fb6e565
commit 28e26b7
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 16 deletions.
diff --git 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
@@ -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,
 				})