From c6ab6b9546aa1ae039070d5a501b30d20d5fb5e4 Mon Sep 17 00:00:00 2001
From: Lukas Holecek <hluk@email.cz>
Date: Apr 08 2022 08:55:03 +0000
Subject: Retry on ResponseNotReady exception during XMLRPC calls


JIRA: RHELWF-6331

---

diff --git a/greenwave/tests/test_xmlrpc_server_proxy.py b/greenwave/tests/test_xmlrpc_server_proxy.py
index f0b4804..bc7369f 100644
--- a/greenwave/tests/test_xmlrpc_server_proxy.py
+++ b/greenwave/tests/test_xmlrpc_server_proxy.py
@@ -1,4 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+
+import http.client
+import uuid
 
 import mock
 import pytest
@@ -6,12 +8,81 @@ import pytest
 from greenwave import xmlrpc_server_proxy
 
 
+def unique_koji_url(protocol='https'):
+    """
+    Generates unique Koji API URL to bypass caching on arguments in
+    xmlrpc_server_proxy().
+    """
+    return f'{protocol}://koji-{uuid.uuid4()}.example.com/kojihub'
+
+
+@pytest.fixture
+def mock_xmlrpc_proxy():
+    with mock.patch(
+            'greenwave.xmlrpc_server_proxy.xmlrpc.client.ServerProxy') as proxy:
+        yield proxy
+
+
+def test_xmlrpc_server_proxy_call(mock_xmlrpc_proxy):
+    proxy = xmlrpc_server_proxy.get_server_proxy(unique_koji_url(), timeout=0)
+    assert mock_xmlrpc_proxy.call_count == 1
+    proxy.getBuild('fake_koji_build')
+    assert mock_xmlrpc_proxy.call_count == 1
+    proxy.proxy.getBuild.assert_called_once_with('fake_koji_build')
+
+
+def test_xmlrpc_server_proxy_failure(mock_xmlrpc_proxy):
+    proxy = xmlrpc_server_proxy.get_server_proxy(unique_koji_url(), timeout=0)
+    assert mock_xmlrpc_proxy.call_count == 1
+    proxy.proxy.getBuild.side_effect = http.client.ResponseNotReady
+    with mock.patch('greenwave.xmlrpc_server_proxy.sleep') as mock_sleep:
+        with pytest.raises(http.client.ResponseNotReady):
+            proxy.getBuild('fake_koji_build')
+        mock_sleep.assert_has_calls([
+            mock.call(1),
+            mock.call(2),
+            mock.call(4),
+        ])
+    assert mock_xmlrpc_proxy.call_count == 4
+    proxy.proxy.getBuild.assert_has_calls([
+        mock.call('fake_koji_build'),
+        mock.call('fake_koji_build'),
+        mock.call('fake_koji_build'),
+        mock.call('fake_koji_build'),
+    ])
+
+
+def test_xmlrpc_server_proxy_reconnect(mock_xmlrpc_proxy):
+    proxy = xmlrpc_server_proxy.get_server_proxy(unique_koji_url(), timeout=0)
+    assert mock_xmlrpc_proxy.call_count == 1
+    proxy.proxy.getBuild.side_effect = (
+        http.client.ResponseNotReady,
+        http.client.ResponseNotReady,
+        http.client.ResponseNotReady,
+        {},
+    )
+    with mock.patch('greenwave.xmlrpc_server_proxy.sleep') as mock_sleep:
+        proxy.getBuild('fake_koji_build')
+        mock_sleep.assert_has_calls([
+            mock.call(1),
+            mock.call(2),
+            mock.call(4),
+        ])
+    assert mock_xmlrpc_proxy.call_count == 4
+    proxy.proxy.getBuild.assert_has_calls([
+        mock.call('fake_koji_build'),
+        mock.call('fake_koji_build'),
+        mock.call('fake_koji_build'),
+        mock.call('fake_koji_build'),
+    ])
+
+
 @pytest.mark.parametrize(
     'url, expected_transport, timeout, expected_timeout',
     (
-        ('http://localhost:5000/api', xmlrpc_server_proxy.Transport, 15, 15),
-        ('https://localhost:5000/api', xmlrpc_server_proxy.SafeTransport, 15, 15),
-        ('https://localhost:5000/api', xmlrpc_server_proxy.SafeTransport, (3, 12), 12),
+        (unique_koji_url('http'), xmlrpc_server_proxy.Transport, 15, 15),
+        (unique_koji_url(), xmlrpc_server_proxy.SafeTransport, 15, 15),
+        (unique_koji_url(), xmlrpc_server_proxy.SafeTransport, (3, 12), 12),
     ),
 )
 @mock.patch('greenwave.xmlrpc_server_proxy.Transport')
