Fix review issues

cmd/webhook/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"github.com/harvester/harvester/pkg/indexeres"
 	"os"
 
 	ctlcni "github.com/harvester/harvester/pkg/generated/controllers/k8s.cni.cncf.io"
@@ -11,15 +12,13 @@ import (
 	ctlcore "github.com/rancher/wrangler/pkg/generated/controllers/core"
 	ctlcorev1 "github.com/rancher/wrangler/pkg/generated/controllers/core/v1"
 	"github.com/rancher/wrangler/pkg/kubeconfig"
-	"github.com/rancher/wrangler/pkg/leader"
 	"github.com/rancher/wrangler/pkg/signals"
 	"github.com/rancher/wrangler/pkg/start"
 	"github.com/sirupsen/logrus"
 	"github.com/urfave/cli"
 	"github.com/yaocw2020/webhook/pkg/config"
 	"github.com/yaocw2020/webhook/pkg/server"
 	"github.com/yaocw2020/webhook/pkg/types"
-	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 
 	ctlnetwork "github.com/harvester/harvester-network-controller/pkg/generated/controllers/network.harvesterhci.io"
@@ -84,7 +83,6 @@ func main() {
 		logrus.Fatal(err)
 	}
 
-	client := kubernetes.NewForConfigOrDie(cfg)
 	ctx := signals.SetupSignalContext()
 
 	app := cli.NewApp()
@@ -96,11 +94,9 @@ func main() {
 		}
 	}
 
