Fixes the reroute_compute decorator in the scheduler API so that it properly:

1) Raises InstanceNotFound when all zones return no instance
2) If an instance is found in a zone, ignore whether other zones may be down.  Return the instance
information, catching the novaclient exceptions from the other zones.
3) If a zone is down and no instances are found in other zones, raise a ZoneRequestError... as it's possible the instance exists in the downed zones.

3 files changed, 139 insertions, 22 deletions

diff --git a/nova/exception.py b/nova/exception.py
index 8e73fdfc8..a3cbb98cf 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -810,3 +810,10 @@ class CannotResizeToSmallerSize(NovaException):
 
 class ImageTooLarge(NovaException):
     message = _("Image is larger than instance type allows")
+
+
+class ZoneRequestError(Error):
+    def __init__(self, message=None):
+        if message is None:
+            message = _("1 or more Zones could not complete the request")
+        super(ZoneRequestError, self).__init__(message=message)
diff --git a/nova/scheduler/api.py b/nova/scheduler/api.py
index 55cea5f8f..719437b73 100644
--- a/nova/scheduler/api.py
+++ b/nova/scheduler/api.py
@@ -103,22 +103,6 @@ def update_service_capabilities(context, service_name, host, capabilities):
     return rpc.fanout_cast(context, 'scheduler', kwargs)
 
 
-def _wrap_method(function, self):
-    """Wrap method to supply self."""
-    def _wrap(*args, **kwargs):
-        return function(self, *args, **kwargs)
-    return _wrap
-
-
-def _process(func, zone):
-    """Worker stub for green thread pool. Give the worker
-    an authenticated nova client and zone info."""
-    nova = novaclient.Client(zone.username, zone.password, None,
-                                zone.api_url)
-    nova.authenticate()
-    return func(nova, zone)
-
-
 def call_zone_method(context, method_name, errors_to_ignore=None,
                      novaclient_collection_name='zones', zones=None,
                      *args, **kwargs):
@@ -166,6 +150,32 @@ def child_zone_helper(zone_list, func):
     For example, if you are calling server.pause(), the list will
     be whatever the response from server.pause() is. One entry
     per child zone called."""
+
+    def _wrap_method(function, arg1):
+        """Wrap method to supply an argument."""
+        def _wrap(*args, **kwargs):
+            return function(arg1, *args, **kwargs)
+        return _wrap
+
+    def _process(func, zone):
+        """Worker stub for green thread pool. Give the worker
+        an authenticated nova client and zone info."""
+        try:
+            nova = novaclient.Client(zone.username, zone.password, None,
+                    zone.api_url)
+            nova.authenticate()
+        except novaclient_exceptions.BadRequest, e:
+            url = zone.api_url
+            LOG.warn(_("Failed request to zone; URL=%(url)s: %(e)s")
+                    % locals())
+            # This is being returned instead of raised, so that when
+            # results are processed in unmarshal_result() after the
+            # greenpool.imap completes, the exception can be raised
+            # there if no other zones had a response.
+            return exception.ZoneRequestError()
+        else:
+            return func(nova, zone)
+
     green_pool = greenpool.GreenPool()
     return [result for result in green_pool.imap(
                     _wrap_method(_process, func), zone_list)]
@@ -260,6 +270,8 @@ class reroute_compute(object):
         if not FLAGS.enable_zone_routing:
             raise exception.InstanceNotFound(instance_id=item_uuid)
 
+        self.item_uuid = item_uuid
+
         zones = db.zone_get_all(context)
         if not zones:
             raise exception.InstanceNotFound(instance_id=item_uuid)
@@ -342,9 +354,13 @@ class reroute_compute(object):
         dict {'server':{k:v}}. Others may return a list of them, like
         {'servers':[{k,v}]}"""
         reduced_response = []
+        found_exception = None
         for zone_response in zone_responses:
             if not zone_response:
                 continue
+            if isinstance(zone_response, BaseException):
+                found_exception = zone_response
+                continue
 
             server = zone_response.__dict__
 
@@ -355,7 +371,9 @@ class reroute_compute(object):
             reduced_response.append(dict(server=server))
         if reduced_response:
             return reduced_response[0]  # first for now.
-        return {}
+        elif found_exception:
+            raise found_exception
+        raise exception.InstanceNotFound(instance_id=self.item_uuid)
 
 
 def redirect_handler(f):
diff --git a/nova/tests/scheduler/test_scheduler.py b/nova/tests/scheduler/test_scheduler.py
index a52dd041a..890348192 100644
--- a/nova/tests/scheduler/test_scheduler.py
+++ b/nova/tests/scheduler/test_scheduler.py
@@ -963,9 +963,14 @@ class FakeZone(object):
         self.password = password
 
 
