Merge lp:~gmb/maas/enlist-mscm-to-RPC into lp:~maas-committers/maas/trunk

[Graham Binns]

Convert the enlist_nodes_from_mscm Celery task to an RPC call.

[Gavin Panella (allenap)]

review: Approve

[Graham Binns (gmb)]

On 11 September 2014 11:28, Gavin Panella <email address hidden> wrote:
>
>> +
>> +class EnlistNodesFromMSCM(amp.Command):
>> + """Probe for and enlist mscm machines attached to the cluster.
>> +
>> + :since 1.7:
>> + """
>> + arguments = [
>> + (b"host", amp.Unicode()),
>> + (b"username", amp.Unicode()),
>> + (b"password", amp.Unicode()),
>> + ]
>> + response = []
>> + errors = {}
>
> No errors at all?

probe_and_enlist_mscm() doesn't appear to raise any, so no. This is
one of those cases where we need a catch-all, because I don't know
what might happen further down the stack (this is the one and only
time I've ever been tempted to miss Java's error declarations…)

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/enlist-mscm-to-RPC into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://security.ubuntu.com trusty-security Release
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl python-paramiko python-pexpect python-pip python-pocket-lint python-psycopg2 python-pyinotify python-seamicroclient python-simplejson python-simplestreams python-sphinx python-s...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~gmb/maas/enlist-mscm-to-RPC into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:2 http://security.ubuntu.com trusty-security Release [59.7 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:5 http://security.ubuntu.com trusty-security/main Sources [43.9 kB]
Get:6 http://security.ubuntu.com trusty-security/universe Sources [10.8 kB]
Get:7 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [118 kB]
Get:8 http://security.ubuntu.com trusty-security/main amd64 Packages [138 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [47.2 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [83.7 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [317 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [201 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,080 kB in 0s (1,756 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl py...

=== modified file 'src/maasserver/api/tests/test_nodegroup.py'
--- src/maasserver/api/tests/test_nodegroup.py	2014-09-10 06:35:46 +0000
+++ src/maasserver/api/tests/test_nodegroup.py	2014-09-11 11:22:16 +0000
@@ -55,7 +55,10 @@
     )
 from mock import Mock
 from provisioningserver.auth import get_recorded_nodegroup_uuid
-from provisioningserver.rpc.cluster import ImportBootImages
+from provisioningserver.rpc.cluster import (
+    EnlistNodesFromMSCM,
+    ImportBootImages,
+    )
 from testresources import FixtureResource
 from testtools.matchers import (
     AllMatch,
@@ -370,7 +373,9 @@
         password = factory.make_name('password')
         self.become_admin()
 
-        mock = self.patch(nodegroup_module, 'enlist_nodes_from_mscm')
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
 
         response = self.client.post(
             reverse('nodegroup_handler', args=[nodegroup.uuid]),
@@ -385,9 +390,11 @@
             httplib.OK, response.status_code,
             explain_unexpected_response(httplib.OK, response))
 
-        args = (host, username, password)
-        matcher = MockCalledOnceWith(queue=nodegroup.uuid, args=args)
-        self.assertThat(mock.apply_async, matcher)
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                EnlistNodesFromMSCM, host=host, username=username,
+                password=password))
 
 
 class TestNodeGroupAPIAuth(MAASServerTestCase):

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2014-09-10 16:01:38 +0000
+++ src/maasserver/models/nodegroup.py	2014-09-11 11:22:16 +0000
@@ -41,13 +41,11 @@
 from provisioningserver.rpc.cluster import (
     AddSeaMicro15k,
     AddVirsh,
+    EnlistNodesFromMSCM,
     ImportBootImages,
     )
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
-from provisioningserver.tasks import (
-    enlist_nodes_from_mscm,
-    enlist_nodes_from_ucsm,
-    )
+from provisioningserver.tasks import enlist_nodes_from_ucsm
 
 
 class NodeGroupManager(Manager):
@@ -314,6 +312,18 @@
         :param host: IP address for the MSCM.
         :param username: username for MSCM.
         :param password: password for MSCM.
