Backport: fix(vmip): fix double create VirtualMachineIPLease (#987)

fix(vmip): fix double create VirtualMachineIPLease (#976)

Signed-off-by: Dmitry Lopatin <dmitry.lopatin@flant.com>
Co-authored-by: Dmitry Lopatin <93423466+LopatinDmitr@users.noreply.github.com>

Author: deckhouse-BOaTswain

images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go

@@ -19,6 +19,7 @@ package vmip
 import (
 	"context"
 	"fmt"
+	"time"
 
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -33,6 +34,7 @@ import (
 	"github.com/deckhouse/virtualization-controller/pkg/controller/indexer"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vmip/internal/state"
+	"github.com/deckhouse/virtualization-controller/pkg/logger"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
 
@@ -152,7 +154,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 	}
 
 	s := state.New(r.client, vmip.Changed())
-
 	rec := reconciler.NewBaseReconciler[Handler](r.handlers)
 	rec.SetHandlerExecutor(func(ctx context.Context, h Handler) (reconcile.Result, error) {
 		return h.Handle(ctx, s)
@@ -163,7 +164,21 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		return vmip.Update(ctx)
 	})
 
-	return rec.Reconcile(ctx)
+	// TODO: This code addresses the issue of creating vmipl within the controller.
+	// The object is saved in etcd but does not get updated in the cache in time.
+	// As a result, we encounter the creation of multiple vmipl during a single reconcile operation.
+	// Adding reconcile.Result{RequeueAfter: 2 * time.Second} helps to fix this issue in 90% of cases.
+	// In the future, this code should be architecturally redesigned to prevent such situations.
+	result, err := rec.Reconcile(ctx)
+	if err != nil {
+		logger.FromContext(ctx).Error("Failed to reconcile VMIP", logger.SlogErr(err))
+		return reconcile.Result{RequeueAfter: 2 * time.Second}, nil
+	}
+	if result.Requeue {
+		return reconcile.Result{RequeueAfter: 2 * time.Second}, nil
+	}
+
+	return result, nil
 }
 
 func (r *Reconciler) factory() *virtv2.VirtualMachineIPAddress {

images/virtualization-artifact/pkg/controller/vmiplease/internal/lifecycle_handler.go

@@ -22,6 +22,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/deckhouse/virtualization-controller/pkg/common/ip"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vmiplease/internal/state"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
@@ -55,7 +56,7 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPLeaseStat
 		return reconcile.Result{}, err
 	}
 
-	if vmip != nil {
+	if vmip != nil && vmip.Status.Address == ip.LeaseNameToIP(lease.Name) {
 		if leaseStatus.Phase != virtv2.VirtualMachineIPAddressLeasePhaseBound {
 			leaseStatus.Phase = virtv2.VirtualMachineIPAddressLeasePhaseBound
 			cb.Status(metav1.ConditionTrue).

images/virtualization-artifact/pkg/controller/vmiplease/internal/protection_handler.go

@@ -22,6 +22,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/deckhouse/virtualization-controller/pkg/common/ip"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vmiplease/internal/state"
 	"github.com/deckhouse/virtualization-controller/pkg/logger"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
@@ -44,7 +45,7 @@ func (h *ProtectionHandler) Handle(ctx context.Context, state state.VMIPLeaseSta
 		return reconcile.Result{}, err
 	}
 
-	if vmip != nil {
+	if vmip != nil && vmip.Status.Address == ip.LeaseNameToIP(lease.Name) {
 		controllerutil.AddFinalizer(lease, virtv2.FinalizerIPAddressLeaseCleanup)
 	} else if lease.GetDeletionTimestamp() == nil {
 		log.Info("Deletion observed: remove cleanup finalizer from VirtualMachineIPAddressLease")

images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go

@@ -23,6 +23,7 @@ import (
 
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	"github.com/deckhouse/virtualization-controller/pkg/common/ip"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vmiplease/internal/state"
 	"github.com/deckhouse/virtualization-controller/pkg/logger"
@@ -52,7 +53,7 @@ func (h *RetentionHandler) Handle(ctx context.Context, state state.VMIPLeaseStat
 		return reconcile.Result{}, err
 	}
 
-	if vmip == nil {
+	if vmip == nil || vmip.Status.Address != ip.LeaseNameToIP(lease.Name) {
 		if lease.Spec.VirtualMachineIPAddressRef.Name != "" {
 			log.Info("VirtualMachineIP not found: remove this ref from the spec and retain VMIPLease")
 			lease.Spec.VirtualMachineIPAddressRef.Name = ""

images/virtualization-artifact/pkg/controller/vmiplease/vmiplease_reconciler.go

@@ -19,6 +19,7 @@ package vmiplease
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"reflect"
 
 	"k8s.io/apimachinery/pkg/types"
@@ -31,7 +32,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
-	"github.com/deckhouse/virtualization-controller/pkg/common/ip"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/reconciler"
 	"github.com/deckhouse/virtualization-controller/pkg/controller/vmiplease/internal/state"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
@@ -70,7 +70,7 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr
 	return ctr.Watch(source.Kind(mgr.GetCache(), &virtv2.VirtualMachineIPAddressLease{}), &handler.EnqueueRequestForObject{})
 }
 
-func (r *Reconciler) enqueueRequestsFromVMIP(_ context.Context, obj client.Object) []reconcile.Request {
+func (r *Reconciler) enqueueRequestsFromVMIP(ctx context.Context, obj client.Object) []reconcile.Request {
 	vmip, ok := obj.(*virtv2.VirtualMachineIPAddress)
 	if !ok {
 		return nil
@@ -80,13 +80,30 @@ func (r *Reconciler) enqueueRequestsFromVMIP(_ context.Context, obj client.Objec
 		return nil
 	}
 
-	return []reconcile.Request{
-		{
-			NamespacedName: types.NamespacedName{
-				Name: ip.IpToLeaseName(vmip.Status.Address),
-			},
-		},
+	var requests []reconcile.Request
+
+	var vmiplList virtv2.VirtualMachineIPAddressLeaseList
+	err := r.client.List(ctx, &vmiplList, &client.ListOptions{})
+	if err != nil {
+		slog.Default().Error(fmt.Sprintf("failed to list vmipl: %s", err))
+		return requests
+	}
+
+	for _, vmipl := range vmiplList.Items {
+		if vmipl.Spec.VirtualMachineIPAddressRef == nil {
+			continue
+		}
+
+		if vmipl.Spec.VirtualMachineIPAddressRef.Name == obj.GetName() {
+			requests = append(requests, reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name: vmipl.Name,
+				},
+			})
+		}
 	}
+
+	return requests
 }
 
 func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
@@ -109,10 +126,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		return h.Handle(ctx, s)
 	})
 	rec.SetResourceUpdater(func(ctx context.Context) error {
-		if s.ShouldDeletion() {
-			return r.client.Delete(ctx, lease.Changed())
-		}
-
 		if !reflect.DeepEqual(lease.Current().Spec, lease.Changed().Spec) {
 			leaseStatus := lease.Changed().Status.DeepCopy()
 			err = r.client.Update(ctx, lease.Changed())
@@ -122,7 +135,16 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 			lease.Changed().Status = *leaseStatus
 		}
 
-		return lease.Update(ctx)
+		err = lease.Update(ctx)
+		if err != nil {
+			return err
+		}
+
+		if s.ShouldDeletion() {
+			return r.client.Delete(ctx, lease.Changed())
+		}
+
+		return nil
 	})
 
 	return rec.Reconcile(ctx)