Check restoration hash in NetBox before updating resource in NetBox

api/v1/iprangeclaim_types.go

@@ -181,7 +181,7 @@ var ConditionIpRangeAssignedFalse = metav1.Condition{
 	Message: "Failed to fetch new IP Range from NetBox",
 }
 
-var ConditionIpRangeAssignedFalseSizeMissmatch = metav1.Condition{
+var ConditionIpRangeAssignedFalseSizeMismatch = metav1.Condition{
 	Type:    "IPRangeAssigned",
 	Status:  "False",
 	Reason:  "IPRangeCRNotCreated",

config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml

@@ -14,7 +14,6 @@ spec:
   prefixLength: "/31"
   parentPrefixSelector:
     tenant: "MY_TENANT"
-    site: "DM-Buffalo"
     family: "IPv4"
     environment: "Production"
-    poolName: "Pool 1"
+    poolName: "Pool 1"

internal/controller/expected_netboxmock_calls_test.go

@@ -41,7 +41,7 @@ func mockIpAddressListWithIpAddressFilter(ipamMock *mock_interfaces.MockIpamInte
 				return &ipam.IpamIPAddressesListOK{Payload: nil}, err
 			}
 			fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList was called with expected input\n")
-			return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressList()}, nil
+			return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHash)}, nil
 		}).MinTimes(1)
 }
 
@@ -92,6 +92,22 @@ func mockIpAddressListWithHashFilter(ipamMock *mock_interfaces.MockIpamInterface
 		}).MinTimes(1)
 }
 
+func mockIpAddressListWithHashFilterMismatch(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
+	ipamMock.EXPECT().IpamIPAddressesList(gomock.Any(), gomock.Any(), gomock.Any()).
+		DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesListOK, error) {
+			got := params.(*ipam.IpamIPAddressesListParams)
+			diff := deep.Equal(got, ExpectedIpAddressListParamsWithIpAddressData)
+			// skip check for the 3rd input parameter as it is a method, method is a non comparable type
+			if len(diff) > 0 {
+				err := fmt.Errorf("netboxmock: unexpected call to ipam.IpamIPAddressesList, diff to expected params diff: %+v", diff)
+				catchUnexpectedParams <- err
+				return &ipam.IpamIPAddressesListOK{Payload: nil}, err
+			}
+			fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList (empty reslut) was called with expected input,\n")
+			return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHashMismatch)}, nil
+		}).MinTimes(1)
+}
+
 func mockPrefixesListWithPrefixFilter(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
 	ipamMock.EXPECT().IpamPrefixesList(gomock.Any(), gomock.Any()).
 		DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamPrefixesListOK, error) {

internal/controller/ipaddress_controller.go

@@ -170,10 +170,30 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	netboxIpAddressModel, err := r.NetboxClient.ReserveOrUpdateIpAddress(ipAddressModel)
 	if err != nil {
-		updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
-			corev1.EventTypeWarning, o.Spec.IpAddress)
-		return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
-			"after reservation of ip in netbox failed: %w", updateStatusErr, err)
+		if errors.Is(err, api.ErrRestorationHashMismatch) && o.Status.IpAddressId == 0 {
+			// if there is a restoration hash mismatch and the IpAddressId status field is not set,
+			// delete the ip address so it can be recreated by the ip address 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 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 {
+					return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
+						"after deletion of ip address cr failed: %w", updateStatusErr, err)
+				}
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, nil
+
+		}
+
+		if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
+			corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
+			return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
+				"after reservation of ip in netbox failed: %w", updateStatusErr, err)
+		}
+		return ctrl.Result{Requeue: true}, nil
 	}
 
 	// 3. unlock lease of parent prefix

internal/controller/ipaddress_controller_test.go

