fixup! fixup! fixup! refactor: updated IP controller for new ipam

liqotech · Nov 18, 2024 · e980fde · e980fde
1 parent 81689fa
commit e980fde
Showing 1 changed file with 80 additions and 52 deletions.
diff --git a/pkg/liqo-controller-manager/networking/ip-controller/ip_controller.go b/pkg/liqo-controller-manager/networking/ip-controller/ip_controller.go
@@ -83,76 +83,98 @@ func (r *IPReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Re
 		return ctrl.Result{}, err
 	}
 
+	// Get the CIDR of the Network referenced by the IP, or default to the
+	// external CIDR of the local cluster.
 	networkRef, cidr, err := r.handleNetworkRef(ctx, &ip)
-	if err != nil {
-		klog.Errorf("error while handling NetworkRef for IP %q: %v", req.NamespacedName, err)
+	if client.IgnoreNotFound(err) != nil {
+		klog.Errorf("error while handling network reference for IP %q: %v", req.NamespacedName, err)
 		return ctrl.Result{}, err
 	}
 
-	if ip.GetDeletionTimestamp().IsZero() {
-		if !controllerutil.ContainsFinalizer(&ip, ipamIPFinalizer) {
-			// Add finalizer to prevent deletion without releasing the IP.
-			controllerutil.AddFinalizer(&ip, ipamIPFinalizer)
+	networkExists := !apierrors.IsNotFound(err)
+	deleting := !ip.GetDeletionTimestamp().IsZero()
+
+	// The resource is being deleted or the referenced network does not exist:
+	// - call the IPAM to release the IP if it set, and empty the status.
+	// - remove eventual finalizers from the resource.
+	if deleting || !networkExists {
+		if err := r.handleIPStatusDeletion(ctx, &ip); err != nil {
+			klog.Errorf("error while handling IP status deletion %q: %v", req.NamespacedName, err)
+			return ctrl.Result{}, err
+		}
+
+		// Remove finalizer from the IP resource.
+		if controllerutil.ContainsFinalizer(&ip, ipamIPFinalizer) {
+			controllerutil.RemoveFinalizer(&ip, ipamIPFinalizer)
 
 			// Update the IP object
 			if err := r.Update(ctx, &ip); err != nil {
-				klog.Errorf("error while adding finalizer to IP %q: %v", req.NamespacedName, err)
+				klog.Errorf("error while removing finalizer from IP %q: %v", req.NamespacedName, err)
 				return ctrl.Result{}, err
 			}
-			klog.Infof("finalizer %q correctly added to IP %q", ipamIPFinalizer, req.NamespacedName)
 
-			// We return immediately and wait for the next reconcile to eventually update the status.
-			return ctrl.Result{}, nil
+			klog.Infof("finalizer %q correctly removed from IP %q", ipamIPFinalizer, req.NamespacedName)
 		}
 
-		// Add network reference to the IP in the labels. This is used to trigger the reconciliation
-		// of the IP by watching deletion events of the father Network.
-		if ip.Labels == nil {
-			ip.Labels = make(map[string]string)
-		}
-		ip.Labels[consts.NetworkNamespaceLabelKey] = networkRef.Namespace
-		ip.Labels[consts.NetworkNameLabelKey] = networkRef.Name
-		if err := r.Update(ctx, &ip); err != nil {
-			klog.Errorf("error while updating IP %q: %v", req.NamespacedName, err)
+		// Raise an error if the IP is referencing a non-existing network and it is not being deleted.
+		if !deleting {
+			err := fmt.Errorf("network referenced by IP %q does not exist", req.NamespacedName)
+			klog.Error(err)
 			return ctrl.Result{}, err
 		}
 
-		if ip.Status.IP == "" {
-			if err := r.forgeIPStatus(ctx, &ip, cidr); err != nil {
-				klog.Errorf("error while forging IP status for IP %q: %v", req.NamespacedName, err)
-				return ctrl.Result{}, err
-			}
+		// We do not have to delete possible service and endpointslice associated, as already deleted by
+		// the Kubernetes garbage collector (since they are owned by the IP resource).
 
-			if err := r.Client.Status().Update(ctx, &ip); err != nil {
-				klog.Errorf("error while updating IP %q: %v", req.NamespacedName, err)
-				return ctrl.Result{}, err
-			}
-			klog.Infof("updated IP %q", req.NamespacedName)
-		}
+		return ctrl.Result{}, nil
+	}
 
-		// Create service and associated endpointslice if the template is defined
-		if err := r.handleAssociatedService(ctx, &ip); err != nil {
-			klog.Errorf("error while handling associated service for IP %q: %v", req.NamespacedName, err)
+	if !controllerutil.ContainsFinalizer(&ip, ipamIPFinalizer) {
+		// Add finalizer to prevent deletion without releasing the IP.
+		controllerutil.AddFinalizer(&ip, ipamIPFinalizer)
+
+		// Update the IP object
+		if err := r.Update(ctx, &ip); err != nil {
+			klog.Errorf("error while adding finalizer to IP %q: %v", req.NamespacedName, err)
 			return ctrl.Result{}, err
 		}
-	} else if controllerutil.ContainsFinalizer(&ip, ipamIPFinalizer) {
-		// the resource is being deleted, but the finalizer is present:
-		// - release the IP from the IPAM.
-		// - remove finalizer from the resource.
-		if err := r.handleDelete(ctx, &ip); err != nil {
-			klog.Errorf("error while handling deletion of IP %q: %v", req.NamespacedName, err)
+		klog.Infof("finalizer %q correctly added to IP %q", ipamIPFinalizer, req.NamespacedName)
+
+		// We return immediately and wait for the next reconcile to eventually update the status.
+		return ctrl.Result{}, nil
+	}
+
+	// Add network reference to the IP in the labels. This is used to trigger the reconciliation
+	// of the IP by watching deletion events of the father Network.
+	if ip.Labels == nil {
+		ip.Labels = make(map[string]string)
+	}
+	ip.Labels[consts.NetworkNamespaceLabelKey] = networkRef.Namespace
+	ip.Labels[consts.NetworkNameLabelKey] = networkRef.Name
+
+	if err := r.Update(ctx, &ip); err != nil {
+		klog.Errorf("error while updating IP %q: %v", req.NamespacedName, err)
+		return ctrl.Result{}, err
+	}
+
+	// Forge the IP status if it is not set yet.
+	if ip.Status.IP == "" {
+		if err := r.forgeIPStatus(ctx, &ip, cidr); err != nil {
+			klog.Errorf("error while forging IP status for IP %q: %v", req.NamespacedName, err)
 			return ctrl.Result{}, err
 		}
 
-		// Update the IP object
-		if err := r.Update(ctx, &ip); err != nil {
-			klog.Errorf("error while removing finalizer from IP %q: %v", req.NamespacedName, err)
+		if err := r.Client.Status().Update(ctx, &ip); err != nil {
+			klog.Errorf("error while updating IP %q: %v", req.NamespacedName, err)
 			return ctrl.Result{}, err
 		}
-		klog.Infof("finalizer %q correctly removed from IP %q", ipamIPFinalizer, req.NamespacedName)
+		klog.Infof("updated IP %q", req.NamespacedName)
+	}
 
-		// We do not have to delete possible service and endpointslice associated, as already deleted by
-		// the Kubernetes garbage collector (since they are owned by the IP resource).
+	// Create service and associated endpointslice if the template is defined
+	if err := r.handleAssociatedService(ctx, &ip); err != nil {
+		klog.Errorf("error while handling associated service for IP %q: %v", req.NamespacedName, err)
+		return ctrl.Result{}, err
 	}
 
 	return ctrl.Result{}, nil
@@ -244,18 +266,24 @@ func (r *IPReconciler) forgeIPStatus(ctx context.Context, ip *ipamv1alpha1.IP, c
 	return nil
 }
 
-// handleDelete handles the deletion of the IP resource. It call the IPAM to release the IP.
-func (r *IPReconciler) handleDelete(ctx context.Context, ip *ipamv1alpha1.IP) error {
+// handleIPStatusDeletion handles the deletion of the IP status. It call the IPAM to release the IP
+// and empties the status.
+func (r *IPReconciler) handleIPStatusDeletion(ctx context.Context, ip *ipamv1alpha1.IP) error {
 	if ip.Status.IP != "" && ip.Status.CIDR != "" {
 		if err := deleteIP(ctx, r.ipamClient, ip.Status.IP, ip.Status.CIDR); err != nil {
 			return err
 		}
-	}
 
-	// Remove status and finalizer, and update the object.
-	ip.Status.IP = ""
-	ip.Status.CIDR = ""
-	controllerutil.RemoveFinalizer(ip, ipamIPFinalizer)
+		// Remove status and finalizer, and update the object.
+		ip.Status = ipamv1alpha1.IPStatus{}
+
+		// Update the IP status
+		if err := r.Client.Status().Update(ctx, ip); err != nil {
+			return err
+		}
+
+		klog.Infof("IP %q status correctly cleaned", client.ObjectKeyFromObject(ip))
+	}
 
 	return nil
 }