Add has_dns_error() and has_tls_error() assert to mgc tests

testsuite/httpx/__init__.py

@@ -1,9 +1,11 @@
 """Common classes for Httpx"""
+# 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
 
 import backoff
-from httpx import Client, Response, ConnectError
+from httpx import Client, ConnectError
 
 from testsuite.certificates import Certificate
 
@@ -18,15 +20,50 @@ def create_tmp_file(content: str):
     return file
 
 
-class UnexpectedResponse(Exception):
-    """Slightly different response attributes were expected or no response was given"""
+class KuadrantResult:
+    """Result from HTTP request"""
 
-    def __init__(self, msg, response):
-        super().__init__(msg)
+    def __init__(self, retry_codes, response=None, error=None):
         self.response = response
-
-
-class HttpxBackoffClient(Client):
+        self.error = error
+        self.retry_codes = retry_codes
+
+    def should_backoff(self):
+        """True, if the Result can be considered an instability and should be retried"""
+        return self.has_dns_error() or (self.success() and self.status_code in self.retry_codes)
+
+    def success(self):
+        """True, if the request succeeded (=didn't throw error)"""
+        return self.response is not None
+
+    def has_error(self):
+        """True, if the request failed and an error was returned"""
+        return self.error is not None
+
+    def has_dns_error(self):
+        """True, if the result failed due to DNS failure"""
+        return (
+            self.has_error()
+            and len(self.error.args) > 0
+            and any("Name or service not known" in arg for arg in self.error.args)
+        )
+
+    def has_tls_error(self):
+        """True, if the result failed due to TLS failure"""
+        return (
+            self.has_error()
+            and len(self.error.args) > 0
+            and any("SSL: CERTIFICATE_VERIFY_FAILED" in arg for arg in self.error.args)
+        )
+
+    def __getattr__(self, item):
+        """For backwards compatibility"""
+        if self.response is not None:
+            return getattr(self.response, item)
+        return None
+
+
+class KuadrantClient(Client):
     """Httpx client which retries unstable requests"""
 
     def __init__(self, *, verify: Union[Certificate, bool] = True, cert: Certificate = None, **kwargs):
@@ -59,7 +96,7 @@ def add_retry_code(self, code):
         self.retry_codes.add(code)
 
     # pylint: disable=too-many-locals
-    @backoff.on_exception(backoff.fibo, UnexpectedResponse, max_tries=8, jitter=None)
+    @backoff.on_predicate(backoff.fibo, lambda result: result.should_backoff(), max_tries=8, jitter=None)
     def request(
         self,
         method: str,
@@ -76,7 +113,7 @@ def request(
         follow_redirects=None,
         timeout=None,
         extensions=None,
-    ) -> Response:
+    ) -> KuadrantResult:
         try:
             response = super().request(
                 method,
@@ -93,18 +130,14 @@ def request(
                 timeout=timeout,
                 extensions=extensions,
             )
-            if response.status_code in self.retry_codes:
-                raise UnexpectedResponse(f"Didn't expect '{response.status_code}' status code", response)
-            return response
+            return KuadrantResult(self.retry_codes, response=response)
         except ConnectError as e:
-            # note: when the code reaches this point, negative caching might have been triggered,
-            # negative caching TTL of SOA record of the zone must be set accordingly,
-            # otherwise retry will fail if the value is too high
-            if len(e.args) > 0 and any("Name or service not known" in arg for arg in e.args):
-                raise UnexpectedResponse("Didn't expect 'Name or service not known' error", None) from e
-            raise
-
-    def get_many(self, url, count, *, params=None, headers=None, auth=None) -> list[Response]:
+            return KuadrantResult(self.retry_codes, error=e)
+
+    def get(self, *args, **kwargs) -> KuadrantResult:
+        return super().get(*args, **kwargs)
+
+    def get_many(self, url, count, *, params=None, headers=None, auth=None) -> list[KuadrantResult]:
         """Send multiple `GET` requests."""
         responses = []
         for _ in range(count):

testsuite/openshift/objects/gateway_api/route.py

@@ -6,7 +6,7 @@
 
 from httpx import Client
 
-from testsuite.httpx import HttpxBackoffClient
+from testsuite.httpx import KuadrantClient
 from testsuite.openshift.client import OpenShiftClient
 from testsuite.openshift.objects import modify, OpenShiftObject
 from testsuite.openshift.objects.route import Route
@@ -23,7 +23,7 @@ class HTTPRoute(OpenShiftObject, Referencable):
 
     def client(self, **kwargs) -> Client:
         """Returns HTTPX client"""
-        return HttpxBackoffClient(base_url=f"http://{self.hostnames[0]}", **kwargs)
+        return KuadrantClient(base_url=f"http://{self.hostnames[0]}", **kwargs)
 
     @classmethod
     def create_instance(
@@ -106,7 +106,7 @@ def hostname(self) -> str:
         return self._hostname
 
     def client(self, **kwargs) -> Client:
-        return HttpxBackoffClient(base_url=f"http://{self.hostname}", **kwargs)
+        return KuadrantClient(base_url=f"http://{self.hostname}", **kwargs)
 
     @property
     def reference(self) -> dict[str, str]:

testsuite/openshift/objects/route.py

@@ -4,7 +4,7 @@
 
 from httpx import Client
 
-from testsuite.httpx import HttpxBackoffClient
+from testsuite.httpx import KuadrantClient
 from testsuite.openshift.objects import OpenShiftObject
 
 
@@ -53,7 +53,7 @@ def client(self, **kwargs) -> Client:
         protocol = "http"
         if "tls" in self.model.spec:
             protocol = "https"
-        return HttpxBackoffClient(base_url=f"{protocol}://{self.hostname}", **kwargs)
+        return KuadrantClient(base_url=f"{protocol}://{self.hostname}", **kwargs)
 
     @cached_property
     def hostname(self):

testsuite/tests/kuadrant/authorino/operator/http/conftest.py

@@ -2,7 +2,7 @@
 import pytest
 
 from testsuite.objects import Value
-from testsuite.httpx import HttpxBackoffClient
+from testsuite.httpx import KuadrantClient
 from testsuite.openshift.objects.auth_config import AuthConfig
 from testsuite.openshift.objects.route import OpenshiftRoute
 
@@ -20,7 +20,7 @@ def authorization(authorization, wildcard_domain, openshift, module_label) -> Au
 @pytest.fixture(scope="module")
 def client(authorization, authorino_route):
     """Returns httpx client to be used for requests, it also commits AuthConfig"""
-    client = HttpxBackoffClient(base_url=f"http://{authorino_route.model.spec.host}", verify=False)
+    client = KuadrantClient(base_url=f"http://{authorino_route.model.spec.host}", verify=False)
     yield client
     client.close()
 

testsuite/tests/mgc/test_basic.py

@@ -15,7 +15,7 @@
 """
 import pytest
 
-from testsuite.httpx import HttpxBackoffClient
+from testsuite.httpx import KuadrantClient
 
 pytestmark = [pytest.mark.mgc]
 
@@ -34,7 +34,9 @@ def test_smoke(route, upstream_gateway):
     tls_cert = upstream_gateway.get_tls_cert()
 
     # assert that tls_cert is used by the server
-    backend_client = HttpxBackoffClient(base_url=f"https://{route.hostnames[0]}", verify=tls_cert)
+    backend_client = KuadrantClient(base_url=f"https://{route.hostnames[0]}", verify=tls_cert)
 
-    response = backend_client.get("get")
-    assert response.status_code == 200
+    result = backend_client.get("get")
+    assert not result.has_dns_error()
+    assert not result.has_tls_error()
+    assert result.status_code == 200