@@ -36,12 +107,14 @@ def test_get_server_proxy(
 
 def test_get_server_proxy_cached():
     """Server proxy objects are cached"""
-    s1 = xmlrpc_server_proxy.get_server_proxy('https://localhost:5000/api', timeout=None)
-    s2 = xmlrpc_server_proxy.get_server_proxy('https://localhost:5000/api', timeout=None)
+    url1 = unique_koji_url()
+    s1 = xmlrpc_server_proxy.get_server_proxy(url1, timeout=None)
+    s2 = xmlrpc_server_proxy.get_server_proxy(url1, timeout=None)
     assert s1 is s2
 
-    s3 = xmlrpc_server_proxy.get_server_proxy('https://localhost:5000/api', timeout=10)
+    s3 = xmlrpc_server_proxy.get_server_proxy(url1, timeout=10)
     assert s1 is not s3
 
-    s4 = xmlrpc_server_proxy.get_server_proxy('https://localhost:5001/api', timeout=None)
+    url2 = unique_koji_url()
+    s4 = xmlrpc_server_proxy.get_server_proxy(url2, timeout=None)
     assert s1 is not s4
diff --git a/greenwave/xmlrpc_server_proxy.py b/greenwave/xmlrpc_server_proxy.py
index 8b166fc..958f49f 100644
--- a/greenwave/xmlrpc_server_proxy.py
+++ b/greenwave/xmlrpc_server_proxy.py
@@ -3,9 +3,58 @@
 """
 Provides an "xmlrpc.client.ServerProxy" object with a timeout on the socket.
 """
+import http.client
+import logging
 import urllib.parse
 import xmlrpc.client
 from functools import lru_cache
+from time import sleep
+
+log = logging.getLogger(__name__)
+
+RETRY_ON_EXCEPTIONS = (
+    http.client.ResponseNotReady,
+)
+MAX_RETRIES = 3
+
+
+class XmlRpcServerProxy:
+    def __init__(self, uri, timeout):
+        self.uri = uri
+        self.timeout = timeout
+        self.reconnect()
+
+    def reconnect(self):
+        parsed_uri = urllib.parse.urlparse(self.uri)
+        if parsed_uri.scheme == 'https':
+            transport = SafeTransport(timeout=self.timeout)
+        else:
+            transport = Transport(timeout=self.timeout)
+
+        self.proxy = xmlrpc.client.ServerProxy(self.uri, transport=transport)
+
+    def __getattr__(self, name):
+        return XmlRpcMethod(self, name)
+
+
+class XmlRpcMethod:
+    def __init__(self, proxy, name):
+        self.proxy = proxy
+        self.name = name
+
+    def __call__(self, *args):
+        retry_counter = 0
+        while True:
+            try:
+                return getattr(self.proxy.proxy, self.name)(*args)
+            except RETRY_ON_EXCEPTIONS as e:
+                retry_counter += 1
+                if retry_counter > MAX_RETRIES:
+                    raise
+
+                log.warning("Retrying XMLRPC call %r on error: %s", self.name, e)
+                sleep(2 ** (retry_counter - 1))
+                self.proxy.reconnect()
 
 
 @lru_cache(maxsize=None)
@@ -20,16 +69,10 @@ def get_server_proxy(uri, timeout):
         timeout (int): The timeout to set on the transport socket.
 
     Returns:
-        xmlrpc.client.ServerProxy: An instance of :py:class:`xmlrpc.client.ServerProxy` with
+        XmlRpcServerProxy: Wrapper for :py:class:`xmlrpc.client.ServerProxy` with
             a socket timeout set.
     """
-    parsed_uri = urllib.parse.urlparse(uri)
-    if parsed_uri.scheme == 'https':
-        transport = SafeTransport(timeout=timeout)
-    else:
-        transport = Transport(timeout=timeout)
-
-    return xmlrpc.client.ServerProxy(uri, transport=transport)
+    return XmlRpcServerProxy(uri, timeout)
 
 
 class Transport(xmlrpc.client.Transport):

