test: tolerate connection drops during NGINX reloads in reload-sensitive e2e tests (#10443)

* test(appprotect): tolerate connection drops during reload in watch-namespace-label test

The watch-namespace-label AppProtect e2e test toggles namespace labels, each
of which triggers an NGINX reload plus an App Protect WAF soft reset. Requests
that land during the reload window hit recycled workers and get a closed
connection (access log status 000), surfacing as an uncaught
requests.exceptions.ConnectionError (RemoteDisconnected) that fails the test.

Add a retry_get_until_body_contains helper that retries the request and
tolerates ConnectionError during reloads, mirroring the transient-handling
already done by ensure_response_from_backend and wait_for_reload. Use it at the
three request sites in test_app_protect_watch_namespace_label.py and drop the
now-unused requests import.

Also add FLAKY_RELOAD_REQUESTS.md documenting the pattern so the same fix can be
applied to other reload-sensitive e2e tests.

Test-only change: no product code, codegen, or snapshot updates.

* test: tolerate connection drops during reloads across reload-sensitive e2e tests

Requests that land while NGINX is reloading (worker recycling, or an App Protect
WAF soft reset) hit a closed connection and surface as an uncaught
requests.exceptions.ConnectionError (RemoteDisconnected, access-log status 000),
failing the test on a timing race rather than a real regression.

Apply the pattern documented in FLAKY_RELOAD_REQUESTS.md to the remaining
reload-sensitive tests instead of wrapping whole tests in @pytest.mark.flaky:

Shared helpers (cover the majority of call sites):
- resources_utils: add retry_get_until_status_code (status-based sibling of
  retry_get_until_body_contains, supports an SNI/client-cert session and
  requests.get kwargs) and retry_get (guards a single request for arbitrary or
  negative assertions); both catch ConnectionError while retrying.
- custom_assertions.wait_and_assert_status_code and
  ap_resources_utils.send_malicious_request_with_retry now tolerate
  ConnectionError, hardening every caller.

Test call sites:
- Replace hand-rolled status-poll loops in test_ingress_mtls.py,
  test_ingress_mtls_ingress.py, test_ingress_mtls_mergeable_ingress.py and
  test_watch_namespace_label.py with retry_get_until_status_code, preserving
  retry budgets and assertions.
- Add except ConnectionError after the load-bearing except SSLError handlers in
  the mTLS tests. Note: SSLError subclasses ConnectionError, so SSLError must be
  caught first.
- Guard single post-reload requests in test_app_protect_integration.py,
  test_app_protect_waf_policies.py, test_app_protect_waf_bundle_source_vs.py,
  test_dos.py and test_virtual_server_dos.py with retry_get.

Drop now-unused requests/mock/wait_before_test imports where applicable and
document the new helpers and the SSLError ordering caveat in
FLAKY_RELOAD_REQUESTS.md.

Test-only change: no product code, codegen, or snapshot updates.

* test: fail explicitly when reload-retry helpers never get a response

retry_get_until_body_contains, retry_get_until_status_code and retry_get could
return None when every attempt hit a ConnectionError. Callers immediately access
resp.text/resp.status_code, so a persistent connection drop surfaced as an
AttributeError that obscured the real failure.

Track the last ConnectionError in each helper and pytest.fail with the URL,
attempt count, and last error when no response was ever obtained. When a response
was received but did not match (wrong body/status), still return it so the
caller's own assertion produces its meaningful message.

Test-only change: no product code, codegen, or snapshot updates.

* test: fail explicitly when send_malicious_request_with_retry gets no response

send_malicious_request_with_retry could return None when every attempt hit a
ConnectionError. Callers treat the return value as a real requests.Response, so
a persistent connection drop surfaced as an AttributeError that hid the root
cause.

Track the last ConnectionError and pytest.fail with the URL, attempt count, and
last error when no response was ever obtained (adding the pytest import). When a
response was received but the WAF did not block, still return it so the caller's
own assertion produces its meaningful message.

Test-only change: no product code, codegen, or snapshot updates.

* test: surface root cause when wait_and_assert_status_code gets no response

When every attempt in wait_and_assert_status_code hit a ConnectionError, resp
stayed None and the final assertion reported only "status_code is still not
<code>", hiding the underlying connection drops.

Track the last ConnectionError and pytest.fail with the URL, attempt count, and
last error when no response was ever received; keep the existing status_code
assertion for the connected-but-wrong-status case.

Test-only change: no product code, codegen, or snapshot updates.

* test: avoid trailing sleep and preserve attempt count in reload-retry loops

The reload-tolerant retry loops slept after every attempt, including the final
one, adding needless wall-clock time (~1s per call across 100+ call sites), and
the while-loop rewrites did one fewer request attempt than the original code.

Switch send_malicious_request_with_retry and wait_and_assert_status_code to
range-based loops that restore the original attempt count (initial request plus
up to N retries) and sleep only when another attempt will follow. Apply the same
"no sleep after the last attempt" fix to retry_get_until_body_contains,
retry_get_until_status_code, and retry_get.

Test-only change: no product code, codegen, or snapshot updates.

* test: derive wait_and_assert_status_code failure message from attempt count

The failure/assertion messages hardcoded "After 30 seconds" while the loop now
runs up to 32 attempts (~31s), making failures misleading. Compute
elapsed_seconds from attempts and use it (plus the attempt count) in both the
no-response pytest.fail and the wrong-status assertion, so the messages stay
accurate if the loop bounds change again.

Test-only change: no product code, codegen, or snapshot updates.

* test: tolerate connection drops in ensure_response_from_backend polling loops

ensure_response_from_backend performed unguarded requests.get/session.get calls
in its polling loops, so a connection dropped by an NGINX reload (worker
recycling) raised an uncaught ConnectionError. Many tests call this helper right
after a config change, leaving reload flakiness unaddressed despite the new retry
helpers.

Catch requests.exceptions.ConnectionError in all three loops and keep retrying
within the existing budgets (SSLError stays first since it subclasses
ConnectionError). Seed resp to None and report "no response (connection kept
dropping)" in the failure message when every attempt dropped, so the root cause
is preserved.

