Merge "Use tenacity for image upload retries"

requirements.txt

@@ -26,3 +26,4 @@ netaddr>=0.7.18 # BSD
 python-zaqarclient>=1.0.0 # Apache-2.0
 python-keystoneclient>=3.8.0 # Apache-2.0
 keystoneauth1>=3.4.0 # Apache-2.0
+tenacity>=3.2.1 # Apache-2.0

tripleo_common/image/image_uploader.py

@@ -24,7 +24,7 @@ import requests
 import six
 from six.moves import urllib
 import subprocess
-import time
+import tenacity
 
 import docker
 try:
@@ -174,18 +174,23 @@ class DockerImageUploader(ImageUploader):
             LOG.info('Skipping upload for image %s' % image_name)
             return []
 
-        DockerImageUploader._pull_retry(dockerc, repo, tag=tag)
+        DockerImageUploader._pull(dockerc, repo, tag=tag)
 
         response = dockerc.tag(image=full_image, repository=new_repo,
                                tag=tag, force=True)
         LOG.debug(response)
 
-        DockerImageUploader._push_retry(dockerc, new_repo, tag=tag)
+        DockerImageUploader._push(dockerc, new_repo, tag=tag)
 
         LOG.info('Completed upload for image %s' % image_name)
         return full_image, full_new_repo
 
     @staticmethod
+    @tenacity.retry(  # Retry up to 5 times with jittered exponential backoff
+        reraise=True,
+        wait=tenacity.wait_random_exponential(multiplier=1, max=10),
+        stop=tenacity.stop_after_attempt(5)
+    )
     def _pull(dockerc, image, tag=None):
         LOG.debug('Pulling %s' % image)
 
@@ -193,24 +198,14 @@ class DockerImageUploader(ImageUploader):
             status = json.loads(line)
             if 'error' in status:
                 LOG.warning('docker pull failed: %s' % status['error'])
-                return 1
-            LOG.debug(status.get('status'))
-        return 0
-
-    @staticmethod
-    def _pull_retry(dockerc, image, tag=None):
-        retval = -1
-        count = 0
-        while retval != 0:
-            if count >= 5:
                 raise ImageUploaderException('Could not pull image %s' % image)
-            count += 1
-            retval = DockerImageUploader._pull(dockerc, image, tag)
-            if retval != 0:
-                time.sleep(3)
-                LOG.warning('retrying pulling image: %s' % image)
 
     @staticmethod
+    @tenacity.retry(  # Retry up to 5 times with jittered exponential backoff
+        reraise=True,
+        wait=tenacity.wait_random_exponential(multiplier=1, max=10),
+        stop=tenacity.stop_after_attempt(5)
+    )
     def _push(dockerc, image, tag=None):
         LOG.debug('Pushing %s' % image)
 
@@ -218,22 +213,7 @@ class DockerImageUploader(ImageUploader):
             status = json.loads(line)
             if 'error' in status:
                 LOG.warning('docker push failed: %s' % status['error'])
-                return 1
-            LOG.debug(status.get('status'))
-        return 0
-
-    @staticmethod
-    def _push_retry(dockerc, image, tag=None):
-        retval = -1
-        count = 0
-        while retval != 0:
-            if count >= 5:
                 raise ImageUploaderException('Could not push image %s' % image)
-            count += 1
-            retval = DockerImageUploader._push(dockerc, image, tag)
-            if retval != 0:
-                time.sleep(3)
-                LOG.warning('retrying pushing image: %s' % image)
 
     @staticmethod
     def _images_match(image1, image2, insecure_registries):

tripleo_common/tests/image/test_image_uploader.py

@@ -18,6 +18,7 @@ import mock
 import operator
 import requests
 import six
+import urllib3
 
 from tripleo_common.image.exception import ImageUploaderException
 from tripleo_common.image import image_uploader
