Filter vmi explicity when query them

Signed-off-by: Jian Wang <[email protected]>

pkg/utils/vmi.go

@@ -19,9 +19,16 @@ func NewVmiGetter(vmiCache ctlkubevirtv1.VirtualMachineInstanceCache) *VmiGetter
 }
 
 // WhoUseNad requires adding network indexer to the vmi cache before invoking it
-func (v *VmiGetter) WhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]*kubevirtv1.VirtualMachineInstance, error) {
+func (v *VmiGetter) WhoUseNad(nad *nadv1.NetworkAttachmentDefinition, filterFlag bool, nodesFilter mapset.Set[string]) ([]*kubevirtv1.VirtualMachineInstance, error) {
 	// multus network name can be <networkName> or <namespace>/<networkName>
 	// ref: https://github.com/kubevirt/client-go/blob/148fa0d1c7e83b7a56606a7ca92394ba6768c9ac/api/v1/schema.go#L1436-L1439
+
+	// note: this is a inclusive filter, the target vmi must be in given nodes
+	// when the nodes are empty, return nil directly
+	if filterFlag && (nodesFilter == nil || nodesFilter.Cardinality() == 0) {
+		return nil, nil
+	}
+
 	networkName := fmt.Sprintf("%s/%s", nad.Namespace, nad.Name)
 	vmis, err := v.VmiCache.GetByIndex(indexeres.VMByNetworkIndex, networkName)
 	if err != nil {
@@ -39,7 +46,8 @@ func (v *VmiGetter) WhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nodesFilte
 		vmis = append(vmis, vmi)
 	}
 
-	if nodesFilter == nil || nodesFilter.Cardinality() == 0 {
+	// no need to filter
+	if !filterFlag {
 		return vmis, nil
 	}
 
@@ -55,11 +63,11 @@ func (v *VmiGetter) WhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nodesFilte
 }
 
 // Get the vmi lists who uses a group of nads, duplicated vmis may exist when they attache to mutli nads
-func (v *VmiGetter) WhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]*kubevirtv1.VirtualMachineInstance, error) {
+func (v *VmiGetter) WhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, filterFlag bool, nodesFilter mapset.Set[string]) ([]*kubevirtv1.VirtualMachineInstance, error) {
 	vmis := make([]*kubevirtv1.VirtualMachineInstance, 0)
 	for _, nad := range nads {
 		// get all vmis on this nad
-		vmisTemp, err := v.WhoUseNad(nad, nodesFilter)
+		vmisTemp, err := v.WhoUseNad(nad, filterFlag, nodesFilter)
 		if err != nil {
 			return nil, err
 		}
@@ -69,8 +77,8 @@ func (v *VmiGetter) WhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, nodesF
 }
 
 // Get the vmi name list who uses the nad
-func (v *VmiGetter) VmiNamesWhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]string, error) {
-	vmis, err := v.WhoUseNad(nad, nodesFilter)
+func (v *VmiGetter) VmiNamesWhoUseNad(nad *nadv1.NetworkAttachmentDefinition, filterFlag bool, nodesFilter mapset.Set[string]) ([]string, error) {
+	vmis, err := v.WhoUseNad(nad, filterFlag, nodesFilter)
 	if err != nil {
 		return nil, err
 	}
@@ -79,8 +87,8 @@ func (v *VmiGetter) VmiNamesWhoUseNad(nad *nadv1.NetworkAttachmentDefinition, no
 }
 
 // Get the vmi name list who uses a group of nads, duplicated names may exist when they attache to mutli nads
-func (v *VmiGetter) VmiNamesWhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]string, error) {
-	vmis, err := v.WhoUseNads(nads, nodesFilter)
+func (v *VmiGetter) VmiNamesWhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, filterFlag bool, nodesFilter mapset.Set[string]) ([]string, error) {
+	vmis, err := v.WhoUseNads(nads, filterFlag, nodesFilter)
 	if err != nil {
 		return nil, err
 	}

pkg/webhook/clusternetwork/validator.go

@@ -206,7 +206,8 @@ func (c *CnValidator) checkMTUOfUpdatedMgmtClusterNetwork(oldCn, newCn *networkv
 	}
 
 	vmiGetter := utils.NewVmiGetter(c.vmiCache)
-	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNads(nads, nil); err != nil {
+	// get all, no filter
+	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNads(nads, false, nil); err != nil {
 		return err
 	} else if len(vmiStrList) > 0 {
 		return fmt.Errorf("the MTU can't be changed from %v to %v as following VMs must be stopped at first: %s", oldMtu, newMtu, strings.Join(vmiStrList, ", "))

pkg/webhook/nad/validator.go

@@ -176,7 +176,8 @@ func (v *Validator) checkRoute(config string) error {
 
 func (v *Validator) checkVmi(nad *cniv1.NetworkAttachmentDefinition) error {
 	vmiGetter := utils.NewVmiGetter(v.vmiCache)
-	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNad(nad, nil); err != nil {
+	// get all, no filter
+	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNad(nad, false, nil); err != nil {
 		return err
 	} else if len(vmiStrList) > 0 {
 		return fmt.Errorf("it's still used by VM(s) %s which must be stopped at first", strings.Join(vmiStrList, ", "))

pkg/webhook/vlanconfig/validator.go

@@ -109,12 +109,7 @@ func (v *Validator) Update(_ *admission.Request, oldObj, newObj runtime.Object)
 		return fmt.Errorf(updateErr, newVc.Name, fmt.Errorf("it refers to a none-existing cluster network %s or error %w", newVc.Spec.ClusterNetwork, err))
 	}
 
-	// TODO: do not allow such change?
-	/*
-		if oldVc.Spec.ClusterNetwork != oldVc.Spec.ClusterNetwork {
-			return fmt.Errorf("cluster network can't be changed from %v to %v", oldVc.Spec.ClusterNetwork, oldVc.Spec.ClusterNetwork)
-		}
-	*/
+	// Harvester UI allows to migration a vlanconfig from one clusternetwork to another
 
 	if err := v.validateMTU(newVc); err != nil {
 		return fmt.Errorf(updateErr, newVc.Name, err)
@@ -212,31 +207,33 @@ func (v *Validator) checkOverlaps(vc *networkv1.VlanConfig, nodes mapset.Set[str
 	return nil
 }
 
-// checkVmi is to confirm if any VMIs will be affected on affected nodes. Those VMIs must be stopped in advance.
+// checkVmi is to confirm if any VMIs exists on the affected nodes. Those VMIs must be stopped in advance.
 func (v *Validator) checkVmi(vc *networkv1.VlanConfig, nodes mapset.Set[string]) error {
+	// note: the vlanconfig's selector may select empty node, e.g. a place-holder vlanconfig
+	// when then given nodes are empty, then no vmi exists
+	if nodes == nil || nodes.Cardinality() == 0 {
+		return nil
+	}
+
 	// The vlanconfig is not allowed to be deleted if it has been applied to some nodes and its clusternetwork is attached by some nads.
 	vss, err := v.vsCache.List(labels.Set(map[string]string{utils.KeyVlanConfigLabel: vc.Name}).AsSelector())
 	if err != nil {
-		return fmt.Errorf("failed to list vlanstatus erro %w", err)
+		return fmt.Errorf("failed to list vlanstatus error %w", err)
 	}
 
 	if len(vss) == 0 {
 		return nil
 	}
 
-	// affect no nodes
-	if nodes == nil || nodes.Cardinality() == 0 {
-		return nil
-	}
-
 	nadGetter := utils.NewNadGetter(v.nadCache)
 	nads, err := nadGetter.ListNadsOnClusterNetwork(vc.Spec.ClusterNetwork)
 	if err != nil {
 		return err
 	}
 
 	vmiGetter := utils.NewVmiGetter(v.vmiCache)
-	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNads(nads, nodes); err != nil {
+	// get vmis on the given nodes
+	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNads(nads, true, nodes); err != nil {
 		return err
 	} else if len(vmiStrList) > 0 {
 		return fmt.Errorf("it is blocked by VM(s) %s which must be stopped at first", strings.Join(vmiStrList, ", "))