Fix Finalizers

api/v1/ipaddress_types.go

@@ -101,3 +101,10 @@ var ConditionIpaddressReadyFalse = metav1.Condition{
 	Reason:  "FailedToReserveIpInNetbox",
 	Message: "Failed to reserve IP in NetBox",
 }
+
+var ConditionIpaddressReadyFalseDeletionFailed = metav1.Condition{
+	Type:    "Ready",
+	Status:  "False",
+	Reason:  "FailedToDeleteIpInNetbox",
+	Message: "Failed to delete IP in NetBox",
+}

api/v1/prefix_types.go

@@ -102,3 +102,10 @@ var ConditionPrefixReadyFalse = metav1.Condition{
 	Reason:  "FailedToReservePrefixInNetbox",
 	Message: "Failed to reserve prefix in NetBox",
 }
+
+var ConditionPrefixReadyFalseDeletionFailed = metav1.Condition{
+	Type:    "Ready",
+	Status:  "False",
+	Reason:  "FailedToDeletePrefixInNetbox",
+	Message: "Failed to delete prefix in Netbox",
+}

internal/controller/expected_netboxmock_calls_test.go

@@ -139,6 +139,21 @@ func mockIpAddressesDelete(ipamMock *mock_interfaces.MockIpamInterface, catchUne
 		}).MinTimes(1)
 }
 
+func mockIpAddressesDeleteFail(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
+	ipamMock.EXPECT().IpamIPAddressesDelete(gomock.Any(), nil).
+		DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesDeleteNoContent, error) {
+			got := params.(*ipam.IpamIPAddressesDeleteParams)
+			diff := deep.Equal(got, ExpectedDeleteFailParams)
+			if len(diff) > 0 {
+				err := fmt.Errorf("netboxmock: unexpected call to ipam.IpamIPAddressesDelete, diff to expected params diff: %+v", diff)
+				catchUnexpectedParams <- err
+				return &ipam.IpamIPAddressesDeleteNoContent{}, err
+			}
+			fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesDelete was called with mock input\n")
+			return nil, ipam.NewIpamIPAddressesDeleteDefault(404)
+		}).MinTimes(1)
+}
+
 func mockIpamIPAddressesUpdate(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
 	ipamMock.EXPECT().IpamIPAddressesUpdate(gomock.Any(), nil).
 		DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesUpdateOK, error) {

internal/controller/ipaddress_controller.go

@@ -81,7 +81,12 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			if !o.Spec.PreserveInNetbox {
 				err := r.NetboxClient.DeleteIpAddress(o.Status.IpAddressId)
 				if err != nil {
-					return ctrl.Result{}, err
+					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)
+					}
+
+					return ctrl.Result{Requeue: true}, nil
 				}
 			}
 
@@ -101,6 +106,15 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{}, nil
 	}
 
+	// if PreserveIpInNetbox flag is false then register finalizer if not yet registered
+	if !o.Spec.PreserveInNetbox && !controllerutil.ContainsFinalizer(o, IpAddressFinalizerName) {
+		debugLogger.Info("adding the finalizer")
+		controllerutil.AddFinalizer(o, IpAddressFinalizerName)
+		if err := r.Update(ctx, o); err != nil {
+			return ctrl.Result{}, 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
@@ -208,15 +222,6 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		r.Recorder.Event(o, corev1.EventTypeWarning, "IpDescriptionTruncated", "ip address was created with truncated description")
 	}
 
-	// if PreserveIpInNetbox flag is false then register finalizer if not yet registered
-	if !o.Spec.PreserveInNetbox && !controllerutil.ContainsFinalizer(o, IpAddressFinalizerName) {
-		debugLogger.Info("adding the finalizer")
-		controllerutil.AddFinalizer(o, IpAddressFinalizerName)
-		if err := r.Update(ctx, o); err != nil {
-			return ctrl.Result{}, err
-		}
-	}
-
 	debugLogger.Info(fmt.Sprintf("reserved ip address in netbox, ip: %s", o.Spec.IpAddress))
 
 	err = r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyTrue, corev1.EventTypeNormal, "")

internal/controller/ipaddress_controller_test.go

