From ae6d868e2f13f90d9f97c982fdbbccdc6fb8b9c9 Mon Sep 17 00:00:00 2001
From: Diana Clarke <diana.joan.clarke@gmail.com>
Date: Tue, 15 Mar 2016 16:17:28 -0400
Subject: [PATCH] Fix retry mechanism for generator results

Both v1 and v2 of the glance client return python generators in some
cases (rather than fully fleshed out lists) which thwarts our retry
mechanism. Convert generator results to a list, so that any potential
exceptions get raised earlier rather than later, allowing for retries.

Change-Id: Ibc84f1596d4eaabdef0a48f6cf4da2d1323843a8
Closes-Bug: #1557584
---
 nova/image/glance.py                 |  8 ++++++-
 nova/tests/unit/image/test_glance.py | 33 ++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/nova/image/glance.py b/nova/image/glance.py
index 777c6a655df7..9291610d064a 100644
--- a/nova/image/glance.py
+++ b/nova/image/glance.py
@@ -18,6 +18,7 @@
 from __future__ import absolute_import
 
 import copy
+import inspect
 import itertools
 import random
 import sys
@@ -246,7 +247,12 @@ class GlanceClientWrapper(object):
             client = self.client or self._create_onetime_client(context,
                                                                 version)
             try:
-                return getattr(client.images, method)(*args, **kwargs)
+                result = getattr(client.images, method)(*args, **kwargs)
+                if inspect.isgenerator(result):
+                    # Convert generator results to a list, so that we can
+                    # catch any potential exceptions now and retry the call.
+                    return list(result)
+                return result
             except retry_excs as e:
                 if attempt < num_attempts:
                     extra = "retrying"
diff --git a/nova/tests/unit/image/test_glance.py b/nova/tests/unit/image/test_glance.py
index 09aee7d552d7..515b4ebddf89 100644
--- a/nova/tests/unit/image/test_glance.py
+++ b/nova/tests/unit/image/test_glance.py
@@ -381,6 +381,39 @@ class TestGlanceClientWrapper(test.NoDBTestCase):
         self.assertEqual(str(client.api_server), "https://host2:9293")
         sleep_mock.assert_called_once_with(1)
 
+    @mock.patch('random.shuffle')
+    @mock.patch('time.sleep')
+    @mock.patch('nova.image.glance._glanceclient_from_endpoint')
+    def test_retry_works_with_generators(self, create_client_mock,
+                                         sleep_mock, shuffle_mock):
+        def some_generator(exception):
+            if exception:
+                raise glanceclient.exc.CommunicationError('Boom!')
+            yield 'something'
+
+        api_servers = [
+            'https://host2:9292',
+            'https://host2:9293',
+            'http://host3:9294'
+        ]
+        client_mock = mock.MagicMock()
+        images_mock = mock.MagicMock()
+        images_mock.list.side_effect = [
+            some_generator(exception=True),
+            some_generator(exception=False),
+        ]
+        type(client_mock).images = mock.PropertyMock(return_value=images_mock)
+        create_client_mock.return_value = client_mock
+
+        self.flags(num_retries=1, group='glance')
+        self.flags(api_servers=api_servers, group='glance')
+
+        ctx = context.RequestContext('fake', 'fake')
+        client = glance.GlanceClientWrapper()
+        client.call(ctx, 1, 'list', 'meow')
+        sleep_mock.assert_called_once_with(1)
+        self.assertEqual(str(client.api_server), 'https://host2:9293')
+
     @mock.patch('oslo_service.sslutils.is_enabled')
     @mock.patch('glanceclient.Client')
     def test_create_glance_client_with_ssl(self, client_mock,