update deletion for finalizers and move finalizer creation up

netbox-community · Oct 7, 2024 · 0a8c6bb · 0a8c6bb
1 parent 51fd430
commit 0a8c6bb
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 6 deletions.
diff --git a/api/v1/ipaddress_types.go b/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",
+}
diff --git a/api/v1/prefix_types.go b/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",
+}
diff --git a/internal/controller/expected_netboxmock_calls_test.go b/internal/controller/expected_netboxmock_calls_test.go
@@ -139,6 +139,21 @@ func mockIpAddressesDelete(ipamMock *mock_interfaces.MockIpamInterface, catchUne
 		}).MinTimes(1)
 }
 
+func mockIpAddressesDeleteUpdateFail(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, ExpectedDeleteUpdateFailParams)
+			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 &ipam.IpamIPAddressesDeleteNoContent{}, nil
+		}).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) {

diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go
@@ -82,15 +82,14 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			if !o.Spec.PreserveInNetbox {
 				err := r.NetboxClient.DeleteIpAddress(o.Status.IpAddressId)
 				if err != nil {
-					if errors.Is(err, utils.ErrNotFound) {
-						setConditionErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse, corev1.EventTypeWarning, err.Error())
+					if !errors.Is(err, utils.ErrNotFound) {
+						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
 					}
-					return ctrl.Result{}, err
 				}
 			}
 

diff --git a/internal/controller/ipaddress_controller_test.go b/internal/controller/ipaddress_controller_test.go
@@ -146,6 +146,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 			[]func(*mock_interfaces.MockIpamInterface, chan error){
 				mockIpAddressListWithIpAddressFilter,
 				mockIpamIPAddressesUpdateFail,
+				mockIpAddressesDeleteUpdateFail,
 			},
 			[]func(*mock_interfaces.MockTenancyInterface, chan error){
 				mockTenancyTenancyTenantsList,

diff --git a/internal/controller/netbox_testdata_test.go b/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 ExpectedDeleteUpdateFailParams = ipam.NewIpamIPAddressesDeleteParams().WithID(expectedIpAddressFailID)
+
 var ExpectedIpAddressStatus = netboxv1.IpAddressStatus{IpAddressId: 1}
 
 var ExpectedIpAddressFailedStatus = netboxv1.IpAddressStatus{IpAddressId: 0}

diff --git a/internal/controller/prefix_controller.go b/internal/controller/prefix_controller.go
@@ -82,15 +82,14 @@ 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 {
-					if errors.Is(err, utils.ErrNotFound) {
-						setConditionErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, err.Error())
+					if !errors.Is(err, utils.ErrNotFound) {
+						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
 					}
-					return ctrl.Result{}, err
 				}
 			}