Test-only change: no product code, codegen, or snapshot updates.

* test: preserve retry pacing and fix misleading timeout messages

Address review feedback on the reload-tolerant retry work:

- ensure_response_from_backend (default branch): the retry-interval log said
  "1 second" and the failure said "after 60 seconds", but the branch sleeps
  wait_before_test() (RECONFIGURATION_DELAY=3s) over 30 iterations (~90s).
  Base both messages on RECONFIGURATION_DELAY so timeouts are interpretable.
- test_watch_namespace_label.py: import RECONFIGURATION_DELAY and pass
  wait_seconds=RECONFIGURATION_DELAY to all six retry_get_until_status_code
  calls, restoring the prior ~3s-per-attempt pacing (the default of 1s
  shortened the tolerance window after label-change reloads).

Test-only change: no product code, codegen, or snapshot updates.

Author: javorszky

tests/suite/FLAKY_RELOAD_REQUESTS.md

@@ -0,0 +1,105 @@
+# Fixing flaky e2e tests: connection drops during NGINX reloads
+
+## Symptom
+
+A test intermittently fails with:
+
+    requests.exceptions.ConnectionError:
+      ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))
+
+Often accompanied in the IC logs by an access-log line with status `000`, and
+around a config change (Ingress/Policy/namespace-label update) that triggers an
+NGINX reload. With App Protect, the WAF "soft reset" recycles workers and can
+take several seconds, widening the window where in-flight connections are closed.
+
+## Root cause
+
+The test sends an HTTP request that lands exactly while NGINX is reloading and
+recycling worker processes. NGINX closes the connection, `requests` raises an
+uncaught `ConnectionError`, and the test crashes instead of retrying. This is a
+timing race, not a product bug -- a dropped connection during a reload is
+expected and should be retried (the same way `ensure_response_from_backend` and
+`wait_for_reload` already tolerate transients).
+
+## When this pattern applies
+
+Apply the fix to any e2e test that BOTH:
+
+1. Mutates config that causes a reload (apply/patch/delete of Ingress, VS/VSR,
+   TransportServer, Policy, ConfigMap, or namespace label/watch changes), AND
+2. Sends traffic (`requests.get/post/...`) shortly after, while a reload may be
+   in progress -- especially inside a body/status polling loop.
+
+Do NOT apply it to tests that only send traffic against already-stable config
+and never reload mid-traffic (e.g. `test_app_protect_watch_namespace.py`).
+
+## The fix
+
+Do not wrap the whole test in `@pytest.mark.flaky` -- that reruns the entire
+expensive test and can mask real regressions. Instead, make the specific request
+resilient.
+
+Use one of the shared helpers in `suite/utils/resources_utils.py`, all of which
+catch `requests.exceptions.ConnectionError` while retrying:
+
+- `retry_get_until_body_contains(req_url, host, expected_body)` -- poll until a
+  substring appears in the body.
+- `retry_get_until_status_code(req_url, host, expected_status, session=..., **kwargs)`
+  -- poll until a status code matches; supports an SNI/client-cert `session` and
+  extra `requests.get` kwargs (`cert`, `verify`, `allow_redirects`, ...).
+- `retry_get(req_url, host, **kwargs)` -- guard a single request whose assertion
+  cannot be expressed as "body contains X" or "status == Y" (e.g. asserting a
+  substring is absent, or an arbitrary status); retries only on `ConnectionError`.
+
+The shared helpers `wait_and_assert_status_code` (`suite/utils/custom_assertions.py`)
+and `send_malicious_request_with_retry` (`suite/utils/ap_resources_utils.py`) also
+tolerate `ConnectionError`, so their many callers are covered automatically.
+
+    ```python
+    from suite.utils.resources_utils import retry_get_until_body_contains
+
+    resp = retry_get_until_body_contains(req_url, ingress_host, expected_body)
+    assert expected_body in resp.text
+    assert resp.status_code == 200
+    ```
+
+Replace hand-rolled loops like:
+
+    ```python
+    resp = requests.get(url, headers={"host": host}, verify=False)
+    retry = 0
+    while expected not in resp.text and retry <= 60:
+        resp = requests.get(url, headers={"host": host}, verify=False)  # can raise ConnectionError
+        retry += 1
+        wait_before_test(1)
+    ```
+
+with a single `retry_get_until_body_contains(...)` call.
+
+If a test needs a method/headers/body the helper doesn't cover, replicate its
+core guarantee: catch `requests.exceptions.ConnectionError` inside the retry loop
+and continue, rather than letting it propagate.
+
+Note: `requests.exceptions.SSLError` is a *subclass* of `ConnectionError`. When a
+test relies on catching `SSLError` (e.g. mTLS client-cert tests), keep the
+`except SSLError` handler **before** any `except ConnectionError` handler so the
+SSL case is not swallowed.
+
+## Reference implementation
+
+See `test_app_protect_watch_namespace_label.py` and the
+`retry_get_until_body_contains` helper in `suite/utils/resources_utils.py`.
+For status-based and single-request variants, see `retry_get_until_status_code`
+and `retry_get` in the same file, and the mTLS tests (`test_ingress_mtls*.py`)
+for the SSLError-ordering pattern.
+
+## Checklist for the AI applying this
+
+- [ ] Confirm the test reloads config AND sends traffic that could overlap it.
+- [ ] Replace unguarded `requests.*` retry loops with `retry_get_until_body_contains`
+      (or add `except requests.exceptions.ConnectionError: continue` to the loop).
+- [ ] Guard single pre-loop requests that immediately follow a config change too.
+- [ ] Preserve existing assertions and the ~60s retry budget.
+- [ ] Remove the now-unused `import requests` if no other usage remains in the file.
+- [ ] Do NOT add `@pytest.mark.flaky`.
+- [ ] Test-only change: no product code, codegen, or snapshot updates.

