feat: [NPM] Adding DPShim layer in controller pods (#1206)

* intial touches to create sets

* adding initial touches to dpshim

* deprecating initialize DP func

* feat: [NPM] Adding DPShim layer in controller pods

* correcting an import error

* Adding some UTs

* adding a UT

* Addressing some comments

* Moving an UT to linux specifc file

* Fixing some issues with controller pod

* Adding some dns poilices and logs for debugging

* Moving aroudn outchannel to help with hydration of new clients

* removeing pass by ref

* Adding http server in daemon for pprof

* Adding a new grpc option to wait

* Fixing 100% cpu in daemon

* Fixing some logic in list management

* Applying some golints

* adding mutex

* Addressing comments and solving a bug. Cyclonus seems to be good now

* Fixing a bug

* Addressing a comment

* fixing an issue and addressing comments

Author: vakalapa

npm/cmd/start_daemon.go

@@ -9,6 +9,8 @@ import (
 	"github.com/Azure/azure-container-networking/common"
 	npmconfig "github.com/Azure/azure-container-networking/npm/config"
 	"github.com/Azure/azure-container-networking/npm/daemon"
+	restserver "github.com/Azure/azure-container-networking/npm/http/server"
+	"github.com/Azure/azure-container-networking/npm/metrics"
 	"github.com/Azure/azure-container-networking/npm/pkg/controlplane/goalstateprocessor"
 	"github.com/Azure/azure-container-networking/npm/pkg/dataplane"
 	"github.com/Azure/azure-container-networking/npm/pkg/models"
@@ -49,6 +51,9 @@ func startDaemon(config npmconfig.Config) error {
 	pod := os.Getenv(podNameEnv)
 	node := os.Getenv(nodeNameEnv)
 
+	klog.Infof("initializing metrics")
+	metrics.InitializeAll()
+
 	addr := config.Transport.Address + ":" + strconv.Itoa(config.Transport.ServicePort)
 	ctx := context.Background()
 	err := initLogging()
@@ -65,6 +70,10 @@ func startDaemon(config npmconfig.Config) error {
 		return fmt.Errorf("failed to create dataplane with error %w", err)
 	}
 
+	dp.RunPeriodicTasks()
+	// TODO Daemon should implement cache encoder
+	go restserver.NPMRestServerListenAndServe(config, nil)
+
 	client, err := transport.NewEventsClient(ctx, pod, node, addr)
 	if err != nil {
 		klog.Errorf("failed to create dataplane events client with error %v", err)
@@ -88,5 +97,6 @@ func startDaemon(config npmconfig.Config) error {
 		klog.Errorf("failed to start dataplane : %v", err)
 		return fmt.Errorf("failed to start dataplane: %w", err)
 	}
+
 	return nil
 }

npm/cmd/start_server.go

@@ -11,7 +11,6 @@ import (
 	"github.com/Azure/azure-container-networking/npm/controller"
 	restserver "github.com/Azure/azure-container-networking/npm/http/server"
 	"github.com/Azure/azure-container-networking/npm/metrics"
-	"github.com/Azure/azure-container-networking/npm/pkg/dataplane"
 	"github.com/Azure/azure-container-networking/npm/pkg/dataplane/dpshim"
 	"github.com/Azure/azure-container-networking/npm/pkg/transport"
 	"github.com/spf13/cobra"
@@ -60,6 +59,9 @@ func startControlplane(config npmconfig.Config, flags npmconfig.Flags) error {
 		return err
 	}
 
+	klog.Infof("initializing metrics")
+	metrics.InitializeAll()
+
 	// Create the kubernetes client
 	var k8sConfig *rest.Config
 	if flags.KubeConfigPath == "" {
@@ -96,16 +98,14 @@ func startControlplane(config npmconfig.Config, flags npmconfig.Flags) error {
 
 	k8sServerVersion := k8sServerVersion(clientset)
 
-	var dp dataplane.GenericDataplane
-
-	mgr := transport.NewEventsServer(context.Background(), config.Transport.Port)
-
-	dp, err = dpshim.NewDPSim(mgr.InputChannel())
+	dp, err := dpshim.NewDPSim(wait.NeverStop)
 	if err != nil {
 		klog.Errorf("failed to create dataplane shim with error: %v", err)
 		return fmt.Errorf("failed to create dataplane with error: %w", err)
 	}
 
+	mgr := transport.NewEventsServer(context.Background(), config.Transport.Port, dp)
+
 	npMgr, err := controller.NewNetworkPolicyServer(config, factory, mgr, dp, version, k8sServerVersion)
 	if err != nil {
 		klog.Errorf("failed to create NPM controlplane manager with error: %v", err)

npm/deploy/kustomize/overlays/daemon/deployment.yaml

@@ -71,6 +71,7 @@ spec:
           - name: azure-npm-config
             mountPath: /etc/azure-npm
       hostNetwork: true
+      dnsPolicy: ClusterFirstWithHostNet
       volumes:
       - name: log
         hostPath:

npm/deploy/manifests/daemon/azure-npm.yaml

@@ -14,24 +14,24 @@ metadata:
   name: azure-npm
   namespace: kube-system
 rules:
-- apiGroups:
-  - ""
-  resources:
-  - pods
-  - nodes
-  - namespaces
-  verbs:
-  - get
-  - list
-  - watch
-- apiGroups:
-  - networking.k8s.io
-  resources:
-  - networkpolicies
-  verbs:
-  - get
-  - list
-  - watch
+  - apiGroups:
+      - ""
+    resources:
+      - pods
+      - nodes
+      - namespaces
+    verbs:
+      - get
+      - list
+      - watch
+  - apiGroups:
+      - networking.k8s.io
+    resources:
+      - networkpolicies
+    verbs:
+      - get
+      - list
+      - watch
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
@@ -45,9 +45,9 @@ roleRef:
   kind: ClusterRole
   name: azure-npm
 subjects:
-- kind: ServiceAccount
-  name: azure-npm
-  namespace: kube-system
+  - kind: ServiceAccount
+    name: azure-npm
+    namespace: kube-system
 ---
 apiVersion: v1
 data:
@@ -80,15 +80,15 @@ metadata:
   labels:
     app: azure-npm
     component: daemon
-  name: npm-deamon-metrics-cluster-service
+  name: npm-daemon-metrics-cluster-service
   namespace: kube-system
 spec:
   ports:
-  - name: metrics 
-    port: 9000
-    targetPort: 10091
+    - name: metrics
+      port: 9000
+      targetPort: 10091
   selector:
-    component: deamon
+    component: daemon
     k8s-app: azure-npm
 ---
 apiVersion: apps/v1
@@ -98,7 +98,7 @@ metadata:
     addonmanager.kubernetes.io/mode: EnsureExists
     app: azure-npm
     component: daemon
-  name: azure-npm-deamon
+  name: azure-npm-daemon
   namespace: kube-system
 spec:
   selector:
@@ -115,72 +115,73 @@ spec:
         k8s-app: azure-npm
     spec:
       containers:
-      - args:
-        - start
-        - daemon
-        command:
-        - azure-npm
-        env:
-        - name: HOSTNAME
-          valueFrom:
-            fieldRef:
-              apiVersion: v1
-              fieldPath: spec.nodeName
-        - name: NPM_CONFIG
-          value: /etc/azure-npm/azure-npm.json
-        - name: DAEMON_POD_NAME
-          valueFrom:
-            fieldRef:
-              fieldPath: metadata.name
-        - name: DAEMON_NODE_NAME
-          valueFrom:
-            fieldRef:
-              fieldPath: spec.nodeName
-        image: azure-npm:v1.4.1
-        name: azure-npm
-        ports:
-        - name: metrics
-          containerPort: 10091
-        resources:
-          limits:
-            cpu: 250m
-            memory: 300Mi
-          requests:
-            cpu: 250m
-        securityContext:
-          privileged: true
-        volumeMounts:
-        - mountPath: /var/log
-          name: log
-        - mountPath: /run/xtables.lock
-          name: xtables-lock
-        - mountPath: /etc/protocols
-          name: protocols
-        - mountPath: /etc/azure-npm
-          name: azure-npm-config
+        - args:
+            - start
+            - daemon
+          command:
+            - azure-npm
+          env:
+            - name: HOSTNAME
+              valueFrom:
+                fieldRef:
+                  apiVersion: v1
+                  fieldPath: spec.nodeName
+            - name: NPM_CONFIG
+              value: /etc/azure-npm/azure-npm.json
+            - name: DAEMON_POD_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.name
+            - name: DAEMON_NODE_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: spec.nodeName
+          image: azure-npm:v1.4.1
+          name: azure-npm
+          ports:
+            - name: metrics
+              containerPort: 10091
+          resources:
+            limits:
+              cpu: 250m
+              memory: 300Mi
+            requests:
+              cpu: 250m
+          securityContext:
+            privileged: true
+          volumeMounts:
+            - mountPath: /var/log
+              name: log
+            - mountPath: /run/xtables.lock
+              name: xtables-lock
+            - mountPath: /etc/protocols
+              name: protocols
+            - mountPath: /etc/azure-npm
+              name: azure-npm-config
       hostNetwork: true
+      dnsPolicy: ClusterFirstWithHostNet
       priorityClassName: system-node-critical
       serviceAccountName: azure-npm
       tolerations:
-      - effect: NoExecute
-        operator: Exists
-      - effect: NoSchedule
-        operator: Exists
-      - key: CriticalAddonsOnly
-        operator: Exists
+        - effect: NoExecute
+          operator: Exists
+        - effect: NoSchedule
+          operator: Exists
+        - key: CriticalAddonsOnly
+          operator: Exists
       volumes:
-      - hostPath:
-          path: /var/log
-          type: Directory
-        name: log
-      - hostPath:
-          path: /run/xtables.lock
-          type: File
-        name: xtables-lock
-      - hostPath:
-          path: /etc/protocols
-          type: File
-        name: protocols
-      - configMap:
+        - hostPath:
+            path: /var/log
+            type: Directory
+          name: log
+        - hostPath:
+            path: /run/xtables.lock
+            type: File
+          name: xtables-lock
+        - hostPath:
+            path: /etc/protocols
+            type: File
+          name: protocols
+        - configMap:
+            name: azure-npm-config
           name: azure-npm-config
-        name: azure-npm-config

npm/http/server/server.go

@@ -32,7 +32,7 @@ func NPMRestServerListenAndServe(config npmconfig.Config, npmEncoder json.Marsha
 		rs.router.Handle(api.ClusterMetricsPath, metrics.GetHandler(metrics.ClusterMetrics))
 	}
 
-	if config.Toggles.EnableHTTPDebugAPI {
+	if config.Toggles.EnableHTTPDebugAPI && npmEncoder != nil {
 		// ACN CLI debug handlerss
 		rs.router.Handle(api.NPMMgrPath, rs.npmCacheHandler(npmEncoder)).Methods(http.MethodGet)
 	}

npm/npm.go

@@ -157,7 +157,7 @@ func (npMgr *NetworkPolicyManager) GetAppVersion() string {
 func (npMgr *NetworkPolicyManager) Start(config npmconfig.Config, stopCh <-chan struct{}) error {
 	if !config.Toggles.EnableV2NPM {
 		// Do initialization of data plane before starting syncup of each controller to avoid heavy call to api-server
-		if err := npMgr.NetPolControllerV1.ResetDataPlane(); err != nil {
+		if err := npMgr.NetPolControllerV1.BootupDataplane(); err != nil {
 			return fmt.Errorf("Failed to initialized data plane with err %w", err)
 		}
 	}

npm/pkg/controlplane/controllers/v1/networkPolicyController.go

@@ -64,9 +64,9 @@ func NewNetworkPolicyController(npInformer networkinginformers.NetworkPolicyInfo
 	return netPolController
 }
 
-// initializeDataPlane do all initialization tasks for data plane
+// BootupDataplane does all initialization tasks for data plane
 // TODO(jungukcho) Need to refactor UninitNpmChains since it assumes it has already AZURE-NPM chains
-func (c *NetworkPolicyController) ResetDataPlane() error {
+func (c *NetworkPolicyController) BootupDataplane() error {
 	klog.Infof("Initiailize data plane. Clean up Azure-NPM chains and start reconcile iptables")
 
 	// TODO(jungukcho): will clean-up error handling codes to initialize iptables and ipset in a separate PR

npm/pkg/controlplane/controllers/v2/podController.go

@@ -589,7 +589,7 @@ func (c *PodController) cleanUpDeletedPod(cachedNpmPodKey string) error {
 	// Get lists of podLabelKey and podLabelKey + podLavelValue ,and then start deleting them from ipsets
 	for labelKey, labelVal := range cachedNpmPod.Labels {
 		labelKeyValue := util.GetIpSetFromLabelKV(labelKey, labelVal)
-		klog.Infof("Deleting pod %s (ip : %s) to ipset %s and %s", cachedNpmPodKey, cachedNpmPod.PodIP, labelKey, labelKeyValue)
+		klog.Infof("Deleting pod %s (ip : %s) from ipsets %s and %s", cachedNpmPodKey, cachedNpmPod.PodIP, labelKey, labelKeyValue)
 		if err = c.dp.RemoveFromSets(
 			[]*ipsets.IPSetMetadata{
 				ipsets.NewIPSetMetadata(labelKey, ipsets.KeyLabelOfPod),