diff --git a/pkg/utils/vmi.go b/pkg/utils/vmi.go index fe213602..3c0b8d97 100644 --- a/pkg/utils/vmi.go +++ b/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 or / // 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 } diff --git a/pkg/webhook/clusternetwork/validator.go b/pkg/webhook/clusternetwork/validator.go index fac9683f..e423fe93 100644 --- a/pkg/webhook/clusternetwork/validator.go +++ b/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, ", ")) diff --git a/pkg/webhook/nad/validator.go b/pkg/webhook/nad/validator.go index d37ac655..819c8620 100644 --- a/pkg/webhook/nad/validator.go +++ b/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, ", ")) diff --git a/pkg/webhook/vlanconfig/validator.go b/pkg/webhook/vlanconfig/validator.go index 7088258b..d63ae33f 100644 --- a/pkg/webhook/vlanconfig/validator.go +++ b/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,23 +207,24 @@ 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 { @@ -236,7 +232,8 @@ func (v *Validator) checkVmi(vc *networkv1.VlanConfig, nodes mapset.Set[string]) } 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, ", "))