tests/suite/test_app_protect_integration.py

@@ -1,5 +1,4 @@
 import pytest
-import requests
 import yaml
 from settings import CRDS, TEST_DATA
 from suite.utils.ap_resources_utils import (
@@ -28,6 +27,7 @@
     get_pod_name_that_contains,
     get_pods_amount,
     get_test_file_name,
+    retry_get,
     scale_deployment,
     wait_before_test,
     wait_until_all_pods_are_ready,
@@ -209,7 +209,7 @@ def test_ap_enable_true_policy_correct(
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host, check404=True)
 
         print("----------------------- Send request ----------------------")
-        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
+        response = retry_get(appprotect_setup.req_url + "/<script>", ingress_host, verify=False)
         print(response.text)
         delete_items_from_yaml(kube_apis, src_ing_yaml, test_namespace)
         assert_invalid_responses(response)
@@ -233,7 +233,7 @@ def test_ap_enable_false_policy_correct(
         wait_before_test(5)
 
         print("----------------------- Send request ----------------------")
-        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
+        response = retry_get(appprotect_setup.req_url + "/<script>", ingress_host, verify=False)
         print(response.text)
         delete_items_from_yaml(kube_apis, src_ing_yaml, test_namespace)
         assert_valid_responses(response)
@@ -260,7 +260,7 @@ def test_ap_enable_true_policy_incorrect(
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host, check404=True)
 
         print("----------------------- Send request ----------------------")
-        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
+        response = retry_get(appprotect_setup.req_url + "/<script>", ingress_host, verify=False)
         print(response.text)
         delete_items_from_yaml(kube_apis, src_ing_yaml, test_namespace)
         assert_invalid_responses(response)
@@ -288,7 +288,7 @@ def test_ap_enable_false_policy_incorrect(
 
         print("----------------------- Send request ----------------------")
         wait_before_test(5)
-        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
+        response = retry_get(appprotect_setup.req_url + "/<script>", ingress_host, verify=False)
         print(response.text)
         delete_items_from_yaml(kube_apis, src_ing_yaml, test_namespace)
         assert_valid_responses(response)
@@ -366,7 +366,7 @@ def test_ap_multi_sec_logs(
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host, check404=True)
 
         print("----------------------- Send request ----------------------")
-        response = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
+        response = retry_get(appprotect_setup.req_url + "/<script>", ingress_host, verify=False)
         print(response.text)
         syslog_pod = get_pod_name_that_contains(kube_apis.v1, test_namespace, "syslog-")
         syslog2_pod = get_pod_name_that_contains(kube_apis.v1, test_namespace, "syslog2")
@@ -434,9 +434,9 @@ def test_ap_enable_true_policy_correct_uds(
         wait_before_test(120)
         ensure_response_from_backend(appprotect_setup.req_url, ingress_host, check404=True)
         print("----------------------- Send request ----------------------")
-        response1 = requests.get(appprotect_setup.req_url, headers={"host": ingress_host}, verify=False, data="kic")
+        response1 = retry_get(appprotect_setup.req_url, ingress_host, verify=False, data="kic")
         print(response1.text)
-        response2 = requests.get(appprotect_setup.req_url + "/<script>", headers={"host": ingress_host}, verify=False)
+        response2 = retry_get(appprotect_setup.req_url + "/<script>", ingress_host, verify=False)
         print(response2.text)
         reload_ms = get_last_reload_time(appprotect_setup.metrics_url, "nginx")
         print(f"last reload duration: {reload_ms} ms")

tests/suite/test_app_protect_waf_bundle_source_vs.py

@@ -1,5 +1,4 @@
 import pytest
-import requests
 from settings import TEST_DATA
 from suite.utils.ap_resources_utils import (
     assert_waf_blocked,
@@ -14,7 +13,7 @@
 )
 from suite.utils.custom_resources_utils import read_custom_resource, wait_for_resource_status
 from suite.utils.policy_resources_utils import delete_policy
-from suite.utils.resources_utils import wait_before_test
+from suite.utils.resources_utils import retry_get, wait_before_test
 from suite.utils.vs_vsr_resources_utils import (
     create_virtual_server_from_yaml,
     delete_virtual_server,
@@ -208,9 +207,10 @@ def test_bundle_source_invalid_url(
         # WAF is inactive so a malicious request should NOT be blocked with
         # the rejection page. The VS may return a 500 or the backend's response
         # depending on how the NGINX config was generated.
-        response = requests.get(
+        # Tolerate a connection dropped by an NGINX reload; retry the single request.
+        response = retry_get(
             virtual_server_setup.backend_1_url + "</script>",
-            headers={"host": virtual_server_setup.vs_host},
+            virtual_server_setup.vs_host,
         )
         assert "The requested URL was rejected" not in response.text
 

tests/suite/test_app_protect_waf_policies.py

@@ -1,5 +1,4 @@
 import pytest
-import requests
 from settings import TEST_DATA
 from suite.utils.ap_resources_utils import (
     create_ap_logconf_from_yaml,
@@ -17,6 +16,7 @@
     create_items_from_yaml,
     get_file_contents,
     get_pod_name_that_contains,
+    retry_get,
     wait_before_test,
 )
 from suite.utils.vs_vsr_resources_utils import (
@@ -197,16 +197,16 @@ def test_ap_waf_policy_block(
         wait_before_test(120)
 
         print("----------------------- Send request with embedded malicious script----------------------")
-        response1 = requests.get(
+        response1 = retry_get(
             virtual_server_setup.backend_1_url + "</script>",
-            headers={"host": virtual_server_setup.vs_host},
+            virtual_server_setup.vs_host,
         )
         print(response1.text)
 
         print("----------------------- Send request with blocked keyword in UDS----------------------")
-        response2 = requests.get(
+        response2 = retry_get(
             virtual_server_setup.backend_1_url,
-            headers={"host": virtual_server_setup.vs_host},
+            virtual_server_setup.vs_host,
             data="kic",
         )
         print(response2.text)
@@ -268,16 +268,16 @@ def test_ap_waf_policy_allow(
         wait_before_test(120)
 
         print("----------------------- Send request with embedded malicious script----------------------")
-        response1 = requests.get(
+        response1 = retry_get(
             virtual_server_setup.backend_1_url + "</script>",
-            headers={"host": virtual_server_setup.vs_host},
+            virtual_server_setup.vs_host,
         )
         print(response1.text)
 
         print("----------------------- Send request with blocked keyword in UDS----------------------")
-        response2 = requests.get(
+        response2 = retry_get(
             virtual_server_setup.backend_1_url,
-            headers={"host": virtual_server_setup.vs_host},
+            virtual_server_setup.vs_host,
             data="kic",
         )
         print(response2.text)
@@ -333,9 +333,9 @@ def test_ap_waf_policy_multi_logs(
         wait_before_test(120)
 
         print("----------------------- Send request with embedded malicious script----------------------")
-        response = requests.get(
+        response = retry_get(
             virtual_server_setup.backend_1_url + "</script>",
-            headers={"host": virtual_server_setup.vs_host},
+            virtual_server_setup.vs_host,
         )
         print(response.text)
         syslog_pod = get_pod_name_that_contains(kube_apis.v1, test_namespace, "syslog")
@@ -449,9 +449,9 @@ def test_ap_waf_policy_block(
         ap_crd_info = read_ap_custom_resource(kube_apis.custom_objects, test_namespace, "appolicies", ap_policy_uds)
         assert_ap_crd_info(ap_crd_info, ap_policy_uds)
         wait_before_test(120)
-        response = requests.get(
+        response = retry_get(
             f'{req_url}{v_s_route_setup.route_m.paths[0]}+"</script>"',
-            headers={"host": v_s_route_setup.vs_host},
+            v_s_route_setup.vs_host,
         )
         print(response.text)
         delete_policy(kube_apis.custom_objects, "waf-policy", v_s_route_setup.route_m.namespace)
@@ -546,9 +546,9 @@ def test_ap_waf_policy_block(
         ap_crd_info = read_ap_custom_resource(kube_apis.custom_objects, test_namespace, "appolicies", ap_policy_uds)
         assert_ap_crd_info(ap_crd_info, ap_policy_uds)
         wait_before_test(120)
-        response = requests.get(
+        response = retry_get(
             f'{req_url}{v_s_route_selector_setup.route_m.paths[0]}+"</script>"',
-            headers={"host": v_s_route_selector_setup.vs_host},
+            v_s_route_selector_setup.vs_host,
         )
         print(response.text)
         delete_policy(kube_apis.custom_objects, "waf-policy", v_s_route_selector_setup.route_m.namespace)

tests/suite/test_app_protect_watch_namespace_label.py

@@ -1,7 +1,6 @@
 import time
 
 import pytest
-import requests
 from settings import TEST_DATA
 from suite.utils.ap_resources_utils import (
     create_ap_logconf_from_yaml,
@@ -20,6 +19,7 @@
     ensure_connection_to_public_endpoint,
     ensure_response_from_backend,
     patch_namespace_with_label,
+    retry_get_until_body_contains,
     wait_before_test,
     wait_until_all_pods_are_ready,
 )
@@ -156,8 +156,8 @@ def test_responses(self, request, kube_apis, crd_ingress_controller_with_ap, bac
         ensure_response_from_backend(backend_setup.req_url, backend_setup.ingress_host, check404=True)
 
         print("----------------------- Send request ----------------------")
-        resp = requests.get(
-            f"{backend_setup.req_url}/test.bat", headers={"host": backend_setup.ingress_host}, verify=False
+        resp = retry_get_until_body_contains(
+            f"{backend_setup.req_url}/test.bat", backend_setup.ingress_host, valid_resp_body
         )
 
         print(resp.text)
@@ -176,17 +176,9 @@ def test_responses(self, request, kube_apis, crd_ingress_controller_with_ap, bac
         ensure_response_from_backend(backend_setup.req_url, backend_setup.ingress_host, check404=True)
 
         print("----------------------- Send request ----------------------")
-        resp = requests.get(
-            f"{backend_setup.req_url}/test.bat", headers={"host": backend_setup.ingress_host}, verify=False
+        resp = retry_get_until_body_contains(
+            f"{backend_setup.req_url}/test.bat", backend_setup.ingress_host, invalid_resp_body
         )
-        retry = 0
-        while invalid_resp_body not in resp.text and retry <= 60:
-            resp = requests.get(
-                f"{backend_setup.req_url}/test.bat", headers={"host": backend_setup.ingress_host}, verify=False
-            )
-            retry += 1
-            wait_before_test(1)
-            print(f"Policy not yet enforced, retrying... #{retry}")
 
         assert invalid_resp_body in resp.text
         assert resp.status_code == 200
@@ -202,17 +194,9 @@ def test_responses(self, request, kube_apis, crd_ingress_controller_with_ap, bac
         ensure_response_from_backend(backend_setup.req_url, backend_setup.ingress_host, check404=True)
 
         print("----------------------- Send request ----------------------")
-        resp = requests.get(
-            f"{backend_setup.req_url}/test.bat", headers={"host": backend_setup.ingress_host}, verify=False
+        resp = retry_get_until_body_contains(
+            f"{backend_setup.req_url}/test.bat", backend_setup.ingress_host, valid_resp_body
         )
-        retry = 0
-        while valid_resp_body not in resp.text and retry <= 60:
-            resp = requests.get(
-                f"{backend_setup.req_url}/test.bat", headers={"host": backend_setup.ingress_host}, verify=False
-            )
-            retry += 1
-            wait_before_test(1)
-            print(f"Policy not yet removed, retrying... #{retry}")
 
         assert valid_resp_body in resp.text
         assert resp.status_code == 200