Implement dynamic selection of parent prefix from a set of custom fields

api/v1/prefixclaim_types.go

@@ -24,14 +24,20 @@ import (
 // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
 
 // PrefixClaimSpec defines the desired state of PrefixClaim
+// TODO: The reason for using a workaround please see https://github.com/netbox-community/netbox-operator/pull/90#issuecomment-2402112475
+// +kubebuilder:validation:XValidation:rule="(!has(self.parentPrefix) && has(self.parentPrefixSelector)) || (has(self.parentPrefix) && !has(self.parentPrefixSelector))"
 type PrefixClaimSpec struct {
+
  // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
  // Important: Run "make" to regenerate code after modifying this file
 
- //+kubebuilder:validation:Required
  //+kubebuilder:validation:Format=cidr
  //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefix' is immutable"
- ParentPrefix string `json:"parentPrefix"`
+ ParentPrefix string `json:"parentPrefix,omitempty"`
+
+ // TODO(henrybear327): validate the key and value are all of type string
+ //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="Field 'parentPrefixSelector' is immutable"
+ ParentPrefixSelector map[string]string `json:"parentPrefixSelector,omitempty"`
 
  //+kubebuilder:validation:Required
  //+kubebuilder:validation:Pattern=`^\/[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]$`
@@ -56,16 +62,19 @@ type PrefixClaimSpec struct {
 type PrefixClaimStatus struct {
  // INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
  // Important: Run "make" to regenerate code after modifying this file
- // Prefix status: container, active, reserved , deprecated
- Prefix string `json:"prefix,omitempty"`
- PrefixName string `json:"prefixName,omitempty"`
- Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
+ // Prefix status: container, active, reserved, deprecated
+
+ ParentPrefix string `json:"parentPrefix,omitempty"` // Due to the fact that we can use ParentPrefixSelector to assign parent prefixes, we use this field to store exactly which parent prefix we are using for prefix allocation
+ Prefix string `json:"prefix,omitempty"`
+ PrefixName string `json:"prefixName,omitempty"`
+ Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
 }
 
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="Prefix",type=string,JSONPath=`.status.prefix`
 // +kubebuilder:printcolumn:name="PrefixAssigned",type=string,JSONPath=`.status.conditions[?(@.type=="PrefixAssigned")].status`
+// +kubebuilder:printcolumn:name="ParentPrefix",type=string,JSONPath=`.status.parentPrefix`
 // +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
 // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`
 // +kubebuilder:resource:shortName=pxc
@@ -118,3 +127,17 @@ var ConditionPrefixAssignedFalse = metav1.Condition{
  Reason: "PrefixCRNotCreated",
  Message: "Failed to fetch new Prefix from NetBox",
 }
+
+var ConditionParentPrefixComputedTrue = metav1.Condition{
+ Type: "ParentPrefixComputed",
+ Status: "True",
+ Reason: "ParentPrefixComputed",
+ Message: "The parent prefix was computed successfully",
+}
+
+var ConditionParentPrefixComputedFalse = metav1.Condition{
+ Type: "ParentPrefixComputed",
+ Status: "False",
+ Reason: "ParentPrefixNotComputed",
+ Message: "The parent prefix was not able to be computed",
+}

config/crd/bases/netbox.dev_prefixclaims.yaml

@@ -23,6 +23,9 @@ spec:
  - jsonPath: .status.conditions[?(@.type=="PrefixAssigned")].status
  name: PrefixAssigned
  type: string
+ - jsonPath: .status.parentPrefix
+ name: ParentPrefix
+ type: string
  - jsonPath: .status.conditions[?(@.type=="Ready")].status
  name: Ready
  type: string
@@ -52,7 +55,9 @@ spec:
  metadata:
  type: object
  spec:
- description: PrefixClaimSpec defines the desired state of PrefixClaim
+ description: |-
+ PrefixClaimSpec defines the desired state of PrefixClaim
+ TODO: The reason for using a workaround please see https://github.com/netbox-community/netbox-operator/pull/90#issuecomment-2402112475
  properties:
  comments:
  type: string
@@ -68,6 +73,15 @@ spec:
  x-kubernetes-validations:
  - message: Field 'parentPrefix' is immutable
  rule: self == oldSelf
+ parentPrefixSelector:
+ additionalProperties:
+ type: string
+ description: 'TODO(henrybear327): validate the key and value are all
+ of type string'
+ type: object
+ x-kubernetes-validations:
+ - message: Field 'parentPrefixSelector' is immutable
+ rule: self == oldSelf
  prefixLength:
  pattern: ^\/[0-9]|[1-9][0-9]|1[01][0-9]|12[0-8]$
  type: string
@@ -84,9 +98,11 @@ spec:
  - message: Field 'tenant' is immutable
  rule: self == oldSelf
  required:
- - parentPrefix
  - prefixLength
  type: object
+ x-kubernetes-validations:
+ - rule: (!has(self.parentPrefix) && has(self.parentPrefixSelector)) ||
+ (has(self.parentPrefix) && !has(self.parentPrefixSelector))
  status:
  description: PrefixClaimStatus defines the observed state of PrefixClaim
  properties:
@@ -159,11 +175,9 @@ spec:
  - type
  type: object
  type: array
+ parentPrefix:
+ type: string
  prefix:
- description: |-
- INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
- Important: Run "make" to regenerate code after modifying this file
- Prefix status: container, active, reserved , deprecated
  type: string
  prefixName:
  type: string

config/samples/kustomization.yaml

@@ -4,4 +4,5 @@ resources:
 - netbox_v1_ipaddressclaim.yaml
 - netbox_v1_prefix.yaml
 - netbox_v1_prefixclaim.yaml
+- netbox_v1_prefixclaim_customfields.yaml
 #+kubebuilder:scaffold:manifestskustomizesamples

config/samples/netbox_v1_prefixclaim_customfields.yaml

@@ -0,0 +1,19 @@
+apiVersion: netbox.dev/v1
+kind: PrefixClaim
+metadata:
+ labels:
+ app.kubernetes.io/name: netbox-operator
+ app.kubernetes.io/managed-by: kustomize
+ name: prefixclaim-customfields-sample
+spec:
+ tenant: "Dunder-Mifflin, Inc."
+ site: "DataCenter"
+ description: "some description"
+ comments: "your comments"
+ preserveInNetbox: true
+ prefixLength: "/31"
+ parentPrefixSelector: # notice that the keys and values are case-sensitive
+ environment: "Production"
+ poolName: "Pool 1"
+ # environment: "production"
+ # poolName: "pool 3"

internal/controller/ipaddress_controller.go

@@ -147,9 +147,8 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  // create lock
  locked := ll.TryLock(lockCtx)
  if !locked {
- logger.Info(fmt.Sprintf("failed to lock parent prefix %s", ipAddressClaim.Spec.ParentPrefix))
- r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
- ipAddressClaim.Spec.ParentPrefix)
+ errorMsg := fmt.Sprintf("failed to lock parent prefix %s", ipAddressClaim.Spec.ParentPrefix)
+ r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
  return ctrl.Result{
  RequeueAfter: 2 * time.Second,
  }, nil

internal/controller/ipaddressclaim_controller.go

@@ -23,7 +23,6 @@ import (
  "time"
 
  netboxv1 "github.com/netbox-community/netbox-operator/api/v1"
- "github.com/netbox-community/netbox-operator/pkg/config"
  "github.com/netbox-community/netbox-operator/pkg/netbox/api"
  "github.com/netbox-community/netbox-operator/pkg/netbox/models"
  "github.com/swisscom/leaselocker"
@@ -111,9 +110,8 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
  locked := ll.TryLock(lockCtx)
  if !locked {
  // lock for parent prefix was not available, rescheduling
- logger.Info(fmt.Sprintf("failed to lock parent prefix %s", o.Spec.ParentPrefix))
- r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
- o.Spec.ParentPrefix)
+ errorMsg := fmt.Sprintf("failed to lock parent prefix %s", o.Spec.ParentPrefix)
+ r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
  return ctrl.Result{
  RequeueAfter: 2 * time.Second,
  }, nil
@@ -122,7 +120,7 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
  // 4. try to reclaim ip address
  h := generateIpAddressRestorationHash(o)
- ipAddressModel, err := r.NetboxClient.RestoreExistingIpByHash(config.GetOperatorConfig().NetboxRestorationHashFieldName, h)
+ ipAddressModel, err := r.NetboxClient.RestoreExistingIpByHash(h)
  if err != nil {
  if setConditionErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpAssignedFalse, corev1.EventTypeWarning, err.Error()); setConditionErr != nil {
  return ctrl.Result{}, fmt.Errorf("error updating status: %w, looking up ip by hash failed: %w", setConditionErr, err)

internal/controller/prefix_controller.go

@@ -132,9 +132,19 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
  return ctrl.Result{}, err
  }
 
+ if prefixClaim.Status.ParentPrefix == "" {
+ // the parent prefix is not computed
+ if err := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, "the parent prefix is not computed"); err != nil {
+ return ctrl.Result{}, err
+ }
+ return ctrl.Result{
+ Requeue: true,
+ }, nil
+ }
+
  // get the name of the parent prefix
  leaseLockerNSN := types.NamespacedName{
- Name: convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix),
+ Name: convertCIDRToLeaseLockName(prefixClaim.Status.ParentPrefix),
  Namespace: r.OperatorNamespace,
  }
  ll, err = leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.NamespacedName.String())
@@ -147,14 +157,13 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
  // create lock
  if locked := ll.TryLock(lockCtx); !locked {
- logger.Info(fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Spec.ParentPrefix))
- r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
- prefixClaim.Spec.ParentPrefix)
+ errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.ParentPrefix)
+ r.Recorder.Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
  return ctrl.Result{
  RequeueAfter: 2 * time.Second,
  }, nil
  }
- debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix)
+ debugLogger.Info("successfully locked parent prefix %s", prefixClaim.Status.ParentPrefix)
  }
 
  /* 2. reserve or update Prefix in netbox */
@@ -218,7 +227,6 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
  }
 
  debugLogger.Info(fmt.Sprintf("reserved prefix in netbox, prefix: %s", prefix.Spec.Prefix))
-
  if err = r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyTrue, corev1.EventTypeNormal, ""); err != nil {
  return ctrl.Result{}, err
  }

internal/controller/prefixclaim_controller.go

@@ -75,7 +75,49 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
  return ctrl.Result{}, nil
  }
 
- /* 1. check if the matching Prefix object exists */
+ /* 1. compute and assign the parent prefix if required */
+ // The current design will use prefixClaim.Status.ParentPrefix for storing the computed parent prefix, and as the source of truth for future parent prefix references
+ if prefixClaim.Status.ParentPrefix == "" /* parent prefix not yet computed/assigned */ {
+ if prefixClaim.Spec.ParentPrefix != "" {
+ // ParentPrefix takes precedence over ParentPrefixSelector
+ 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.ParentPrefixSelector, prefixClaim.Spec.Tenant, prefixClaim.Spec.PrefixLength)
+ 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 exhausation
+ return ctrl.Result{Requeue: true}, nil
+ }
+
+ // set prefixClaim.Spec.ParentPrefix
+ // 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 {
+ return ctrl.Result{}, err
+ }
+ return ctrl.Result{Requeue: false}, nil
+ }
+
+ // set status, and condition field
+ msg := fmt.Sprintf("parentPrefix is computed: %v", prefixClaim.Status.ParentPrefix)
+ if err := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionParentPrefixComputedTrue, corev1.EventTypeNormal, msg); err != nil {
+ return ctrl.Result{}, err
+ }
+ }
+
+ /* 2. check if the matching Prefix object exists */
  prefix := &netboxv1.Prefix{}
  prefixName := prefixClaim.ObjectMeta.Name
  prefixLookupKey := types.NamespacedName{
@@ -89,9 +131,9 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
  }
  debugLogger.Info("the prefix was not found, will create a new prefix object now")
 
