From 358c40eeac5145169f43e73d496e6d8be0c1bb64 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 5 May 2025 22:21:07 +0300 Subject: [PATCH 01/17] update protection_handler.go Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/protection_handler.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go index 26b53f9a0f..6966b6220d 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go @@ -45,11 +45,6 @@ func (h *ProtectionHandler) Handle(ctx context.Context, state state.VMIPState) ( vmip := state.VirtualMachineIP() - vm, err := state.VirtualMachine(ctx) - if err != nil { - return reconcile.Result{}, err - } - configuredVms, err := h.getConfiguredVM(ctx, vmip) if err != nil { return reconcile.Result{}, err @@ -66,6 +61,11 @@ func (h *ProtectionHandler) Handle(ctx context.Context, state state.VMIPState) ( log.Debug("VirtualMachineIPAddress deletion is delayed: it's protected by virtual machines") } + vm, err := state.VirtualMachine(ctx) + if err != nil { + return reconcile.Result{}, err + } + if vm == nil || vm.DeletionTimestamp != nil { log.Info("VirtualMachineIP is no longer attached to any VM: remove cleanup finalizer", "VirtualMachineIPName", vmip.Name) controllerutil.RemoveFinalizer(vmip, virtv2.FinalizerIPAddressCleanup) From 85fc2f129bfaca62f4b678c42c4b022640b1b493 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Sat, 10 May 2025 16:18:49 +0300 Subject: [PATCH 02/17] minor fix Signed-off-by: Dmitry Lopatin --- .../vmip/internal/lifecycle_handler.go | 33 +++++++++---------- .../vmip/internal/protection_handler.go | 2 +- .../vmiplease/internal/lifecycle_handler.go | 3 +- .../vmiplease/internal/protection_handler.go | 3 +- .../vmiplease/internal/retention_handler.go | 3 +- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go index 50ff935621..657d10ae42 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go @@ -51,7 +51,6 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r log := logger.FromContext(ctx).With(logger.SlogHandler(LifecycleHandlerName)) vmip := state.VirtualMachineIP() - vmipStatus := &vmip.Status vm, err := state.VirtualMachine(ctx) if err != nil { @@ -69,12 +68,12 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r Status(metav1.ConditionUnknown) defer func() { - conditions.SetCondition(conditionBound, &vmipStatus.Conditions) - conditions.SetCondition(conditionAttach, &vmipStatus.Conditions) + conditions.SetCondition(conditionBound, &vmip.Status.Conditions) + conditions.SetCondition(conditionAttach, &vmip.Status.Conditions) }() if vm == nil || vm.DeletionTimestamp != nil { - vmipStatus.VirtualMachine = "" + vmip.Status.VirtualMachine = "" conditionAttach.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineNotFound). Message("Virtual machine not found") @@ -88,35 +87,35 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r needRequeue := false switch { - case lease == nil && vmipStatus.Address != "": - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending + case lease == nil && vmip.Status.Address != "": + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending conditionBound.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineIPAddressLeaseLost). Message(fmt.Sprintf("VirtualMachineIPAddressLease %s doesn't exist", - ip.IpToLeaseName(vmipStatus.Address))) + ip.IpToLeaseName(vmip.Status.Address))) case lease == nil: - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending conditionBound.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineIPAddressLeaseNotFound). Message("VirtualMachineIPAddressLease is not found") case util.IsBoundLease(lease, vmip): - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhaseBound - vmipStatus.Address = ip.LeaseNameToIP(lease.Name) + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhaseBound + vmip.Status.Address = ip.LeaseNameToIP(lease.Name) conditionBound.Status(metav1.ConditionTrue). Reason(vmipcondition.Bound) if vm != nil && vm.GetDeletionTimestamp().IsZero() { - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhaseAttached - vmipStatus.VirtualMachine = vm.Name + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhaseAttached + vmip.Status.VirtualMachine = vm.Name conditionAttach.Status(metav1.ConditionTrue). Reason(vmipcondition.Attached) h.recorder.Eventf(vmip, corev1.EventTypeNormal, virtv2.ReasonAttached, "VirtualMachineIPAddress is attached to %q/%q.", vm.Namespace, vm.Name) } case lease.Status.Phase == virtv2.VirtualMachineIPAddressLeasePhaseBound: - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending + vmip.Status.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)) conditionBound.Status(metav1.ConditionFalse). @@ -125,23 +124,23 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r lease.Name)) case lease.Spec.VirtualMachineIPAddressRef.Namespace != vmip.Namespace: - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending conditionBound.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists). Message(fmt.Sprintf("The VirtualMachineIPLease %s belongs to a different namespace", lease.Name)) needRequeue = true default: - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending conditionBound.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineIPAddressLeaseNotReady). Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is not ready", lease.Name)) } - log.Debug("Set VirtualMachineIP phase", "phase", vmipStatus.Phase) + log.Debug("Set VirtualMachineIP phase", "phase", vmip.Status.Phase) - vmipStatus.ObservedGeneration = vmip.GetGeneration() + vmip.Status.ObservedGeneration = vmip.GetGeneration() if !needRequeue { return reconcile.Result{}, nil } else { diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go index 6966b6220d..e9834bdf75 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go @@ -70,8 +70,8 @@ func (h *ProtectionHandler) Handle(ctx context.Context, state state.VMIPState) ( log.Info("VirtualMachineIP is no longer attached to any VM: remove cleanup finalizer", "VirtualMachineIPName", vmip.Name) controllerutil.RemoveFinalizer(vmip, virtv2.FinalizerIPAddressCleanup) } else if vmip.GetDeletionTimestamp() == nil { - controllerutil.AddFinalizer(vmip, virtv2.FinalizerIPAddressCleanup) log.Info("VirtualMachineIP is still attached, finalizer added", "VirtualMachineIPName", vmip.Name) + controllerutil.AddFinalizer(vmip, virtv2.FinalizerIPAddressCleanup) } return reconcile.Result{}, nil 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 61c579f63e..09df427174 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/lifecycle_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmiplease/internal/lifecycle_handler.go @@ -22,7 +22,6 @@ 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 +54,7 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPLeaseStat return reconcile.Result{}, err } - if vmip != nil && vmip.Status.Address == ip.LeaseNameToIP(lease.Name) { + if vmip != nil { leaseStatus.Phase = virtv2.VirtualMachineIPAddressLeasePhaseBound cb.Status(metav1.ConditionTrue). Reason(vmiplcondition.Bound) 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 7323a3e63e..6ff435d94e 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/protection_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmiplease/internal/protection_handler.go @@ -22,7 +22,6 @@ 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" @@ -45,7 +44,7 @@ func (h *ProtectionHandler) Handle(ctx context.Context, state state.VMIPLeaseSta return reconcile.Result{}, err } - if vmip != nil && vmip.Status.Address == ip.LeaseNameToIP(lease.Name) { + if vmip != nil { controllerutil.AddFinalizer(lease, virtv2.FinalizerIPAddressLeaseCleanup) } else { 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 71082fe0f6..17aad7ef50 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go @@ -23,7 +23,6 @@ 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 +51,7 @@ func (h *RetentionHandler) Handle(ctx context.Context, state state.VMIPLeaseStat return reconcile.Result{}, err } - if vmip == nil || vmip.Status.Address != ip.LeaseNameToIP(lease.Name) { + if vmip == nil { if lease.Spec.VirtualMachineIPAddressRef.Name != "" { log.Debug("VirtualMachineIP not found: remove this ref from the spec and retain VMIPLease") lease.Spec.VirtualMachineIPAddressRef.Name = "" From 6ec736c24bb440ebbf78e4ab6e12e3f4ea062c68 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Sat, 10 May 2025 17:10:38 +0300 Subject: [PATCH 03/17] refactor lifecycle_handler.go Signed-off-by: Dmitry Lopatin --- .../vmip/internal/lifecycle_handler.go | 86 +++++++++++-------- 1 file changed, 52 insertions(+), 34 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go index 657d10ae42..91dd893904 100644 --- 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" + "log/slog" "time" corev1 "k8s.io/api/core/v1" @@ -72,48 +73,73 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r conditions.SetCondition(conditionAttach, &vmip.Status.Conditions) }() + lease, err := state.VirtualMachineIPLease(ctx) + if err != nil { + return reconcile.Result{}, err + } + + needRequeue := false + switch { + case lease == nil && vmip.Status.Address != "": + h.handleLostLease(vmip, conditionBound) + case lease == nil: + h.handleNotFoundLease(vmip, conditionBound) + default: + needRequeue = h.handleBoundLease(lease, vmip, log, conditionBound) + h.handleAttachedLease(vmip, vm, conditionAttach) + } + + log.Debug("Set VirtualMachineIP phase", "phase", vmip.Status.Phase) + vmip.Status.ObservedGeneration = vmip.GetGeneration() + if !needRequeue { + return reconcile.Result{}, nil + } else { + // TODO add requeue with with exponential BackOff time interval using condition Bound -> probeTime + return reconcile.Result{RequeueAfter: 30 * time.Second}, nil + } +} + +func (h *LifecycleHandler) handleNotFoundLease(vmip *virtv2.VirtualMachineIPAddress, conditionBound *conditions.ConditionBuilder) { + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending + conditionBound.Status(metav1.ConditionFalse). + Reason(vmipcondition.VirtualMachineIPAddressLeaseNotFound). + Message("VirtualMachineIPAddressLease is not found") +} + +func (h *LifecycleHandler) handleLostLease(vmip *virtv2.VirtualMachineIPAddress, conditionBound *conditions.ConditionBuilder) { + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending + conditionBound.Status(metav1.ConditionFalse). + Reason(vmipcondition.VirtualMachineIPAddressLeaseLost). + Message(fmt.Sprintf("VirtualMachineIPAddressLease %s doesn't exist", + ip.IpToLeaseName(vmip.Status.Address))) +} + +func (h *LifecycleHandler) handleAttachedLease(vmip *virtv2.VirtualMachineIPAddress, vm *virtv2.VirtualMachine, conditionAttach *conditions.ConditionBuilder) { if vm == nil || vm.DeletionTimestamp != nil { vmip.Status.VirtualMachine = "" conditionAttach.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineNotFound). Message("Virtual machine not found") h.recorder.Event(vmip, corev1.EventTypeWarning, virtv2.ReasonNotAttached, "Virtual machine not found.") + return } - lease, err := state.VirtualMachineIPLease(ctx) - if err != nil { - return reconcile.Result{}, err - } + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhaseAttached + vmip.Status.VirtualMachine = vm.Name + conditionAttach.Status(metav1.ConditionTrue). + Reason(vmipcondition.Attached) + h.recorder.Eventf(vmip, corev1.EventTypeNormal, virtv2.ReasonAttached, "VirtualMachineIPAddress is attached to %q/%q.", vm.Namespace, vm.Name) +} +func (h *LifecycleHandler) handleBoundLease(lease *virtv2.VirtualMachineIPAddressLease, vmip *virtv2.VirtualMachineIPAddress, log *slog.Logger, conditionBound *conditions.ConditionBuilder) bool { needRequeue := false switch { - case lease == nil && vmip.Status.Address != "": - vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending - conditionBound.Status(metav1.ConditionFalse). - Reason(vmipcondition.VirtualMachineIPAddressLeaseLost). - Message(fmt.Sprintf("VirtualMachineIPAddressLease %s doesn't exist", - ip.IpToLeaseName(vmip.Status.Address))) - - case lease == nil: - vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending - conditionBound.Status(metav1.ConditionFalse). - Reason(vmipcondition.VirtualMachineIPAddressLeaseNotFound). - Message("VirtualMachineIPAddressLease is not found") - case util.IsBoundLease(lease, vmip): vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhaseBound vmip.Status.Address = ip.LeaseNameToIP(lease.Name) conditionBound.Status(metav1.ConditionTrue). Reason(vmipcondition.Bound) - if vm != nil && vm.GetDeletionTimestamp().IsZero() { - vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhaseAttached - vmip.Status.VirtualMachine = vm.Name - conditionAttach.Status(metav1.ConditionTrue). - Reason(vmipcondition.Attached) - h.recorder.Eventf(vmip, corev1.EventTypeNormal, virtv2.ReasonAttached, "VirtualMachineIPAddress is attached to %q/%q.", vm.Namespace, vm.Name) - } - case lease.Status.Phase == virtv2.VirtualMachineIPAddressLeasePhaseBound: vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending log.Warn(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress resource: %s/%s", @@ -138,15 +164,7 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r lease.Name)) } - log.Debug("Set VirtualMachineIP phase", "phase", vmip.Status.Phase) - - vmip.Status.ObservedGeneration = vmip.GetGeneration() - if !needRequeue { - return reconcile.Result{}, nil - } else { - // TODO add requeue with with exponential BackOff time interval using condition Bound -> probeTime - return reconcile.Result{RequeueAfter: 30 * time.Second}, nil - } + return needRequeue } func (h *LifecycleHandler) Name() string { From 323b01f13022c4455ed0d2cbe4035fc84ad7c214 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Sun, 11 May 2025 16:33:38 +0300 Subject: [PATCH 04/17] fix iplease_handler.go Signed-off-by: Dmitry Lopatin --- .../vmip/internal/iplease_handler.go | 88 ++++++++++--------- 1 file changed, 47 insertions(+), 41 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 index 992add325a..b897701995 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -20,8 +20,11 @@ import ( "context" "errors" "fmt" + "log/slog" + "time" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -57,19 +60,13 @@ func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reco log, ctx := logger.GetHandlerContext(ctx, IpLeaseHandlerName) vmip := state.VirtualMachineIP() - vmipStatus := &vmip.Status lease, err := state.VirtualMachineIPLease(ctx) if err != nil { return reconcile.Result{}, err } - condition, _ := conditions.GetCondition(vmipcondition.BoundType, vmipStatus.Conditions) switch { - case lease == nil && vmipStatus.Address != "" && condition.Reason != vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists.String(): - log.Info("Lease by name not found: waiting for the lease to be available") - return reconcile.Result{}, nil - case lease == nil: log.Info("No Lease for VirtualMachineIP: create the new one", "type", vmip.Spec.Type, "address", vmip.Spec.StaticIP) return h.createNewLease(ctx, state) @@ -78,8 +75,12 @@ func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reco log.Info("Lease is not ready: waiting for the lease") return reconcile.Result{}, nil + case vmip.Status.Address == "": + vmip.Status.Address = ip.LeaseNameToIP(lease.Name) + return reconcile.Result{}, nil + case util.IsBoundLease(lease, vmip): - log.Info("Lease already exists, VirtualMachineIP ref is valid") + log.Info("Lease is bound, VirtualMachineIP ref is valid") return reconcile.Result{}, nil case lease.Status.Phase == virtv2.VirtualMachineIPAddressLeasePhaseBound: @@ -87,83 +88,83 @@ func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reco return reconcile.Result{}, nil default: - log.Info("Lease is released: set binding") + return h.updateLease(ctx, lease, vmip, log) + } +} - if lease.Spec.VirtualMachineIPAddressRef.Namespace != vmip.Namespace { - log.Warn(fmt.Sprintf("The VirtualMachineIPLease belongs to a different namespace: %s", lease.Spec.VirtualMachineIPAddressRef.Namespace)) - h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists.String(), "The VirtualMachineIPLease belongs to a different namespace") +func (h IPLeaseHandler) updateLease(ctx context.Context, lease *virtv2.VirtualMachineIPAddressLease, vmip *virtv2.VirtualMachineIPAddress, log *slog.Logger) (reconcile.Result, error) { + log.Info("Lease is released: set binding") - return reconcile.Result{}, nil - } + if lease.Spec.VirtualMachineIPAddressRef.Namespace != vmip.Namespace { + log.Warn(fmt.Sprintf("The VirtualMachineIPLease belongs to a different namespace: %s", lease.Spec.VirtualMachineIPAddressRef.Namespace)) + h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists.String(), "The VirtualMachineIPLease belongs to a different namespace") - lease.Spec.VirtualMachineIPAddressRef = &virtv2.VirtualMachineIPAddressLeaseIpAddressRef{ - Name: vmip.Name, - Namespace: vmip.Namespace, - } + return reconcile.Result{}, nil + } - err := h.client.Update(ctx, lease) - if err != nil { - return reconcile.Result{}, err - } + lease.Spec.VirtualMachineIPAddressRef = &virtv2.VirtualMachineIPAddressLeaseIpAddressRef{ + Name: vmip.Name, + Namespace: vmip.Namespace, + } - vmipStatus.Address = ip.LeaseNameToIP(lease.Name) - return reconcile.Result{}, nil + err := h.client.Update(ctx, lease) + if err != nil { + return reconcile.Result{}, err } + // fixme dlopatin delete this line + vmip.Status.Address = ip.LeaseNameToIP(lease.Name) + return reconcile.Result{}, nil } func (h IPLeaseHandler) createNewLease(ctx context.Context, state state.VMIPState) (reconcile.Result, error) { log := logger.FromContext(ctx) vmip := state.VirtualMachineIP() - vmipStatus := &vmip.Status - + ipAddress := "" if vmip.Spec.Type == virtv2.VirtualMachineIPAddressTypeAuto { log.Info("allocate the new VirtualMachineIP address") var err error - vmipStatus.Address, err = h.ipService.AllocateNewIP(state.AllocatedIPs()) + ipAddress, err = h.ipService.AllocateNewIP(state.AllocatedIPs()) if err != nil { return reconcile.Result{}, err } } else { - vmipStatus.Address = vmip.Spec.StaticIP + ipAddress = vmip.Spec.StaticIP } - err := h.ipService.IsAvailableAddress(vmipStatus.Address, state.AllocatedIPs()) + err := h.ipService.IsAvailableAddress(ipAddress, state.AllocatedIPs()) if err != nil { - vmipStatus.Address = "" msg := fmt.Sprintf("the VirtualMachineIP cannot be created: %s", err.Error()) - log.Info(msg) + log.Warn(msg) conditionBound := conditions.NewConditionBuilder(vmipcondition.BoundType). Generation(vmip.GetGeneration()) switch { case errors.Is(err, service.ErrIPAddressOutOfRange): - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending - msg := fmt.Sprintf("The requested address %s is out of the valid range.", vmip.Spec.StaticIP) + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending + msg = fmt.Sprintf("The requested address %s is out of the valid range.", vmip.Spec.StaticIP) conditionBound.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineIPAddressIsOutOfTheValidRange). Message(msg) h.recorder.Event(vmip, corev1.EventTypeWarning, virtv2.ReasonFailed, msg) case errors.Is(err, service.ErrIPAddressAlreadyExist): - vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending - msg := fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress.", - ip.IpToLeaseName(vmipStatus.Address)) + vmip.Status.Phase = virtv2.VirtualMachineIPAddressPhasePending + msg = fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress.", + ip.IpToLeaseName(ipAddress)) conditionBound.Status(metav1.ConditionFalse). Reason(vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists). Message(msg) h.recorder.Event(vmip, corev1.EventTypeWarning, virtv2.ReasonBound, msg) } - conditions.SetCondition(conditionBound, &vmipStatus.Conditions) + conditions.SetCondition(conditionBound, &vmip.Status.Conditions) return reconcile.Result{}, nil } - leaseName := ip.IpToLeaseName(vmipStatus.Address) + leaseName := ip.IpToLeaseName(ipAddress) - log.Info("Create lease", - "leaseName", leaseName, - "refName", vmip.Name, - "refNamespace", vmip.Namespace, + log.Info("Create lease", "leaseName", leaseName, + "refName", vmip.Name, "refNamespace", vmip.Namespace, ) err = h.client.Create(ctx, &virtv2.VirtualMachineIPAddressLease{ @@ -178,6 +179,11 @@ func (h IPLeaseHandler) createNewLease(ctx context.Context, state state.VMIPStat }, }) if err != nil { + if k8serrors.IsAlreadyExists(err) { + log.Warn("Lease already exists: requeue 2s") + return reconcile.Result{RequeueAfter: 2 * time.Second}, nil + } + return reconcile.Result{}, err } From 7b500d39969bb18c049b8c3031b5fc00511e45a4 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Sun, 11 May 2025 17:33:24 +0300 Subject: [PATCH 05/17] fix state.go Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/state/state.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index 2304c938bb..7bb213082e 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -64,8 +64,11 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi } var err error + leaseName := ip.IpToLeaseName(s.vmip.Spec.StaticIP) - leaseName := ip.IpToLeaseName(s.vmip.Status.Address) + if leaseName == "" { + leaseName = ip.IpToLeaseName(s.vmip.Status.Address) + } if leaseName != "" { leaseKey := types.NamespacedName{Name: leaseName} @@ -78,7 +81,6 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi if s.lease == nil { var leases virtv2.VirtualMachineIPAddressLeaseList err = s.client.List(ctx, &leases, - client.InNamespace(s.vmip.Namespace), &client.MatchingFields{ indexer.IndexFieldVMIPLeaseByVMIP: s.vmip.Name, }) From 798440eeec865520f479ac61356cfbe4da58671f Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Sun, 11 May 2025 17:57:28 +0300 Subject: [PATCH 06/17] fix to worked Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/state/state.go | 5 +++++ .../pkg/controller/vmiplease/internal/retention_handler.go | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index 7bb213082e..fd3b66ca6a 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -30,6 +30,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/controller/ipam" "github.com/deckhouse/virtualization-controller/pkg/controller/vmip/internal/util" + "github.com/deckhouse/virtualization-controller/pkg/logger" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmiplcondition" ) @@ -88,6 +89,10 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi return nil, err } + if len(leases.Items) > 1 { + logger.FromContext(ctx).Warn("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) + } + for i, lease := range leases.Items { boundCondition, exist := conditions.GetCondition(vmiplcondition.BoundType, lease.Status.Conditions) if exist && boundCondition.Status == metav1.ConditionTrue { 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 17aad7ef50..c41d58dd5c 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" @@ -51,7 +52,7 @@ func (h *RetentionHandler) Handle(ctx context.Context, state state.VMIPLeaseStat return reconcile.Result{}, err } - if vmip == nil { + if vmip == nil || vmip.Status.Address != "" && vmip.Status.Address != ip.LeaseNameToIP(lease.Name) { if lease.Spec.VirtualMachineIPAddressRef.Name != "" { log.Debug("VirtualMachineIP not found: remove this ref from the spec and retain VMIPLease") lease.Spec.VirtualMachineIPAddressRef.Name = "" From 2fadb8bb1092500d3275741f6517cc94108a3899 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 13:15:54 +0300 Subject: [PATCH 07/17] right way Signed-off-by: Dmitry Lopatin --- .../pkg/common/annotations/annotations.go | 3 +++ .../pkg/controller/indexer/indexer.go | 13 ------------- .../controller/vmip/internal/iplease_handler.go | 7 +++++-- .../pkg/controller/vmip/internal/state/state.go | 14 ++++++++------ 4 files changed, 16 insertions(+), 21 deletions(-) diff --git a/images/virtualization-artifact/pkg/common/annotations/annotations.go b/images/virtualization-artifact/pkg/common/annotations/annotations.go index 6c5998908f..2be408717d 100644 --- a/images/virtualization-artifact/pkg/common/annotations/annotations.go +++ b/images/virtualization-artifact/pkg/common/annotations/annotations.go @@ -97,6 +97,9 @@ const ( // LabelVirtualMachineUID is a label to link VirtualMachineIPAddress to VirtualMachine. LabelVirtualMachineUID = LabelsPrefix + "/virtual-machine-uid" + // LabelVirtualMachineIPAddressUID is a label to link VirtualMachineIPAddressLease to VirtualMachineIPAddress. + LabelVirtualMachineIPAddressUID = LabelsPrefix + "/virtual-machine-ip-address-uid" + UploaderServiceLabel = "service" // AppKubernetesManagedByLabel is the Kubernetes recommended managed-by label. diff --git a/images/virtualization-artifact/pkg/controller/indexer/indexer.go b/images/virtualization-artifact/pkg/controller/indexer/indexer.go index 7122f105a1..f3421a24a8 100644 --- a/images/virtualization-artifact/pkg/controller/indexer/indexer.go +++ b/images/virtualization-artifact/pkg/controller/indexer/indexer.go @@ -36,8 +36,6 @@ const ( IndexFieldVMByCVI = "spec.blockDeviceRefs.ClusterVirtualImage" IndexFieldVMByNode = "status.node" - IndexFieldVMIPLeaseByVMIP = "spec.virtualMachineIPAddressRef.Name" - IndexFieldVDByVDSnapshot = "vd,spec.DataSource.ObjectRef.Name,.Kind=VirtualDiskSnapshot" IndexFieldVIByVDSnapshot = "vi,spec.DataSource.ObjectRef.Name,.Kind=VirtualDiskSnapshot" IndexFieldCVIByVDSnapshot = "cvi,spec.DataSource.ObjectRef.Name,.Kind=VirtualDiskSnapshot" @@ -62,7 +60,6 @@ var IndexGetters = []IndexGetter{ IndexVMByVI, IndexVMByCVI, IndexVMByNode, - IndexVMIPLeaseByVMIP, IndexVMSnapshotByVM, IndexVMSnapshotByVDSnapshot, IndexVMRestoreByVMSnapshot, @@ -140,13 +137,3 @@ func getBlockDeviceNamesByKind(obj client.Object, kind virtv2.BlockDeviceKind) [ } return res } - -func IndexVMIPLeaseByVMIP() (obj client.Object, field string, extractValue client.IndexerFunc) { - return &virtv2.VirtualMachineIPAddressLease{}, IndexFieldVMIPLeaseByVMIP, func(object client.Object) []string { - lease, ok := object.(*virtv2.VirtualMachineIPAddressLease) - if !ok || lease == nil { - return nil - } - return []string{lease.Spec.VirtualMachineIPAddressRef.Name} - } -} diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go index b897701995..53e40a5014 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/ip" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" "github.com/deckhouse/virtualization-controller/pkg/controller/service" @@ -111,8 +112,7 @@ func (h IPLeaseHandler) updateLease(ctx context.Context, lease *virtv2.VirtualMa if err != nil { return reconcile.Result{}, err } - // fixme dlopatin delete this line - vmip.Status.Address = ip.LeaseNameToIP(lease.Name) + return reconcile.Result{}, nil } @@ -169,6 +169,9 @@ func (h IPLeaseHandler) createNewLease(ctx context.Context, state state.VMIPStat err = h.client.Create(ctx, &virtv2.VirtualMachineIPAddressLease{ ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + annotations.LabelVirtualMachineIPAddressUID: string(vmip.GetUID()), + }, Name: leaseName, }, Spec: virtv2.VirtualMachineIPAddressLeaseSpec{ diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index fd3b66ca6a..034e15b702 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -21,13 +21,14 @@ import ( "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/deckhouse/virtualization-controller/pkg/common/annotations" "github.com/deckhouse/virtualization-controller/pkg/common/ip" "github.com/deckhouse/virtualization-controller/pkg/common/object" "github.com/deckhouse/virtualization-controller/pkg/controller/conditions" - "github.com/deckhouse/virtualization-controller/pkg/controller/indexer" "github.com/deckhouse/virtualization-controller/pkg/controller/ipam" "github.com/deckhouse/virtualization-controller/pkg/controller/vmip/internal/util" "github.com/deckhouse/virtualization-controller/pkg/logger" @@ -80,11 +81,12 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi } if s.lease == nil { - var leases virtv2.VirtualMachineIPAddressLeaseList - err = s.client.List(ctx, &leases, - &client.MatchingFields{ - indexer.IndexFieldVMIPLeaseByVMIP: s.vmip.Name, - }) + leases := &virtv2.VirtualMachineIPAddressLeaseList{} + + err = s.client.List(ctx, leases, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{annotations.LabelVirtualMachineIPAddressUID: string(s.vmip.GetUID())}), + }) + if err != nil { return nil, err } From afcf7d3ac6027ab2c7c50e0e6da9370a4524b838 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 14:18:10 +0300 Subject: [PATCH 08/17] right way part 2 Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/iplease_handler.go | 1 + .../pkg/controller/vmiplease/internal/retention_handler.go | 6 ++++-- 2 files changed, 5 insertions(+), 2 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 index 53e40a5014..e83a7efbb3 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -107,6 +107,7 @@ func (h IPLeaseHandler) updateLease(ctx context.Context, lease *virtv2.VirtualMa Name: vmip.Name, Namespace: vmip.Namespace, } + lease.Labels[annotations.LabelVirtualMachineIPAddressUID] = string(vmip.GetUID()) err := h.client.Update(ctx, lease) if err != nil { 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 c41d58dd5c..d5e9ba4b99 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/annotations" "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" @@ -53,9 +54,10 @@ func (h *RetentionHandler) Handle(ctx context.Context, state state.VMIPLeaseStat } if vmip == nil || vmip.Status.Address != "" && vmip.Status.Address != ip.LeaseNameToIP(lease.Name) { - if lease.Spec.VirtualMachineIPAddressRef.Name != "" { - log.Debug("VirtualMachineIP not found: remove this ref from the spec and retain VMIPLease") + if lease.Spec.VirtualMachineIPAddressRef.Name != "" || lease.Labels[annotations.LabelVirtualMachineIPAddressUID] != "" { + log.Debug("VirtualMachineIP not found: remove this ref from the spec, delete label value and retain VMIPLease") lease.Spec.VirtualMachineIPAddressRef.Name = "" + lease.Labels[annotations.LabelVirtualMachineIPAddressUID] = "" return reconcile.Result{RequeueAfter: h.retentionDuration}, nil } From e8ec9f5d517e7206c30fb18c9111e139270fe395 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 16:28:28 +0300 Subject: [PATCH 09/17] right way part 3 Signed-off-by: Dmitry Lopatin --- .../cmd/virtualization-controller/main.go | 2 +- .../controller/vmip/internal/state/state.go | 6 ++-- .../pkg/controller/vmip/vmip_controller.go | 5 ++- .../pkg/controller/vmip/vmip_reconciler.go | 33 ++++++------------- 4 files changed, 19 insertions(+), 27 deletions(-) diff --git a/images/virtualization-artifact/cmd/virtualization-controller/main.go b/images/virtualization-artifact/cmd/virtualization-controller/main.go index 4e8ffa798a..2c86e84209 100644 --- a/images/virtualization-artifact/cmd/virtualization-controller/main.go +++ b/images/virtualization-artifact/cmd/virtualization-controller/main.go @@ -300,7 +300,7 @@ func main() { } vmipLogger := logger.NewControllerLogger(vmip.ControllerName, logLevel, logOutput, logDebugVerbosity, logDebugControllerList) - if _, err = vmip.NewController(ctx, mgr, vmipLogger, virtualMachineCIDRs); err != nil { + if _, err = vmip.NewController(ctx, mgr, virtClient, vmipLogger, virtualMachineCIDRs); err != nil { log.Error(err.Error()) os.Exit(1) } diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index 034e15b702..a9be3f448f 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -32,6 +32,7 @@ import ( "github.com/deckhouse/virtualization-controller/pkg/controller/ipam" "github.com/deckhouse/virtualization-controller/pkg/controller/vmip/internal/util" "github.com/deckhouse/virtualization-controller/pkg/logger" + "github.com/deckhouse/virtualization/api/client/kubeclient" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" "github.com/deckhouse/virtualization/api/core/v1alpha2/vmiplcondition" ) @@ -46,14 +47,15 @@ type VMIPState interface { type state struct { client client.Client + virtClient kubeclient.Client vmip *virtv2.VirtualMachineIPAddress lease *virtv2.VirtualMachineIPAddressLease vm *virtv2.VirtualMachine allocatedIPs ip.AllocatedIPs } -func New(c client.Client, vmip *virtv2.VirtualMachineIPAddress) VMIPState { - return &state{client: c, vmip: vmip} +func New(c client.Client, virtClient kubeclient.Client, vmip *virtv2.VirtualMachineIPAddress) VMIPState { + return &state{client: c, virtClient: virtClient, vmip: vmip} } func (s *state) VirtualMachineIP() *virtv2.VirtualMachineIPAddress { diff --git a/images/virtualization-artifact/pkg/controller/vmip/vmip_controller.go b/images/virtualization-artifact/pkg/controller/vmip/vmip_controller.go index e548d4528e..2704c28236 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/vmip_controller.go +++ b/images/virtualization-artifact/pkg/controller/vmip/vmip_controller.go @@ -26,10 +26,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/manager" "github.com/deckhouse/deckhouse/pkg/log" + "github.com/deckhouse/virtualization-controller/pkg/controller/service" "github.com/deckhouse/virtualization-controller/pkg/controller/vmip/internal" "github.com/deckhouse/virtualization-controller/pkg/eventrecord" "github.com/deckhouse/virtualization-controller/pkg/logger" + "github.com/deckhouse/virtualization/api/client/kubeclient" "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -40,6 +42,7 @@ const ( func NewController( ctx context.Context, mgr manager.Manager, + virtClient kubeclient.Client, log *log.Logger, virtualMachineCIDRs []string, ) (controller.Controller, error) { @@ -52,7 +55,7 @@ func NewController( internal.NewLifecycleHandler(recorder), } - r, err := NewReconciler(mgr.GetClient(), handlers...) + r, err := NewReconciler(mgr.GetClient(), virtClient, handlers...) if err != nil { return nil, err } diff --git a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go index 804c72f494..57a1afcea3 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go @@ -19,7 +19,6 @@ package vmip import ( "context" "fmt" - "time" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,7 +33,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" + "github.com/deckhouse/virtualization/api/client/kubeclient" virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2" ) @@ -44,14 +43,16 @@ type Handler interface { } type Reconciler struct { - handlers []Handler - client client.Client + handlers []Handler + client client.Client + virtClient kubeclient.Client } -func NewReconciler(client client.Client, handlers ...Handler) (*Reconciler, error) { +func NewReconciler(client client.Client, virtClient kubeclient.Client, handlers ...Handler) (*Reconciler, error) { return &Reconciler{ - client: client, - handlers: handlers, + client: client, + virtClient: virtClient, + handlers: handlers, }, nil } @@ -153,7 +154,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return reconcile.Result{}, nil } - s := state.New(r.client, vmip.Changed()) + s := state.New(r.client, r.virtClient, vmip.Changed()) rec := reconciler.NewBaseReconciler[Handler](r.handlers) rec.SetHandlerExecutor(func(ctx context.Context, h Handler) (reconcile.Result, error) { return h.Handle(ctx, s) @@ -164,21 +165,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco return vmip.Update(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 + return rec.Reconcile(ctx) } func (r *Reconciler) factory() *virtv2.VirtualMachineIPAddress { From d98af9ec766a8e13fbbc9b67dc6fb3b15cc13115 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 17:23:25 +0300 Subject: [PATCH 10/17] right way part 4 Signed-off-by: Dmitry Lopatin --- .../vmip/internal/iplease_handler.go | 4 ++++ .../vmip/internal/lifecycle_handler.go | 4 ++++ .../controller/vmip/internal/state/state.go | 24 ++++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go index e83a7efbb3..3661fbeba2 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -64,6 +64,10 @@ func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reco lease, err := state.VirtualMachineIPLease(ctx) if err != nil { + if err.Error() == "VirtualMachineIPAddressLease found in kubeclient without cache" { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go index 91dd893904..08f7f99a30 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go @@ -75,6 +75,10 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r lease, err := state.VirtualMachineIPLease(ctx) if err != nil { + if err.Error() == "VirtualMachineIPAddressLease found in kubeclient without cache" { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err } diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index a9be3f448f..c869adee73 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -18,6 +18,7 @@ package state import ( "context" + "errors" "fmt" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -82,6 +83,8 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi } } + logger := logger.FromContext(ctx) + if s.lease == nil { leases := &virtv2.VirtualMachineIPAddressLeaseList{} @@ -94,7 +97,9 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi } if len(leases.Items) > 1 { - logger.FromContext(ctx).Warn("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) + logger.Warn("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) + } else if len(leases.Items) == 0 { + logger.Warn("VirtualMachineIPAddressLease not found -> kubeclient", "vmip", s.vmip.Name) } for i, lease := range leases.Items { @@ -106,6 +111,23 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi } } + if s.lease == nil { + leases, err := s.virtClient.VirtualMachineIPAddressLeases().List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", annotations.LabelVirtualMachineIPAddressUID, string(s.vmip.GetUID())), + }) + if err != nil { + return nil, err + } + + if len(leases.Items) != 0 { + if len(leases.Items) > 1 { + logger.Warn("More than one VirtualMachineIPAddressLease found in kubeclient without cache", "count", len(leases.Items)) + } + logger.Warn("VirtualMachineIPAddressLease found in kubeclient without cache", "vmip", s.vmip.Name) + return nil, errors.New("VirtualMachineIPAddressLease found in kubeclient without cache") + } + } + if s.lease == nil { s.allocatedIPs, err = util.GetAllocatedIPs(ctx, s.client, s.vmip.Spec.Type) if err != nil { From 44e04353d310e177eb48667b72a2e702b4e9b6fc Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 20:47:07 +0300 Subject: [PATCH 11/17] polishing Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/vmip_reconciler.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go index 57a1afcea3..8b3a1f04b0 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go @@ -81,7 +81,18 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr return fmt.Errorf("error setting watch on vms: %w", err) } - return ctr.Watch(source.Kind(mgr.GetCache(), &virtv2.VirtualMachineIPAddress{}), &handler.EnqueueRequestForObject{}) + return ctr.Watch( + source.Kind(mgr.GetCache(), &virtv2.VirtualMachineIPAddress{}), + &handler.EnqueueRequestForObject{}, + predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return true }, + DeleteFunc: func(e event.DeleteEvent) bool { return false }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldVmip := e.ObjectOld.(*virtv2.VirtualMachineIPAddress) + newVmip := e.ObjectNew.(*virtv2.VirtualMachineIPAddress) + return oldVmip.Spec != newVmip.Spec + }, + }) } func (r *Reconciler) enqueueRequestsFromVMs(ctx context.Context, obj client.Object) []reconcile.Request { From 2d860374db26d1c6cca466b842a2df1fcb66cb62 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 20:51:34 +0300 Subject: [PATCH 12/17] polishing 2 Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/vmip_reconciler.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go index 8b3a1f04b0..42730c3d0c 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go @@ -74,8 +74,15 @@ func (r *Reconciler) SetupController(_ context.Context, mgr manager.Manager, ctr handler.EnqueueRequestsFromMapFunc(r.enqueueRequestsFromVMs), predicate.Funcs{ CreateFunc: func(e event.CreateEvent) bool { return false }, - DeleteFunc: func(e event.DeleteEvent) bool { return true }, - UpdateFunc: func(e event.UpdateEvent) bool { return true }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldVm := e.ObjectOld.(*virtv2.VirtualMachine) + newVm := e.ObjectNew.(*virtv2.VirtualMachine) + return newVm.Spec.VirtualMachineIPAddress != oldVm.Spec.VirtualMachineIPAddress || + newVm.Status.VirtualMachineIPAddress != oldVm.Status.VirtualMachineIPAddress + }, }, ); err != nil { return fmt.Errorf("error setting watch on vms: %w", err) From 9cd5cf19408b3392c839474cb44e8df0bd5016d6 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 21:24:08 +0300 Subject: [PATCH 13/17] polishing 3 Signed-off-by: Dmitry Lopatin --- .../controller/service/ip_address_service.go | 2 + .../vmip/internal/iplease_handler.go | 11 +-- .../vmip/internal/lifecycle_handler.go | 16 +---- .../vmip/internal/protection_handler.go | 5 +- .../controller/vmip/internal/state/state.go | 70 +++++++++++-------- .../pkg/controller/vmip/vmip_reconciler.go | 9 +++ 6 files changed, 55 insertions(+), 58 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/service/ip_address_service.go b/images/virtualization-artifact/pkg/controller/service/ip_address_service.go index 6c608bd44f..deccc1cdaf 100644 --- a/images/virtualization-artifact/pkg/controller/service/ip_address_service.go +++ b/images/virtualization-artifact/pkg/controller/service/ip_address_service.go @@ -25,6 +25,7 @@ import ( k8snet "k8s.io/utils/net" "github.com/deckhouse/deckhouse/pkg/log" + "github.com/deckhouse/virtualization-controller/pkg/common/ip" ) @@ -94,6 +95,7 @@ func (s IpAddressService) AllocateNewIP(allocatedIPs ip.AllocatedIPs) (string, e } if _, ok := allocatedIPs[ip.String()]; !ok { + return ip.String(), nil } } diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go index 3661fbeba2..709f62933f 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -60,16 +60,7 @@ func NewIPLeaseHandler(client client.Client, ipAddressService *service.IpAddress func (h IPLeaseHandler) Handle(ctx context.Context, state state.VMIPState) (reconcile.Result, error) { log, ctx := logger.GetHandlerContext(ctx, IpLeaseHandlerName) - vmip := state.VirtualMachineIP() - - lease, err := state.VirtualMachineIPLease(ctx) - if err != nil { - if err.Error() == "VirtualMachineIPAddressLease found in kubeclient without cache" { - return reconcile.Result{}, nil - } - - return reconcile.Result{}, err - } + vmip, lease := state.VirtualMachineIP(), state.VirtualMachineIPLease() switch { case lease == nil: diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go index 08f7f99a30..b23ab81ee8 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go @@ -51,12 +51,7 @@ func NewLifecycleHandler(recorder eventrecord.EventRecorderLogger) *LifecycleHan func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (reconcile.Result, error) { log := logger.FromContext(ctx).With(logger.SlogHandler(LifecycleHandlerName)) - vmip := state.VirtualMachineIP() - - vm, err := state.VirtualMachine(ctx) - if err != nil { - return reconcile.Result{}, err - } + vmip, vm := state.VirtualMachineIP(), state.VirtualMachine() conditionBound := conditions.NewConditionBuilder(vmipcondition.BoundType). Generation(vmip.GetGeneration()). @@ -73,14 +68,7 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r conditions.SetCondition(conditionAttach, &vmip.Status.Conditions) }() - lease, err := state.VirtualMachineIPLease(ctx) - if err != nil { - if err.Error() == "VirtualMachineIPAddressLease found in kubeclient without cache" { - return reconcile.Result{}, nil - } - - return reconcile.Result{}, err - } + lease := state.VirtualMachineIPLease() needRequeue := false switch { diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go index e9834bdf75..e4468c6bfd 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/protection_handler.go @@ -61,10 +61,7 @@ func (h *ProtectionHandler) Handle(ctx context.Context, state state.VMIPState) ( log.Debug("VirtualMachineIPAddress deletion is delayed: it's protected by virtual machines") } - vm, err := state.VirtualMachine(ctx) - if err != nil { - return reconcile.Result{}, err - } + vm := state.VirtualMachine() if vm == nil || vm.DeletionTimestamp != nil { log.Info("VirtualMachineIP is no longer attached to any VM: remove cleanup finalizer", "VirtualMachineIPName", vmip.Name) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index c869adee73..e93ae819e4 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -39,9 +39,10 @@ import ( ) type VMIPState interface { + Reload(ctx context.Context) error VirtualMachineIP() *virtv2.VirtualMachineIPAddress - VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachineIPAddressLease, error) - VirtualMachine(ctx context.Context) (*virtv2.VirtualMachine, error) + VirtualMachineIPLease() *virtv2.VirtualMachineIPAddressLease + VirtualMachine() *virtv2.VirtualMachine AllocatedIPs() ip.AllocatedIPs } @@ -59,15 +60,35 @@ func New(c client.Client, virtClient kubeclient.Client, vmip *virtv2.VirtualMach return &state{client: c, virtClient: virtClient, vmip: vmip} } +func (s *state) Reload(ctx context.Context) error { + if err := s.reloadVirtualMachineIPLease(ctx); err != nil { + return err + } + + if err := s.reloadVirtualMachine(ctx); err != nil { + return err + } + + return nil +} + func (s *state) VirtualMachineIP() *virtv2.VirtualMachineIPAddress { return s.vmip } -func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachineIPAddressLease, error) { - if s.lease != nil { - return s.lease, nil - } +func (s *state) VirtualMachineIPLease() *virtv2.VirtualMachineIPAddressLease { + return s.lease +} +func (s *state) VirtualMachine() *virtv2.VirtualMachine { + return s.vm +} + +func (s *state) AllocatedIPs() ip.AllocatedIPs { + return s.allocatedIPs +} + +func (s *state) reloadVirtualMachineIPLease(ctx context.Context) error { var err error leaseName := ip.IpToLeaseName(s.vmip.Spec.StaticIP) @@ -79,12 +100,11 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi leaseKey := types.NamespacedName{Name: leaseName} s.lease, err = object.FetchObject(ctx, leaseKey, s.client, &virtv2.VirtualMachineIPAddressLease{}) if err != nil { - return nil, fmt.Errorf("unable to get Lease %s: %w", leaseKey, err) + return fmt.Errorf("unable to get Lease %s: %w", leaseKey, err) } } logger := logger.FromContext(ctx) - if s.lease == nil { leases := &virtv2.VirtualMachineIPAddressLeaseList{} @@ -93,13 +113,11 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi }) if err != nil { - return nil, err + return err } if len(leases.Items) > 1 { - logger.Warn("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) - } else if len(leases.Items) == 0 { - logger.Warn("VirtualMachineIPAddressLease not found -> kubeclient", "vmip", s.vmip.Name) + logger.Error("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) } for i, lease := range leases.Items { @@ -116,43 +134,39 @@ func (s *state) VirtualMachineIPLease(ctx context.Context) (*virtv2.VirtualMachi LabelSelector: fmt.Sprintf("%s=%s", annotations.LabelVirtualMachineIPAddressUID, string(s.vmip.GetUID())), }) if err != nil { - return nil, err + return err } if len(leases.Items) != 0 { if len(leases.Items) > 1 { - logger.Warn("More than one VirtualMachineIPAddressLease found in kubeclient without cache", "count", len(leases.Items)) + logger.Error("More than one VirtualMachineIPAddressLease found in kubeclient without cache", "count", len(leases.Items)) } logger.Warn("VirtualMachineIPAddressLease found in kubeclient without cache", "vmip", s.vmip.Name) - return nil, errors.New("VirtualMachineIPAddressLease found in kubeclient without cache") + return errors.New("VirtualMachineIPAddressLease found in kubeclient without cache") } } if s.lease == nil { s.allocatedIPs, err = util.GetAllocatedIPs(ctx, s.client, s.vmip.Spec.Type) if err != nil { - return nil, err + return err } } - return s.lease, nil + return nil } -func (s *state) VirtualMachine(ctx context.Context) (*virtv2.VirtualMachine, error) { - if s.vm != nil { - return s.vm, nil - } - +func (s *state) reloadVirtualMachine(ctx context.Context) error { var err error if s.vmip.Status.VirtualMachine != "" { vmKey := types.NamespacedName{Name: s.vmip.Status.VirtualMachine, Namespace: s.vmip.Namespace} vm, err := object.FetchObject(ctx, vmKey, s.client, &virtv2.VirtualMachine{}) if err != nil { - return nil, fmt.Errorf("unable to get VM %s: %w", vmKey, err) + return fmt.Errorf("unable to get VM %s: %w", vmKey, err) } if vm == nil { - return s.vm, nil + return nil } if vm.Status.VirtualMachineIPAddress == s.vmip.Name && vm.Status.IPAddress == s.vmip.Status.Address { @@ -166,7 +180,7 @@ func (s *state) VirtualMachine(ctx context.Context) (*virtv2.VirtualMachine, err Namespace: s.vmip.Namespace, }) if err != nil { - return nil, err + return err } for i, vm := range vms.Items { @@ -177,9 +191,5 @@ func (s *state) VirtualMachine(ctx context.Context) (*virtv2.VirtualMachine, err } } - return s.vm, nil -} - -func (s *state) AllocatedIPs() ip.AllocatedIPs { - return s.allocatedIPs + return nil } diff --git a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go index 42730c3d0c..32b2e48813 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/vmip_reconciler.go @@ -173,6 +173,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco } s := state.New(r.client, r.virtClient, vmip.Changed()) + err = s.Reload(ctx) + if err != nil { + if err.Error() == "VirtualMachineIPAddressLease found in kubeclient without cache" { + return reconcile.Result{}, nil + } + + return reconcile.Result{}, err + } + rec := reconciler.NewBaseReconciler[Handler](r.handlers) rec.SetHandlerExecutor(func(ctx context.Context, h Handler) (reconcile.Result, error) { return h.Handle(ctx, s) From 4eb54fe05b8de536f17346b6b048b275150a4dd0 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Mon, 12 May 2025 21:27:01 +0300 Subject: [PATCH 14/17] polishing 3 Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/state/state.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go index e93ae819e4..5d99424b36 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/state/state.go @@ -104,7 +104,7 @@ func (s *state) reloadVirtualMachineIPLease(ctx context.Context) error { } } - logger := logger.FromContext(ctx) + log := logger.FromContext(ctx) if s.lease == nil { leases := &virtv2.VirtualMachineIPAddressLeaseList{} @@ -117,7 +117,7 @@ func (s *state) reloadVirtualMachineIPLease(ctx context.Context) error { } if len(leases.Items) > 1 { - logger.Error("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) + log.Error("More than one VirtualMachineIPAddressLease found", "count", len(leases.Items)) } for i, lease := range leases.Items { @@ -139,9 +139,9 @@ func (s *state) reloadVirtualMachineIPLease(ctx context.Context) error { if len(leases.Items) != 0 { if len(leases.Items) > 1 { - logger.Error("More than one VirtualMachineIPAddressLease found in kubeclient without cache", "count", len(leases.Items)) + log.Error("More than one VirtualMachineIPAddressLease found in kubeclient without cache", "count", len(leases.Items)) } - logger.Warn("VirtualMachineIPAddressLease found in kubeclient without cache", "vmip", s.vmip.Name) + log.Warn("VirtualMachineIPAddressLease found in kubeclient without cache", "vmip", s.vmip.Name) return errors.New("VirtualMachineIPAddressLease found in kubeclient without cache") } } From 8184123a59e43906365d490c099a92752f569c3d Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin <93423466+LopatinDmitr@users.noreply.github.com> Date: Wed, 14 May 2025 10:54:27 +0300 Subject: [PATCH 15/17] Update images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go Co-authored-by: Yaroslav Borbat <86148689+yaroslavborbat@users.noreply.github.com> Signed-off-by: Dmitry Lopatin <93423466+LopatinDmitr@users.noreply.github.com> --- .../pkg/controller/vmip/internal/iplease_handler.go | 7 +------ 1 file changed, 1 insertion(+), 6 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 index 709f62933f..01d3102da8 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -104,12 +104,7 @@ func (h IPLeaseHandler) updateLease(ctx context.Context, lease *virtv2.VirtualMa } lease.Labels[annotations.LabelVirtualMachineIPAddressUID] = string(vmip.GetUID()) - err := h.client.Update(ctx, lease) - if err != nil { - return reconcile.Result{}, err - } - - return reconcile.Result{}, nil + return reconcile.Result{}, h.client.Update(ctx, lease) } func (h IPLeaseHandler) createNewLease(ctx context.Context, state state.VMIPState) (reconcile.Result, error) { From 72496d71967b27ecfc9ffd4150776432078b0528 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Wed, 14 May 2025 17:51:40 +0300 Subject: [PATCH 16/17] fix after review Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/lifecycle_handler.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go index b23ab81ee8..750dc1f066 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/lifecycle_handler.go @@ -51,7 +51,8 @@ func NewLifecycleHandler(recorder eventrecord.EventRecorderLogger) *LifecycleHan func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (reconcile.Result, error) { log := logger.FromContext(ctx).With(logger.SlogHandler(LifecycleHandlerName)) - vmip, vm := state.VirtualMachineIP(), state.VirtualMachine() + vmip := state.VirtualMachineIP() + vm := state.VirtualMachine() conditionBound := conditions.NewConditionBuilder(vmipcondition.BoundType). Generation(vmip.GetGeneration()). From 70d9c0a9a69bfbc41307cc8f8b585a5da1d2bd00 Mon Sep 17 00:00:00 2001 From: Dmitry Lopatin Date: Wed, 14 May 2025 17:53:47 +0300 Subject: [PATCH 17/17] fix after review Signed-off-by: Dmitry Lopatin --- .../pkg/controller/vmip/internal/iplease_handler.go | 2 +- .../pkg/controller/vmiplease/internal/retention_handler.go | 2 +- 2 files changed, 2 insertions(+), 2 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 index 01d3102da8..4d611cba96 100644 --- a/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmip/internal/iplease_handler.go @@ -102,7 +102,7 @@ func (h IPLeaseHandler) updateLease(ctx context.Context, lease *virtv2.VirtualMa Name: vmip.Name, Namespace: vmip.Namespace, } - lease.Labels[annotations.LabelVirtualMachineIPAddressUID] = string(vmip.GetUID()) + annotations.AddLabel(lease, annotations.LabelVirtualMachineIPAddressUID, string(vmip.GetUID())) return reconcile.Result{}, h.client.Update(ctx, lease) } 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 d5e9ba4b99..10d6fff63b 100644 --- a/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go +++ b/images/virtualization-artifact/pkg/controller/vmiplease/internal/retention_handler.go @@ -57,7 +57,7 @@ func (h *RetentionHandler) Handle(ctx context.Context, state state.VMIPLeaseStat if lease.Spec.VirtualMachineIPAddressRef.Name != "" || lease.Labels[annotations.LabelVirtualMachineIPAddressUID] != "" { log.Debug("VirtualMachineIP not found: remove this ref from the spec, delete label value and retain VMIPLease") lease.Spec.VirtualMachineIPAddressRef.Name = "" - lease.Labels[annotations.LabelVirtualMachineIPAddressUID] = "" + annotations.AddLabel(lease, annotations.LabelVirtualMachineIPAddressUID, "") return reconcile.Result{RequeueAfter: h.retentionDuration}, nil }