Merge "Allow retrying network allocations separately"

2 files changed, 147 insertions, 19 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 0c6d4f640..82a71e448 100755
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -106,6 +106,9 @@ compute_opts = [
                 default=False,
                 help='Whether to start guests that were running before the '
                      'host rebooted'),
+    cfg.IntOpt('network_allocate_retries',
+               default=0,
+               help="Number of times to retry network allocation on failures"),
     ]
 
 interval_opts = [
@@ -1151,6 +1154,50 @@ class ComputeManager(manager.SchedulerDependentManager):
                               expected_task_state=(task_states.SCHEDULING,
                                                    None))
 
+    def _allocate_network_async(self, context, instance, requested_networks,
+                                macs, security_groups, is_vpn):
+        """Method used to allocate networks in the background.
+
+        Broken out for testing.
+        """
+        LOG.debug(_("Allocating IP information in the background."),
+                  instance=instance)
+        retries = CONF.network_allocate_retries
+        if retries < 0:
+            LOG.warn(_("Treating negative config value (%(retries)s) for "
+                       "'network_allocate_retries' as 0."),
+                     {'retries': retries})
+        attempts = retries > 1 and retries + 1 or 1
+        retry_time = 1
+        for attempt in range(1, attempts + 1):
+            try:
+                nwinfo = self.network_api.allocate_for_instance(
+                        context, instance, vpn=is_vpn,
+                        requested_networks=requested_networks,
+                        macs=macs,
+                        conductor_api=self.conductor_api,
+                        security_groups=security_groups)
+                LOG.debug(_('Instance network_info: |%s|'), nwinfo,
+                          instance=instance)
+                return nwinfo
+            except Exception:
+                exc_info = sys.exc_info()
+                log_info = {'attempt': attempt,
+                            'attempts': attempts}
+                if attempt == attempts:
+                    LOG.exception(_('Instance failed network setup '
+                                    'after %(attempts)d attempt(s)'),
+                                  log_info)
+                    raise exc_info[0], exc_info[1], exc_info[2]
+                LOG.warn(_('Instance failed network setup '
+                           '(attempt %(attempt)d of %(attempts)d)'),
+                         log_info, instance=instance)
+                time.sleep(retry_time)
+                retry_time *= 2
+                if retry_time > 30:
+                    retry_time = 30
+        # Not reached.
+
     def _allocate_network(self, context, instance, requested_networks, macs,
                           security_groups):
         """Start network allocation asynchronously.  Return an instance
@@ -1165,25 +1212,9 @@ class ComputeManager(manager.SchedulerDependentManager):
                                          task_state=task_states.NETWORKING,
                                          expected_task_state=None)
         is_vpn = pipelib.is_vpn_image(instance['image_ref'])
-
-        def async_alloc():
-            LOG.debug(_("Allocating IP information in the background."),
-                      instance=instance)
-            try:
-                nwinfo = self.network_api.allocate_for_instance(
-                        context, instance, vpn=is_vpn,
-                        requested_networks=requested_networks,
-                        macs=macs,
-                        conductor_api=self.conductor_api,
-                        security_groups=security_groups)
-            except Exception:
-                with excutils.save_and_reraise_exception():
-                    LOG.exception(_('Instance failed network setup'),
-                                  instance=instance)
-            LOG.debug(_('Instance network_info: |%s|'), nwinfo,
-                      instance=instance)
-            return nwinfo
-        return network_model.NetworkInfoAsyncWrapper(async_alloc)
+        return network_model.NetworkInfoAsyncWrapper(
+                self._allocate_network_async, context, instance,
+                requested_networks, macs, security_groups, is_vpn)
 
     def _prep_block_device(self, context, instance, bdms):
         """Set up the block device for an instance with error logging."""
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 21d9db54b..4fbf805f1 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -1088,6 +1088,103 @@ class ComputeTestCase(BaseTestCase):
         self._assert_state({'vm_state': vm_states.ERROR,
                             'task_state': None})
 
+    def test_allocate_network_succeeds_after_retries(self):
+        # Undo setUp() stubs as this is a true unit test
+        self.stubs.UnsetAll()
+        self.flags(network_allocate_retries=8)
+
+        nwapi = self.compute.network_api
+        self.mox.StubOutWithMock(nwapi, 'allocate_for_instance')
+        self.mox.StubOutWithMock(time, 'sleep')
+
+        instance = {}
+        is_vpn = 'fake-is-vpn'
+        req_networks = 'fake-req-networks'
+        macs = 'fake-macs'
+        sec_groups = 'fake-sec-groups'
+        final_result = 'meow'
+
+        expected_sleep_times = [1, 2, 4, 8, 16, 30, 30, 30]
+
+        for sleep_time in expected_sleep_times:
+            nwapi.allocate_for_instance(
+                    self.context, instance, vpn=is_vpn,
+                    requested_networks=req_networks, macs=macs,
+                    conductor_api=self.compute.conductor_api,
+                    security_groups=sec_groups).AndRaise(
+                            test.TestingException())
+            time.sleep(sleep_time)
+
+        nwapi.allocate_for_instance(
+                self.context, instance, vpn=is_vpn,
+                requested_networks=req_networks, macs=macs,
+                conductor_api=self.compute.conductor_api,
+                security_groups=sec_groups).AndReturn(final_result)
+
+        self.mox.ReplayAll()
+
+        res = self.compute._allocate_network_async(self.context, instance,
+                                                   req_networks,
+                                                   macs,
+                                                   sec_groups,
+                                                   is_vpn)
+        self.assertEqual(final_result, res)
+
+    def test_allocate_network_fails(self):
+        # Undo setUp() stubs as this is a true unit test
+        self.stubs.UnsetAll()
+        self.flags(network_allocate_retries=0)
+
+        nwapi = self.compute.network_api
+        self.mox.StubOutWithMock(nwapi, 'allocate_for_instance')
+
+        instance = {}
+        is_vpn = 'fake-is-vpn'
+        req_networks = 'fake-req-networks'
+        macs = 'fake-macs'
+        sec_groups = 'fake-sec-groups'
+
+        nwapi.allocate_for_instance(
+                self.context, instance, vpn=is_vpn,
+                requested_networks=req_networks, macs=macs,
+                conductor_api=self.compute.conductor_api,
+                security_groups=sec_groups).AndRaise(test.TestingException())
+
+        self.mox.ReplayAll()
+
+        self.assertRaises(test.TestingException,
+                          self.compute._allocate_network_async,
+                          self.context, instance, req_networks, macs,
+                          sec_groups, is_vpn)
+
+    def test_allocate_network_neg_conf_value_treated_as_zero(self):
+        # Undo setUp() stubs as this is a true unit test
+        self.stubs.UnsetAll()
+        self.flags(network_allocate_retries=-1)
+
+        nwapi = self.compute.network_api
+        self.mox.StubOutWithMock(nwapi, 'allocate_for_instance')
+
+        instance = {}
+        is_vpn = 'fake-is-vpn'
+        req_networks = 'fake-req-networks'
+        macs = 'fake-macs'
+        sec_groups = 'fake-sec-groups'
+
+        # Only attempted once.
+        nwapi.allocate_for_instance(
+                self.context, instance, vpn=is_vpn,
+                requested_networks=req_networks, macs=macs,
+                conductor_api=self.compute.conductor_api,
+                security_groups=sec_groups).AndRaise(test.TestingException())
+
+        self.mox.ReplayAll()
+
+        self.assertRaises(test.TestingException,
+                          self.compute._allocate_network_async,
+                          self.context, instance, req_networks, macs,
+                          sec_groups, is_vpn)
+
     def test_run_instance_dealloc_network_instance_not_found(self):
         """spawn network deallocate test.