chore: Batch reloads runtime

docs/content/intro/how-nginx-ingress-controller-works.md

@@ -315,6 +315,7 @@ A reload involves multiple steps:
 
 The Ingress Controller reloads NGINX every time the Control Loop processes a change that affects the generated NGINX configuration. In general, every time a resource is changed, the Ingress Controller will regenerate the configuration and reload NGINX. A resource could be of any type the Ingress Controller monitors -- see [The Ingress Controller is a Kubernetes Controller](#the-ingress-controller-is-a-kubernetes-controller) section.
 
-There are two special cases:
+There are three special cases:
 - *Start*. When the Ingress Controller starts, it processes all resources in the cluster and only then reloads NGINX. This avoids creating a "reload storm" by reloading only once.
+- *Batch updates*. When the Ingress Controller receives a number of concurrent requests from the Kubernetes API, it will pause NGINX reloads untill the task queue is empty. This reduces the number of reloads to minimize the impact of batch updates and reduce the risk of OOM errors.
 - *NGINX Plus*. If the Ingress Controller uses NGINX Plus, it will not reload NGINX Plus for changes to the Endpoints resources. In this case, the Ingress Controller will use the NGINX Plus API to update the corresponding upstreams and skip reloading.

internal/configs/configurator.go

@@ -1039,6 +1039,11 @@ func (cnf *Configurator) EnableReloads() {
 	cnf.isReloadsEnabled = true
 }
 
+// DisableReloads disables NGINX reloads meaning that configuration changes will not be followed by a reload.
+func (cnf *Configurator) DisableReloads() {
+	cnf.isReloadsEnabled = false
+}
+
 func (cnf *Configurator) reload(isEndpointsUpdate bool) error {
 	if !cnf.isReloadsEnabled {
 		return nil

internal/k8s/controller.go

@@ -166,6 +166,7 @@ type LoadBalancerController struct {
 	configMap                     *api_v1.ConfigMap
 	certManagerController         *cm_controller.CmController
 	externalDNSController         *ed_controller.ExtDNSController
+	batchSyncEnabled              bool
 }
 
 var keyFunc = cache.DeletionHandlingMetaNamespaceKeyFunc
@@ -764,6 +765,11 @@ func (lbc *LoadBalancerController) syncConfigMap(task task) {
 		return
 	}
 
+	if lbc.batchSyncEnabled {
+		glog.V(3).Infof("Skipping ConfigMap update because batch sync is on")
+		return
+	}
+
 	lbc.updateAllConfigs()
 }
 
@@ -836,6 +842,12 @@ func (lbc *LoadBalancerController) preSyncSecrets() {
 }
 
 func (lbc *LoadBalancerController) sync(task task) {
+	if lbc.isNginxReady && lbc.syncQueue.Len() > 1 && !lbc.batchSyncEnabled {
+		lbc.configurator.DisableReloads()
+		lbc.batchSyncEnabled = true
+
+		glog.V(3).Infof("Batch processing %v items", lbc.syncQueue.Len())
+	}
 	glog.V(3).Infof("Syncing %v", task.Key)
 	if lbc.spiffeCertFetcher != nil {
 		lbc.syncLock.Lock()
@@ -892,6 +904,14 @@ func (lbc *LoadBalancerController) sync(task task) {
 		lbc.isNginxReady = true
 		glog.V(3).Infof("NGINX is ready")
 	}
+
+	if lbc.batchSyncEnabled && lbc.syncQueue.Len() == 0 {
+		lbc.batchSyncEnabled = false
+		lbc.configurator.EnableReloads()
+		lbc.updateAllConfigs()
+
+		glog.V(3).Infof("Batch sync completed")
+	}
 }
 
 func (lbc *LoadBalancerController) syncIngressLink(task task) {

tests/Makefile

@@ -72,3 +72,8 @@ create-kind-cluster: ## Create Kind cluster
 .PHONY: delete-kind-cluster
 delete-kind-cluster: ## Delete Kind cluster
 	kind delete cluster
+
+.PHONY: test-lint
+test-lint: ## Run Python linting tools
+	isort .
+	black .

tests/conftest.py

@@ -5,6 +5,7 @@
 import pytest
 from kubernetes.config.kube_config import KUBE_CONFIG_DEFAULT_LOCATION
 from settings import (
+    BATCH_RELOAD_NUMBER,
     BATCH_RESOURCES,
     BATCH_START,
     DEFAULT_DEPLOYMENT_TYPE,
@@ -95,6 +96,12 @@ def pytest_addoption(parser) -> None:
         default=BATCH_RESOURCES,
         help="Number of VS/Ingress resources to deploy",
     )
+    parser.addoption(
+        "--batch-reload-number",
+        action="store",
+        default=BATCH_RELOAD_NUMBER,
+        help="Number of reloads expected for batch reload test",
+    )
     parser.addoption(
         "--ns-count",
         action="store",

tests/settings.py

@@ -22,7 +22,9 @@
 """Settings below are test specific"""
 # Determines if batch reload tests will be ran or not
 BATCH_START = "False"
-# Number of Ingress/VS resources to deploy based on BATCH_START value, used in test_batch_startup_times.py
+# Number of Ingress/VS resources to deploy based on BATCH_START value, used in test_batch_startup_times.py and test_batch_reloads.py
 BATCH_RESOURCES = 1
+# Threshold for batch reloads (reloads for batch requests should be at or below this number). Used in test_batch_reloads.py
+BATCH_RELOAD_NUMBER = 2
 # Number of namespaces to deploy to measure Pod performance, used in test_multiple_ns_perf.py
 NS_COUNT = 0

tests/suite/resources_utils.py

@@ -1209,21 +1209,24 @@ def create_items_from_yaml(kube_apis, yaml_manifest, namespace) -> {}:
     print("Load yaml:")
     with open(yaml_manifest) as f:
         docs = yaml.safe_load_all(f)
-        for doc in docs:
-            if doc["kind"] == "Secret":
-                res["Secret"] = create_secret(kube_apis.v1, namespace, doc)
-            elif doc["kind"] == "ConfigMap":
-                res["ConfigMap"] = create_configmap(kube_apis.v1, namespace, doc)
-            elif doc["kind"] == "Ingress":
-                res["Ingress"] = create_ingress(kube_apis.networking_v1, namespace, doc)
-            elif doc["kind"] == "Service":
-                res["Service"] = create_service(kube_apis.v1, namespace, doc)
-            elif doc["kind"] == "Deployment":
-                res["Deployment"] = create_deployment(kube_apis.apps_v1_api, namespace, doc)
-            elif doc["kind"] == "DaemonSet":
-                res["DaemonSet"] = create_daemon_set(kube_apis.apps_v1_api, namespace, doc)
-            elif doc["kind"] == "Namespace":
-                res["Namespace"] = create_namespace(kube_apis.v1, doc)
+        try:
+            for doc in docs:
+                if doc["kind"] == "Secret":
+                    res["Secret"] = create_secret(kube_apis.v1, namespace, doc)
+                elif doc["kind"] == "ConfigMap":
+                    res["ConfigMap"] = create_configmap(kube_apis.v1, namespace, doc)
+                elif doc["kind"] == "Ingress":
+                    res["Ingress"] = create_ingress(kube_apis.networking_v1, namespace, doc)
+                elif doc["kind"] == "Service":
+                    res["Service"] = create_service(kube_apis.v1, namespace, doc)
+                elif doc["kind"] == "Deployment":
+                    res["Deployment"] = create_deployment(kube_apis.apps_v1_api, namespace, doc)
+                elif doc["kind"] == "DaemonSet":
+                    res["DaemonSet"] = create_daemon_set(kube_apis.apps_v1_api, namespace, doc)
+                elif doc["kind"] == "Namespace":
+                    res["Namespace"] = create_namespace(kube_apis.v1, doc)
+        except Exception:
+            pass
 
     return res
 
@@ -1322,21 +1325,24 @@ def delete_items_from_yaml(kube_apis, yaml_manifest, namespace) -> None:
     print("Load yaml:")
     with open(yaml_manifest) as f:
         docs = yaml.safe_load_all(f)
-        for doc in docs:
-            if doc["kind"] == "Namespace":
-                delete_namespace(kube_apis.v1, doc["metadata"]["name"])
-            elif doc["kind"] == "Secret":
-                delete_secret(kube_apis.v1, doc["metadata"]["name"], namespace)
-            elif doc["kind"] == "Ingress":
-                delete_ingress(kube_apis.networking_v1, doc["metadata"]["name"], namespace)
-            elif doc["kind"] == "Service":
-                delete_service(kube_apis.v1, doc["metadata"]["name"], namespace)
-            elif doc["kind"] == "Deployment":
-                delete_deployment(kube_apis.apps_v1_api, doc["metadata"]["name"], namespace)
-            elif doc["kind"] == "DaemonSet":
-                delete_daemon_set(kube_apis.apps_v1_api, doc["metadata"]["name"], namespace)
-            elif doc["kind"] == "ConfigMap":
-                delete_configmap(kube_apis.v1, doc["metadata"]["name"], namespace)
+        try:
+            for doc in docs:
+                if doc["kind"] == "Namespace":
+                    delete_namespace(kube_apis.v1, doc["metadata"]["name"])
+                elif doc["kind"] == "Secret":
+                    delete_secret(kube_apis.v1, doc["metadata"]["name"], namespace)
+                elif doc["kind"] == "Ingress":
+                    delete_ingress(kube_apis.networking_v1, doc["metadata"]["name"], namespace)
+                elif doc["kind"] == "Service":
+                    delete_service(kube_apis.v1, doc["metadata"]["name"], namespace)
+                elif doc["kind"] == "Deployment":
+                    delete_deployment(kube_apis.apps_v1_api, doc["metadata"]["name"], namespace)
+                elif doc["kind"] == "DaemonSet":
+                    delete_daemon_set(kube_apis.apps_v1_api, doc["metadata"]["name"], namespace)
+                elif doc["kind"] == "ConfigMap":
+                    delete_configmap(kube_apis.v1, doc["metadata"]["name"], namespace)
+        except Exception:
+            pass
 
 
 def ensure_connection(request_url, expected_code=404, headers={}) -> None:

tests/suite/test_app_protect_integration.py

@@ -228,6 +228,7 @@ def test_ap_enable_false_policy_correct(
         ap_crd_info = read_ap_custom_resource(kube_apis.custom_objects, test_namespace, "appolicies", ap_policy)
         assert_ap_crd_info(ap_crd_info, ap_policy)
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host, check404=True)
+        wait_before_test(5)
 
         print("----------------------- Send request ----------------------")
         response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)