+
+        :raises NoConnectionsAvailable: If no connections to the cluster
+            are available.
         """
-        args = (host, username, password)
-        enlist_nodes_from_mscm.apply_async(queue=self.uuid, args=args)
+        try:
+            client = getClientFor(self.uuid, timeout=1)
+        except NoConnectionsAvailable:
+            # No connection to the cluster so we can't do anything. We
+            # let the caller handle the error, since we don't want to
+            # just drop it.
+            raise
+        else:
+            return client(
+                EnlistNodesFromMSCM, host=host, username=username,
+                password=password)

=== modified file 'src/maasserver/models/tests/test_nodegroup.py'
--- src/maasserver/models/tests/test_nodegroup.py	2014-09-10 16:01:38 +0000
+++ src/maasserver/models/tests/test_nodegroup.py	2014-09-11 11:22:16 +0000
@@ -49,6 +49,7 @@
 from provisioningserver.rpc.cluster import (
     AddSeaMicro15k,
     AddVirsh,
+    EnlistNodesFromMSCM,
     ImportBootImages,
     )
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
@@ -513,3 +514,52 @@
         self.assertRaises(
             NoConnectionsAvailable, nodegroup.add_seamicro15k,
             mac, username, password, power_control)
+
+    def test_enlist_nodes_from_mscm_end_to_end(self):
+        nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ACCEPTED)
+
+        self.useFixture(RegionEventLoopFixture("rpc"))
+        self.useFixture(RunningEventLoopFixture())
+        fixture = self.useFixture(MockLiveRegionToClusterRPCFixture())
+        protocol = fixture.makeCluster(nodegroup, EnlistNodesFromMSCM)
+        protocol.EnlistNodesFromMSCM.return_value = defer.succeed({})
+
+        host = factory.make_name('host')
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        nodegroup.enlist_nodes_from_mscm(
+            host, username, password).wait(10)
+
+        self.expectThat(
+            protocol.EnlistNodesFromMSCM,
+            MockCalledOnceWith(
+                ANY, host=host, username=username, password=password))
+
+    def test_enlist_nodes_from_mscm_calls_client_with_resource_endpoint(self):
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
+
+        host = factory.make_name('host')
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        nodegroup.enlist_nodes_from_mscm(
+            host, username, password).wait(10)
+
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                EnlistNodesFromMSCM, host=host, username=username,
+                password=password))
+
+    def test_enlist_nodes_from_mscm_raises_if_no_connection_to_cluster(self):
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        getClientFor.side_effect = NoConnectionsAvailable()
+        nodegroup = factory.make_NodeGroup()
+
+        host = factory.make_name('host')
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        self.assertRaises(
+            NoConnectionsAvailable, nodegroup.enlist_nodes_from_mscm,
+            host, username, password)

=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-09-10 16:44:28 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-09-11 11:22:16 +0000
@@ -377,3 +377,17 @@
     errors = {
         exceptions.NoIPFoundForMACAddress: b"NoIPFoundForMACAddress",
     }
+
+
+class EnlistNodesFromMSCM(amp.Command):
+    """Probe for and enlist mscm machines attached to the cluster.
+
+    :since: 1.7
+    """
+    arguments = [
+        (b"host", amp.Unicode()),
+        (b"username", amp.Unicode()),
+        (b"password", amp.Unicode()),
+    ]
+    response = []
+    errors = {}

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-09-10 23:00:52 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-09-11 11:22:16 +0000
@@ -29,6 +29,7 @@
     ArchitectureRegistry,
     PowerTypeRegistry,
     )
+from provisioningserver.drivers.hardware.mscm import probe_and_enlist_mscm
 from provisioningserver.drivers.hardware.seamicro import (
     probe_seamicro15k_and_enlist,
     )
@@ -266,6 +267,16 @@
             raise exceptions.NoIPFoundForMACAddress(message)
         return {}
 
+    @cluster.EnlistNodesFromMSCM.responder
+    def enlist_nodes_from_mscm(self, host, username, password):
+        """enlist_nodes_from_mscm()
+
+        Implemention of
+        :py:class:`~provisioningserver.rpc.cluster.EnlistNodesFromMSCM`.
+        """
+        probe_and_enlist_mscm(host, username, password)
+        return {}
+
 
 @implementer(IConnection)
 class ClusterClient(Cluster):

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-10 23:00:52 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-11 11:22:16 +0000
@@ -1256,3 +1256,30 @@
             probe_seamicro15k_and_enlist, MockCalledOnceWith(
                 find_ip_via_arp.return_value, username, password,
                 power_control=power_control))
+
+
+class TestClusterProtocol_EnlistNodesFromMSCM(MAASTestCase):
+
+    def test__is_registered(self):
+        protocol = Cluster()
+        responder = protocol.locateResponder(
+            cluster.EnlistNodesFromMSCM.commandName)
+        self.assertIsNot(responder, None)
+
+    def test__calls_probe_and_enlist_mscm(self):
+        probe_and_enlist_mscm = self.patch_autospec(
+            clusterservice, 'probe_and_enlist_mscm')
+
+        host = factory.make_name('host')
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+
+        call_responder(Cluster(), cluster.EnlistNodesFromMSCM, {
+            'host': host,
+            'username': username,
+            'password': password,
+        })
+
+        self.assertThat(
+            probe_and_enlist_mscm, MockCalledOnceWith(
+                host, username, password))

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2014-09-10 17:00:43 +0000
+++ src/provisioningserver/tasks.py	2014-09-11 11:22:16 +0000
@@ -42,7 +42,6 @@
     set_up_options_conf,
     setup_rndc,
     )
-from provisioningserver.drivers.hardware.mscm import probe_and_enlist_mscm
 from provisioningserver.drivers.hardware.ucsm import probe_and_enlist_ucsm
 from provisioningserver.logger import get_maas_logger
 
@@ -309,11 +308,3 @@
 def enlist_nodes_from_ucsm(url, username, password):
     """ See `maasserver.api.NodeGroupHandler.enlist_nodes_from_ucsm`. """
     probe_and_enlist_ucsm(url, username, password)
-
-
-@task
-@log_task_events()
-@log_exception_text
-def enlist_nodes_from_mscm(host, username, password):
-    """ See `maasserver.api.NodeGroupHandler.enlist_nodes_from_mscm`. """
-    probe_and_enlist_mscm(host, username, password)

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2014-09-10 17:00:43 +0000
+++ src/provisioningserver/tests/test_tasks.py	2014-09-11 11:22:16 +0000
@@ -55,7 +55,6 @@
     )
 from provisioningserver.tags import MissingCredentials
 from provisioningserver.tasks import (
-    enlist_nodes_from_mscm,
     enlist_nodes_from_ucsm,
     refresh_secrets,
     report_boot_images,
@@ -403,14 +402,3 @@
         mock = self.patch(tasks, 'probe_and_enlist_ucsm')
         enlist_nodes_from_ucsm(url, username, password)
         self.assertThat(mock, MockCalledOnceWith(url, username, password))
-
-
-class TestAddMSCM(PservTestCase):
-
-    def test_enlist_nodes_from_mscm(self):
-        host = 'host'
-        username = 'username'
-        password = 'password'
-        mock = self.patch(tasks, 'probe_and_enlist_mscm')
-        enlist_nodes_from_mscm(host, username, password)
-        self.assertThat(mock, MockCalledOnceWith(host, username, password))