Consolidate Status and Condition reporting

README.md

@@ -30,7 +30,10 @@ There are several ways to install NetBox Operator on your Cluster:
 
 ## Running both NetBox Operator and NetBox on a local kind cluster
 
-Note: This requires Docker BuildKit.
+> **Note:** This requires Docker BuildKit.
+
+> **Note:** There is currently a bug where if Docker uses containerd for pulling and storing images, kind fails to load the image into the cluster ([GitHub Issue](https://github.com/kubernetes-sigs/kind/issues/3795)).
+To resolve this, temporarily disable this option in the Docker settings: "General > Use containerd for pulling and storing images"
 
 - Create kind cluster with a NetBox deployment: `make create-kind`
 - Deploy the NetBox Operator on the local kind cluster: `make deploy-kind` (In case you're using podman use `CONTAINER_TOOL=podman make deploy-kind`)

api/v1/ipaddress_types.go

@@ -99,6 +99,10 @@ type IpAddress struct {
 	Status IpAddressStatus `json:"status,omitempty"`
 }
 
+func (i *IpAddress) Conditions() *[]metav1.Condition {
+	return &i.Status.Conditions
+}
+
 //+kubebuilder:object:root=true
 
 // IpAddressList contains a list of IpAddress

api/v1/ipaddressclaim_types.go

@@ -104,6 +104,10 @@ type IpAddressClaim struct {
 	Status IpAddressClaimStatus `json:"status,omitempty"`
 }
 
+func (i *IpAddressClaim) Conditions() *[]metav1.Condition {
+	return &i.Status.Conditions
+}
+
 //+kubebuilder:object:root=true
 
 // IpAddressClaimList contains a list of IpAddressClaim

api/v1/iprange_types.go

@@ -108,6 +108,10 @@ type IpRange struct {
 	Status IpRangeStatus `json:"status,omitempty"`
 }
 
+func (i *IpRange) Conditions() *[]metav1.Condition {
+	return &i.Status.Conditions
+}
+
 //+kubebuilder:object:root=true
 
 // IpRangeList contains a list of IpRange

api/v1/iprangeclaim_types.go

@@ -133,6 +133,10 @@ type IpRangeClaim struct {
 	Status IpRangeClaimStatus `json:"status,omitempty"`
 }
 
+func (i *IpRangeClaim) Conditions() *[]metav1.Condition {
+	return &i.Status.Conditions
+}
+
 //+kubebuilder:object:root=true
 
 // IpRangeClaimList contains a list of IpRangeClaim

api/v1/prefix_types.go

@@ -104,6 +104,10 @@ type Prefix struct {
 	Status PrefixStatus `json:"status,omitempty"`
 }
 
+func (p *Prefix) Conditions() *[]metav1.Condition {
+	return &p.Status.Conditions
+}
+
 // +kubebuilder:object:root=true
 
 // PrefixList contains a list of Prefix

api/v1/prefixclaim_types.go

@@ -132,6 +132,10 @@ type PrefixClaim struct {
 	Status PrefixClaimStatus `json:"status,omitempty"`
 }
 
+func (p *PrefixClaim) Conditions() *[]metav1.Condition {
+	return &p.Status.Conditions
+}
+
 // +kubebuilder:object:root=true
 
 // PrefixClaimList contains a list of PrefixClaim

api/v1/shared_conditions.go

@@ -0,0 +1,26 @@
+/*
+Copyright 2024 Swisscom (Schweiz) AG.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1
+
+import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+var ConditionReadyFalseNewResource = metav1.Condition{
+	Type:    "Ready",
+	Status:  "False",
+	Reason:  "NewResource",
+	Message: "Pending Reconciliation",
+}

cmd/main.go

@@ -168,67 +168,67 @@ func main() {
 	}
 
 	if err = (&controller.IpAddressReconciler{
-		Client:            mgr.GetClient(),
-		Scheme:            mgr.GetScheme(),
-		Recorder:          mgr.GetEventRecorderFor("ip-address-controller"),
-		NetboxClient:      netboxClient,
-		OperatorNamespace: operatorNamespace,
-		RestConfig:        mgr.GetConfig(),
+		Client:              mgr.GetClient(),
+		Scheme:              mgr.GetScheme(),
+		EventStatusRecorder: controller.NewEventStatusRecorder(mgr.GetClient(), mgr.GetEventRecorderFor("ip-address-controller")),
+		NetboxClient:        netboxClient,
+		OperatorNamespace:   operatorNamespace,
+		RestConfig:          mgr.GetConfig(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "IpAddress")
 		os.Exit(1)
 	}
 	if err = (&controller.IpAddressClaimReconciler{
-		Client:            mgr.GetClient(),
-		Scheme:            mgr.GetScheme(),
-		Recorder:          mgr.GetEventRecorderFor("ip-address-claim-controller"),
-		NetboxClient:      netboxClient,
-		OperatorNamespace: operatorNamespace,
-		RestConfig:        mgr.GetConfig(),
+		Client:              mgr.GetClient(),
+		Scheme:              mgr.GetScheme(),
+		EventStatusRecorder: controller.NewEventStatusRecorder(mgr.GetClient(), mgr.GetEventRecorderFor("ip-address-claim-controller")),
+		NetboxClient:        netboxClient,
+		OperatorNamespace:   operatorNamespace,
+		RestConfig:          mgr.GetConfig(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "IpAddressClaim")
 		os.Exit(1)
 	}
 	if err = (&controller.PrefixReconciler{
-		Client:            mgr.GetClient(),
-		Scheme:            mgr.GetScheme(),
-		Recorder:          mgr.GetEventRecorderFor("prefix-controller"),
-		NetboxClient:      netboxClient,
-		OperatorNamespace: operatorNamespace,
-		RestConfig:        mgr.GetConfig(),
+		Client:              mgr.GetClient(),
+		Scheme:              mgr.GetScheme(),
+		EventStatusRecorder: controller.NewEventStatusRecorder(mgr.GetClient(), mgr.GetEventRecorderFor("prefix-controller")),
+		NetboxClient:        netboxClient,
+		OperatorNamespace:   operatorNamespace,
+		RestConfig:          mgr.GetConfig(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "Prefix")
 		os.Exit(1)
 	}
 	if err = (&controller.PrefixClaimReconciler{
-		Client:            mgr.GetClient(),
-		Scheme:            mgr.GetScheme(),
-		Recorder:          mgr.GetEventRecorderFor("prefix-claim-controller"),
-		NetboxClient:      netboxClient,
-		OperatorNamespace: operatorNamespace,
-		RestConfig:        mgr.GetConfig(),
+		Client:              mgr.GetClient(),
+		Scheme:              mgr.GetScheme(),
+		EventStatusRecorder: controller.NewEventStatusRecorder(mgr.GetClient(), mgr.GetEventRecorderFor("prefix-claim-controller")),
+		NetboxClient:        netboxClient,
+		OperatorNamespace:   operatorNamespace,
+		RestConfig:          mgr.GetConfig(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "PrefixClaim")
 		os.Exit(1)
 	}
 	if err = (&controller.IpRangeClaimReconciler{
-		Client:            mgr.GetClient(),
-		Scheme:            mgr.GetScheme(),
-		Recorder:          mgr.GetEventRecorderFor("ip-range-claim-controller"),
-		NetboxClient:      netboxClient,
-		OperatorNamespace: operatorNamespace,
-		RestConfig:        mgr.GetConfig(),
+		Client:              mgr.GetClient(),
+		Scheme:              mgr.GetScheme(),
+		EventStatusRecorder: controller.NewEventStatusRecorder(mgr.GetClient(), mgr.GetEventRecorderFor("ip-range-claim-controller")),
+		NetboxClient:        netboxClient,
+		OperatorNamespace:   operatorNamespace,
+		RestConfig:          mgr.GetConfig(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "IpRangeClaim")
 		os.Exit(1)
 	}
 	if err = (&controller.IpRangeReconciler{
-		Client:            mgr.GetClient(),
-		Scheme:            mgr.GetScheme(),
-		Recorder:          mgr.GetEventRecorderFor("ip-range-controller"),
-		NetboxClient:      netboxClient,
-		OperatorNamespace: operatorNamespace,
-		RestConfig:        mgr.GetConfig(),
+		Client:              mgr.GetClient(),
+		Scheme:              mgr.GetScheme(),
+		EventStatusRecorder: controller.NewEventStatusRecorder(mgr.GetClient(), mgr.GetEventRecorderFor("ip-range-controller")),
+		NetboxClient:        netboxClient,
+		OperatorNamespace:   operatorNamespace,
+		RestConfig:          mgr.GetConfig(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "IpRange")
 		os.Exit(1)

internal/controller/interfaces.go

@@ -0,0 +1,27 @@
+/*
+Copyright 2024 Swisscom (Schweiz) AG.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+type ObjectWithConditions interface {
+	client.Object
+	Conditions() *[]metav1.Condition
+}

internal/controller/ipaddress_controller.go

@@ -33,11 +33,9 @@ import (
 	"github.com/swisscom/leaselocker"
 	corev1 "k8s.io/api/core/v1"
 	apismeta "k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -50,11 +48,11 @@ const IPManagedCustomFieldsAnnotationName = "ipaddress.netbox.dev/managed-custom
 // IpAddressReconciler reconciles a IpAddress object
 type IpAddressReconciler struct {
 	client.Client
-	Scheme            *runtime.Scheme
-	NetboxClient      *api.NetboxClient
-	Recorder          record.EventRecorder
-	OperatorNamespace string
-	RestConfig        *rest.Config
+	Scheme              *runtime.Scheme
+	NetboxClient        *api.NetboxClient
+	EventStatusRecorder *EventStatusRecorder
+	OperatorNamespace   string
+	RestConfig          *rest.Config
 }
 
 //+kubebuilder:rbac:groups=netbox.dev,resources=ipaddresses,verbs=get;list;watch;create;update;patch;delete
@@ -81,11 +79,9 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			if !o.Spec.PreserveInNetbox {
 				err := r.NetboxClient.DeleteIpAddress(o.Status.IpAddressId)
 				if err != nil {
-					setConditionErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalseDeletionFailed, corev1.EventTypeWarning, err.Error())
-					if setConditionErr != nil {
-						return ctrl.Result{}, fmt.Errorf("error updating status: %w, when deleting IPAddress failed: %w", setConditionErr, err)
+					if errReport := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionIpaddressReadyFalseDeletionFailed, corev1.EventTypeWarning, err); errReport != nil {
+						return ctrl.Result{}, errReport
 					}
-
 					return ctrl.Result{Requeue: true}, nil
 				}
 			}
@@ -115,6 +111,14 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		}
 	}
 
+	// Set ready to false initially
+	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)
+		}
+	}
+
 	// 1. try to lock lease of parent prefix if IpAddress status condition is not true
 	// and IpAddress is owned by an IpAddressClaim
 	or := o.ObjectMeta.OwnerReferences
@@ -148,7 +152,7 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		locked := ll.TryLock(lockCtx)
 		if !locked {
 			errorMsg := fmt.Sprintf("failed to lock parent prefix %s", ipAddressClaim.Spec.ParentPrefix)
-			r.Recorder.Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
+			r.EventStatusRecorder.Recorder().Eventf(o, corev1.EventTypeWarning, "FailedToLockParentPrefix", errorMsg)
 			return ctrl.Result{
 				RequeueAfter: 2 * time.Second,
 			}, nil
@@ -177,8 +181,8 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			logger.Info("restoration hash mismatch, deleting ip address custom resource", "ipaddress", o.Spec.IpAddress)
 			err = r.Client.Delete(ctx, o)
 			if err != nil {
-				if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
-					corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
+				if updateStatusErr := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
+					corev1.EventTypeWarning, err); updateStatusErr != nil {
 					return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
 						"after deletion of ip address cr failed: %w", updateStatusErr, err)
 				}
@@ -188,8 +192,8 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 		}
 
-		if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
-			corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
+		if updateStatusErr := r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
+			corev1.EventTypeWarning, err, o.Spec.IpAddress); updateStatusErr != nil {
 			return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
 				"after reservation of ip in netbox failed: %w", updateStatusErr, err)
 		}
@@ -232,12 +236,12 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	// check if created ip address contains entire description from spec
 	_, found := strings.CutPrefix(netboxIpAddressModel.Description, req.NamespacedName.String()+" // "+o.Spec.Description)
 	if !found {
-		r.Recorder.Event(o, corev1.EventTypeWarning, "IpDescriptionTruncated", "ip address was created with truncated description")
+		r.EventStatusRecorder.Recorder().Event(o, corev1.EventTypeWarning, "IpDescriptionTruncated", "ip address was created with truncated description")
 	}
 
 	debugLogger.Info(fmt.Sprintf("reserved ip address in netbox, ip: %s", o.Spec.IpAddress))
 
-	err = r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyTrue, corev1.EventTypeNormal, "")
+	err = r.EventStatusRecorder.Report(ctx, o, netboxv1.ConditionIpaddressReadyTrue, corev1.EventTypeNormal, nil)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -254,21 +258,6 @@ func (r *IpAddressReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *IpAddressReconciler) SetConditionAndCreateEvent(ctx context.Context, o *netboxv1.IpAddress, condition metav1.Condition, eventType string, conditionMessageAppend string) error {
-	if len(conditionMessageAppend) > 0 {
-		condition.Message = condition.Message + ". " + conditionMessageAppend
-	}
-	conditionChanged := apismeta.SetStatusCondition(&o.Status.Conditions, condition)
-	if conditionChanged {
-		r.Recorder.Event(o, eventType, condition.Reason, condition.Message)
-	}
-	err := r.Client.Status().Update(ctx, o)
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 func generateNetboxIpAddressModelFromIpAddressSpec(spec *netboxv1.IpAddressSpec, req ctrl.Request, lastIpAddressMetadata string) (*models.IPAddress, error) {
 	// unmarshal lastIpAddressMetadata json string to map[string]string
 	lastAppliedCustomFields := make(map[string]string)