@@ -28,6 +28,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	netboxv1 "github.com/netbox-community/netbox-operator/api/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
 var _ = Describe("IpAddress Controller", Ordered, func() {
@@ -51,6 +52,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 		cr *netboxv1.IpAddress, // our CR as typed object
 		IpamMocksIpAddress []func(*mock_interfaces.MockIpamInterface, chan error),
 		TenancyMocks []func(*mock_interfaces.MockTenancyInterface, chan error),
+		restorationHashMismatch bool, // To check for deletion if restoration hash does not match
 		expectedConditionReady bool, // Expected state of the ConditionReady condition
 		expectedCRStatus netboxv1.IpAddressStatus, // Expected status of the CR
 	) {
@@ -81,31 +83,40 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 		By("Creating IpAddress CR")
 		Eventually(k8sClient.Create(ctx, cr), timeout, interval).Should(Succeed())
 
-		// check that reconcile loop did run a least once by checking that conditions are set
 		createdCR := &netboxv1.IpAddress{}
-		Eventually(func() bool {
-			err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
-			return err == nil && len(createdCR.Status.Conditions) > 0
-		}, timeout, interval).Should(BeTrue())
-
-		// Now check if conditions are set as expected
-		Eventually(func() bool {
-			err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
-			return err == nil &&
-				apismeta.IsStatusConditionTrue(createdCR.Status.Conditions, netboxv1.ConditionIpaddressReadyTrue.Type) == expectedConditionReady
-		}, timeout, interval).Should(BeTrue())
-
-		// Check that the expected ip address is present in the status
-		Expect(createdCR.Status.IpAddressId).To(Equal(expectedCRStatus.IpAddressId))
-
-		// Cleanup the netbox resources
-		Expect(k8sClient.Delete(ctx, createdCR)).Should(Succeed())
-
-		// Wait until the resource is deleted to make sure that it will not interfere with the next test case
-		Eventually(func() bool {
-			err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
-			return err != client.IgnoreNotFound(err)
-		}, timeout, interval).Should(BeTrue())
+
+		if restorationHashMismatch {
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
+				return apierrors.IsNotFound(err)
+			}, timeout, interval).Should(BeTrue())
+		} else {
+
+			// check that reconcile loop did run a least once by checking that conditions are set
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
+				return err == nil && len(createdCR.Status.Conditions) > 0
+			}, timeout, interval).Should(BeTrue())
+
+			// Now check if conditions are set as expected
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
+				return err == nil &&
+					apismeta.IsStatusConditionTrue(createdCR.Status.Conditions, netboxv1.ConditionIpaddressReadyTrue.Type) == expectedConditionReady
+			}, timeout, interval).Should(BeTrue())
+
+			// Check that the expected ip address is present in the status
+			Expect(createdCR.Status.IpAddressId).To(Equal(expectedCRStatus.IpAddressId))
+
+			// Cleanup the netbox resources
+			Expect(k8sClient.Delete(ctx, createdCR)).Should(Succeed())
+
+			// Wait until the resource is deleted to make sure that it will not interfere with the next test case
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
+				return err != client.IgnoreNotFound(err)
+			}, timeout, interval).Should(BeTrue())
+		}
 
 		catchCtxCancel()
 	},
@@ -119,7 +130,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 			[]func(*mock_interfaces.MockTenancyInterface, chan error){
 				mockTenancyTenancyTenantsList,
 			},
