From d38e11ea8a170500e3f4aeb79b949506f64c2464 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Budai?= <ondrej@budai.cz>
Date: Wed, 20 Jul 2022 14:57:44 +0200
Subject: [PATCH] builder: add retries to composer API calls

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.
---
 plugins/builder/osbuild.py |  9 +++++++
 test/unit/test_builder.py  | 49 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/plugins/builder/osbuild.py b/plugins/builder/osbuild.py
index 8cd4a60..a4bc9e7 100644
--- a/plugins/builder/osbuild.py
+++ b/plugins/builder/osbuild.py
@@ -29,6 +29,7 @@ from typing import Dict, List, Optional, Union
 import requests
 import koji
 
+from requests.adapters import HTTPAdapter, Retry
 from koji.daemon import fast_incremental_upload
 from koji.tasks import BaseTaskHandler
 
@@ -336,6 +337,14 @@ class Client:
         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(',')]
diff --git a/test/unit/test_builder.py b/test/unit/test_builder.py
index 890335a..11f0c24 100644
--- a/test/unit/test_builder.py
+++ b/test/unit/test_builder.py
@@ -300,6 +300,46 @@ class MockComposer:  # pylint: disable=too-many-instance-attributes
         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
 
@@ -1168,3 +1208,12 @@ class TestBuilderPlugin(PluginTest): # pylint: disable=too-many-public-methods
                 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)