update cluster network name to overlay VM Network

Signed-off-by: Renuka Devi Rajendran <renuka.rajendran@suse.com>

Author: rrajendran17

cmd/webhook/main.go

@@ -137,7 +137,7 @@ func run(ctx context.Context, cfg *rest.Config, options *config.Options) error {
 	webhookServer := server.NewWebhookServer(ctx, cfg, name, options)
 
 	if err := webhookServer.RegisterMutators(
-		nad.NewNadMutator(c.cnCache, c.vcCache),
+		nad.NewNadMutator(c.cnCache, c.vcCache, c.nodeCache),
 		vlanconfig.NewVlanConfigMutator(c.nodeCache),
 	); err != nil {
 		return fmt.Errorf("failed to register mutators: %v", err)

pkg/controller/agent/clusternetwork/controller.go

@@ -23,20 +23,18 @@ const (
 )
 
 type Handler struct {
-	cnCache   ctlnetworkv1.ClusterNetworkCache
-	cnClient  ctlnetworkv1.ClusterNetworkClient
-	nadCache  ctlcniv1.NetworkAttachmentDefinitionCache
-	nadClient ctlcniv1.NetworkAttachmentDefinitionClient
+	cnCache  ctlnetworkv1.ClusterNetworkCache
+	cnClient ctlnetworkv1.ClusterNetworkClient
+	nadCache ctlcniv1.NetworkAttachmentDefinitionCache
 }
 
 func Register(ctx context.Context, management *config.Management) error {
 	cns := management.HarvesterNetworkFactory.Network().V1beta1().ClusterNetwork()
 	nads := management.CniFactory.K8s().V1().NetworkAttachmentDefinition()
 	handler := Handler{
-		cnCache:   cns.Cache(),
-		cnClient:  cns,
-		nadClient: nads,
-		nadCache:  nads.Cache(),
+		cnCache:  cns.Cache(),
+		cnClient: cns,
+		nadCache: nads.Cache(),
 	}
 
 	cns.OnChange(ctx, controllerName, handler.OnChange)

pkg/controller/agent/hostnetworkconfig/controller.go

@@ -131,7 +131,13 @@ func (h *Handler) OnChange(_ string, hnc *networkv1.HostNetworkConfig) (*network
 
 	// node selector matches and host network interface already exists, skip processing
 	if intfExists {
-		logrus.Infof("hostnetwork config %s has been applied on this node already, update nodestatus and skip", hnc.Name)
+		logrus.Infof("hostnetwork config %s has been applied on this node already, update nodestatus,tunnel interface annotation and skip", hnc.Name)
+
+		// intf exists but there could be change in underlay, need to update node annotation with new interface if needed
+		if err := h.addNodeAnnotation(utils.GetClusterNetworkVlanDevice(hnc.Spec.ClusterNetwork, hnc.Spec.VlanID), hnc.Spec.Underlay); err != nil {
+			return nil, fmt.Errorf("add node annotation to node %s for host network config %s failed, error: %w", h.nodeName, hnc.Name, err)
+		}
+
 		err := h.setHostNetworkPerNodeStatus(hnc, true, nil)
 		if err != nil {
 			return nil, err

pkg/controller/manager/node/controller.go

@@ -14,8 +14,10 @@ import (
 
 	networkv1 "github.com/harvester/harvester-network-controller/pkg/apis/network.harvesterhci.io/v1beta1"
 	"github.com/harvester/harvester-network-controller/pkg/config"
+	ctlcniv1 "github.com/harvester/harvester-network-controller/pkg/generated/controllers/k8s.cni.cncf.io/v1"
 	ctlnetworkv1 "github.com/harvester/harvester-network-controller/pkg/generated/controllers/network.harvesterhci.io/v1beta1"
 	"github.com/harvester/harvester-network-controller/pkg/utils"
+	cniv1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 )
 
 const controllerName = "harvester-network-manager-node-controller"
@@ -30,6 +32,8 @@ type Handler struct {
 	lmClient                ctlnetworkv1.LinkMonitorClient
 	hostNetworkConfigClient ctlnetworkv1.HostNetworkConfigClient
 	hostNetworkConfigCache  ctlnetworkv1.HostNetworkConfigCache
+	nadCache                ctlcniv1.NetworkAttachmentDefinitionCache
+	nadClient               ctlcniv1.NetworkAttachmentDefinitionClient
 }
 
 func Register(ctx context.Context, management *config.Management) error {
@@ -38,6 +42,7 @@ func Register(ctx context.Context, management *config.Management) error {
 	vss := management.HarvesterNetworkFactory.Network().V1beta1().VlanStatus()
 	lms := management.HarvesterNetworkFactory.Network().V1beta1().LinkMonitor()
 	hns := management.HarvesterNetworkFactory.Network().V1beta1().HostNetworkConfig()
+	nads := management.CniFactory.K8s().V1().NetworkAttachmentDefinition()
 
 	h := Handler{
 		nodeClient:              nodes,
@@ -49,6 +54,8 @@ func Register(ctx context.Context, management *config.Management) error {
 		lmClient:                lms,
 		hostNetworkConfigClient: hns,
 		hostNetworkConfigCache:  hns.Cache(),
+		nadClient:               nads,
+		nadCache:                nads.Cache(),
 	}
 
 	nodes.OnChange(ctx, controllerName, h.OnChange)
@@ -86,6 +93,10 @@ func (h Handler) OnChange(_ string, node *corev1.Node) (*corev1.Node, error) {
 		return nil, fmt.Errorf("failed to update host network annotations, node: %s, error: %w", node.Name, err)
 	}
 
+	if err := h.updateUnderlayForOverlayNetwork(node); err != nil {
+		return nil, fmt.Errorf("failed to update underlay for overlay network, node: %s, error: %w", node.Name, err)
+	}
+
 	return node, nil
 }
 
@@ -305,3 +316,53 @@ func (h Handler) updateHostNetworkAnnotation(node *corev1.Node) error {
 
 	return nil
 }
+
+func (h Handler) setNadClusterNetworkLabel(bridge string) (err error) {
+	var nadList []*cniv1.NetworkAttachmentDefinition
+	nadGetter := utils.NewNadGetter(h.nadCache)
+	if nadList, err = nadGetter.ListAllNads(); err != nil {
+		return fmt.Errorf("failed to list nads err %w", err)
+	}
+
+	for _, nad := range nadList {
+		if nad.DeletionTimestamp != nil {
+			continue
+		}
+
+		if !utils.IsOverlayNad(nad) {
+			continue
+		}
+
+		if utils.GetNadLabel(nad, utils.KeyClusterNetworkLabel) == bridge {
+			continue
+		}
+		nadCopy := nad.DeepCopy()
+		utils.SetNadLabel(nadCopy, utils.KeyClusterNetworkLabel, bridge)
+		if _, err := h.nadClient.Update(nadCopy); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (h Handler) updateUnderlayForOverlayNetwork(node *corev1.Node) error {
+	if node.Annotations == nil {
+		return nil
+	}
+
+	iface, ok := node.Annotations[utils.KeyUnderlayIntf]
+	if !ok || iface == "" {
+		return nil
+	}
+	bridge, err := utils.GetClusterNetworkName(iface)
+	if err != nil {
+		return fmt.Errorf("failed to get bridge name from node %s annotation, error: %w", node.Name, err)
+	}
+
+	if err := h.setNadClusterNetworkLabel(bridge); err != nil {
+		return fmt.Errorf("failed to set NAD cluster network label for node %s, error: %w", node.Name, err)
+	}
+
+	return nil
+}

pkg/utils/clusternetwork.go

@@ -2,6 +2,7 @@ package utils
 
 import (
 	"fmt"
+	"strings"
 
 	networkv1 "github.com/harvester/harvester-network-controller/pkg/apis/network.harvesterhci.io/v1beta1"
 )
@@ -34,3 +35,11 @@ func SetClusterNetworkVlanAnnotations(cn *networkv1.ClusterNetwork, vidstr, vidh
 	cn.Annotations[KeyVlanIDSetStr] = vidstr
 	cn.Annotations[KeyVlanIDSetStrHash] = vidhash
 }
+
+func GetClusterNetworkName(iface string) (string, error) {
+	cnName, _, found := strings.Cut(iface, BridgeSuffix)
+	if !found {
+		return "", fmt.Errorf("invalid interface format: %s", iface)
+	}
+	return cnName, nil
+}

pkg/webhook/hostnetworkconfig/validator.go

@@ -165,13 +165,9 @@ func (v *Validator) Update(_ *admission.Request, oldObj, newObj runtime.Object)
 func (v *Validator) Delete(_ *admission.Request, oldObj runtime.Object) error {
 	hnc := oldObj.(*networkv1.HostNetworkConfig)
 
-	if !hnc.Spec.Underlay {
-		//delete hostnetworkconfig if its not serving as underlay
-		return nil
-	}
-
-	if err := v.checkifVMExistsForOverlayNADs(); err != nil {
-		return fmt.Errorf(deleteErr, hnc.Name, err)
+	//since ovn is already using the interface as underlay, user should disable it first in hostnetworkconfig before deleting it.
+	if hnc.Spec.Underlay {
+		return fmt.Errorf(deleteErr, hnc.Name, fmt.Errorf("underlay is enabled, disable underlay first"))
 	}
 
 	return nil

pkg/webhook/hostnetworkconfig/validator_test.go

@@ -4,7 +4,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -547,7 +546,6 @@ func TestCreateHostNetworkConfig(t *testing.T) {
 			validator := NewHostNetworkConfigValidator(nadCache, cnCache, hncCache, vcCache, vsCache, nodeCache, vmCache)
 
 			err := validator.Create(nil, tc.newHostNetworkConfig)
-			logrus.Infof("test case %s returned error: %v", tc.name, err)
 			assert.True(t, tc.returnErr == (err != nil))
 			if tc.returnErr {
 				assert.NotNil(t, err)
@@ -932,7 +930,6 @@ func TestUpdateHostNetworkConfig(t *testing.T) {
 			validator := NewHostNetworkConfigValidator(nadCache, cnCache, hncCache, vcCache, vsCache, nodeCache, vmCache)
 
 			err := validator.Update(nil, tc.currentHostNetworkConfig, tc.newHostNetworkConfig)
-			logrus.Infof("test case %s returned error: %v", tc.name, err)
 			assert.True(t, tc.returnErr == (err != nil))
 			if tc.returnErr {
 				assert.NotNil(t, err)
@@ -947,13 +944,10 @@ func TestDeleteHostNetworkConfig(t *testing.T) {
 		name                     string
 		returnErr                bool
 		errKey                   string
-		currentCN                *networkv1.ClusterNetwork
-		currentNAD               *cniv1.NetworkAttachmentDefinition
 		currentHostNetworkConfig *networkv1.HostNetworkConfig
-		currentVM                *kubevirtv1.VirtualMachine
 	}{
 		{
-			name:      "delete hostnetworkconfig successfully when not used by any overlay VMs",
+			name:      "delete hostnetworkconfig successfully",
 			returnErr: false,
 			errKey:    "",
 			currentHostNetworkConfig: &networkv1.HostNetworkConfig{
@@ -962,62 +956,15 @@ func TestDeleteHostNetworkConfig(t *testing.T) {
 					VlanID:         2012,
 					Mode:           "static",
 					HostIPs:        map[string]networkv1.IPAddr{"node1": "192.168.1.100/24"},
-					Underlay:       true,
+					Underlay:       false,
 				},
 			},
 		},
 		{
-			name:      "cannot delete hostnetworkconfig when used by overlay VMs",
+			name:      "cannot delete hostnetworkconfig when underlay is enabled",
 			returnErr: true,
-			errKey:    "it's still used by VM(s)",
-			currentCN: &networkv1.ClusterNetwork{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:        testCnName,
-					Annotations: map[string]string{"test": "test"},
-				},
-			},
-			currentNAD: &cniv1.NetworkAttachmentDefinition{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:        testNadName,
-					Namespace:   testNamespace,
-					Annotations: map[string]string{"test": "test"},
-					Labels:      map[string]string{utils.KeyClusterNetworkLabel: testCnName, utils.KeyNetworkType: string(utils.OverlayNetwork)},
-				},
-				Spec: cniv1.NetworkAttachmentDefinitionSpec{
-					Config: testNadConfig,
-				},
-			},
-			currentVM: &kubevirtv1.VirtualMachine{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      testVMName,
-					Namespace: testNamespace,
-				},
-				Spec: kubevirtv1.VirtualMachineSpec{
-					Template: &kubevirtv1.VirtualMachineInstanceTemplateSpec{
-						Spec: kubevirtv1.VirtualMachineInstanceSpec{
-							Networks: []kubevirtv1.Network{
-								{
-									Name: "nic-1",
-									NetworkSource: kubevirtv1.NetworkSource{
-										Multus: &kubevirtv1.MultusNetwork{
-											NetworkName: testNamespace + "/" + testNadName, // same with nad namesapce
-										},
-									},
-								},
-							},
-							Domain: kubevirtv1.DomainSpec{
-								Devices: kubevirtv1.Devices{
-									Interfaces: []kubevirtv1.Interface{
-										{
-											Name: "nic-1",
-										},
-									},
-								},
-							},
-						}, // vmi.spec
-					},
-				},
-			},
+			errKey:    "disable underlay first",
+
 			currentHostNetworkConfig: &networkv1.HostNetworkConfig{
 				Spec: networkv1.HostNetworkConfigSpec{
 					ClusterNetwork: testCnName,
@@ -1044,33 +991,10 @@ func TestDeleteHostNetworkConfig(t *testing.T) {
 
 			// client to inject test data
 			cnCache := fakeclients.ClusterNetworkCache(nchclientset.NetworkV1beta1().ClusterNetworks)
-			cnClient := fakeclients.ClusterNetworkClient(nchclientset.NetworkV1beta1().ClusterNetworks)
 			vcCache := fakeclients.VlanConfigCache(nchclientset.NetworkV1beta1().VlanConfigs)
 			vsCache := fakeclients.VlanStatusCache(nchclientset.NetworkV1beta1().VlanStatuses)
 			nodeCache := fakeclients.NodeCache(nchclientset.CoreV1().Nodes)
 
-			if tc.currentVM != nil {
-				err := nchclientset.Tracker().Add(tc.currentVM)
-				assert.Nil(t, err, "mock resource vm should add into fake controller tracker")
-			}
-
-			if tc.currentCN != nil {
-				_, err := cnClient.Create(tc.currentCN)
-				assert.NoError(t, err)
-			}
-
-			if tc.currentNAD != nil {
-				nadGvr := schema.GroupVersionResource{
-					Group:    "k8s.cni.cncf.io",
-					Version:  "v1",
-					Resource: "network-attachment-definitions",
-				}
-
-				if err := nchclientset.Tracker().Create(nadGvr, tc.currentNAD.DeepCopy(), tc.currentNAD.Namespace); err != nil {
-					t.Fatalf("failed to add nad %+v", tc.currentNAD)
-				}
-			}
-
 			if tc.currentHostNetworkConfig != nil {
 				hncClient := fakeclients.HostNetworkConfigClient(nchclientset.NetworkV1beta1().HostNetworkConfigs)
 				_, err := hncClient.Create(tc.currentHostNetworkConfig)