fix(vmip): fix bug with create VirtualMachineIPAddress in different n…

…amespace, when VirtualMahineIPAddressLease 'Released' - add reque after 30s - fix message in recording event Signed-off-by: Dmitry Lopatin <[email protected]>
deckhouse · Aug 17, 2024 · 2773161 · 2773161
1 parent 8b5551d
commit 2773161
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 10 deletions.
diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go
@@ -63,9 +63,10 @@ func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reco
 	if err != nil {
 		return reconcile.Result{}, err
 	}
+	condition, _ := service.GetCondition(vmipcondition.BoundType, vmipStatus.Conditions)
 
 	switch {
-	case lease == nil && vmipStatus.Address != "":
+	case lease == nil && vmipStatus.Address != "" && condition.Reason != vmipcondition.VirtualMachineIPAddressLeaseAlready:
 		log.Info("Lease by name not found: waiting for the lease to be available")
 		return reconcile.Result{}, nil
 
@@ -89,9 +90,9 @@ func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reco
 		log.Info("Lease is released: set binding")
 
 		if lease.Spec.VirtualMachineIPAddressRef.Namespace != vmip.Namespace {
-			msg := fmt.Sprintf("the selected VirtualMachineIP lease belongs to a different namespace: %s", lease.Spec.VirtualMachineIPAddressRef.Namespace)
-			log.Error(msg)
-			h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseNotFound, msg)
+			log.Warn(fmt.Sprintf("The VirtualMachineIPLease belongs to a different namespace: %s", lease.Spec.VirtualMachineIPAddressRef.Namespace))
+			h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseAlready, "The VirtualMachineIPLease belongs to a different namespace")
+
 			return reconcile.Result{}, nil
 		}
 
@@ -150,7 +151,7 @@ func (h IPLeaseHandler) createNewLease(ctx context.Context, state state.VMIPStat
 			vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending
 			mgr.Update(conditionBound.Status(metav1.ConditionFalse).
 				Reason(vmipcondition.VirtualMachineIPAddressLeaseAlready).
-				Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIP",
+				Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress",
 					common.IpToLeaseName(vmipStatus.Address))).
 				Condition())
 			h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseAlready, msg)

diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go
@@ -19,6 +19,7 @@ package internal
 import (
 	"context"
 	"fmt"
+	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -76,6 +77,7 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r
 		return reconcile.Result{}, err
 	}
 
+	needReque := false
 	switch {
 	case lease == nil && vmipStatus.Address != "":
 		if vmipStatus.Phase != virtv2.VirtualMachineIPAddressPhasePending {
@@ -108,29 +110,44 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r
 	case lease.Status.Phase == virtv2.VirtualMachineIPAddressLeasePhaseBound:
 		if vmipStatus.Phase != virtv2.VirtualMachineIPAddressPhasePending {
 			vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending
+			log.Warn(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress resource: %s/%s",
+				lease.Name, lease.Spec.VirtualMachineIPAddressRef.Name, lease.Spec.VirtualMachineIPAddressRef.Namespace))
 			mgr.Update(conditionBound.Status(metav1.ConditionFalse).
 				Reason(vmipcondition.VirtualMachineIPAddressLeaseAlready).
-				Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIP",
-					common.IpToLeaseName(vmipStatus.Address))).
+				Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress resource",
+					lease.Name)).
+				Condition())
+		}
+
+	case lease.Spec.VirtualMachineIPAddressRef.Namespace != vmip.Namespace:
+		if vmipStatus.Phase != virtv2.VirtualMachineIPAddressPhasePending {
+			vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending
+			mgr.Update(conditionBound.Status(metav1.ConditionFalse).
+				Reason(vmipcondition.VirtualMachineIPAddressLeaseAlready).
+				Message(fmt.Sprintf("The VirtualMachineIPLease %s belongs to a different namespace", lease.Name)).
 				Condition())
 		}
+		needReque = true
 
 	default:
 		if vmipStatus.Phase != virtv2.VirtualMachineIPAddressPhasePending {
 			vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending
 			mgr.Update(conditionBound.Status(metav1.ConditionFalse).
 				Reason(vmipcondition.VirtualMachineIPAddressLeaseNotReady).
 				Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is not ready",
-					common.IpToLeaseName(vmipStatus.Address))).
+					lease.Name)).
 				Condition())
 		}
 	}
 
 	log.Info("Set VirtualMachineIP phase", "phase", vmipStatus.Phase)
 	vmipStatus.Conditions = mgr.Generate()
 	vmipStatus.ObservedGeneration = vmip.GetGeneration()
-
-	return reconcile.Result{}, nil
+	if !needReque {
+		return reconcile.Result{}, nil
+	} else {
+		return reconcile.Result{RequeueAfter: 30 * time.Second}, nil
+	}
 }
 
 func (h *LifecycleHandler) Name() string {

diff --git a/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go b/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go
@@ -57,6 +57,8 @@ func (h *RetentionHandler) Handle(ctx context.Context, state state.VMIPLeaseStat
 		if lease.Spec.VirtualMachineIPAddressRef.Name != "" {
 			log.Info("VirtualMachineIP not found: remove this ref from the spec and retain VMIPLease")
 			lease.Spec.VirtualMachineIPAddressRef.Name = ""
+
+			// TODO add requeue with with exponential BackOff time interval using condition Bound -> probeTime
 			return reconcile.Result{RequeueAfter: h.retentionDuration}, nil
 		}