diff --git a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go index 880d028441..804c72f494 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go +++ b/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 { diff --git a/images/virtualization-artifact/pkg/controller/vmiplease/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmiplease/internal/lifecycle_handler.go index 18ef101d13..18568021aa 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/lifecycle_handler.go +++ b/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). diff --git a/images/virtualization-artifact/pkg/controller/vmiplease/internal/protection_handler.go b/images/virtualization-artifact/pkg/controller/vmiplease/internal/protection_handler.go index deaa6fef70..5dc4c96997 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/protection_handler.go +++ b/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") diff --git a/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go b/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go index cbdf591573..8450d4a417 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go +++ b/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 = "" diff --git a/images/virtualization-artifact/pkg/controller/vmiplease/vmiplease_reconciler.go b/images/virtualization-artifact/pkg/controller/vmiplease/vmiplease_reconciler.go index c72be9b318..4ad384a094 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/vmiplease_reconciler.go +++ b/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)