perf: [WIN-NPM] apply ipsets in background (#1875)

* wip

* fix UT by applying dataplane immediately for RemovePolicy()

* configmap options for apply in background

* fix deadlocks

* better logging

* rename config variables, update default config, change shouldApply check

* update configmap values

* FIXME: remove tmp commit overriding applyDP config (using for pipeline tests)

* optimize applying ipsets for add policy

* cleanup code and finalize apply ipsets for netpols

* flip order of if statement

* UTs. address comments. fix netpol behavior by waiting to start pod controller

* all UTs except ones related to issue #1729

* remove bootup phase stuff

* fix lints and move applyinbackground to toggle

* fix lint

* don't check isWindows every time

* use diff var for applyinbackground

* fix lint

Author: huntergregory

npm/azure-npm.yaml

@@ -151,11 +151,15 @@ data:
         "ResyncPeriodInMinutes": 15,
         "ListeningPort":         10091,
         "ListeningAddress":      "0.0.0.0",
+        "ApplyMaxBatches": 100,
+        "ApplyIntervalInMilliseconds": 500,
         "Toggles": {
             "EnablePrometheusMetrics": true,
             "EnablePprof":             true,
             "EnableHTTPDebugAPI":      true,
             "EnableV2NPM":             true,
-            "PlaceAzureChainFirst":    true
+            "PlaceAzureChainFirst":    true,
+            "ApplyIPSetsOnNeed":       false,
+            "ApplyInBackground": true
         }
     }

npm/cmd/start.go

