From 0d454c6e64ef84f6e5159217ebd75b3d667297f3 Mon Sep 17 00:00:00 2001 From: Joel Studler Date: Tue, 15 Jul 2025 12:23:11 +0200 Subject: [PATCH] Alignment of controllers for variable names (o), spacing of kubebuilder markers, RequeueAfter --- api/v1/ipaddress_types.go | 2 +- api/v1/ipaddressclaim_types.go | 2 +- api/v1/iprange_types.go | 2 +- api/v1/iprangeclaim_types.go | 2 +- api/v1/prefix_types.go | 18 +-- api/v1/prefixclaim_types.go | 16 +-- internal/controller/ipaddress_controller.go | 1 - internal/controller/iprange_controller.go | 28 +++-- internal/controller/prefix_controller.go | 83 ++++++------- internal/controller/prefixclaim_controller.go | 112 +++++++++--------- 10 files changed, 135 insertions(+), 131 deletions(-) diff --git a/api/v1/ipaddress_types.go b/api/v1/ipaddress_types.go index a038ac0e..32f75bed 100644 --- a/api/v1/ipaddress_types.go +++ b/api/v1/ipaddress_types.go @@ -88,7 +88,7 @@ type IpAddressStatus struct { //+kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id` //+kubebuilder:printcolumn:name="URL",type=string,JSONPath=`.status.url` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` -// +kubebuilder:resource:shortName=ip +//+kubebuilder:resource:shortName=ip // IpAddress allows to create a NetBox IP Address. More info about NetBox IP Addresses: https://github.com/netbox-community/netbox/blob/main/docs/models/ipam/ipaddress.md type IpAddress struct { diff --git a/api/v1/ipaddressclaim_types.go b/api/v1/ipaddressclaim_types.go index f290facb..66bbe64e 100644 --- a/api/v1/ipaddressclaim_types.go +++ b/api/v1/ipaddressclaim_types.go @@ -89,7 +89,7 @@ type IpAddressClaimStatus struct { //+kubebuilder:printcolumn:name="IpAssigned",type=string,JSONPath=`.status.conditions[?(@.type=="IPAssigned")].status` //+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` -// +kubebuilder:resource:shortName=ipc +//+kubebuilder:resource:shortName=ipc // IpAddressClaim allows to claim a NetBox IP Address from an existing Prefix. // The IpAddressClaim Controller will try to assign an available IP Address diff --git a/api/v1/iprange_types.go b/api/v1/iprange_types.go index f521d0ab..2ad4e164 100644 --- a/api/v1/iprange_types.go +++ b/api/v1/iprange_types.go @@ -97,7 +97,7 @@ type IpRangeStatus struct { //+kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id` //+kubebuilder:printcolumn:name="URL",type=string,JSONPath=`.status.url` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` -// +kubebuilder:resource:shortName=ipr +//+kubebuilder:resource:shortName=ipr // IpRange allows to create a NetBox IP Range. More info about NetBox IP Ranges: https://github.com/netbox-community/netbox/blob/main/docs/models/ipam/iprange.md type IpRange struct { diff --git a/api/v1/iprangeclaim_types.go b/api/v1/iprangeclaim_types.go index 5c58f725..ad3c0946 100644 --- a/api/v1/iprangeclaim_types.go +++ b/api/v1/iprangeclaim_types.go @@ -118,7 +118,7 @@ type IpRangeClaimStatus struct { //+kubebuilder:printcolumn:name="IpRangeAssigned",type=string,JSONPath=`.status.conditions[?(@.type=="IPRangeAssigned")].status` //+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` //+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` -// +kubebuilder:resource:shortName=iprc +//+kubebuilder:resource:shortName=iprc // IpRangeClaim allows to claim a NetBox IP Range from an existing Prefix. // The IpRangeClaim Controller will try to assign an available IP Range diff --git a/api/v1/prefix_types.go b/api/v1/prefix_types.go index 57663582..baad5af4 100644 --- a/api/v1/prefix_types.go +++ b/api/v1/prefix_types.go @@ -86,14 +86,14 @@ type PrefixStatus struct { 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=`.spec.prefix` -// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` -// +kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id` -// +kubebuilder:printcolumn:name="URL",type=string,JSONPath=`.status.url` -// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` -// +kubebuilder:resource:shortName=px +//+kubebuilder:object:root=true +//+kubebuilder:subresource:status +//+kubebuilder:printcolumn:name="Prefix",type=string,JSONPath=`.spec.prefix` +//+kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` +//+kubebuilder:printcolumn:name="ID",type=string,JSONPath=`.status.id` +//+kubebuilder:printcolumn:name="URL",type=string,JSONPath=`.status.url` +//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +//+kubebuilder:resource:shortName=px // Prefix allows to create a NetBox Prefix. More info about NetBox Prefixes: https://github.com/netbox-community/netbox/blob/main/docs/models/ipam/prefix.md type Prefix struct { @@ -108,7 +108,7 @@ func (p *Prefix) Conditions() *[]metav1.Condition { return &p.Status.Conditions } -// +kubebuilder:object:root=true +//+kubebuilder:object:root=true // PrefixList contains a list of Prefix type PrefixList struct { diff --git a/api/v1/prefixclaim_types.go b/api/v1/prefixclaim_types.go index 54cfe7f7..24f14516 100644 --- a/api/v1/prefixclaim_types.go +++ b/api/v1/prefixclaim_types.go @@ -110,13 +110,13 @@ type PrefixClaimStatus struct { 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="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` -// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` -// +kubebuilder:resource:shortName=pxc +//+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="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status` +//+kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` +//+kubebuilder:resource:shortName=pxc // PrefixClaim allows to claim a NetBox Prefix from an existing Prefix // (parentPrefix) or a dynamically selected Prefix (parentPrefixSelector). @@ -136,7 +136,7 @@ func (p *PrefixClaim) Conditions() *[]metav1.Condition { return &p.Status.Conditions } -// +kubebuilder:object:root=true +//+kubebuilder:object:root=true // PrefixClaimList contains a list of PrefixClaim type PrefixClaimList struct { diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go index 03217331..71c313ca 100644 --- a/internal/controller/ipaddress_controller.go +++ b/internal/controller/ipaddress_controller.go @@ -273,7 +273,6 @@ func generateNetboxIpAddressModelFromIpAddressSpec(spec *netboxv1.IpAddressSpec, } // if a custom field was removed from the spec, add it with an empty value - for key := range lastAppliedCustomFields { _, ok := netboxCustomFields[key] if !ok { diff --git a/internal/controller/iprange_controller.go b/internal/controller/iprange_controller.go index 4a80d53e..4d08a79a 100644 --- a/internal/controller/iprange_controller.go +++ b/internal/controller/iprange_controller.go @@ -24,6 +24,7 @@ import ( "maps" "strconv" "strings" + "time" netboxv1 "github.com/netbox-community/netbox-operator/api/v1" "github.com/netbox-community/netbox-operator/pkg/config" @@ -37,6 +38,7 @@ import ( "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -73,16 +75,17 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // if being deleted if !o.ObjectMeta.DeletionTimestamp.IsZero() { - if !o.Spec.PreserveInNetbox { - err := r.NetboxClient.DeleteIpRange(o.Status.IpRangeId) - if err != nil { - err = r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionIpRangeReadyFalseDeletionFailed, - corev1.EventTypeWarning, err) + if controllerutil.ContainsFinalizer(o, IpRangeFinalizerName) { + if !o.Spec.PreserveInNetbox { + err := r.NetboxClient.DeleteIpRange(o.Status.IpRangeId) if err != nil { - return ctrl.Result{}, err + err = r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionIpRangeReadyFalseDeletionFailed, + corev1.EventTypeWarning, err) + if err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{Requeue: true}, nil } - - return ctrl.Result{Requeue: true}, nil } } @@ -127,10 +130,11 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct // create lock locked := ll.TryLock(lockCtx) if !locked { - logger.Info(fmt.Sprintf("failed to lock parent prefix %s", parentPrefix)) - r.EventStatusRecorder.Recorder().Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", "failed to lock parent prefix %s", - parentPrefix) - return ctrl.Result{Requeue: true}, nil + errorMsg := fmt.Sprintf("failed to lock parent prefix %s", parentPrefix) + r.EventStatusRecorder.Recorder().Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) + return ctrl.Result{ + RequeueAfter: 2 * time.Second, + }, nil } logger.V(4).Info(fmt.Sprintf("successfully locked parent prefix %s", parentPrefix)) } diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go index 2b132d3e..2d41cde5 100644 --- a/internal/controller/prefix_controller.go +++ b/internal/controller/prefix_controller.go @@ -56,9 +56,9 @@ type PrefixReconciler struct { RestConfig *rest.Config } -// +kubebuilder:rbac:groups=netbox.dev,resources=prefixes,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=netbox.dev,resources=prefixes/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=netbox.dev,resources=prefixes/finalizers,verbs=update +//+kubebuilder:rbac:groups=netbox.dev,resources=prefixes,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=netbox.dev,resources=prefixes/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=netbox.dev,resources=prefixes/finalizers,verbs=update // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -66,20 +66,20 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr logger := log.FromContext(ctx) debugLogger := logger.V(4) - logger.Info("prefix reconcile loop started") + logger.Info("reconcile loop started") /* 0. check if the matching Prefix object exists */ - prefix := &netboxv1.Prefix{} - if err := r.Client.Get(ctx, req.NamespacedName, prefix); err != nil { + o := &netboxv1.Prefix{} + if err := r.Client.Get(ctx, req.NamespacedName, o); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } // if being deleted - if !prefix.ObjectMeta.DeletionTimestamp.IsZero() { - if controllerutil.ContainsFinalizer(prefix, PrefixFinalizerName) { - if !prefix.Spec.PreserveInNetbox { - if err := r.NetboxClient.DeletePrefix(prefix.Status.PrefixId); err != nil { - if errReport := r.EventStatusRecorder.Report(ctx, prefix, netboxv1.ConditionPrefixReadyFalseDeletionFailed, corev1.EventTypeWarning, err); errReport != nil { + if !o.ObjectMeta.DeletionTimestamp.IsZero() { + if controllerutil.ContainsFinalizer(o, PrefixFinalizerName) { + if !o.Spec.PreserveInNetbox { + if err := r.NetboxClient.DeletePrefix(o.Status.PrefixId); err != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixReadyFalseDeletionFailed, corev1.EventTypeWarning, err); errReport != nil { return ctrl.Result{}, errReport } @@ -88,11 +88,11 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } debugLogger.Info("removing the finalizer") - if removed := controllerutil.RemoveFinalizer(prefix, PrefixFinalizerName); !removed { + if removed := controllerutil.RemoveFinalizer(o, PrefixFinalizerName); !removed { return ctrl.Result{}, errors.New("failed to remove the finalizer") } - if err := r.Update(ctx, prefix); err != nil { + if err := r.Update(ctx, o); err != nil { return ctrl.Result{}, err } } @@ -102,18 +102,18 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } // Set ready to false initially - if apismeta.FindStatusCondition(prefix.Status.Conditions, netboxv1.ConditionReadyFalseNewResource.Type) == nil { - err := r.EventStatusRecorder.Report(ctx, prefix, netboxv1.ConditionReadyFalseNewResource, corev1.EventTypeNormal, nil) + if apismeta.FindStatusCondition(o.Status.Conditions, netboxv1.ConditionReadyFalseNewResource.Type) == nil { + err := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionReadyFalseNewResource, corev1.EventTypeNormal, nil) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to initialise Ready condition: %w, ", err) } } // register finalizer if not yet registered - if !prefix.Spec.PreserveInNetbox && !controllerutil.ContainsFinalizer(prefix, PrefixFinalizerName) { + if !o.Spec.PreserveInNetbox && !controllerutil.ContainsFinalizer(o, PrefixFinalizerName) { debugLogger.Info("adding the finalizer") - controllerutil.AddFinalizer(prefix, PrefixFinalizerName) - if err := r.Update(ctx, prefix); err != nil { + controllerutil.AddFinalizer(o, PrefixFinalizerName) + if err := r.Update(ctx, o); err != nil { return ctrl.Result{}, err } } @@ -123,10 +123,10 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr - the prefix is owned by at least 1 prefixClaim - the prefix status condition is not ready */ - ownerReferences := prefix.ObjectMeta.OwnerReferences + ownerReferences := o.ObjectMeta.OwnerReferences var ll *leaselocker.LeaseLocker var err error - if len(ownerReferences) > 0 /* len(nil array) = 0 */ && !apismeta.IsStatusConditionTrue(prefix.Status.Conditions, "Ready") { + if len(ownerReferences) > 0 /* len(nil array) = 0 */ && !apismeta.IsStatusConditionTrue(o.Status.Conditions, "Ready") { // get prefixClaim ownerReferencesLookupKey := types.NamespacedName{ Name: ownerReferences[0].Name, // TODO(henrybear327): Under what condition would we have more than 1 ownerReferences? What should we do with it? @@ -139,7 +139,7 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if prefixClaim.Status.SelectedParentPrefix == "" { // the parent prefix is not selected - if errReport := r.EventStatusRecorder.Report(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, fmt.Errorf("%s", "the parent prefix is not selected")); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, fmt.Errorf("%s", "the parent prefix is not selected")); errReport != nil { return ctrl.Result{}, errReport } return ctrl.Result{ @@ -164,9 +164,10 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr defer cancel() // create lock - if locked := ll.TryLock(lockCtx); !locked { + locked := ll.TryLock(lockCtx) + if !locked { errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.SelectedParentPrefix) - r.EventStatusRecorder.Recorder().Eventf(prefix, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) + r.EventStatusRecorder.Recorder().Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil @@ -177,26 +178,26 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr /* 2. reserve or update Prefix in netbox */ accessor := apismeta.NewAccessor() - annotations, err := accessor.Annotations(prefix) + annotations, err := accessor.Annotations(o) if err != nil { return ctrl.Result{}, err } - prefixModel, err := generateNetboxPrefixModelFromPrefixSpec(&prefix.Spec, req, annotations[PXManagedCustomFieldsAnnotationName]) + prefixModel, err := generateNetboxPrefixModelFromPrefixSpec(&o.Spec, req, annotations[PXManagedCustomFieldsAnnotationName]) if err != nil { return ctrl.Result{}, err } netboxPrefixModel, err := r.NetboxClient.ReserveOrUpdatePrefix(prefixModel) if err != nil { - if errors.Is(err, api.ErrRestorationHashMismatch) && prefix.Status.PrefixId == 0 { + if errors.Is(err, api.ErrRestorationHashMismatch) && o.Status.PrefixId == 0 { // if there is a restoration hash mismatch and the PrefixId status field is not set, // delete the prefix so it can be recreated by the prefix claim controller // this will only affect resources that are created by a claim controller (and have a restoration hash custom field - logger.Info("restoration hash mismatch, deleting prefix custom resource", "prefix", prefix.Spec.Prefix) - err = r.Client.Delete(ctx, prefix) + logger.Info("restoration hash mismatch, deleting prefix custom resource", "prefix", o.Spec.Prefix) + err = r.Client.Delete(ctx, o) if err != nil { - if updateStatusErr := r.EventStatusRecorder.Report(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, + if updateStatusErr := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, err); updateStatusErr != nil { return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+ "after deletion of prefix cr failed: %w", updateStatusErr, err) @@ -206,7 +207,7 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, nil } - if updateStatusErr := r.EventStatusRecorder.Report(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, + if updateStatusErr := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, err); updateStatusErr != nil { return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+ "after reservation of prefix netbox failed: %w", updateStatusErr, err) @@ -220,9 +221,9 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } /* 4. update status fields */ - prefix.Status.PrefixId = netboxPrefixModel.ID - prefix.Status.PrefixUrl = config.GetBaseUrl() + "/ipam/prefixes/" + strconv.FormatInt(netboxPrefixModel.ID, 10) - err = r.Client.Status().Update(ctx, prefix) + o.Status.PrefixId = netboxPrefixModel.ID + o.Status.PrefixUrl = config.GetBaseUrl() + "/ipam/prefixes/" + strconv.FormatInt(netboxPrefixModel.ID, 10) + err = r.Client.Status().Update(ctx, o) if err != nil { return ctrl.Result{}, err } @@ -231,33 +232,33 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr annotations = make(map[string]string, 1) } - annotations[PXManagedCustomFieldsAnnotationName], err = generateManagedCustomFieldsAnnotation(prefix.Spec.CustomFields) + annotations[PXManagedCustomFieldsAnnotationName], err = generateManagedCustomFieldsAnnotation(o.Spec.CustomFields) if err != nil { logger.Error(err, "failed to update last metadata annotation") return ctrl.Result{Requeue: true}, nil } - err = accessor.SetAnnotations(prefix, annotations) + err = accessor.SetAnnotations(o, annotations) if err != nil { return ctrl.Result{}, err } // update object to store lastIpAddressMetadata annotation - if err := r.Update(ctx, prefix); err != nil { + if err := r.Update(ctx, o); err != nil { return ctrl.Result{}, err } // check if the created prefix contains the entire description from spec - if _, found := strings.CutPrefix(netboxPrefixModel.Description, req.NamespacedName.String()+" // "+prefix.Spec.Description); !found { - r.EventStatusRecorder.Recorder().Event(prefix, corev1.EventTypeWarning, "PrefixDescriptionTruncated", "prefix was created with truncated description") + if _, found := strings.CutPrefix(netboxPrefixModel.Description, req.NamespacedName.String()+" // "+o.Spec.Description); !found { + r.EventStatusRecorder.Recorder().Event(o, corev1.EventTypeWarning, "PrefixDescriptionTruncated", "prefix was created with truncated description") } - debugLogger.Info(fmt.Sprintf("reserved prefix in netbox, prefix: %s", prefix.Spec.Prefix)) - if err = r.EventStatusRecorder.Report(ctx, prefix, netboxv1.ConditionPrefixReadyTrue, corev1.EventTypeNormal, nil); err != nil { + debugLogger.Info(fmt.Sprintf("reserved prefix in netbox, prefix: %s", o.Spec.Prefix)) + if err = r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixReadyTrue, corev1.EventTypeNormal, nil); err != nil { return ctrl.Result{}, err } - logger.Info("prefix reconcile loop finished") + logger.Info("reconcile loop finished") return ctrl.Result{}, nil } diff --git a/internal/controller/prefixclaim_controller.go b/internal/controller/prefixclaim_controller.go index 839fb2a7..1695a98f 100644 --- a/internal/controller/prefixclaim_controller.go +++ b/internal/controller/prefixclaim_controller.go @@ -53,10 +53,10 @@ type PrefixClaimReconciler struct { RestConfig *rest.Config } -// +kubebuilder:rbac:groups=netbox.dev,resources=prefixclaims,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=netbox.dev,resources=prefixclaims/status,verbs=get;update;patch -// +kubebuilder:rbac:groups=netbox.dev,resources=prefixclaims/finalizers,verbs=update -// +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch +//+kubebuilder:rbac:groups=netbox.dev,resources=prefixclaims,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=netbox.dev,resources=prefixclaims/status,verbs=get;update;patch +//+kubebuilder:rbac:groups=netbox.dev,resources=prefixclaims/finalizers,verbs=update +//+kubebuilder:rbac:groups=core,resources=events,verbs=create;patch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -64,23 +64,23 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) logger := log.FromContext(ctx) debugLogger := logger.V(4) - logger.Info("prefixClaim reconcile loop started") + logger.Info("reconcile loop started") /* 0. check if the matching PrefixClaim object exists */ - prefixClaim := &netboxv1.PrefixClaim{} - if err := r.Client.Get(ctx, req.NamespacedName, prefixClaim); err != nil { + o := &netboxv1.PrefixClaim{} + if err := r.Client.Get(ctx, req.NamespacedName, o); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } // if being deleted - if !prefixClaim.ObjectMeta.DeletionTimestamp.IsZero() { + if !o.ObjectMeta.DeletionTimestamp.IsZero() { // end loop if deletion timestamp is not zero return ctrl.Result{}, nil } // Set ready to false initially - if apismeta.FindStatusCondition(prefixClaim.Status.Conditions, netboxv1.ConditionReadyFalseNewResource.Type) == nil { - err := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionReadyFalseNewResource, corev1.EventTypeNormal, nil) + if apismeta.FindStatusCondition(o.Status.Conditions, netboxv1.ConditionReadyFalseNewResource.Type) == nil { + err := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionReadyFalseNewResource, corev1.EventTypeNormal, nil) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to initialise Ready condition: %w, ", err) } @@ -89,24 +89,24 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) /* 1. compute and assign the parent prefix if required */ // The current design will use prefixClaim.Status.ParentPrefix for storing the selected parent prefix, // and as the source of truth for future parent prefix references - if prefixClaim.Status.SelectedParentPrefix == "" /* parent prefix not yet selected/assigned */ { - if prefixClaim.Spec.ParentPrefix != "" { - prefixClaim.Status.SelectedParentPrefix = prefixClaim.Spec.ParentPrefix + if o.Status.SelectedParentPrefix == "" /* parent prefix not yet selected/assigned */ { + if o.Spec.ParentPrefix != "" { + o.Status.SelectedParentPrefix = o.Spec.ParentPrefix // set status, and condition field - msg := fmt.Sprintf("parentPrefix is provided in CR: %v", prefixClaim.Status.SelectedParentPrefix) - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, nil, msg); errReport != nil { + msg := fmt.Sprintf("parentPrefix is provided in CR: %v", o.Status.SelectedParentPrefix) + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, nil, msg); errReport != nil { return ctrl.Result{}, errReport } - } else if len(prefixClaim.Spec.ParentPrefixSelector) > 0 { + } else if len(o.Spec.ParentPrefixSelector) > 0 { // we first check if a prefix can be restored from the netbox // since the parent prefix is not part of the restoration hash computation // we can quickly check to see if the prefix with the restoration hash is matched in NetBox - h := generatePrefixRestorationHash(prefixClaim) + h := generatePrefixRestorationHash(o) canBeRestored, err := r.NetboxClient.RestoreExistingPrefixByHash(h) if err != nil { - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, fmt.Errorf("failed to look up prefix by hash: %w", err)); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, fmt.Errorf("failed to look up prefix by hash: %w", err)); errReport != nil { return ctrl.Result{}, errReport } @@ -142,9 +142,9 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) // since we can't infer the parent prefix // we write a special string in the ParentPrefix status field indicating the situation - prefixClaim.Status.SelectedParentPrefix = msgCanNotInferParentPrefix + o.Status.SelectedParentPrefix = msgCanNotInferParentPrefix - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, nil, msgCanNotInferParentPrefix); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, nil, msgCanNotInferParentPrefix); errReport != nil { return ctrl.Result{}, errReport } } else { @@ -154,18 +154,18 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) // The existing algorithm for prefix allocation within a ParentPrefix remains unchanged // fetch available prefixes from netbox - parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixesByParentPrefixSelector(&prefixClaim.Spec) + parentPrefixCandidates, err := r.NetboxClient.GetAvailablePrefixesByParentPrefixSelector(&o.Spec) if err != nil { - r.EventStatusRecorder.Recorder().Event(prefixClaim, corev1.EventTypeWarning, netboxv1.ConditionPrefixAssignedFalse.Reason, netboxv1.ConditionPrefixAssignedFalse.Message+": "+err.Error()) - if err := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); err != nil { + r.EventStatusRecorder.Recorder().Event(o, corev1.EventTypeWarning, netboxv1.ConditionPrefixAssignedFalse.Reason, netboxv1.ConditionPrefixAssignedFalse.Message+": "+err.Error()) + if err := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); err != nil { return ctrl.Result{}, err } return ctrl.Result{Requeue: true}, nil } if len(parentPrefixCandidates) == 0 { message := "no parent prefix found matching the parentPrefixSelector" - r.EventStatusRecorder.Recorder().Event(prefixClaim, corev1.EventTypeWarning, netboxv1.ConditionPrefixAssignedFalse.Reason, netboxv1.ConditionPrefixAssignedFalse.Message+": "+message) - if err := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, errors.New(message)); err != nil { + r.EventStatusRecorder.Recorder().Event(o, corev1.EventTypeWarning, netboxv1.ConditionPrefixAssignedFalse.Reason, netboxv1.ConditionPrefixAssignedFalse.Message+": "+message) + if err := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, errors.New(message)); err != nil { return ctrl.Result{}, err } // we requeue as this might be a temporary prefix exhausation @@ -174,17 +174,17 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) // TODO(henrybear327): use best-fit algorithm to pick a parent prefix parentPrefixCandidate := parentPrefixCandidates[0] - prefixClaim.Status.SelectedParentPrefix = parentPrefixCandidate.Prefix + o.Status.SelectedParentPrefix = parentPrefixCandidate.Prefix // set status, and condition field - msg := fmt.Sprintf("parentPrefix is selected: %v", prefixClaim.Status.SelectedParentPrefix) - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, nil, msg); errReport != nil { + msg := fmt.Sprintf("parentPrefix is selected: %v", o.Status.SelectedParentPrefix) + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionParentPrefixSelectedTrue, corev1.EventTypeNormal, nil, msg); errReport != nil { return ctrl.Result{}, errReport } } } else { // this case should not be triggered anymore, as we have validation rules put in place on the CR - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, fmt.Errorf("%s", "either ParentPrefixSelector or ParentPrefix needs to be set")); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionParentPrefixSelectedFalse, corev1.EventTypeWarning, fmt.Errorf("%s", "either ParentPrefixSelector or ParentPrefix needs to be set")); errReport != nil { return ctrl.Result{}, errReport } return ctrl.Result{}, nil @@ -193,10 +193,10 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) /* 2. check if the matching Prefix object exists */ prefix := &netboxv1.Prefix{} - prefixName := prefixClaim.ObjectMeta.Name + prefixName := o.ObjectMeta.Name prefixLookupKey := types.NamespacedName{ Name: prefixName, - Namespace: prefixClaim.Namespace, + Namespace: o.Namespace, } if err := r.Client.Get(ctx, prefixLookupKey, prefix); err != nil { // if not nil (likely the Prefix object is not found) /* return error if not a notfound error */ @@ -205,12 +205,12 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) } debugLogger.Info("the prefix was not found, will create a new prefix object now") - if prefixClaim.Status.SelectedParentPrefix != msgCanNotInferParentPrefix { + if o.Status.SelectedParentPrefix != msgCanNotInferParentPrefix { // we can't restore from the restoration hash /* 3. check if the lease for parent prefix is available */ leaseLockerNSN := types.NamespacedName{ - Name: convertCIDRToLeaseLockName(prefixClaim.Status.SelectedParentPrefix), + Name: convertCIDRToLeaseLockName(o.Status.SelectedParentPrefix), Namespace: r.OperatorNamespace, } ll, err := leaselocker.NewLeaseLocker(r.RestConfig, leaseLockerNSN, req.Namespace+"/"+prefixName) @@ -225,23 +225,23 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) locked := ll.TryLock(lockCtx) if !locked { // lock for parent prefix was not available, rescheduling - errorMsg := fmt.Sprintf("failed to lock parent prefix %s", prefixClaim.Status.SelectedParentPrefix) - r.EventStatusRecorder.Recorder().Eventf(prefixClaim, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) + errorMsg := fmt.Sprintf("failed to lock parent prefix %s", o.Status.SelectedParentPrefix) + r.EventStatusRecorder.Recorder().Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg) return ctrl.Result{ RequeueAfter: 2 * time.Second, }, nil } - debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", prefixClaim.Status.SelectedParentPrefix)) + debugLogger.Info(fmt.Sprintf("successfully locked parent prefix %s", o.Status.SelectedParentPrefix)) } // else { // we can restore from the restoration hash // we skip directly to try to reclaim Prefix using restorationHash // } // 5. try to reclaim Prefix using restorationHash - h := generatePrefixRestorationHash(prefixClaim) + h := generatePrefixRestorationHash(o) prefixModel, err := r.NetboxClient.RestoreExistingPrefixByHash(h) if err != nil { - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); errReport != nil { return ctrl.Result{}, errReport } return ctrl.Result{Requeue: true}, nil @@ -254,26 +254,26 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) // get available Prefix under parent prefix in netbox with equal mask length prefixModel, err = r.NetboxClient.GetAvailablePrefixByClaim( &models.PrefixClaim{ - ParentPrefix: prefixClaim.Status.SelectedParentPrefix, - PrefixLength: prefixClaim.Spec.PrefixLength, + ParentPrefix: o.Status.SelectedParentPrefix, + PrefixLength: o.Spec.PrefixLength, Metadata: &models.NetboxMetadata{ - Tenant: prefixClaim.Spec.Tenant, - Site: prefixClaim.Spec.Site, + Tenant: o.Spec.Tenant, + Site: o.Spec.Site, }, }) if err != nil { if errors.Is(err, api.ErrParentPrefixExhausted) { - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, fmt.Errorf("%w, will restart the parent prefix selection process", err)); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, fmt.Errorf("%w, will restart the parent prefix selection process", err)); errReport != nil { return ctrl.Result{}, errReport } // we reset the selected parent prefix, since this one is already exhausted - prefixClaim.Status.SelectedParentPrefix = "" + o.Status.SelectedParentPrefix = "" return ctrl.Result{Requeue: true}, nil } - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); errReport != nil { return ctrl.Result{}, errReport } return ctrl.Result{Requeue: true}, nil @@ -287,20 +287,20 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) } /* 7.a create the Prefix object */ - prefixResource := generatePrefixFromPrefixClaim(prefixClaim, prefixModel.Prefix, logger) - err = controllerutil.SetControllerReference(prefixClaim, prefixResource, r.Scheme) + prefixResource := generatePrefixFromPrefixClaim(o, prefixModel.Prefix, logger) + err = controllerutil.SetControllerReference(o, prefixResource, r.Scheme) if err != nil { return ctrl.Result{}, err } err = r.Client.Create(ctx, prefixResource) if err != nil { - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err); errReport != nil { return ctrl.Result{}, errReport } return ctrl.Result{}, err } - if err = r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedTrue, corev1.EventTypeNormal, nil); err != nil { + if err = r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixAssignedTrue, corev1.EventTypeNormal, nil); err != nil { return ctrl.Result{}, err } } else { // Prefix object exists @@ -310,7 +310,7 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, err } - updatedPrefixSpec := generatePrefixSpec(prefixClaim, prefix.Spec.Prefix, logger) + updatedPrefixSpec := generatePrefixSpec(o, prefix.Spec.Prefix, logger) if _, err = ctrl.CreateOrUpdate(ctx, r.Client, prefix, func() error { // only add the mutable fields here prefix.Spec.Site = updatedPrefixSpec.Site @@ -318,7 +318,7 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) prefix.Spec.Description = updatedPrefixSpec.Description prefix.Spec.Comments = updatedPrefixSpec.Comments prefix.Spec.PreserveInNetbox = updatedPrefixSpec.PreserveInNetbox - err = controllerutil.SetControllerReference(prefixClaim, prefix, r.Scheme) + err = controllerutil.SetControllerReference(o, prefix, r.Scheme) if err != nil { return err } @@ -332,22 +332,22 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) if apismeta.IsStatusConditionTrue(prefix.Status.Conditions, "Ready") { debugLogger.Info("prefix status ready true") - prefixClaim.Status.Prefix = prefix.Spec.Prefix - prefixClaim.Status.PrefixName = prefix.Name + o.Status.Prefix = prefix.Spec.Prefix + o.Status.PrefixName = prefix.Name - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixClaimReadyTrue, corev1.EventTypeNormal, nil); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixClaimReadyTrue, corev1.EventTypeNormal, nil); errReport != nil { return ctrl.Result{}, errReport } } else { debugLogger.Info("prefix status ready false") - if errReport := r.EventStatusRecorder.Report(ctx, prefixClaim, netboxv1.ConditionPrefixClaimReadyFalse, corev1.EventTypeWarning, nil); errReport != nil { + if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionPrefixClaimReadyFalse, corev1.EventTypeWarning, nil); errReport != nil { return ctrl.Result{}, errReport } return ctrl.Result{Requeue: true}, nil } - logger.Info("prefixClaim reconcile loop finished") + logger.Info("reconcile loop finished") return ctrl.Result{}, nil }