Merge pull request Kuadrant#399 from pehala/fix_reconciliation

Fix flaky reconciliation tests

testsuite/httpx/__init__.py

@@ -3,7 +3,7 @@
 # I change return type of HTTPX client to Kuadrant Result
 # mypy: disable-error-code="override, return-value"
 from tempfile import NamedTemporaryFile
-from typing import Union
+from typing import Union, Iterable
 
 import backoff
 from httpx import Client, RequestError
@@ -84,9 +84,16 @@ def assert_all(self, status_code):
 class KuadrantClient(Client):
     """Httpx client which retries unstable requests"""
 
-    def __init__(self, *, verify: Union[Certificate, bool] = True, cert: Certificate = None, **kwargs):
+    def __init__(
+        self,
+        *,
+        verify: Union[Certificate, bool] = True,
+        cert: Certificate = None,
+        retry_codes: Iterable[int] = None,
+        **kwargs,
+    ):
         self.files = []
-        self.retry_codes = {503}
+        self.retry_codes = retry_codes or {503}
         _verify = None
         if isinstance(verify, Certificate):
             verify_file = create_tmp_file(verify.chain)

testsuite/tests/kuadrant/reconciliation/conftest.py

@@ -1,6 +1,5 @@
 """Conftest for reconciliation tests"""
 
-import backoff
 import pytest
 
 
@@ -10,15 +9,3 @@ def commit(request, authorization):
     request.addfinalizer(authorization.delete)
     authorization.commit()
     authorization.wait_for_ready()
-
-
-@pytest.fixture(scope="module")
-def resilient_request(client):
-    """Fixture which allows to send retrying requests until the expected status code is returned"""
-
-    def _request(path, method="get", expected_status=200, http_client=client, max_tries=4):
-        return backoff.on_predicate(
-            backoff.expo, lambda x: x.status_code == expected_status, max_tries=max_tries, jitter=None
-        )(lambda: getattr(http_client, method)(path))()
-
-    return _request

testsuite/tests/kuadrant/reconciliation/test_httproute_delete.py

@@ -6,7 +6,7 @@
 
 
 @pytest.mark.issue("https://github.com/Kuadrant/kuadrant-operator/issues/124")
-def test_delete(client, route, authorization, resilient_request):
+def test_delete(client, route, hostname, authorization):
     """
     Tests that after deleting HTTPRoute, status.conditions shows it missing:
       * Test that that client works
@@ -21,8 +21,9 @@ def test_delete(client, route, authorization, resilient_request):
 
     route.delete()
 
-    response = resilient_request("/get", http_client=client, expected_status=404)
-    assert response.status_code == 404, "Removing HTTPRoute was not reconciled"
+    with hostname.client(retry_codes={200}) as failing_client:
+        response = failing_client.get("/get")
+        assert response.status_code == 404, "Removing HTTPRoute was not reconciled"
 
     authorization.refresh()
     condition = authorization.model.status.conditions[0]

testsuite/tests/kuadrant/reconciliation/test_httproute_hosts.py

@@ -14,12 +14,12 @@ def second_hostname(exposer, gateway, blame):
 @pytest.fixture
 def client2(second_hostname):
     """Client for a second hostname to HTTPRoute"""
-    client = second_hostname.client()
+    client = second_hostname.client(retry_codes={404})
     yield client
     client.close()
 
 
-def test_add_host(client, client2, second_hostname, route, resilient_request):
+def test_add_host(client, client2, second_hostname, route):
     """
     Tests that HTTPRoute spec.hostnames changes are reconciled when changed:
       * Test that both hostnames work
@@ -38,10 +38,11 @@ def test_add_host(client, client2, second_hostname, route, resilient_request):
 
     route.remove_hostname(second_hostname.hostname)
 
-    response = resilient_request("/get", http_client=client2, expected_status=404)
-    assert response.status_code == 404, "Removing host was not reconciled"
+    with second_hostname.client(retry_codes={200}) as failing_client:
+        response = failing_client.get("/get")
+        assert response.status_code == 404, "Removing host was not reconciled"
 
     route.add_hostname(second_hostname.hostname)
 
-    response = resilient_request("/get", http_client=client2)
+    response = client2.get("/get")
     assert response.status_code == 200, "Adding host was not reconciled"

testsuite/tests/kuadrant/reconciliation/test_httproute_matches.py

@@ -7,7 +7,7 @@
 pytestmark = [pytest.mark.kuadrant_only]
 
 
-def test_matches(client, backend, route, resilient_request):
+def test_matches(client, backend, route, hostname):
     """
     Tests that HTTPRoute spec.routes.matches changes are reconciled when changed
       * Test that /get works
@@ -21,8 +21,9 @@ def test_matches(client, backend, route, resilient_request):
     route.remove_all_rules()
     route.add_rule(backend, RouteMatch(path=PathMatch(value="/anything")))
 
-    response = resilient_request("/get", expected_status=404)
-    assert response.status_code == 404, "Matches were not reconciled"
+    with hostname.client(retry_codes={200}) as failing_client:
+        response = failing_client.get("/get")
+        assert response.status_code == 404, "Matches were not reconciled"
 
     response = client.get("/anything/get")
     assert response.status_code == 200