fix(vmip): refactor vmip and vmiplease controllers (#1081)

Refactored the controllers for vmip and vmiplease.

vmip:

- Introduced a new handler, BoundHandler, with a Steps-based architecture and three corresponding steps for managing the respective condition;
- Added a new handler, AttachedHandler, for managing the respective condition;
- The indexer IndexVMIPByVM now also looks at ownerRef;
- Removed the reconciliationstate;
- Removed the IPLeaseHandler. Management of the Lease resource is now handled by BoundHandler;
- Watchers have been moved to internal/watchers, and the event subscriptions and filtering have been updated;
- LifeCycleHandler now only sets the phase;
- The IPAddressService has become an internal service of vmip.

vmiplease:

- The order of handlers has changed;
- Removed the reconciliation state;
- Watchers have been moved to internal/watchers, and the event subscriptions and filtering have been updated;
- Resource deletion has been moved to RetentionHandler.

Signed-off-by: Isteb4k <dmitry.rakitin@flant.com>

Author: Isteb4k

api/core/v1alpha2/events.go

@@ -124,6 +124,8 @@ const (
 	// ReasonNotAttached is event reason that VirtualMachineIPAddress is not attached to VirtualMachine.
 	ReasonNotAttached = "NotAttached"
 
+	// ReasonIPAddressHasBeenAllocated is the event reason indicating that a new IP address has been allocated.
+	ReasonIPAddressHasBeenAllocated = "IPAddressHasBeenAllocated"
 	// ReasonBound is the event reason indicating that a VirtualMachineIPLease is bound to a VirtualMachineIPAddress.
 	ReasonBound = "Bound"
 	// ReasonFailed is the event reason indicating that the binding of a VirtualMachineIPLease to a VirtualMachineIPAddress has failed.

api/core/v1alpha2/vmipcondition/condition.go

@@ -49,25 +49,19 @@ func (r AttachedReason) String() string {
 const (
 	// VirtualMachineIPAddressIsOutOfTheValidRange is a BoundReason indicating when specified IP address is out of the range in controller settings.
 	VirtualMachineIPAddressIsOutOfTheValidRange BoundReason = "VirtualMachineIPAddressIsOutOfTheValidRange"
-
 	// 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"
-
 	// VirtualMachineIPAddressLeaseNotFound is a BoundReason indicating the IP address lease was not found.
 	VirtualMachineIPAddressLeaseNotFound BoundReason = "VirtualMachineIPAddressLeaseNotFound"
-
 	// VirtualMachineIPAddressLeaseNotReady is a BoundReason indicating the IP address lease was not ready.
 	VirtualMachineIPAddressLeaseNotReady BoundReason = "VirtualMachineIPAddressLeaseNotReady"
-
 	// Bound is a BoundReason indicating the IP address lease is successfully bound.
 	Bound BoundReason = "Bound"
 
 	// VirtualMachineNotFound is an AttachedReason indicating the Virtual Machine was not found.
 	VirtualMachineNotFound AttachedReason = "VirtualMachineNotFound"
-
 	// Attached is an AttachedReason indicating the IP address was successfully attached to the Virtual Machine.
 	Attached AttachedReason = "Attached"
 )

images/virtualization-artifact/pkg/common/blockdevice/runner.go

@@ -26,7 +26,7 @@ import (
 )
 
 type Resource interface {
-	*virtv2.VirtualDisk | *virtv2.VirtualImage
+	*virtv2.VirtualDisk | *virtv2.VirtualImage | *virtv2.VirtualMachineIPAddress
 }
 
 type StepTaker[R Resource] interface {

images/virtualization-artifact/pkg/common/ip/ip.go

@@ -19,15 +19,13 @@ package ip
 import (
 	"net"
 	"strings"
-
-	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
 
 const ipPrefix = "ip-"
 
-type AllocatedIPs map[string]*virtv2.VirtualMachineIPAddressLease
+type AllocatedIPs map[string]struct{}
 
-// IpToLeaseName generate the Virtual Machine IP Address Lease's name from the ip address
+// IpToLeaseName generate the Virtual Machine IP Address Lease's name from the ip address.
 func IpToLeaseName(ip string) string {
 	addr := net.ParseIP(ip)
 	if addr.To4() != nil {
@@ -38,7 +36,7 @@ func IpToLeaseName(ip string) string {
 	return ""
 }
 
-// LeaseNameToIP generate the ip address from the Virtual Machine IP Address Lease's name
+// LeaseNameToIP generate the ip address from the Virtual Machine IP Address Lease's name.
 func LeaseNameToIP(leaseName string) string {
 	if strings.HasPrefix(leaseName, ipPrefix) && len(leaseName) > len(ipPrefix) {
 		return strings.ReplaceAll(leaseName[len(ipPrefix):], "-", ".")

images/virtualization-artifact/pkg/controller/indexer/vmip_indexer.go

@@ -28,7 +28,25 @@ func IndexVMIPByVM() (obj client.Object, field string, extractValue client.Index
 		if !ok || vmip == nil {
 			return nil
 		}
-		return []string{vmip.Status.VirtualMachine}
+
+		var vmNames []string
+		if vmip.Status.VirtualMachine != "" {
+			vmNames = append(vmNames, vmip.Status.VirtualMachine)
+		}
+
+		for _, ownerRef := range vmip.OwnerReferences {
+			if ownerRef.Kind != virtv2.VirtualMachineKind {
+				continue
+			}
+
+			if ownerRef.Name == "" || ownerRef.Name == vmip.Status.VirtualMachine {
+				continue
+			}
+
+			vmNames = append(vmNames, ownerRef.Name)
+		}
+
+		return vmNames
 	}
 }
 

images/virtualization-artifact/pkg/controller/ipam/ipam.go

@@ -25,7 +25,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/deckhouse/virtualization-controller/pkg/common/annotations"
+	"github.com/deckhouse/virtualization-controller/pkg/controller/conditions"
 	virtv2 "github.com/deckhouse/virtualization/api/core/v1alpha2"
+	"github.com/deckhouse/virtualization/api/core/v1alpha2/vmipcondition"
 )
 
 const AnnoIPAddressCNIRequest = "cni.cilium.io/ipAddress"
@@ -41,7 +43,13 @@ func (m IPAM) IsBound(vmName string, vmip *virtv2.VirtualMachineIPAddress) bool
 		return false
 	}
 
-	if vmip.Status.Phase != virtv2.VirtualMachineIPAddressPhaseBound && vmip.Status.Phase != virtv2.VirtualMachineIPAddressPhaseAttached {
+	boundCondition, _ := conditions.GetCondition(vmipcondition.BoundType, vmip.Status.Conditions)
+	if boundCondition.Status != metav1.ConditionTrue || !conditions.IsLastUpdated(boundCondition, vmip) {
+		return false
+	}
+
+	attachedCondition, _ := conditions.GetCondition(vmipcondition.AttachedType, vmip.Status.Conditions)
+	if attachedCondition.Status != metav1.ConditionTrue || !conditions.IsLastUpdated(attachedCondition, vmip) {
 		return false
 	}
 

images/virtualization-artifact/pkg/controller/reconciler/resource.go

@@ -210,15 +210,14 @@ func (r *Resource[T, ST]) Update(ctx context.Context) error {
 			return nil
 		}
 
-		return fmt.Errorf("error patching metadata: %w", err)
+		return fmt.Errorf("error patching metadata (%s): %w", string(metadataPatchBytes), err)
 	}
 
 	return nil
 }
 
 func (r *Resource[T, ST]) JSONPatchOpsForFinalizers() []patch.JsonPatchOperation {
 	return []patch.JsonPatchOperation{
-		patch.NewJsonPatchOperation(patch.PatchTestOp, "/metadata/finalizers", r.currentObj.GetFinalizers()),
 		patch.NewJsonPatchOperation(patch.PatchReplaceOp, "/metadata/finalizers", r.changedObj.GetFinalizers()),
 	}
 }

images/virtualization-artifact/pkg/controller/service/errors.go

@@ -26,8 +26,3 @@ var (
 	ErrDataVolumeNotRunning               = errors.New("pvc importer is not running")
 	ErrDataVolumeProvisionerUnschedulable = errors.New("provisioner unschedulable")
 )
-
-var (
-	ErrIPAddressAlreadyExist = errors.New("the IP address is already allocated")
-	ErrIPAddressOutOfRange   = errors.New("the IP address is out of range")
-)

images/virtualization-artifact/pkg/controller/service/ip_address_service.go

@@ -25,17 +25,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/deckhouse/virtualization-controller/pkg/common/object"
-	"github.com/deckhouse/virtualization-controller/pkg/controller/vm/internal"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 )
 
 type IPAMValidator struct {
-	ipam   internal.IPAM
 	client client.Client
 }
 
-func NewIPAMValidator(ipam internal.IPAM, client client.Client) *IPAMValidator {
-	return &IPAMValidator{ipam: ipam, client: client}
+func NewIPAMValidator(client client.Client) *IPAMValidator {
+	return &IPAMValidator{client: client}
 }
 
 func (v *IPAMValidator) ValidateCreate(ctx context.Context, vm *v1alpha2.VirtualMachine) (admission.Warnings, error) {
@@ -60,7 +58,7 @@ func (v *IPAMValidator) ValidateCreate(ctx context.Context, vm *v1alpha2.Virtual
 			" already exists: set spec.virtualMachineIPAddress field to %s to use IP %s", vmip.Name, vmip.Status.Address)
 	}
 
-	return nil, v.ipam.CheckIpAddressAvailableForBinding(vm.Name, vmip)
+	return nil, nil
 }
 
 func (v *IPAMValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alpha2.VirtualMachine) (admission.Warnings, error) {
@@ -82,5 +80,5 @@ func (v *IPAMValidator) ValidateUpdate(ctx context.Context, oldVM, newVM *v1alph
 		return nil, nil
 	}
 
-	return nil, v.ipam.CheckIpAddressAvailableForBinding(newVM.Name, vmip)
+	return nil, nil
 }