Reflect NICs defined in the VirtualMachine resource to the corresponding VirtualMachineNetworkConfig resource (backport #43)

chart/crds/network.harvesterhci.io_virtualmachinenetworkconfigs.yaml

@@ -24,6 +24,12 @@ spec:
     - jsonPath: .status.conditions[?(@.type=='Allocated')].status
       name: ALLOCATED
       type: string
+    - jsonPath: .status.conditions[?(@.type=='Disabled')].status
+      name: DISABLED
+      type: string
+    - jsonPath: .status.conditions[?(@.type=='InSynced')].status
+      name: INSYNCED
+      type: string
     - jsonPath: .metadata.creationTimestamp
       name: AGE
       type: date
@@ -68,9 +74,6 @@ spec:
                   type: object
                 maxItems: 4
                 type: array
-                x-kubernetes-validations:
-                - message: NetworkConfig may only be added
-                  rule: oldSelf.all(x, x in self)
               paused:
                 type: boolean
               vmName:

pkg/apis/network.harvesterhci.io/v1alpha1/virtualmachinenetworkconfig.go

@@ -9,11 +9,13 @@ import (
 const (
 	AllocatedState NetworkConfigState = "Allocated"
 	PendingState   NetworkConfigState = "Pending"
+	StaleState     NetworkConfigState = "Stale"
 )
 
 var (
 	Allocated condition.Cond = "Allocated"
 	Disabled  condition.Cond = "Disabled"
+	InSynced  condition.Cond = "InSynced"
 )
 
 type NetworkConfigState string
@@ -24,6 +26,8 @@ type NetworkConfigState string
 // +kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="VMNAME",type=string,JSONPath=`.spec.vmName`
 // +kubebuilder:printcolumn:name="ALLOCATED",type=string,JSONPath=`.status.conditions[?(@.type=='Allocated')].status`
+// +kubebuilder:printcolumn:name="DISABLED",type=string,JSONPath=`.status.conditions[?(@.type=='Disabled')].status`
+// +kubebuilder:printcolumn:name="INSYNCED",type=string,JSONPath=`.status.conditions[?(@.type=='InSynced')].status`
 // +kubebuilder:printcolumn:name="AGE",type="date",JSONPath=`.metadata.creationTimestamp`
 
 type VirtualMachineNetworkConfig struct {
@@ -42,7 +46,6 @@ type VirtualMachineNetworkConfigSpec struct {
 
 	// +optional
 	// +kubebuilder:validation:Optional
-	// +kubebuilder:validation:XValidation:rule="oldSelf.all(x, x in self)",message="NetworkConfig may only be added"
 	// +kubebuilder:validation:MaxItems=4
 	NetworkConfigs []NetworkConfig `json:"networkConfigs,omitempty"`
 

pkg/controller/vm/controller.go

@@ -5,6 +5,7 @@ import (
 	"reflect"
 
 	"github.com/sirupsen/logrus"
+	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -23,6 +24,7 @@ const (
 )
 
 type Handler struct {
+	vmController   ctlkubevirtv1.VirtualMachineController
 	vmClient       ctlkubevirtv1.VirtualMachineClient
 	vmCache        ctlkubevirtv1.VirtualMachineCache
 	vmnetcfgClient ctlnetworkv1.VirtualMachineNetworkConfigClient
@@ -34,6 +36,7 @@ func Register(ctx context.Context, management *config.Management) error {
 	vmnetcfgs := management.HarvesterNetworkFactory.Network().V1alpha1().VirtualMachineNetworkConfig()
 
 	handler := &Handler{
+		vmController:   vms,
 		vmClient:       vms,
 		vmCache:        vms.Cache(),
 		vmnetcfgClient: vmnetcfgs,
@@ -103,11 +106,33 @@ func (h *Handler) OnChange(key string, vm *kubevirtv1.VirtualMachine) (*kubevirt
 	vmNetCfgCpy := oldVmNetCfg.DeepCopy()
 	vmNetCfgCpy.Spec.NetworkConfigs = vmNetCfg.Spec.NetworkConfigs
 
-	if !reflect.DeepEqual(vmNetCfgCpy, oldVmNetCfg) {
-		logrus.Infof("(vm.OnChange) update vmnetcfg %s/%s", vmNetCfgCpy.Namespace, vmNetCfgCpy.Name)
-		if _, err := h.vmnetcfgClient.Update(vmNetCfgCpy); err != nil {
+	// The following block is a two-step process. Ideally,
+	// 1. if the network config of the VirtualMachine has been changed, update the status of the VirtualMachineNetworkConfig
+	//   to out-of-sync so that the vmnetcfg-controller can handle it accordingly, and
+	// 2. since the spec of the VirtualMachineNetworkConfig hasn't been changed, update it to reflect the new network config.
+	// This is to throttle the vmnetcfg-controller and to avoid allocate-before-deallocate from happening.
+	if !reflect.DeepEqual(vmNetCfgCpy.Spec.NetworkConfigs, oldVmNetCfg.Spec.NetworkConfigs) {
+		if networkv1.InSynced.IsFalse(oldVmNetCfg) {
+			logrus.Infof("(vm.OnChange) vmnetcfg %s/%s is deemed out-of-sync, updating it", vmNetCfgCpy.Namespace, vmNetCfgCpy.Name)
+			if _, err := h.vmnetcfgClient.Update(vmNetCfgCpy); err != nil {
+				return vm, err
+			}
+			return vm, nil
+		}
+
+		logrus.Infof("(vm.OnChange) update vmnetcfg %s/%s status as out-of-sync due to network config changes", vmNetCfgCpy.Namespace, vmNetCfgCpy.Name)
+
+		// Mark the VirtualMachineNetworkConfig as out-of-sync so that the vmnetcfg-controller can handle it accordingly
+		networkv1.InSynced.SetStatus(vmNetCfgCpy, string(corev1.ConditionFalse))
+		networkv1.InSynced.Reason(vmNetCfgCpy, "NetworkConfigChanged")
+		networkv1.InSynced.Message(vmNetCfgCpy, "Network configuration of the upstrem virtual machine has been changed")
+
+		if _, err := h.vmnetcfgClient.UpdateStatus(vmNetCfgCpy); err != nil {
 			return vm, err
 		}
+
+		// Enqueue the VirtualMachine in order to update the network config of its corresponding VirtualMachineNetworkConfig
+		h.vmController.Enqueue(vm.Namespace, vm.Name)
 	}
 
 	return vm, nil

pkg/controller/vmnetcfg/common.go

@@ -7,6 +7,12 @@ import (
 	networkv1 "github.com/harvester/vm-dhcp-controller/pkg/apis/network.harvesterhci.io/v1alpha1"
 )
 
+func updateAllNetworkConfigState(ncStatuses []networkv1.NetworkConfigStatus, state networkv1.NetworkConfigState) {
+	for i := range ncStatuses {
+		ncStatuses[i].State = state
+	}
+}
+
 func setAllocatedCondition(vmNetCfg *networkv1.VirtualMachineNetworkConfig, status corev1.ConditionStatus, reason, message string) {
 	networkv1.Allocated.SetStatus(vmNetCfg, string(status))
 	networkv1.Allocated.Reason(vmNetCfg, reason)
@@ -19,6 +25,12 @@ func setDisabledCondition(vmNetCfg *networkv1.VirtualMachineNetworkConfig, statu
 	networkv1.Disabled.Message(vmNetCfg, message)
 }
 
+func setInSyncedCondition(vmNetCfg *networkv1.VirtualMachineNetworkConfig, status corev1.ConditionStatus, reason, message string) {
+	networkv1.InSynced.SetStatus(vmNetCfg, string(status))
+	networkv1.InSynced.Reason(vmNetCfg, reason)
+	networkv1.InSynced.Message(vmNetCfg, message)
+}
+
 type vmNetCfgBuilder struct {
 	vmNetCfg *networkv1.VirtualMachineNetworkConfig
 }
@@ -79,6 +91,11 @@ func (b *vmNetCfgBuilder) DisabledCondition(status corev1.ConditionStatus, reaso
 	return b
 }
 
+func (b *vmNetCfgBuilder) InSyncedCondition(status corev1.ConditionStatus, reason, message string) *vmNetCfgBuilder {
+	setInSyncedCondition(b.vmNetCfg, status, reason, message)
+	return b
+}
+
 func (b *vmNetCfgBuilder) Build() *networkv1.VirtualMachineNetworkConfig {
 	return b.vmNetCfg
 }
@@ -104,6 +121,13 @@ func (b *vmNetCfgStatusBuilder) WithNetworkConfigStatus(ipAddress, macAddress, n
 	return b
 }
 
+func (b *vmNetCfgStatusBuilder) InSyncedCondition(status corev1.ConditionStatus, reason, message string) *vmNetCfgStatusBuilder {
+	networkv1.InSynced.SetStatus(&b.vmNetCfgStatus, string(status))
+	networkv1.InSynced.Reason(&b.vmNetCfgStatus, reason)
+	networkv1.InSynced.Message(&b.vmNetCfgStatus, message)
+	return b
+}
+
 func (b *vmNetCfgStatusBuilder) Build() networkv1.VirtualMachineNetworkConfigStatus {
 	return b.vmNetCfgStatus
 }