@@ -123,6 +123,18 @@ func start(config npmconfig.Config, flags npmconfig.Flags) error {
 	stopChannel := wait.NeverStop
 	if config.Toggles.EnableV2NPM {
 		// update the dataplane config
+		npmV2DataplaneCfg.ApplyInBackground = config.Toggles.ApplyInBackground
+		if config.ApplyMaxBatches > 0 {
+			npmV2DataplaneCfg.ApplyMaxBatches = config.ApplyMaxBatches
+		} else {
+			npmV2DataplaneCfg.ApplyMaxBatches = npmconfig.DefaultConfig.ApplyMaxBatches
+		}
+		if config.ApplyIntervalInMilliseconds > 0 {
+			npmV2DataplaneCfg.ApplyInterval = time.Duration(config.ApplyIntervalInMilliseconds * int(time.Millisecond))
+		} else {
+			npmV2DataplaneCfg.ApplyInterval = time.Duration(npmconfig.DefaultConfig.ApplyIntervalInMilliseconds * int(time.Millisecond))
+		}
+
 		if config.WindowsNetworkName == "" {
 			npmV2DataplaneCfg.NetworkName = util.AzureNetworkName
 		} else {

npm/config/config.go

@@ -4,6 +4,8 @@ import "github.com/Azure/azure-container-networking/npm/util"
 
 const (
 	defaultResyncPeriod    = 15
+	defaultApplyMaxBatches = 100
+	defaultApplyInterval   = 500
 	defaultListeningPort   = 10091
 	defaultGrpcPort        = 10092
 	defaultGrpcServicePort = 9002
@@ -16,7 +18,6 @@ const (
 
 // DefaultConfig is the guaranteed configuration NPM can run in out of the box
 var DefaultConfig = Config{
-	WindowsNetworkName:    util.AzureNetworkName,
 	ResyncPeriodInMinutes: defaultResyncPeriod,
 
 	ListeningPort:    defaultListeningPort,
@@ -28,13 +29,18 @@ var DefaultConfig = Config{
 		ServicePort: defaultGrpcServicePort,
 	},
 
+	WindowsNetworkName:          util.AzureNetworkName,
+	ApplyMaxBatches:             defaultApplyMaxBatches,
+	ApplyIntervalInMilliseconds: defaultApplyInterval,
+
 	Toggles: Toggles{
 		EnablePrometheusMetrics: true,
 		EnablePprof:             true,
 		EnableHTTPDebugAPI:      true,
 		EnableV2NPM:             true,
 		PlaceAzureChainFirst:    util.PlaceAzureChainFirst,
 		ApplyIPSetsOnNeed:       false,
+		ApplyInBackground:       true,
 	},
 }
 
@@ -48,14 +54,17 @@ type GrpcServerConfig struct {
 }
 
 type Config struct {
-	// WindowsNetworkName can be either 'azure' or 'Calico' (case sensitive).
-	// It can also be the empty string, which results in the default value of 'azure'.
-	WindowsNetworkName    string           `json:"WindowsNetworkName,omitempty"`
 	ResyncPeriodInMinutes int              `json:"ResyncPeriodInMinutes,omitempty"`
 	ListeningPort         int              `json:"ListeningPort,omitempty"`
 	ListeningAddress      string           `json:"ListeningAddress,omitempty"`
 	Transport             GrpcServerConfig `json:"Transport,omitempty"`
-	Toggles               Toggles          `json:"Toggles,omitempty"`
+	// WindowsNetworkName can be either 'azure' or 'Calico' (case sensitive).
+	// It can also be the empty string, which results in the default value of 'azure'.
+	WindowsNetworkName string `json:"WindowsNetworkName,omitempty"`
+	// Apply options for Windows only. Relevant when ApplyInBackground is true.
+	ApplyMaxBatches             int     `json:"ApplyDataPlaneMaxBatches,omitempty"`
+	ApplyIntervalInMilliseconds int     `json:"ApplyDataPlaneMaxWaitInMilliseconds,omitempty"`
+	Toggles                     Toggles `json:"Toggles,omitempty"`
 }
 
 type Toggles struct {
@@ -65,6 +74,8 @@ type Toggles struct {
 	EnableV2NPM             bool
 	PlaceAzureChainFirst    bool
 	ApplyIPSetsOnNeed       bool
+	// ApplyInBackground applies for Windows only
+	ApplyInBackground bool
 }
 
 type Flags struct {

npm/examples/windows/azure-npm.yaml

@@ -143,17 +143,15 @@ data:
         "ResyncPeriodInMinutes": 15,
         "ListeningPort":         10091,
         "ListeningAddress":      "0.0.0.0",
+        "ApplyMaxBatches": 100,
+        "ApplyIntervalInMilliseconds": 500,
         "Toggles": {
             "EnablePrometheusMetrics": true,
             "EnablePprof":             true,
             "EnableHTTPDebugAPI":      true,
             "EnableV2NPM":             true,
             "PlaceAzureChainFirst":    true,
-            "ApplyIPSetsOnNeed":       false
-        },
-        "Transport": {
-          "Address": "azure-npm.kube-system.svc.cluster.local",
-          "Port": 10092,
-          "ServicePort": 9001
+            "ApplyIPSetsOnNeed":       false,
+            "ApplyInBackground": true
         }
     }

npm/npm.go

@@ -198,6 +198,7 @@ func (npMgr *NetworkPolicyManager) Start(config npmconfig.Config, stopCh <-chan
 		go npMgr.PodControllerV2.Run(stopCh)
 		go npMgr.NamespaceControllerV2.Run(stopCh)
 		go npMgr.NetPolControllerV2.Run(stopCh)
+
 		return nil
 	}
 

npm/pkg/dataplane/dataplane-test-cases_windows_test.go

@@ -1,6 +1,8 @@
 package dataplane
 
 import (
+	"time"
+
 	"github.com/Azure/azure-container-networking/network/hnswrapper"
 	"github.com/Azure/azure-container-networking/npm/pkg/dataplane/ipsets"
 	"github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies"
@@ -12,11 +14,12 @@ import (
 
 // tags
 const (
-	podCrudTag    Tag = "pod-crud"
-	nsCrudTag     Tag = "namespace-crud"
-	netpolCrudTag Tag = "netpol-crud"
-	reconcileTag  Tag = "reconcile"
-	calicoTag     Tag = "calico"
+	podCrudTag           Tag = "pod-crud"
+	nsCrudTag            Tag = "namespace-crud"
+	netpolCrudTag        Tag = "netpol-crud"
+	reconcileTag         Tag = "reconcile"
+	calicoTag            Tag = "calico"
+	applyInBackgroundTag Tag = "apply-in-background"
 )
 
 const (
@@ -2255,3 +2258,37 @@ func getAllMultiJobTests() []*MultiJobTestCase {
 		},
 	}
 }
+
+func applyInBackgroundTests() []*SerialTestCase {
+	allTests := make([]*SerialTestCase, 0)
+	allTests = append(allTests, basicTests()...)
+	allTests = append(allTests, capzCalicoTests()...)
+	allTests = append(allTests, updatePodTests()...)
+
+	for _, test := range allTests {
+		test.TestCaseMetadata.Tags = append(test.TestCaseMetadata.Tags, applyInBackgroundTag)
+		cfg := *test.DpCfg
+		cfg.ApplyInBackground = true
+		cfg.ApplyMaxBatches = 3
+		cfg.ApplyInterval = time.Duration(50 * time.Millisecond)
+		test.DpCfg = &cfg
+	}
+
+	return allTests
+}
+
+func multiJobApplyInBackgroundTests() []*MultiJobTestCase {
+	allTests := make([]*MultiJobTestCase, 0)
+	allTests = append(allTests, getAllMultiJobTests()...)
+
+	for _, test := range allTests {
+		test.TestCaseMetadata.Tags = append(test.TestCaseMetadata.Tags, applyInBackgroundTag)
+		cfg := *test.DpCfg
+		cfg.ApplyInBackground = true
+		cfg.ApplyMaxBatches = 3
+		cfg.ApplyInterval = time.Duration(50 * time.Millisecond)
+		test.DpCfg = &cfg
+	}
+
+	return allTests
+}