- /* 2. check if the lease for parent prefix is available */
+ /* 3. check if the lease for parent prefix is available */
  leaseLockerNSN := types.NamespacedName{
- Name: convertCIDRToLeaseLockName(prefixClaim.Spec.ParentPrefix),
+ Name: convertCIDRToLeaseLockName(prefixClaim.Status.ParentPrefix),
  Namespace: r.OperatorNamespace,
  }
  ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+prefixName)
@@ -102,20 +144,19 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
  lockCtx, cancel := context.WithCancel(ctx)
  defer cancel()
 
- /* 3. try to lock the lease for the parent prefix */
+ /* 4. try to lock the lease for the parent prefix */
  locked := ll.TryLock(lockCtx)
  if !locked {
  // lock for parent prefix was not available, rescheduling
- logger.Info(fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Spec.ParentPrefix))
- r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s",
- prefixClaim.Spec.ParentPrefix)
+ errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.ParentPrefix)
+ r.Recorder.Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
  return ctrl.Result{
  RequeueAfter: 2 * time.Second,
  }, nil
  }
- debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Spec.ParentPrefix))
+ debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Status.ParentPrefix))
 
- // 4. try to reclaim Prefix using restorationHash
+ // 5. try to reclaim Prefix using restorationHash
  h := generatePrefixRestorationHash(prefixClaim)
  prefixModel, err := r.NetboxClient.RestoreExistingPrefixByHash(h)
  if err != nil {
@@ -127,12 +168,12 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
  if prefixModel == nil {
  // Prefix cannot be restored from netbox
- // 5.a assign new available Prefix
+ // 6.a assign new available Prefix
 
  // get available Prefix under parent prefix in netbox with equal mask length
  prefixModel, err = r.NetboxClient.GetAvailablePrefixByClaim(
  &models.PrefixClaim{
- ParentPrefix: prefixClaim.Spec.ParentPrefix,
+ ParentPrefix: prefixClaim.Status.ParentPrefix,
  PrefixLength: prefixClaim.Spec.PrefixLength,
  Metadata: &models.NetboxMetadata{
  Tenant: prefixClaim.Spec.Tenant,
@@ -146,13 +187,13 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
  }
  debugLogger.Info(fmt.Sprintf("prefix is not reserved in netbox, assignined new prefix: %s", prefixModel.Prefix))
  } else {
- // 5.b reassign reserved Prefix from netbox
+ // 6.b reassign reserved Prefix from netbox
 
  // do nothing, Prefix restored
  debugLogger.Info(fmt.Sprintf("reassign reserved prefix from netbox, prefix: %s", prefixModel.Prefix))
  }
 
- /* 6-1, create the Prefix object */
+ /* 7.a create the Prefix object */
  prefixResource := generatePrefixFromPrefixClaim(prefixClaim, prefixModel.Prefix, logger)
  err = controllerutil.SetControllerReference(prefixClaim, prefixResource, r.Scheme)
  if err != nil {
@@ -170,7 +211,7 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
  return ctrl.Result{}, err
  }
  } else { // Prefix object exists
- /* 6-2. update fields of the Prefix object */
+ /* 7.b update fields of the Prefix object */
  debugLogger.Info("update prefix resource")
  if err := r.Client.Get(ctx, prefixLookupKey, prefix); err != nil {
  return ctrl.Result{}, err