Merge lp:~allenap/maas/rpc-resilience-when-advertising into lp:~maas-committers/maas/trunk

[Gavin Panella]

Do not be swayed from starting RegionAdvertisingService by mere errors. Fight the good fight until it works.

[Graham Binns (gmb)]

review: Approve

=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py	2014-09-30 13:32:27 +0000
+++ src/maasserver/rpc/regionservice.py	2014-10-01 16:49:21 +0000
@@ -62,6 +62,7 @@
     asynchronous,
     callOut,
     deferWithTimeout,
+    pause,
     synchronous,
     )
 from twisted.application import service
@@ -560,9 +561,11 @@
 
     @asynchronous
     def startService(self):
-        self.starting = deferToThread(self.prepare)
-        self.starting.addCallback(lambda ignore: (
-            super(RegionAdvertisingService, self).startService()))
+        self.starting = self._prepareService()
+
+        def prepared(_):
+            return super(RegionAdvertisingService, self).startService()
+        self.starting.addCallback(prepared)
 
         def ignore_cancellation(failure):
             failure.trap(defer.CancelledError)
@@ -585,6 +588,32 @@
             self.starting.cancel()
             return self.starting
 
+    @inlineCallbacks
+    def _prepareService(self):
+        """Keep calling `prepare` until it works.
+
+        The call to `prepare` can sometimes fail, particularly after starting
+        the region for the first time on a fresh MAAS installation, but the
+        mechanism is not yet understood. We take a pragmatic stance and just
+        keep trying until it works.
+
+        Each failure will be logged, and there will be a pause of 5 seconds
+        between each attempt.
+
+        """
+        while True:
+            try:
+                yield deferToThread(self.prepare)
+            except defer.CancelledError:
+                raise
+            except Exception as e:
+                log.err(e, (
+                    "Preparation of %s failed; will try again in "
+                    "5 seconds." % self.__class__.__name__))
+                yield pause(5)
+            else:
+                break
+
     @synchronous
     @synchronised(lock)
     @transactional

=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2014-10-01 12:16:33 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2014-10-01 16:49:21 +0000
@@ -1417,25 +1417,25 @@
     @wait_for_reactor
     def test_start_up_errors_are_logged(self):
         service = RegionAdvertisingService()
-
+        # Prevent real pauses.
+        self.patch_autospec(regionservice, "pause").return_value = None
         # Ensure that service.prepare fails with a obvious error.
         exception = ValueError("You don't vote for kings!")
-        self.patch(service, "prepare").side_effect = exception
-
-        err_calls = []
-        self.patch(log, "err", err_calls.append)
-
-        err_calls_expected = [
-            AfterPreprocessing(
-                (lambda failure: failure.value),
-                Is(exception)),
-        ]
-
-        def check(ignore):
-            self.assertThat(err_calls, MatchesListwise(err_calls_expected))
+        self.patch(service, "prepare").side_effect = [exception, None]
+        # Capture all Twisted logs.
+        logger = self.useFixture(TwistedLoggerFixture())
+
+        def check_logs(ignore):
+            self.assertDocTestMatches(
+                """\
+                Preparation of ... failed; will try again in 5 seconds.
+                Traceback (most recent call last):...
+                Failure: exceptions.ValueError: You don't vote for kings!
+                """,
+                logger.dump())
 
         service.startService()
-        service.starting.addCallback(check)
+        service.starting.addCallback(check_logs)
         return service.starting
 
     @wait_for_reactor