-	leader.RunOrDie(ctx, options.Namespace, name, client, func(ctx context.Context) {
-		if err := app.Run(os.Args); err != nil {
-			logrus.Fatalf("run webhook server failed: %v", err)
-		}
-	})
+	if err := app.Run(os.Args); err != nil {
+		logrus.Fatalf("run webhook server failed: %v", err)
+	}
 }
 
 func run(ctx context.Context, cfg *rest.Config, options *config.Options) error {
@@ -144,13 +140,15 @@ func newCaches(ctx context.Context, cfg *rest.Config, threadiness int) (*caches,
 	starters = append(starters, harvesterNetworkFactory)
 	coreFactory := ctlcore.NewFactoryFromConfigOrDie(cfg)
 	starters = append(starters, coreFactory)
-	// must declare cache before starting the factories
+	// must declare cache before starting informers
 	c := &caches{
 		vmCache:   kubevirtFactory.Kubevirt().V1().VirtualMachine().Cache(),
 		nadCache:  cniFactory.K8s().V1().NetworkAttachmentDefinition().Cache(),
 		vsCache:   harvesterNetworkFactory.Network().V1beta1().VlanStatus().Cache(),
 		nodeCache: coreFactory.Core().V1().Node().Cache(),
 	}
+	// Indexer must be added before starting the informer, otherwise panic `cannot add indexers to running index` happens
+	c.vmCache.AddIndexer(indexeres.VMByNetworkIndex, indexeres.VMByNetwork)
 
 	if err := start.All(ctx, threadiness, starters...); err != nil {
 		return nil, err

manifests/crds/network.harvesterhci.io_vlanstatuses.yaml

@@ -91,16 +91,16 @@ spec:
                       type: string
                     masterIndex:
                       type: integer
+                    name:
+                      type: string
                     promiscuous:
                       type: boolean
                     state:
                       type: string
-                    string:
-                      type: string
                     type:
                       type: string
                   required:
-                  - string
+                  - name
                   type: object
                 type: array
               localAreas:

pkg/apis/network.harvesterhci.io/v1beta1/vlanstatus.go

@@ -30,14 +30,20 @@ type VlStatus struct {
 
 	Node string `json:"node"`
 	// +optional
-	VLANIDs []uint16 `json:"vlanIds,omitempty"`
+	LocalAreas []LocalArea `json:"localAreas,omitempty"`
 	// +optional
 	LinkStatus []LinkStatus `json:"linkStatus,omitempty"`
 	// +optional
 	Conditions []Condition `json:"conditions,omitempty"`
 }
 
+type LocalArea struct {
+	VID  uint16 `json:"vlanID"`
+	CIDR string `json:"cidr,omitempty"`
+}
+
 type LinkStatus struct {
+	Name string `json:"name"`
 	// +optional
 	Index int `json:"index,omitempty"`
 	// +optional

pkg/controller/agent/nad/controller.go

@@ -36,7 +36,7 @@ func Register(ctx context.Context, management *config.Management) error {
 	return nil
 }
 
-func (h Handler) OnChange(_ string, nad *nadv1.NetworkAttachmentDefinition) (*nadv1.NetworkAttachmentDefinition, error) {
+func (h Handler) OnChange(key string, nad *nadv1.NetworkAttachmentDefinition) (*nadv1.NetworkAttachmentDefinition, error) {
 	if nad == nil || nad.DeletionTimestamp != nil {
 		return nil, nil
 	}
@@ -54,7 +54,7 @@ func (h Handler) OnChange(_ string, nad *nadv1.NetworkAttachmentDefinition) (*na
 	return nad, nil
 }
 
-func (h Handler) OnRemove(_ string, nad *nadv1.NetworkAttachmentDefinition) (*nadv1.NetworkAttachmentDefinition, error) {
+func (h Handler) OnRemove(key string, nad *nadv1.NetworkAttachmentDefinition) (*nadv1.NetworkAttachmentDefinition, error) {
 	if nad == nil {
 		return nil, nil
 	}
@@ -78,26 +78,19 @@ func (h Handler) OnRemove(_ string, nad *nadv1.NetworkAttachmentDefinition) (*na
 }
 
 func (h Handler) addLocalArea(nad *nadv1.NetworkAttachmentDefinition) error {
-	vlanID, err := strconv.Atoi(nad.Labels[utils.KeyVlanLabel])
-	if err != nil {
-		return fmt.Errorf("invalid vlanconfig %s", nad.Labels[utils.KeyVlanLabel])
-	}
-
 	v, err := vlan.GetVlan(nad.Labels[utils.KeyClusterNetworkLabel])
 	if err != nil && !errors.As(err, &netlink.LinkNotFoundError{}) {
 		return err
 	} else if errors.As(err, &netlink.LinkNotFoundError{}) {
 		return nil
 	}
 
-	layer3NetworkConf := &utils.Layer3NetworkConf{}
-	if nad.Annotations != nil && nad.Annotations[utils.KeyNetworkConf] != "" {
-		if layer3NetworkConf, err = utils.NewLayer3NetworkConf(nad.Annotations[utils.KeyNetworkConf]); err != nil {
-			return err
-		}
+	localArea, err := GetLocalArea(nad)
+	if err != nil {
+		return fmt.Errorf("failed to get local area from nad %s/%s, error: %w", nad.Namespace, nad.Name, err)
 	}
 
-	return v.AddLocalArea(uint16(vlanID), layer3NetworkConf.CIDR)
+	return v.AddLocalArea(localArea)
 }
 
 func (h Handler) existDuplicateNad(vlanIdStr, cn string) (bool, error) {
@@ -113,25 +106,36 @@ func (h Handler) existDuplicateNad(vlanIdStr, cn string) (bool, error) {
 }
 
 func (h Handler) removeLocalArea(nad *nadv1.NetworkAttachmentDefinition) error {
-	vlanID, err := strconv.Atoi(nad.Labels[utils.KeyVlanLabel])
-	if err != nil {
-		return fmt.Errorf("invalid vlanconfig %s", nad.Labels[utils.KeyVlanLabel])
-	}
-
 	v, err := vlan.GetVlan(nad.Labels[utils.KeyClusterNetworkLabel])
 	if err != nil && !errors.As(err, &netlink.LinkNotFoundError{}) {
 		return err
 	} else if errors.As(err, &netlink.LinkNotFoundError{}) {
 		return nil
 	}
 
+	localArea, err := GetLocalArea(nad)
+	if err != nil {
+		return fmt.Errorf("failed to get local area from nad %s/%s, error: %w", nad.Namespace, nad.Name, err)
+	}
+
+	return v.RemoveLocalArea(localArea)
+}
+
+func GetLocalArea(nad *nadv1.NetworkAttachmentDefinition) (*vlan.LocalArea, error) {
+	vlanID, err := strconv.Atoi(nad.Labels[utils.KeyVlanLabel])
+	if err != nil {
+		return nil, fmt.Errorf("invalid vlanconfig %s", nad.Labels[utils.KeyVlanLabel])
+	}
+
 	layer3NetworkConf := &utils.Layer3NetworkConf{}
 	if nad.Annotations != nil && nad.Annotations[utils.KeyNetworkConf] != "" {
-		layer3NetworkConf, err = utils.NewLayer3NetworkConf(nad.Annotations[utils.KeyNetworkConf])
-		if err != nil {
-			return err
+		if layer3NetworkConf, err = utils.NewLayer3NetworkConf(nad.Annotations[utils.KeyNetworkConf]); err != nil {
+			return nil, err
 		}
 	}
 
-	return v.RemoveLocalArea(uint16(vlanID), layer3NetworkConf.CIDR)
+	return &vlan.LocalArea{
+		Vid:  uint16(vlanID),
+		Cidr: layer3NetworkConf.CIDR,
+	}, nil
 }