Skip to content

Commit

Permalink
fix(vmip): fix bug with create VirtualMachineIPAddress in different n…
Browse files Browse the repository at this point in the history
…amespace, when VirtualMahineIPAddressLease 'Released' (#296)

* fix(vmip): fix bug with create VirtualMachineIPAddress in different namespace, when VirtualMahineIPAddressLease 'Released'

- Add reque after 30s
- Fix message in recording event
- Add log for case with create VirtualMachineIPAddress in different namespace

Signed-off-by: Dmitry Lopatin <[email protected]>
  • Loading branch information
LopatinDmitr authored Aug 19, 2024
1 parent 4392f0f commit 4425e79
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 15 deletions.
4 changes: 2 additions & 2 deletions api/core/v1alpha2/vmipcondition/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ const (
// VirtualMachineIPAddressIsOutOfTheValidRange is a BoundReason indicating when specified IP address is out of the range in controller settings.
VirtualMachineIPAddressIsOutOfTheValidRange BoundReason = "VirtualMachineIPAddressIsOutOfTheValidRange"

// VirtualMachineIPAddressLeaseAlready is a BoundReason indicating the IP address lease already exists.
VirtualMachineIPAddressLeaseAlready BoundReason = "VirtualMachineIPAddressLeaseAlready"
// VirtualMachineIPAddressLeaseAlreadyExists is a BoundReason indicating the IP address lease already exists.
VirtualMachineIPAddressLeaseAlreadyExists BoundReason = "VirtualMachineIPAddressLeaseAlreadyExists"

// VirtualMachineIPAddressLeaseLost is a BoundReason indicating the IP address lease was lost.
VirtualMachineIPAddressLeaseLost BoundReason = "VirtualMachineIPAddressLeaseLost"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.VirtualMachineIPAddressLeaseAlreadyExists:
log.Info("Lease by name not found: waiting for the lease to be available")
return reconcile.Result{}, nil

Expand All @@ -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.VirtualMachineIPAddressLeaseAlreadyExists, "The VirtualMachineIPLease belongs to a different namespace")

return reconcile.Result{}, nil
}

Expand Down Expand Up @@ -149,11 +150,11 @@ func (h IPLeaseHandler) createNewLease(ctx context.Context, state state.VMIPStat
case errors.Is(err, service.ErrIPAddressAlreadyExist):
vmipStatus.Phase = virtv2.VirtualMachineIPAddressPhasePending
mgr.Update(conditionBound.Status(metav1.ConditionFalse).
Reason(vmipcondition.VirtualMachineIPAddressLeaseAlready).
Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIP",
Reason(vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists).
Message(fmt.Sprintf("VirtualMachineIPAddressLease %s is bound to another VirtualMachineIPAddress",
common.IpToLeaseName(vmipStatus.Address))).
Condition())
h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseAlready, msg)
h.recorder.Event(vmip, corev1.EventTypeWarning, vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists, msg)
}

vmipStatus.Conditions = mgr.Generate()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -76,6 +77,7 @@ func (h *LifecycleHandler) Handle(ctx context.Context, state state.VMIPState) (r
return reconcile.Result{}, err
}

needRequeue := false
switch {
case lease == nil && vmipStatus.Address != "":
if vmipStatus.Phase != virtv2.VirtualMachineIPAddressPhasePending {
Expand Down Expand Up @@ -108,29 +110,45 @@ 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))).
Reason(vmipcondition.VirtualMachineIPAddressLeaseAlreadyExists).
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.VirtualMachineIPAddressLeaseAlreadyExists).
Message(fmt.Sprintf("The VirtualMachineIPLease %s belongs to a different namespace", lease.Name)).
Condition())
}
needRequeue = 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 !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) Name() string {
Expand Down

0 comments on commit 4425e79

Please sign in to comment.