@@ -146,6 +146,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 			[]func(*mock_interfaces.MockIpamInterface, chan error){
 				mockIpAddressListWithIpAddressFilter,
 				mockIpamIPAddressesUpdateFail,
+				mockIpAddressesDeleteFail,
 			},
 			[]func(*mock_interfaces.MockTenancyInterface, chan error){
 				mockTenancyTenancyTenantsList,

internal/controller/netbox_testdata_test.go

@@ -219,6 +219,7 @@ func mockedResponseTenancyTenantsList() *tenancy.TenancyTenantsListOKBody {
 var nsn = namespace + "/" + name + " // "
 var warningComment = " // managed by netbox-operator, please don't edit it in Netbox unless you know what you're doing"
 var expectedIpAddressID = int64(1)
+var expectedIpAddressFailID = int64(0)
 
 var expectedIpToUpdate = &netboxModels.WritableIPAddress{
 	Address:  &ipAddress,
@@ -270,6 +271,9 @@ var ExpectedIpAddressesCreateWithHashParams = ipam.NewIpamIPAddressesCreateParam
 // expected inputs for ipam.IpamIPAddressesDelete method
 var ExpectedDeleteParams = ipam.NewIpamIPAddressesDeleteParams().WithID(expectedIpAddressID)
 
+// expected inputs for ipam.IpamIPAddressesDelete method when update fails
+var ExpectedDeleteFailParams = ipam.NewIpamIPAddressesDeleteParams().WithID(expectedIpAddressFailID)
+
 var ExpectedIpAddressStatus = netboxv1.IpAddressStatus{IpAddressId: 1}
 
 var ExpectedIpAddressFailedStatus = netboxv1.IpAddressStatus{IpAddressId: 0}

internal/controller/prefix_controller.go

@@ -81,7 +81,12 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		if controllerutil.ContainsFinalizer(prefix, PrefixFinalizerName) {
 			if !prefix.Spec.PreserveInNetbox {
 				if err := r.NetboxClient.DeletePrefix(prefix.Status.PrefixId); err != nil {
-					return ctrl.Result{}, err
+					setConditionErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalseDeletionFailed, corev1.EventTypeWarning, err.Error())
+					if setConditionErr != nil {
+						return ctrl.Result{}, fmt.Errorf("error updating status: %w, when deleting Prefix failed: %w", setConditionErr, err)
+					}
+
+					return ctrl.Result{Requeue: true}, nil
 				}
 			}
 
@@ -99,6 +104,15 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		return ctrl.Result{}, nil
 	}
 
+	// register finalizer if not yet registered
+	if !prefix.Spec.PreserveInNetbox && !controllerutil.ContainsFinalizer(prefix, PrefixFinalizerName) {
+		debugLogger.Info("adding the finalizer")
+		controllerutil.AddFinalizer(prefix, PrefixFinalizerName)
+		if err := r.Update(ctx, prefix); err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
 	/*
 		1. try to lock the lease of the parent prefix if all of the following conditions are met
 			- the prefix is owned by at least 1 prefixClaim
@@ -203,15 +217,6 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		r.Recorder.Event(prefix, corev1.EventTypeWarning, "PrefixDescriptionTruncated", "prefix was created with truncated description")
 	}
 
-	// register finalizer if not yet registered
-	if !prefix.Spec.PreserveInNetbox && !controllerutil.ContainsFinalizer(prefix, PrefixFinalizerName) {
-		debugLogger.Info("adding the finalizer")
-		controllerutil.AddFinalizer(prefix, PrefixFinalizerName)
-		if err := r.Update(ctx, prefix); err != nil {
-			return ctrl.Result{}, err
-		}
-	}
-
 	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 {

pkg/netbox/api/ip_address.go

@@ -17,6 +17,7 @@ limitations under the License.
 package api
 
 import (
+	"net/http"
 	"unicode/utf8"
 
 	"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
@@ -100,7 +101,14 @@ func (r *NetboxClient) DeleteIpAddress(ipAddressId int64) error {
 	requestDeleteIp := ipam.NewIpamIPAddressesDeleteParams().WithID(ipAddressId)
 	_, err := r.Ipam.IpamIPAddressesDelete(requestDeleteIp, nil)
 	if err != nil {
-		return utils.NetboxError("Failed to delete IP Address from Netbox", err)
+		switch typedErr := err.(type) {
+		case *ipam.IpamIPAddressesDeleteDefault:
+			if typedErr.IsCode(http.StatusNotFound) {
+				return nil
+			}
+		default:
+			return utils.NetboxError("Failed to delete IP Address from Netbox", err)
+		}
 	}
 	return nil
 }

pkg/netbox/api/prefix.go

@@ -17,6 +17,8 @@ limitations under the License.
 package api
 
 import (
+	"net/http"
+
 	"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
 	netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models"
 
@@ -101,7 +103,14 @@ func (r *NetboxClient) DeletePrefix(prefixId int64) error {
 	requestDeletePrefix := ipam.NewIpamPrefixesDeleteParams().WithID(prefixId)
 	_, err := r.Ipam.IpamPrefixesDelete(requestDeletePrefix, nil)
 	if err != nil {
-		return utils.NetboxError("failed to delete Prefix", err)
+		switch typedErr := err.(type) {
+		case *ipam.IpamPrefixesDeleteDefault:
+			if typedErr.IsCode(http.StatusNotFound) {
+				return nil
+			}
+		default:
+			return utils.NetboxError("Failed to delete IP Address from Netbox", err)
+		}
 	}
 	return nil
 }