launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1055523-cluster-uuid into lp:maas.

Commit message:
When starting a cluster controller, send its UUID to the region controller.  Also, have celery listen on the eponymous rabbit queue.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1055523 in MAAS: "Cluster controllers don't have their own amqp queues yet"
  https://bugs.launchpad.net/maas/+bug/1055523

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1055523-cluster-uuid/+merge/126645

See commit message.  Discussed with Julian and Raphael.  This branch will require an accompanying packaging branch (also on the way) which will make postinst write a uuid into the local celery config.  We want the variable in the config because the celery config itself will reference it when defining its beat schedule.

Production code can reasonably assume that the new CLUSTER_UUID setting has been configured.  Non-packaged cluster setups (for dev testing and demos) will still need a UUID to be set.

The initial value is None, not the empty string, to avoid it being mistaken for a proper UUID.  Also, the new line in the local celery config must be matched exactly by the new postinst code.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1055523-cluster-uuid/+merge/126645
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1055523-cluster-uuid into lp:maas.

=== modified file 'contrib/maas_local_celeryconfig.py'
--- contrib/maas_local_celeryconfig.py	2012-09-20 12:53:00 +0000
+++ contrib/maas_local_celeryconfig.py	2012-09-27 09:40:32 +0000
@@ -1,3 +1,6 @@
-# Broken connection information.
+# Broker connection information.
 # Format: transport://userid:password@hostname:port/virtual_host
 BROKER_URL = ''
+
+# UUID identifying the running cluster controller.
+CLUSTER_UUID = None

=== modified file 'etc/celeryconfig.py'
--- etc/celeryconfig.py	2012-09-25 15:09:58 +0000
+++ etc/celeryconfig.py	2012-09-27 09:40:32 +0000
@@ -50,6 +50,10 @@
 # this setting.
 BROKER_URL = 'amqp://guest:guest@localhost:5672//'
 
+# Cluster UUID.  Will be overridden by the customized setting in the
+# local MAAS Celery config.
+CLUSTER_UUID = None
+
 
 WORKER_QUEUE_DNS = 'celery'
 WORKER_QUEUE_BOOT_IMAGES = 'celery'

=== modified file 'src/provisioningserver/start_cluster_controller.py'
--- src/provisioningserver/start_cluster_controller.py	2012-09-27 05:33:56 +0000
+++ src/provisioningserver/start_cluster_controller.py	2012-09-27 09:40:32 +0000
@@ -30,6 +30,7 @@
     MAASDispatcher,
     NoAuth,
     )
+from celeryconfig import CLUSTER_UUID
 from provisioningserver.logging import task_logger
 from provisioningserver.network import discover_networks
 
@@ -55,14 +56,13 @@
     return MAASClient(NoAuth(), MAASDispatcher(), server_url)
 
 
-def register(server_url, uuid):
+def register(server_url):
     """Request Rabbit connection details from the domain controller.
 
     Offers this machine to the region controller as a potential cluster
     controller.
 
     :param server_url: URL to the region controller's MAAS API.
-    :param uuid: UUID for this cluster controller.
     :return: A dict of connection details if this cluster controller has been
         accepted, or `None` if there is no definite response yet.  If there
         is no definite response, retry this call later.
@@ -74,10 +74,9 @@
     interfaces = json.dumps(discover_networks())
     client = make_anonymous_api_client(server_url)
     try:
-        # XXX JeroenVermeulen 2012-09-27, bug=1055523: Pass uuid=uuid.
         response = client.post(
             'api/1.0/nodegroups/', 'register',
-            interfaces=interfaces)
+            interfaces=interfaces, uuid=CLUSTER_UUID)
     except HTTPError as e:
         status_code = e.code
         if e.code not in known_responses:
@@ -108,18 +107,13 @@
 def start_celery(connection_details):
     broker_url = connection_details['BROKER_URL']
 
-    # XXX JeroenVermeulen 2012-09-24, bug=1055523: Fill in proper
-    # cluster-specific queue name once we have those (based on cluster
-    # uuid).
-    queue = 'celery'
-
     # Copy environment, but also tell celeryd what broker to listen to.
     env = dict(os.environ, CELERY_BROKER_URL=broker_url)
 
     queues = [
         'common',
         'update_dhcp_queue',
-        queue,
+        CLUSTER_UUID,
         ]
     command = [
         'celeryd',
@@ -158,9 +152,7 @@
     If this system is still awaiting approval as a cluster controller, this
     command will keep looping until it gets a definite answer.
     """
-    # XXX JeroenVermeulen 2012-09-27, bug=1055523: Get uuid.
-    uuid = None
-    connection_details = register(args.server_url, uuid)
+    connection_details = register(args.server_url)
     while connection_details is None:
         sleep(60)
         connection_details = register(args.server_url)

=== modified file 'src/provisioningserver/tests/test_start_cluster_controller.py'
--- src/provisioningserver/tests/test_start_cluster_controller.py	2012-09-27 05:33:56 +0000
+++ src/provisioningserver/tests/test_start_cluster_controller.py	2012-09-27 09:40:32 +0000
@@ -79,6 +79,8 @@
         self.patch(start_cluster_controller, 'sleep').side_effect = Sleeping()
         self.patch(start_cluster_controller, 'Popen').side_effect = (
             Executing())
+        self.patch(
+            start_cluster_controller, 'CLUSTER_UUID', factory.getRandomUUID())
 
     def make_connection_details(self):
         return {
@@ -166,7 +168,8 @@
 
     def test_register_passes_cluster_information(self):
         self.prepare_success_response()
-        uuid = factory.getRandomUUID()
+        self.patch(
+            start_cluster_controller, 'CLUSTER_UUID', factory.getRandomUUID())
         interface = {
             'interface': factory.make_name('eth'),
             'ip': factory.getRandomIPAddress(),
@@ -175,14 +178,13 @@
         discover = self.patch(start_cluster_controller, 'discover_networks')
         discover.return_value = [interface]
 
-        start_cluster_controller.register(make_url(), uuid)
+        start_cluster_controller.register(make_url())
 
         (args, kwargs) = MAASDispatcher.dispatch_query.call_args
         headers, body = kwargs["headers"], kwargs["data"]
         post, files = self.parse_headers_and_body(headers, body)
         self.assertEqual([interface], json.loads(post['interfaces']))
-        # XXX JeroenVermeulen 2012-09-27, bug=1055523: Reinstate this.
-        #self.assertEqual(uuid, post['uuid'])
+        self.assertEqual(start_cluster_controller.CLUSTER_UUID, post['uuid'])
 
     def test_starts_up_once_accepted(self):
         self.patch(start_cluster_controller, 'start_up')