Reconcile delete logic #111
Conversation
anton-paulovich
commented
Jan 29, 2026
anton-paulovich
commented
- If CR does not have finalizer - we add it and call another reconciliation
- If CR is going to be deleted we remove IP from NetBox and remove finalizer
| nbIP, err := r.netBox.IPAM().GetIPAddressByAddress(ipStr) | ||
| if err != nil { | ||
| logger.Error(err, "unable to find IP netbox ip address, NetBox IP will not be removed") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Maybe we should check for ErrNotFound here and if so we handle it as a valid case that IP was already removed from NetBox by someone
And if there is another kind of err (400/500/502) we should return err, so Reconcile() will be called again
But I did not found any existing implementation of IsErrNotFound() in NetBox client.
Should I add this check by strings parsing or smth for better error handling here?
Or we do not allow the case when IP was already deleted by someone, so we should return error here anyways?
There was a problem hiding this comment.
yes, we should not handle it as error case if the IPAddress already removed from netbox and also not assigned to the LAG interface.
There was a problem hiding this comment.
not sure about how, I would ask Peter Gehrt for such details.
| nbIP, err := r.netBox.IPAM().GetIPAddressByAddress(ipStr) | ||
| if err != nil { | ||
| logger.Error(err, "unable to find IP netbox ip address, NetBox IP will not be removed") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
yes, we should not handle it as error case if the IPAddress already removed from netbox and also not assigned to the LAG interface.
| nbIP, err := r.netBox.IPAM().GetIPAddressByAddress(ipStr) | ||
| if err != nil { | ||
| logger.Error(err, "unable to find IP netbox ip address, NetBox IP will not be removed") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
not sure about how, I would ask Peter Gehrt for such details.
| return ctrl.Result{}, err | ||
| } | ||
|
|
||
| controllerutil.RemoveFinalizer(ipAddress, ipAddressFinalizer) |
There was a problem hiding this comment.
perhaps only update if we actually removed a finalizer?
| logger := log.FromContext(ctx) | ||
| logger.Info("deleting IP Address") | ||
|
|
||
| if !controllerutil.ContainsFinalizer(ipAddr, ipAddressFinalizer) { |
There was a problem hiding this comment.
I would just try to clean independent if we have the finalizer or not
| } | ||
|
|
||
| controllerutil.RemoveFinalizer(ipAddress, ipAddressFinalizer) | ||
| if err = r.k8sClient.Update(ctx, ipAddress); err != nil { |
There was a problem hiding this comment.
I would either use .Patch() or include
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
on updates, to avoid conflicts.
2 participants
