From b9fd6390777b8bcb8f2591ed31540c8c2d9d831e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Andr=C3=A9?= <m.andre@redhat.com>
Date: Wed, 20 Sep 2017 17:03:26 +0200
Subject: [PATCH] Add retry loop for docker pull

The prepare command also needs a retry loop for docker pull in order to
accommodate with the not so infrequent I/O failures.

Change-Id: I14d5ef226808e8bf2e983b74c9918ea4d61a4aa1
Related-Bug: #1715136
(cherry picked from commit 2ff0842fcb27af43903d1738bff26bec558c966a)
---
 tripleo_common/image/image_uploader.py        | 24 ++++++++--
 .../tests/image/test_image_uploader.py        | 47 ++++++++++++-------
 2 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/tripleo_common/image/image_uploader.py b/tripleo_common/image/image_uploader.py
index 7212c6f6c..e130536f5 100644
--- a/tripleo_common/image/image_uploader.py
+++ b/tripleo_common/image/image_uploader.py
@@ -19,6 +19,7 @@ import json
 import logging
 import netifaces
 import six
+import time
 
 try:
     from docker import APIClient as Client
@@ -119,7 +120,7 @@ class DockerImageUploader(ImageUploader):
         else:
             repo = image
 
-        self._pull(dockerc, repo, tag=tag)
+        self._pull_retry(dockerc, repo, tag=tag)
 
         full_image = repo + ':' + tag
         new_repo = push_destination + '/' + repo.partition('/')[2]
@@ -138,9 +139,22 @@ class DockerImageUploader(ImageUploader):
         for line in dockerc.pull(image, tag=tag, stream=True):
             status = json.loads(line)
             if 'error' in status:
-                raise ImageUploaderException('Could not pull image: %s' %
-                                             status['error'])
+                self.logger.warning('docker pull failed: %s' % status['error'])
+                return 1
             self.logger.debug(status.get('status'))
+        return 0
+
+    def _pull_retry(self, 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 = self._pull(dockerc, image, tag)
+            if retval != 0:
+                time.sleep(3)
+                self.logger.warning('retrying pulling image: %s' % image)
 
     def discover_image_tag(self, image, tag_from_label=None):
         dockerc = Client(base_url='unix://var/run/docker.sock', version='auto')
@@ -150,7 +164,7 @@ class DockerImageUploader(ImageUploader):
             tag = 'latest'
         image = '%s:%s' % (image_name, tag)
 
-        self._pull(dockerc, image)
+        self._pull_retry(dockerc, image)
         i = dockerc.inspect_image(image)
         labels = i['Config']['Labels']
 
@@ -171,5 +185,5 @@ class DockerImageUploader(ImageUploader):
         # confirm the tag exists by pulling it, which should be fast
         # because that image has just been pulled
         versioned_image = '%s:%s' % (image_name, tag_label)
-        self._pull(dockerc, versioned_image)
+        self._pull_retry(dockerc, versioned_image)
         return tag_label
diff --git a/tripleo_common/tests/image/test_image_uploader.py b/tripleo_common/tests/image/test_image_uploader.py
index c33f8ba37..ecbb4fe1c 100644
--- a/tripleo_common/tests/image/test_image_uploader.py
+++ b/tripleo_common/tests/image/test_image_uploader.py
@@ -234,26 +234,41 @@ class TestDockerImageUploader(base.TestCase):
         self.assertRaises(ImageUploaderException,
                           self.uploader.discover_image_tag, image, 'foo')
 
-    def test_discover_image_tag_missing_tag(self):
-        image = 'docker.io/tripleoupstream/heat-docker-agents-centos:latest'
-        vimage = 'docker.io/tripleoupstream/heat-docker-agents-centos:1.2.3'
+    @mock.patch('time.sleep')
+    def test_pull_retry(self, sleep_mock):
+        image = 'docker.io/tripleoupstream/heat-docker-agents-centos'
 
         dockerc = self.dockermock.return_value
         dockerc.pull.side_effect = [
-            ['{"status": "done"}'],  # First pull, :latest
-            ['{"error": "ouch"}'],  # Second pull, :1.2.3
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+            ['{"status": "done"}']
         ]
-        dockerc.inspect_image.return_value = {
-            'Config': {'Labels': {'image-version': '1.2.3'}}
-        }
-        self.assertRaises(ImageUploaderException,
-                          self.uploader.discover_image_tag, image,
-                          'image-version')
-
-        self.dockermock.assert_called_once_with(
-            base_url='unix://var/run/docker.sock', version='auto')
+        self.uploader._pull_retry(dockerc, image)
 
+        self.assertEqual(sleep_mock.call_count, 4)
         dockerc.pull.assert_has_calls([
-            mock.call(image, tag=None, stream=True),
-            mock.call(vimage, tag=None, stream=True),
+            mock.call(image, tag=None, stream=True)
+        ])
+
+    @mock.patch('time.sleep')
+    def test_pull_retry_failure(self, sleep_mock):
+        image = 'docker.io/tripleoupstream/heat-docker-agents-centos'
+
+        dockerc = self.dockermock.return_value
+        dockerc.pull.side_effect = [
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+            ['{"error": "ouch"}'],
+        ]
+        self.assertRaises(ImageUploaderException,
+                          self.uploader._pull_retry, dockerc, image)
+
+        self.assertEqual(sleep_mock.call_count, 5)
+        dockerc.pull.assert_has_calls([
+            mock.call(image, tag=None, stream=True)
         ])