Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 50 additions & 21 deletions testsuite/httpx/__init__.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -18,15 +20,46 @@ def create_tmp_file(content: str):
return file


class UnexpectedResponse(Exception):
"""Slightly different response attributes were expected or no response was given"""
class Result:
"""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 (not self.has_error() and self.status_code in self.retry_codes)

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):
Expand Down Expand Up @@ -59,7 +92,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,
Expand All @@ -76,7 +109,7 @@ def request(
follow_redirects=None,
timeout=None,
extensions=None,
) -> Response:
) -> Result:
try:
response = super().request(
method,
Expand All @@ -93,18 +126,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 Result(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 Result(self.retry_codes, error=e)

def get(self, *args, **kwargs) -> Result:
return super().get(*args, **kwargs)

def get_many(self, url, count, *, params=None, headers=None, auth=None) -> list[Result]:
"""Send multiple `GET` requests."""
responses = []
for _ in range(count):
Expand Down
6 changes: 3 additions & 3 deletions testsuite/openshift/objects/gateway_api/route.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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]:
Expand Down
4 changes: 2 additions & 2 deletions testsuite/openshift/objects/route.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from httpx import Client

from testsuite.httpx import HttpxBackoffClient
from testsuite.httpx import KuadrantClient
from testsuite.openshift.objects import OpenShiftObject


Expand Down Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions testsuite/tests/kuadrant/authorino/operator/http/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import pytest

from testsuite.objects import Value, JsonResponse
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

Expand All @@ -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()

Expand Down
10 changes: 6 additions & 4 deletions testsuite/tests/mgc/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"""
import pytest

from testsuite.httpx import HttpxBackoffClient
from testsuite.httpx import KuadrantClient

pytestmark = [pytest.mark.mgc]

Expand All @@ -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