diff --git a/api/v1/ipaddress_types.go b/api/v1/ipaddress_types.go index 06569808..c80106f8 100644 --- 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 index 96e31eac..d9d78b66 100644 --- 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 index 7d45c4c8..e8c049ac 100644 --- 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 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) { diff --git a/internal/controller/ipaddress_controller.go b/internal/controller/ipaddress_controller.go index ad51a8cc..c54d74ef 100644 --- a/internal/controller/ipaddress_controller.go +++ b/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, "") diff --git a/internal/controller/ipaddress_controller_test.go b/internal/controller/ipaddress_controller_test.go index a5dedfa0..b5f8f56c 100644 --- 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, + mockIpAddressesDeleteFail, }, []func(*mock_interfaces.MockTenancyInterface, chan error){ mockTenancyTenancyTenantsList, diff --git a/internal/controller/netbox_testdata_test.go b/internal/controller/netbox_testdata_test.go index b79570d3..670c5012 100644 --- 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 ExpectedDeleteFailParams = 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 index 7837d859..0f9d0ad1 100644 --- a/internal/controller/prefix_controller.go +++ b/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 { diff --git a/pkg/netbox/api/ip_address.go b/pkg/netbox/api/ip_address.go index 225d14d8..c9fbdbee 100644 --- a/pkg/netbox/api/ip_address.go +++ b/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 } diff --git a/pkg/netbox/api/prefix.go b/pkg/netbox/api/prefix.go index 5d3def2e..46d58baa 100644 --- a/pkg/netbox/api/prefix.go +++ b/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 }