Update nad and vlanconfig webhook

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

pkg/utils/nad.go

@@ -6,6 +6,11 @@ import (
 	"net"
 	"strings"
 
+	ctlcniv1 "github.com/harvester/harvester/pkg/generated/controllers/k8s.cni.cncf.io/v1"
+
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/labels"
+
 	cniv1 "github.com/containernetworking/cni/pkg/types"
 	nadv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 
@@ -138,7 +143,7 @@ func isMaskZero(ipnet *net.IPNet) bool {
 
 // if this nad is a storagenetwork nad
 func IsStorageNetworkNad(nad *nadv1.NetworkAttachmentDefinition) bool {
-	if nad == nil {
+	if nad == nil || nad.Namespace != harvesterutil.HarvesterSystemNamespaceName {
 		return false
 	}
 
@@ -148,9 +153,79 @@ func IsStorageNetworkNad(nad *nadv1.NetworkAttachmentDefinition) bool {
 	}
 
 	// check name
-	if nad.Namespace == harvesterutil.HarvesterSystemNamespaceName && strings.HasPrefix(nad.Name, StorageNetworkNetAttachDefPrefix) {
+	if strings.HasPrefix(nad.Name, StorageNetworkNetAttachDefPrefix) {
 		return true
 	}
 
 	return false
 }
+
+// filter the first storage network nad from a list of nads
+func FilterFirstActiveStorageNetworkNad(nads []*nadv1.NetworkAttachmentDefinition) *nadv1.NetworkAttachmentDefinition {
+	if len(nads) == 0 {
+		return nil
+	}
+	for _, nad := range nads {
+		if IsStorageNetworkNad(nad) && nad.DeletionTimestamp == nil {
+			return nad
+		}
+	}
+	return nil
+}
+
+type NadGetter struct {
+	nadCache ctlcniv1.NetworkAttachmentDefinitionCache
+}
+
+func NewNadGetter(nadCache ctlcniv1.NetworkAttachmentDefinitionCache) *NadGetter {
+	return &NadGetter{nadCache: nadCache}
+}
+
+// list all Nads attached to a cluster network
+func (n *NadGetter) ListNadsOnClusterNetwork(cnName string) ([]*nadv1.NetworkAttachmentDefinition, error) {
+	nads, err := n.nadCache.List(corev1.NamespaceAll, labels.Set(map[string]string{
+		KeyClusterNetworkLabel: cnName,
+	}).AsSelector())
+	if err != nil {
+		return nil, err
+	}
+
+	if len(nads) == 0 {
+		return nil, nil
+	}
+	return nads, nil
+}
+
+func (n *NadGetter) GetFirstActiveStorageNetworkNadOnClusterNetwork(cnName string) (*nadv1.NetworkAttachmentDefinition, error) {
+	nads, err := n.nadCache.List(harvesterutil.HarvesterSystemNamespaceName, labels.Set(map[string]string{
+		KeyClusterNetworkLabel: cnName,
+	}).AsSelector())
+	if err != nil {
+		return nil, err
+	}
+
+	if len(nads) == 0 {
+		return nil, nil
+	}
+
+	return FilterFirstActiveStorageNetworkNad(nads), nil
+}
+
+func (n *NadGetter) NadNamesOnClusterNetwork(cnName string) ([]string, error) {
+	nads, err := n.ListNadsOnClusterNetwork(cnName)
+	if err != nil {
+		return nil, err
+	}
+	return generateNadNameList(nads), nil
+}
+
+func generateNadNameList(nads []*nadv1.NetworkAttachmentDefinition) []string {
+	if len(nads) == 0 {
+		return nil
+	}
+	nadStrList := make([]string, len(nads))
+	for i, nad := range nads {
+		nadStrList[i] = nad.Namespace + "/" + nad.Name
+	}
+	return nadStrList
+}

pkg/utils/vmi.go

@@ -14,6 +14,10 @@ type VmiGetter struct {
 	VmiCache ctlkubevirtv1.VirtualMachineInstanceCache
 }
 
+func NewVmiGetter(vmiCache ctlkubevirtv1.VirtualMachineInstanceCache) *VmiGetter {
+	return &VmiGetter{VmiCache: vmiCache}
+}
+
 // 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) {
 	// multus network name can be <networkName> or <namespace>/<networkName>
@@ -65,7 +69,7 @@ func (v *VmiGetter) WhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, nodesF
 }
 
 // Get the vmi name list who uses the nad
-func (v *VmiGetter) VmiNameWhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]string, error) {
+func (v *VmiGetter) VmiNamesWhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]string, error) {
 	vmis, err := v.WhoUseNad(nad, nodesFilter)
 	if err != nil {
 		return nil, err
@@ -75,7 +79,7 @@ func (v *VmiGetter) VmiNameWhoUseNad(nad *nadv1.NetworkAttachmentDefinition, nod
 }
 
 // Get the vmi name list who uses a group of nads, duplicated names may exist when they attache to mutli nads
-func (v *VmiGetter) VmiNameWhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]string, error) {
+func (v *VmiGetter) VmiNamesWhoUseNads(nads []*nadv1.NetworkAttachmentDefinition, nodesFilter mapset.Set[string]) ([]string, error) {
 	vmis, err := v.WhoUseNads(nads, nodesFilter)
 	if err != nil {
 		return nil, err

pkg/webhook/clusternetwork/validator.go

@@ -8,7 +8,6 @@ import (
 	ctlkubevirtv1 "github.com/harvester/harvester/pkg/generated/controllers/kubevirt.io/v1"
 	"github.com/harvester/webhook/pkg/server/admission"
 	admissionregv1 "k8s.io/api/admissionregistration/v1"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 
@@ -103,25 +102,18 @@ func (c *CnValidator) Delete(_ *admission.Request, oldObj runtime.Object) error
 	}
 
 	// all related nads should be deleted
-	nads, err := c.nadCache.List(corev1.NamespaceAll, labels.Set(map[string]string{
-		utils.KeyClusterNetworkLabel: cn.Name,
-	}).AsSelector())
+	nadGetter := utils.NewNadGetter(c.nadCache)
+	nadStrList, err := nadGetter.NadNamesOnClusterNetwork(cn.Name)
 	if err != nil {
-		return fmt.Errorf(deleteErr, cn.Name, err)
-	}
-
-	if len(nads) == 0 {
-		return nil
+		return err
 	}
 
-	nadStrList := make([]string, len(nads))
-	for i, nad := range nads {
-		nadStrList[i] = nad.Namespace + "/" + nad.Name
+	if len(nadStrList) > 0 {
+		return fmt.Errorf(deleteErr, cn.Name, fmt.Errorf("nads(s) %v under this clusternetwork are still existing", strings.Join(nadStrList, ", ")))
 	}
 
-	return fmt.Errorf(deleteErr, cn.Name, fmt.Errorf("nads(s) %v under this clusternetwork are still existing", strings.Join(nadStrList, ", ")))
-
-	// TODO: check vmi, vm as well?
+	// TODO: check vmi, vm as well?  vmi is tied with NAD;  vm will also be tied with NAD
+	return nil
 }
 
 func (c *CnValidator) Resource() admission.Resource {
@@ -203,25 +195,18 @@ func (c *CnValidator) checkMTUOfUpdatedMgmtClusterNetwork(oldCn, newCn *networkv
 	}
 
 	// for mgmt network, the nad is not tied to any vlanconfig, check nad directly
-	nads, err := c.nadCache.List(corev1.NamespaceAll, labels.Set(map[string]string{
-		utils.KeyClusterNetworkLabel: newCn.Name,
-	}).AsSelector())
+	nadGetter := utils.NewNadGetter(c.nadCache)
+	nads, err := nadGetter.ListNadsOnClusterNetwork(newCn.Name)
 	if err != nil {
-		return fmt.Errorf(updateErr, newCn.Name, err)
-	}
-
-	if len(nads) == 0 {
-		return nil
+		return err
 	}
 
-	for _, nad := range nads {
-		if utils.IsStorageNetworkNad(nad) && nad.DeletionTimestamp == nil {
-			return fmt.Errorf(updateErr, newCn.Name, fmt.Errorf("the MTU can't be changed from %v to %v as storage network nad %s is still attached", oldMtu, newMtu, nad.Name))
-		}
+	if nad := utils.FilterFirstActiveStorageNetworkNad(nads); nad != nil {
+		return fmt.Errorf(updateErr, newCn.Name, fmt.Errorf("the MTU can't be changed from %v to %v as storage network nad %s is still attached", oldMtu, newMtu, nad.Name))
 	}
 
-	vmiGetter := utils.VmiGetter{VmiCache: c.vmiCache}
-	if vmiStrList, err := vmiGetter.VmiNameWhoUseNads(nads, nil); err != nil {
+	vmiGetter := utils.NewVmiGetter(c.vmiCache)
+	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNads(nads, nil); err != nil {
 		return err
 	} else if len(vmiStrList) > 0 {
 		return fmt.Errorf(updateErr, newCn.Name, 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

@@ -175,18 +175,11 @@ func (v *Validator) checkRoute(config string) error {
 }
 
 func (v *Validator) checkVmi(nad *cniv1.NetworkAttachmentDefinition) error {
-	vmiGetter := utils.VmiGetter{VmiCache: v.vmiCache}
-	vmis, err := vmiGetter.WhoUseNad(nad, nil)
-	if err != nil {
+	vmiGetter := utils.NewVmiGetter(v.vmiCache)
+	if vmiStrList, err := vmiGetter.VmiNamesWhoUseNad(nad, nil); err != nil {
 		return err
-	}
-
-	if len(vmis) > 0 {
-		vmiNameList := make([]string, len(vmis), len(vmis))
-		for i, vmi := range vmis {
-			vmiNameList[i] = vmi.Namespace + "/" + vmi.Name
-		}
-		return fmt.Errorf("it's still used by VM(s) %s which must be stopped at first", strings.Join(vmiNameList, ", "))
+	} 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, ", "))
 	}
 
 	return nil

pkg/webhook/nad/validator_test.go

@@ -391,7 +391,7 @@ func TestDeleteNAD(t *testing.T) {
 			currentNAD: &cniv1.NetworkAttachmentDefinition{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        testNadName,
-					Namespace:   testNamespace,
+					Namespace:   harvesterutil.HarvesterSystemNamespaceName, // storagenetwork nad is in given namespace
 					Annotations: map[string]string{utils.StorageNetworkAnnotation: "true"},
 					Labels:      map[string]string{utils.KeyClusterNetworkLabel: testCnName},
 				},