-			true, ExpectedIpAddressStatus),
+			false, true, ExpectedIpAddressStatus),
 		Entry("Create IpAddress CR, ip address already reserved in NetBox, preserved in netbox, ",
 			defaultIpAddressCR(true),
 			[]func(*mock_interfaces.MockIpamInterface, chan error){
@@ -129,7 +140,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 			[]func(*mock_interfaces.MockTenancyInterface, chan error){
 				mockTenancyTenancyTenantsList,
 			},
-			true, ExpectedIpAddressStatus),
+			false, true, ExpectedIpAddressStatus),
 		Entry("Create IpAddress CR, ip address already reserved in NetBox",
 			defaultIpAddressCR(false),
 			[]func(*mock_interfaces.MockIpamInterface, chan error){
@@ -140,7 +151,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 			[]func(*mock_interfaces.MockTenancyInterface, chan error){
 				mockTenancyTenancyTenantsList,
 			},
-			true, ExpectedIpAddressStatus),
+			false, true, ExpectedIpAddressStatus),
 		Entry("Create IpAddress CR, reserve or update failure",
 			defaultIpAddressCR(false),
 			[]func(*mock_interfaces.MockIpamInterface, chan error){
@@ -151,6 +162,15 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
 			[]func(*mock_interfaces.MockTenancyInterface, chan error){
 				mockTenancyTenancyTenantsList,
 			},
-			false, ExpectedIpAddressFailedStatus),
+			false, false, ExpectedIpAddressFailedStatus),
+		Entry("Create IpAddress CR, restoration hash mismatch",
+			defaultIpAddressCreatedByClaim(true),
+			[]func(*mock_interfaces.MockIpamInterface, chan error){
+				mockIpAddressListWithHashFilterMismatch,
+			},
+			[]func(*mock_interfaces.MockTenancyInterface, chan error){
+				mockTenancyTenancyTenantsList,
+			},
+			true, false, nil),
 	)
 })

internal/controller/iprange_controller.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"maps"
 	"strconv"
@@ -142,13 +143,29 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 
 	netboxIpRangeModel, err := r.NetboxClient.ReserveOrUpdateIpRange(ipRangeModel)
 	if err != nil {
-		if loggingErr := r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
-			corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); loggingErr != nil {
-			return ctrl.Result{}, fmt.Errorf("logging error: %w. Original error from ReserveOrUpdateIpRange: %w", loggingErr, err)
+		if errors.Is(err, api.ErrRestorationHashMismatch) && o.Status.IpRangeId == 0 {
+			// if there is a restoration hash mismatch and the IpRangeId status field is not set,
+			// delete the ip range so it can be recreated by the ip range 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 ip range custom resource", "ip-range-start", o.Spec.StartAddress, "ip-range-end", o.Spec.EndAddress)
+			err = r.Client.Delete(ctx, o)
+			if err != nil {
+				if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
+					corev1.EventTypeWarning, "", err); err != nil {
+					return ctrl.Result{}, err
+				}
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, nil
+		}
+
+		if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
+			corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); err != nil {
+			return ctrl.Result{}, err
 		}
 
 		// The decision to not return the error message (just logging it) is to not trigger printing the stacktrace on api errors
-		return ctrl.Result{}, nil
+		return ctrl.Result{Requeue: true}, nil
 	}
 
 	// 3. unlock lease of parent prefix

internal/controller/iprangeclaim_controller.go

@@ -322,7 +322,7 @@ func (r *IpRangeClaimReconciler) restoreOrAssignIpRangeAndSetCondition(ctx conte
 		availableIpRanges, err := r.NetboxClient.GetAvailableIpAddressesByIpRange(ipRangeModel.Id)
 		if len(availableIpRanges.Payload) != o.Spec.Size {
 			ll.Unlock()
-			err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalseSizeMissmatch, corev1.EventTypeWarning, "", err)
+			err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalseSizeMismatch, corev1.EventTypeWarning, "", err)
 			if err != nil {
 				return nil, ctrl.Result{}, err
 			}

internal/controller/netbox_testdata_test.go

@@ -53,8 +53,11 @@ var tenantSlug = "test-tenant-slug"
 
 var restorationHash = "6f6c67651f0b43b2969ba2ae35c74fc91815513b"
 
-var customFields = map[string]string{"example_field": "example value"}
-var customFieldsWithHash = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash}
+var customFieldsCR = map[string]string{"example_field": "example value"}
+var customFieldsWithHashCR = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash}
+
+var customFieldsWithHash = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash}
+var customFieldsWithHashMismatch = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": "different hash"}
 
 var netboxLabel = "Status"
 var value = "active"
