Merge lp:~jk0/nova/lp661472 into lp:~hudson-openstack/nova/trunk

[Josh Kearney]

Check for running AMQP instances.

Fixes bug #661472.

[Vish Ishaya (vishvananda)]

A couple of issues issues:

1. This won't pass pep8

2. We may need something better than time.sleep(30) on fetch. This will block the reactor while we're waiting to reconnect, stopping us from doing any other work like checking the current state of the instances and doing recovery or updating the db. Is there a reason we can't keep the failed connection flag and just add the self.declare() to the existing code?

[Matt Dietz (cerberus)]

Is there some reason we don't want to actually log the exceptions on #22 and #58?

[lp:~jk0/nova/lp661472]

409. By Josh Kearney on 2010-11-19

Reverted some changes

[Vish Ishaya (vishvananda)]

LGTM. Consider changing logging.warning to logging.exception in the init code. Might be good to have traceback in case it is a configuration issue instead of rabbit actually being down.

review: Approve

[lp:~jk0/nova/lp661472]

410. By Josh Kearney on 2010-11-20

Use logging.exception instead

[Joshua McKenty (joshua-mckenty)]

LGTM. Would consider adding a maximum number of retries, or a decay on the 30 second limit. The 30 second should be a FLAG or a CONST as well, IMO.

review: Approve

[Josh Kearney (jk0)]

I like the idea of making it a constant with a maximum number of retries. Would the best course of action be to sys.exit() once that limit is hit (giving a friendly error message), or should we let it continue to run?

Also -- is 30 seconds ideal and what should the retry max be?

[Vish Ishaya (vishvananda)]

lgtm.

review: Approve

[Joshua McKenty (joshua-mckenty)]

I would make it 10 seconds (I think a lot of Rabbit errors are transient network issues), with a maximum 12 tries for a total of 2 minutes. (OR, a bunch more config FLAGS.)

And I'd definitely sys.exit(<nonzero>), so that a monitoring system like monit, etc. can show the shutdown as a failure.

=== modified file 'nova/rpc.py'
--- nova/rpc.py	2010-11-03 23:04:01 +0000
+++ nova/rpc.py	2010-11-20 18:19:13 +0000
@@ -24,6 +24,7 @@
 import json
 import logging
 import sys
+import time
 import uuid
 
 from carrot import connection as carrot_connection
@@ -83,7 +84,18 @@
     """
     def __init__(self, *args, **kwargs):
         self.failed_connection = False
-        super(Consumer, self).__init__(*args, **kwargs)
+
+        while True:
+            try:
+                super(Consumer, self).__init__(*args, **kwargs)
+                break
+            except:  # Catching all because carrot sucks
+                logging.exception("AMQP server on %s:%d is unreachable. " \
+                                "Trying again in 30 seconds." % (
+                                FLAGS.rabbit_host,
+                                FLAGS.rabbit_port))
+                time.sleep(30)
+                continue
 
     def fetch(self, no_ack=None, auto_ack=None, enable_callbacks=False):
         """Wraps the parent fetch with some logic for failed connections"""
@@ -94,8 +106,9 @@
                 # NOTE(vish): conn is defined in the parent class, we can
                 #             recreate it as long as we create the backend too
                 # pylint: disable-msg=W0201
-                self.conn = Connection.recreate()
-                self.backend = self.conn.create_backend()
+                self.connection = Connection.recreate()
+                self.backend = self.connection.create_backend()
+                self.declare()
             super(Consumer, self).fetch(no_ack, auto_ack, enable_callbacks)
             if self.failed_connection:
                 logging.error("Reconnected to queue")