+ZONE_API_URL1 = "http://1.example.com"
+ZONE_API_URL2 = "http://2.example.com"
+
+
 def zone_get_all(context):
     return [
-                FakeZone(1, 'http://example.com', 'bob', 'xxx'),
+                FakeZone(1, ZONE_API_URL1, 'bob', 'xxx'),
+                FakeZone(2, ZONE_API_URL2, 'bob', 'xxx'),
            ]
 
 
@@ -1065,7 +1070,9 @@ class ZoneRedirectTest(test.TestCase):
 
     def test_unmarshal_single_server(self):
         decorator = api.reroute_compute("foo")
-        self.assertEquals(decorator.unmarshall_result([]), {})
+        decorator.item_uuid = 'fake_uuid'
+        self.assertRaises(exception.InstanceNotFound,
+                decorator.unmarshall_result, [])
         self.assertEquals(decorator.unmarshall_result(
                 [FakeResource(dict(a=1, b=2)), ]),
                 dict(server=dict(a=1, b=2)))
@@ -1079,6 +1086,90 @@ class ZoneRedirectTest(test.TestCase):
                 [FakeResource(dict(_a=1, manager=2)), ]),
                 dict(server={}))
 
+    def test_one_zone_down_no_instances(self):
+
+        def _fake_issue_novaclient_command(nova, zone, *args, **kwargs):
+            return None
+
+        class FakeNovaClientWithFailure(object):
+            def __init__(self, username, password, method, api_url):
+                self.api_url = api_url
+
+            def authenticate(self):
+                if self.api_url == ZONE_API_URL2:
+                    raise novaclient_exceptions.BadRequest('foo')
+
+        self.stubs.Set(api, '_issue_novaclient_command',
+                _fake_issue_novaclient_command)
+        self.stubs.Set(api.novaclient, 'Client', FakeNovaClientWithFailure)
+
+        @api.reroute_compute("get")
+        def do_get(self, context, uuid):
+            pass
+
+        self.assertRaises(exception.ZoneRequestError,
+                do_get, None, {}, FAKE_UUID)
+
+    def test_one_zone_down_got_instance(self):
+
+        def _fake_issue_novaclient_command(nova, zone, *args, **kwargs):
+            class FakeServer(object):
+                def __init__(self):
+                    self.id = FAKE_UUID
+                    self.test = '1234'
+            return FakeServer()
+
+        class FakeNovaClientWithFailure(object):
+            def __init__(self, username, password, method, api_url):
+                self.api_url = api_url
+
+            def authenticate(self):
+                if self.api_url == ZONE_API_URL2:
+                    raise novaclient_exceptions.BadRequest('foo')
+
+        self.stubs.Set(api, '_issue_novaclient_command',
+                _fake_issue_novaclient_command)
+        self.stubs.Set(api.novaclient, 'Client', FakeNovaClientWithFailure)
+
+        @api.reroute_compute("get")
+        def do_get(self, context, uuid):
+            pass
+
+        try:
+            do_get(None, {}, FAKE_UUID)
+        except api.RedirectResult, e:
+            results = e.results
+            self.assertIn('server', results)
+            self.assertEqual(results['server']['id'], FAKE_UUID)
+            self.assertEqual(results['server']['test'], '1234')
+        except Exception, e:
+            self.fail(_("RedirectResult should have been raised"))
+        else:
+            self.fail(_("RedirectResult should have been raised"))
+
+    def test_zones_up_no_instances(self):
+
+        def _fake_issue_novaclient_command(nova, zone, *args, **kwargs):
+            return None
+
+        class FakeNovaClientNoFailure(object):
+            def __init__(self, username, password, method, api_url):
+                pass
+
+            def authenticate(self):
+                return
+
+        self.stubs.Set(api, '_issue_novaclient_command',
+                _fake_issue_novaclient_command)
+        self.stubs.Set(api.novaclient, 'Client', FakeNovaClientNoFailure)
+
+        @api.reroute_compute("get")
+        def do_get(self, context, uuid):
+            pass
+
+        self.assertRaises(exception.InstanceNotFound,
+                do_get, None, {}, FAKE_UUID)
+
 
 class FakeServerCollection(object):
     def get(self, instance_id):
@@ -1097,7 +1188,7 @@ class FakeEmptyServerCollection(object):
 
 
 class FakeNovaClient(object):
-    def __init__(self, collection):
+    def __init__(self, collection, *args, **kwargs):
         self.servers = collection
 
 
@@ -1162,8 +1253,9 @@ class CallZoneMethodTest(test.TestCase):
         context = {}
         method = 'do_something'
         results = api.call_zone_method(context, method)
-        expected = [(1, 42)]
-        self.assertEqual(expected, results)
+        self.assertEqual(len(results), 2)
+        self.assertIn((1, 42), results)
+        self.assertIn((2, 42), results)
 
     def test_call_zone_method_not_present(self):
         context = {}