Skip to content

Commit

Permalink
builder: add retries to composer API calls
Browse files Browse the repository at this point in the history
The status calls are sometimes failing on:
upstream connect error or disconnect/reset before headers. reset reason:
connection termination

Since all requests are going through the company proxy, I think that the
networking isn't working 100% reliably. This commit adds a retry mechanism
provided by the urllib3 library. It will retry on all networking issues and
also on some 5xx errors that makes sense to retry (like gateway failures).

A test is added that runs the compose waiting code against a mock server
that fails every second request. This is imho sufficient to mimick a flaky
networking.
  • Loading branch information
ondrejbudai authored and gicmo committed Aug 5, 2022
1 parent 99062e8 commit d38e11e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
9 changes: 9 additions & 0 deletions plugins/builder/osbuild.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import requests
import koji

from requests.adapters import HTTPAdapter, Retry
from koji.daemon import fast_incremental_upload
from koji.tasks import BaseTaskHandler

Expand Down Expand Up @@ -336,6 +337,14 @@ def __init__(self, url):
self.url = urllib.parse.urljoin(url, API_BASE)
self.http = requests.Session()

retries = Retry(total=5,
backoff_factor=0.1,
status_forcelist=[500, 502, 503, 504],
raise_on_status=False
)

self.http.mount(self.server, HTTPAdapter(max_retries=retries))

@staticmethod
def parse_certs(string):
certs = [s.strip() for s in string.split(',')]
Expand Down
49 changes: 49 additions & 0 deletions test/unit/test_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,46 @@ def oauth_activate(self, token_url: str):
print("OAuth active!")


# MockComposerStatus is a simple class for mocking just the GET /composer/{id}
# route of composer. In comparison with MockComposer, it can also mock the
# route being flaky. The current implementation fails every other request with
# error 500 which nicely simulates the fact the networking doesn't always work
# 100% reliably.
class MockComposerStatus:
calls = 0

def __init__(self, compose_id, calls_until_success=10):
self.calls_until_success = calls_until_success
self.compose_id = compose_id

def compose_status(self, _request, _uri, response_headers):
self.calls += 1
if self.calls % 2 == 0:
return [500, response_headers, "I'm flaky!"]

status = "success" if self.calls > 10 else "pending"

result = {
"status": status,
"koji_status": {
"build_id": 42,
},
"image_statuses": [
{
"status": status
}
]
}

return [200, response_headers, json.dumps(result)]

def httpretty_register(self):
httpretty.register_uri(
httpretty.GET,
urllib.parse.urljoin(f"http://localhost/{API_BASE}", f"composes/{self.compose_id}"),
body=self.compose_status
)

class UploadTracker:
"""Mock koji file uploading and keep track of uploaded files
Expand Down Expand Up @@ -1168,3 +1208,12 @@ def test_compose_repo_complex(self):
if baseurl.startswith("https://first.repo"):
ps = r.get("package_sets")
assert ps and ps == ["a", "b", "c", "d"]

@httpretty.activate
def test_compose_status_retry(self):
compose_id = "43e57e63-ab32-4a8d-854d-3bbc117fdce3"

MockComposerStatus(compose_id).httpretty_register()

client = self.plugin.Client("http://localhost")
client.wait_for_compose(compose_id, sleep_time=0.1)

0 comments on commit d38e11e

Please sign in to comment.