@@ -402,47 +403,50 @@ class TestDockerImageUploader(base.TestCase):
                 ('docker.io/t/baz', 'rdo_release', set())
             ])
 
-    @mock.patch('time.sleep')
-    def test_pull_retry(self, sleep_mock):
+    @mock.patch('tenacity.wait.wait_random_exponential.__call__')
+    def test_pull_retry(self, mock_wait):
+        mock_wait.return_value = 0
         image = 'docker.io/tripleomaster/heat-docker-agents-centos'
 
         dockerc = self.dockermock.return_value
         dockerc.pull.side_effect = [
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
             ['{"error": "ouch"}'],
             ['{"error": "ouch"}'],
             ['{"status": "done"}']
         ]
-        self.uploader._pull_retry(dockerc, image)
+        self.uploader._pull(dockerc, image)
 
-        self.assertEqual(sleep_mock.call_count, 4)
+        self.assertEqual(dockerc.pull.call_count, 5)
         dockerc.pull.assert_has_calls([
             mock.call(image, tag=None, stream=True)
         ])
 
-    @mock.patch('time.sleep')
-    def test_pull_retry_failure(self, sleep_mock):
+    @mock.patch('tenacity.wait.wait_random_exponential.__call__')
+    def test_pull_retry_failure(self, mock_wait):
+        mock_wait.return_value = 0
         image = 'docker.io/tripleomaster/heat-docker-agents-centos'
 
         dockerc = self.dockermock.return_value
         dockerc.pull.side_effect = [
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
-            ['{"error": "ouch"}'],
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
+            urllib3.exceptions.ReadTimeoutError('p', '/foo', 'ouch'),
         ]
-        self.assertRaises(ImageUploaderException,
-                          self.uploader._pull_retry, dockerc, image)
+        self.assertRaises(urllib3.exceptions.ReadTimeoutError,
+                          self.uploader._pull, dockerc, image)
 
-        self.assertEqual(sleep_mock.call_count, 5)
+        self.assertEqual(dockerc.pull.call_count, 5)
         dockerc.pull.assert_has_calls([
             mock.call(image, tag=None, stream=True)
         ])
 
-    @mock.patch('time.sleep')
-    def test_push_retry(self, sleep_mock):
+    @mock.patch('tenacity.wait.wait_random_exponential.__call__')
+    def test_push_retry(self, mock_wait):
+        mock_wait.return_value = 0
         image = 'docker.io/tripleoupstream/heat-docker-agents-centos'
 
         dockerc = self.dockermock.return_value
@@ -453,15 +457,16 @@ class TestDockerImageUploader(base.TestCase):
             ['{"error": "ouch"}'],
             ['{"status": "done"}']
         ]
-        self.uploader._push_retry(dockerc, image)
+        self.uploader._push(dockerc, image)
 
-        self.assertEqual(sleep_mock.call_count, 4)
+        self.assertEqual(dockerc.push.call_count, 5)
         dockerc.push.assert_has_calls([
             mock.call(image, tag=None, stream=True)
         ])
 
-    @mock.patch('time.sleep')
-    def test_push_retry_failure(self, sleep_mock):
+    @mock.patch('tenacity.wait.wait_random_exponential.__call__')
+    def test_push_retry_failure(self, mock_wait):
+        mock_wait.return_value = 0
         image = 'docker.io/tripleoupstream/heat-docker-agents-centos'
 
         dockerc = self.dockermock.return_value
@@ -473,9 +478,9 @@ class TestDockerImageUploader(base.TestCase):
             ['{"error": "ouch"}'],
         ]
         self.assertRaises(ImageUploaderException,
-                          self.uploader._push_retry, dockerc, image)
+                          self.uploader._push, dockerc, image)
 
-        self.assertEqual(sleep_mock.call_count, 5)
+        self.assertEqual(dockerc.push.call_count, 5)
         dockerc.push.assert_has_calls([
             mock.call(image, tag=None, stream=True)
         ])