@@ -72,7 +75,7 @@ func defaultIpAddressCR(preserveInNetbox bool) *netboxv1.IpAddress {
 		Spec: netboxv1.IpAddressSpec{
 			IpAddress:        ipAddress,
 			Tenant:           tenant,
-			CustomFields:     customFields,
+			CustomFields:     customFieldsCR,
 			Comments:         comments,
 			Description:      description,
 			PreserveInNetbox: preserveInNetbox,
@@ -89,7 +92,7 @@ func defaultIpAddressCreatedByClaim(preserveInNetbox bool) *netboxv1.IpAddress {
 		Spec: netboxv1.IpAddressSpec{
 			IpAddress:        ipAddress,
 			Tenant:           tenant,
-			CustomFields:     customFieldsWithHash,
+			CustomFields:     customFieldsWithHashCR,
 			Comments:         comments,
 			Description:      description,
 			PreserveInNetbox: preserveInNetbox,
@@ -106,7 +109,7 @@ func defaultIpAddressClaimCR() *netboxv1.IpAddressClaim {
 		Spec: netboxv1.IpAddressClaimSpec{
 			ParentPrefix:     parentPrefix,
 			Tenant:           tenant,
-			CustomFields:     customFields,
+			CustomFields:     customFieldsCR,
 			Comments:         comments,
 			Description:      description,
 			PreserveInNetbox: false,
@@ -176,6 +179,22 @@ func mockedResponsePrefixList() *ipam.IpamPrefixesListOKBody {
 	}
 }
 
+func mockedResponseIPAddressListWithHash(customFields map[string]interface{}) *ipam.IpamIPAddressesListOKBody {
+	return &ipam.IpamIPAddressesListOKBody{
+		Results: []*netboxModels.IPAddress{
+			{
+				ID:           mockedResponseIPAddress().ID,
+				Address:      mockedResponseIPAddress().Address,
+				Comments:     mockedResponseIPAddress().Comments,
+				CustomFields: customFields,
+				Description:  mockedResponseIPAddress().Description,
+				Display:      mockedResponseIPAddress().Display,
+				Tenant:       mockedResponseIPAddress().Tenant,
+			},
+		},
+	}
+}
+
 func mockedResponseIPAddressList() *ipam.IpamIPAddressesListOKBody {
 	return &ipam.IpamIPAddressesListOKBody{
 		Results: []*netboxModels.IPAddress{

internal/controller/prefix_controller.go

@@ -184,8 +184,29 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 
 	netboxPrefixModel, err := r.NetboxClient.ReserveOrUpdatePrefix(prefixModel)
 	if err != nil {
-		updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, prefix.Spec.Prefix)
-		return ctrl.Result{}, fmt.Errorf("failed at update prefix status: %w, "+"after reservation of prefix in netbox failed: %w", updateStatusErr, err)
+		if errors.Is(err, api.ErrRestorationHashMismatch) && prefix.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)
+			if err != nil {
+				if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse,
+					corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
+					return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
+						"after deletion of prefix cr failed: %w", updateStatusErr, err)
+				}
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, nil
+		}
+
+		if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse,
+			corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
+			return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
+				"after reservation of prefix netbox failed: %w", updateStatusErr, err)
+		}
+		return ctrl.Result{Requeue: true}, nil
 	}
 
 	/* 3. unlock lease of parent prefix */

pkg/netbox/api/errors.go

@@ -23,4 +23,5 @@ var (
 	ErrParentPrefixNotFound            = errors.New("parent prefix not found")
 	ErrWrongMatchingPrefixSubnetFormat = errors.New("wrong matchingPrefix subnet format")
 	ErrInvalidIpFamily                 = errors.New("invalid IP Family")
+	ErrRestorationHashMismatch         = errors.New("restoration hash mismatch")
 )