From 733fe3d799b3e072e9590390f830828f20755fc4 Mon Sep 17 00:00:00 2001 From: Zespre Chang Date: Sat, 20 Jan 2024 21:46:02 +0800 Subject: [PATCH] feat(controller): support vmnetcfg disabled mode When an VirtualMachineNetworkConfig is disabled, it will free up the slot it occupied in IPAM and remove the MAC cache entry. Also, the referencing IPPool's status will be updated. However, the allocation record still remain on its status field. When the VirtualMachineNetworkConfig is enabled again, the same IP address will be allocated. Signed-off-by: Zespre Chang --- .../v1alpha1/virtualmachinenetworkconfig.go | 23 ++-- pkg/controller/vmnetcfg/controller.go | 102 ++++++++++++++---- pkg/metrics/metrics.go | 8 +- 3 files changed, 104 insertions(+), 29 deletions(-) diff --git a/pkg/apis/network.harvesterhci.io/v1alpha1/virtualmachinenetworkconfig.go b/pkg/apis/network.harvesterhci.io/v1alpha1/virtualmachinenetworkconfig.go index 61b9fd8..3aefb71 100644 --- a/pkg/apis/network.harvesterhci.io/v1alpha1/virtualmachinenetworkconfig.go +++ b/pkg/apis/network.harvesterhci.io/v1alpha1/virtualmachinenetworkconfig.go @@ -6,11 +6,18 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + AllocatedState NetworkConfigState = "Allocated" + PendingState NetworkConfigState = "Pending" +) + var ( Allocated condition.Cond = "Allocated" Disabled condition.Cond = "Disabled" ) +type NetworkConfigState string + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +kubebuilder:resource:shortName=vmnetcfg;vmnetcfgs,scope=Namespaced @@ -29,25 +36,29 @@ type VirtualMachineNetworkConfig struct { type VirtualMachineNetworkConfigSpec struct { VMName string `json:"vmName,omitempty"` NetworkConfig []NetworkConfig `json:"networkConfig,omitempty"` + + // +optional + Paused *bool `json:"paused,omitempty"` } type NetworkConfig struct { NetworkName string `json:"networkName,omitempty"` MACAddress string `json:"macAddress,omitempty"` + // +optional IPAddress *string `json:"ipAddress,omitempty"` } type VirtualMachineNetworkConfigStatus struct { NetworkConfig []NetworkConfigStatus `json:"networkConfig,omitempty"` - // Conditions is a list of Wrangler conditions that describe the state - // of the VirtualMachineNetworkConfigStatus. + + // +optional Conditions []genericcondition.GenericCondition `json:"conditions,omitempty"` } type NetworkConfigStatus struct { - AllocatedIPAddress string `json:"allocatedIPAddress,omitempty"` - MACAddress string `json:"macAddress,omitempty"` - NetworkName string `json:"networkName,omitempty"` - Status string `json:"status,omitempty"` + AllocatedIPAddress string `json:"allocatedIPAddress,omitempty"` + MACAddress string `json:"macAddress,omitempty"` + NetworkName string `json:"networkName,omitempty"` + State NetworkConfigState `json:"state,omitempty"` } diff --git a/pkg/controller/vmnetcfg/controller.go b/pkg/controller/vmnetcfg/controller.go index bcd1e9f..7b7e428 100644 --- a/pkg/controller/vmnetcfg/controller.go +++ b/pkg/controller/vmnetcfg/controller.go @@ -59,14 +59,50 @@ func Register(ctx context.Context, management *config.Management) error { handler.Allocate, ) + vmnetcfgs.OnChange(ctx, controllerName, handler.OnChange) vmnetcfgs.OnRemove(ctx, controllerName, handler.OnRemove) return nil } +func (h *Handler) OnChange(key string, vmNetCfg *networkv1.VirtualMachineNetworkConfig) (*networkv1.VirtualMachineNetworkConfig, error) { + if vmNetCfg == nil || vmNetCfg.DeletionTimestamp != nil { + return nil, nil + } + + logrus.Debugf("(vmnetcfg.OnChange) vmnetcfg configuration %s has been changed: %+v", key, vmNetCfg.Spec.NetworkConfig) + + vmNetCfgCpy := vmNetCfg.DeepCopy() + + // Check if the VirtualMachineNetworkConfig is administratively disabled + if vmNetCfg.Spec.Paused != nil && *vmNetCfg.Spec.Paused { + logrus.Infof("(vmnetcfg.OnChange) try to cleanup ipam and cache, and update ippool status for vmnetcfg %s", key) + if err := h.cleanup(vmNetCfg); err != nil { + return vmNetCfg, err + } + networkv1.Disabled.True(vmNetCfgCpy) + updateAllNetworkConfigState(vmNetCfgCpy.Status.NetworkConfig) + if !reflect.DeepEqual(vmNetCfgCpy, vmNetCfg) { + return h.vmnetcfgClient.UpdateStatus(vmNetCfgCpy) + } + return vmNetCfg, nil + } + networkv1.Disabled.False(vmNetCfgCpy) + + if !reflect.DeepEqual(vmNetCfgCpy, vmNetCfg) { + return h.vmnetcfgClient.UpdateStatus(vmNetCfgCpy) + } + + return vmNetCfg, nil +} + func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, status networkv1.VirtualMachineNetworkConfigStatus) (networkv1.VirtualMachineNetworkConfigStatus, error) { logrus.Debugf("(vmnetcfg.Allocate) allocate ip for vmnetcfg %s/%s", vmNetCfg.Namespace, vmNetCfg.Name) + if vmNetCfg.Spec.Paused != nil && *vmNetCfg.Spec.Paused { + return status, fmt.Errorf("vmnetcfg %s/%s was administratively disabled", vmNetCfg.Namespace, vmNetCfg.Name) + } + var ncStatuses []networkv1.NetworkConfigStatus for _, nc := range vmNetCfg.Spec.NetworkConfig { exists, err := h.cacheAllocator.HasMAC(nc.NetworkName, nc.MACAddress) @@ -75,6 +111,7 @@ func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, stat } if exists { // Recover IP from cache + ip, err := h.cacheAllocator.GetIPByMAC(nc.NetworkName, nc.MACAddress) if err != nil { return status, err @@ -85,7 +122,7 @@ func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, stat AllocatedIPAddress: ip, MACAddress: nc.MACAddress, NetworkName: nc.NetworkName, - Status: "Allocated", + State: networkv1.AllocatedState, } ncStatuses = append(ncStatuses, ncStatus) @@ -95,7 +132,7 @@ func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, stat ncStatus.NetworkName, ncStatus.MACAddress, ncStatus.AllocatedIPAddress, - ncStatus.Status, + string(ncStatus.State), ) // Update IPPool status @@ -139,10 +176,15 @@ func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, stat } // Allocate new IP + dIP := util.UnspecifiedIPAddress if nc.IPAddress != nil { dIP = *nc.IPAddress } + // Recover IP from status (resume from paused state) + if oIP, err := findIPAddressFromNetworkConfigStatusByMACAddress(vmNetCfg.Status.NetworkConfig, nc.MACAddress); err == nil { + dIP = oIP + } ip, err := h.ipAllocator.AllocateIP(nc.NetworkName, dIP) if err != nil { @@ -158,7 +200,7 @@ func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, stat AllocatedIPAddress: ip, MACAddress: nc.MACAddress, NetworkName: nc.NetworkName, - Status: "Allocated", + State: networkv1.AllocatedState, } ncStatuses = append(ncStatuses, ncStatus) @@ -168,7 +210,7 @@ func (h *Handler) Allocate(vmNetCfg *networkv1.VirtualMachineNetworkConfig, stat ncStatus.NetworkName, ncStatus.MACAddress, ncStatus.AllocatedIPAddress, - ncStatus.Status, + string(ncStatus.State), ) // Update IPPool status @@ -221,32 +263,40 @@ func (h *Handler) OnRemove(key string, vmNetCfg *networkv1.VirtualMachineNetwork logrus.Debugf("(vmnetcfg.OnRemove) vmnetcfg configuration %s/%s has been removed", vmNetCfg.Namespace, vmNetCfg.Name) - h.metricsAllocator.DeleteVmNetCfgStatus(key) + if err := h.cleanup(vmNetCfg); err != nil { + return vmNetCfg, err + } + + return vmNetCfg, nil +} + +func (h *Handler) cleanup(vmNetCfg *networkv1.VirtualMachineNetworkConfig) error { + h.metricsAllocator.DeleteVmNetCfgStatus(vmNetCfg.Namespace + "/" + vmNetCfg.Name) - for _, nc := range vmNetCfg.Status.NetworkConfig { + for _, ncStatus := range vmNetCfg.Status.NetworkConfig { // Deallocate IP address from IPAM - isAllocated, err := h.ipAllocator.IsAllocated(nc.NetworkName, nc.AllocatedIPAddress) + isAllocated, err := h.ipAllocator.IsAllocated(ncStatus.NetworkName, ncStatus.AllocatedIPAddress) if err != nil { - return vmNetCfg, err + return err } if isAllocated { - if err := h.ipAllocator.DeallocateIP(nc.NetworkName, nc.AllocatedIPAddress); err != nil { - return vmNetCfg, err + if err := h.ipAllocator.DeallocateIP(ncStatus.NetworkName, ncStatus.AllocatedIPAddress); err != nil { + return err } } // Remove entry from cache - exists, err := h.cacheAllocator.HasMAC(nc.NetworkName, nc.MACAddress) + exists, err := h.cacheAllocator.HasMAC(ncStatus.NetworkName, ncStatus.MACAddress) if err != nil { - return vmNetCfg, err + return err } if exists { - if err := h.cacheAllocator.DeleteMAC(nc.NetworkName, nc.MACAddress); err != nil { - return vmNetCfg, err + if err := h.cacheAllocator.DeleteMAC(ncStatus.NetworkName, ncStatus.MACAddress); err != nil { + return err } } - ipPoolNamespace, ipPoolName := kv.RSplit(nc.NetworkName, "/") + ipPoolNamespace, ipPoolName := kv.RSplit(ncStatus.NetworkName, "/") if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { ipPool, err := h.ippoolCache.Get(ipPoolNamespace, ipPoolName) if err != nil { @@ -256,10 +306,10 @@ func (h *Handler) OnRemove(key string, vmNetCfg *networkv1.VirtualMachineNetwork ipPoolCpy := ipPool.DeepCopy() // Remove record in IPPool status - delete(ipPoolCpy.Status.IPv4.Allocated, nc.AllocatedIPAddress) + delete(ipPoolCpy.Status.IPv4.Allocated, ncStatus.AllocatedIPAddress) if !reflect.DeepEqual(ipPoolCpy, ipPool) { - logrus.Infof("(vmnetcfg.OnRemove) update ippool %s/%s", ipPool.Namespace, ipPool.Name) + logrus.Infof("(vmnetcfg.cleanup) update ippool %s/%s", ipPool.Namespace, ipPool.Name) ipPoolCpy.Status.LastUpdate = metav1.Now() _, err := h.ippoolClient.UpdateStatus(ipPoolCpy) return err @@ -267,9 +317,23 @@ func (h *Handler) OnRemove(key string, vmNetCfg *networkv1.VirtualMachineNetwork return nil }); err != nil { - return vmNetCfg, err + return err } } + return nil +} - return vmNetCfg, nil +func findIPAddressFromNetworkConfigStatusByMACAddress(ncStatuses []networkv1.NetworkConfigStatus, macAddress string) (ipAddress string, err error) { + for _, ncStatus := range ncStatuses { + if ncStatus.MACAddress == macAddress && ncStatus.AllocatedIPAddress != "" { + return ncStatus.AllocatedIPAddress, nil + } + } + return util.UnspecifiedIPAddress, fmt.Errorf("could not find allocated ip for mac %s", macAddress) +} + +func updateAllNetworkConfigState(ncStatuses []networkv1.NetworkConfigStatus) { + for i := range ncStatuses { + ncStatuses[i].State = networkv1.PendingState + } } diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 2a89522..d859d4f 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -16,7 +16,7 @@ var ( LabelVmNetCfgName = "vmnetcfg" LabelMACAddress = "mac" LabelIPAddress = "ip" - LabelStatus = "status" + LabelState = "state" ) type MetricsAllocator struct { @@ -60,7 +60,7 @@ func NewMetricsAllocator() *MetricsAllocator { LabelNetworkName, LabelMACAddress, LabelIPAddress, - LabelStatus, + LabelState, }, ), } @@ -103,13 +103,13 @@ func (a *MetricsAllocator) DeleteIPPool(name string, cidr string, networkName st }) } -func (a *MetricsAllocator) UpdateVmNetCfgStatus(name, networkName, macAddress, ipAddress, status string) { +func (a *MetricsAllocator) UpdateVmNetCfgStatus(name, networkName, macAddress, ipAddress, state string) { a.vmNetCfgStatus.With(prometheus.Labels{ LabelVmNetCfgName: name, LabelNetworkName: networkName, LabelMACAddress: macAddress, LabelIPAddress: ipAddress, - LabelStatus: status, + LabelState: state, }).Set(float64(1)) }