Merge lp:~allenap/maas/check-for-rpc-connection-before-registering into lp:~maas-committers/maas/trunk

[Gavin Panella]

Check for an RPC connection before registering a new cluster.

[Raphaël Badin (rvb)]

Looks good, just one remark about the choice of httplib.GATEWAY_TIMEOUT.

review: Approve

[Gavin Panella (allenap)]

Thanks!

=== modified file 'src/maasserver/api/node_groups.py'
--- src/maasserver/api/node_groups.py	2014-09-24 14:20:51 +0000
+++ src/maasserver/api/node_groups.py	2014-10-02 01:12:08 +0000
@@ -55,11 +55,13 @@
 from maasserver.models.node import Node
 from maasserver.models.nodegroup import NodeGroup
 from maasserver.models.nodeprobeddetails import get_probed_details
+from maasserver.rpc import getClientFor
 from maasserver.utils import (
     build_absolute_uri,
     get_local_cluster_UUID,
     )
 from maasserver.utils.orm import get_one
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
 
 
 DISPLAYED_NODEGROUP_FIELDS = ('uuid', 'status', 'name', 'cluster_name')
@@ -180,7 +182,17 @@
                 # send it from now on.
                 update_nodegroup_maas_url(nodegroup, request)
         else:
-            nodegroup = register_nodegroup(request, uuid)
+            # An RPC connection to the target cluster is required for
+            # registration to work, so we wait up to 10 seconds for a
+            # connection to be established.
+            try:
+                getClientFor(uuid, timeout=10)
+            except NoConnectionsAvailable:
+                return HttpResponse(
+                    "Waiting for RPC connection.",
+                    status=httplib.SERVICE_UNAVAILABLE)
+            else:
+                nodegroup = register_nodegroup(request, uuid)
 
         return compose_nodegroup_register_response(nodegroup, already_existed)
 

=== modified file 'src/maasserver/api/tests/test_register.py'
--- src/maasserver/api/tests/test_register.py	2014-09-24 14:20:51 +0000
+++ src/maasserver/api/tests/test_register.py	2014-10-02 01:12:08 +0000
@@ -42,7 +42,11 @@
     ANY,
     Mock,
     )
-from testtools.matchers import MatchesStructure
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
+from testtools.matchers import (
+    Equals,
+    MatchesStructure,
+    )
 from testtools.testcase import ExpectedException
 
 
@@ -210,6 +214,11 @@
     This method can be called anonymously.
     """
 
+    def setUp(self):
+        super(TestRegisterAPI, self).setUp()
+        self.getClientFor = self.patch_autospec(
+            nodegroups_module, "getClientFor")
+
     def test_register_creates_nodegroup_and_interfaces(self):
         create_configured_master()
         name = factory.make_name('cluster')
@@ -443,3 +452,14 @@
         self.assertEqual([], update_maas_url.call_args_list)
         # The master's maas_url field remains empty.
         self.assertEqual("", master.maas_url)
+
+    def test_register_nodegroup_when_rpc_not_connected(self):
+        create_configured_master()
+        self.getClientFor.side_effect = NoConnectionsAvailable
+        response = self.client.post(
+            reverse('nodegroups_handler'),
+            {'op': 'register', 'uuid': factory.make_UUID()})
+        self.expectThat(
+            response.status_code, Equals(httplib.SERVICE_UNAVAILABLE))
+        self.expectThat(
+            response.content, Equals("Waiting for RPC connection."))

=== modified file 'src/provisioningserver/start_cluster_controller.py'
--- src/provisioningserver/start_cluster_controller.py	2014-09-23 20:16:54 +0000
+++ src/provisioningserver/start_cluster_controller.py	2014-10-02 01:12:08 +0000
@@ -72,7 +72,12 @@
     :raise ClusterControllerRejected: if this system has been rejected as a
         cluster controller.
     """
-    known_responses = {httplib.OK, httplib.FORBIDDEN, httplib.ACCEPTED}
+    known_responses = {
+        httplib.ACCEPTED,
+        httplib.FORBIDDEN,
+        httplib.OK,
+        httplib.SERVICE_UNAVAILABLE,
+    }
 
     interfaces = json.dumps(discover_networks())
     client = make_anonymous_api_client(server_url)
@@ -100,6 +105,9 @@
     elif status_code == httplib.ACCEPTED:
         # Our application is still waiting for approval.  Keep trying.
         return None
+    elif status_code == httplib.SERVICE_UNAVAILABLE:
+        # Our cluster has not yet established RPC connections.
+        return None
     elif status_code == httplib.FORBIDDEN:
         # Our application has been rejected.  Give up.
         raise ClusterControllerRejected(

=== modified file 'src/provisioningserver/tests/test_start_cluster_controller.py'
--- src/provisioningserver/tests/test_start_cluster_controller.py	2014-09-23 20:21:50 +0000
+++ src/provisioningserver/tests/test_start_cluster_controller.py	2014-10-02 01:12:08 +0000
@@ -123,6 +123,10 @@
         """Prepare to return "request pending" from API request."""
         self.prepare_response(httplib.ACCEPTED)
 
+    def prepare_rpc_wait_response(self):
+        """Prepare to return "waiting for RPC" from API request."""
+        self.prepare_response(httplib.SERVICE_UNAVAILABLE)
+
     def test_run_command(self):
         # We can't really run the script, but we can verify that (with
         # the right system functions patched out) we can run it
@@ -154,6 +158,12 @@
             Sleeping,
             start_cluster_controller.run, make_args())
 
+    def test_polls_on_rpc_wait(self):
+        self.prepare_rpc_wait_response()
+        self.assertRaises(
+            Sleeping,
+            start_cluster_controller.run, make_args())
+
     def test_polls_on_unexpected_errors(self):
         self.patch(MAASDispatcher, 'dispatch_query').side_effect = HTTPError(
             make_url(), httplib.REQUEST_TIMEOUT, "Timeout.", '', BytesIO())