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

[Graham Binns]

Convert the enlist_nodes_from_ucsm() task to use RPC.

[Raphaël Badin (rvb)]

Looks good. (These branches are a bit boring aren't they?)

review: Approve

[Graham Binns (gmb)]

On 11 September 2014 11:35, Raphaël Badin <email address hidden> wrote:
>
>
> When the method is called from the API it will be transformed into a 500 error. This is probably what we want but could you check that the error message in that case is meaningful? (I mean, I'd like to be sure it mentions at least the UUID problematic cluster.)

Yes, I'll check. I'll fix that in a followup branch if necessary;
there are several sites that follow this pattern and they could all do
with the same love :)

[MAAS Lander (maas-lander)]

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

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
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...

=== modified file 'src/maasserver/api/tests/test_nodegroup.py'
--- src/maasserver/api/tests/test_nodegroup.py	2014-09-11 11:21:49 +0000
+++ src/maasserver/api/tests/test_nodegroup.py	2014-09-11 12:18:08 +0000
@@ -57,6 +57,7 @@
 from provisioningserver.auth import get_recorded_nodegroup_uuid
 from provisioningserver.rpc.cluster import (
     EnlistNodesFromMSCM,
+    EnlistNodesFromUCSM,
     ImportBootImages,
     )
 from testresources import FixtureResource
@@ -347,7 +348,9 @@
         password = factory.make_name('password')
         self.become_admin()
 
-        mock = self.patch(nodegroup_module, 'enlist_nodes_from_ucsm')
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
 
         response = self.client.post(
             reverse('nodegroup_handler', args=[nodegroup.uuid]),
@@ -362,9 +365,11 @@
             httplib.OK, response.status_code,
             explain_unexpected_response(httplib.OK, response))
 
-        args = (url, username, password)
-        matcher = MockCalledOnceWith(queue=nodegroup.uuid, args=args)
-        self.assertThat(mock.apply_async, matcher)
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                EnlistNodesFromUCSM, url=url, username=username,
+                password=password))
 
     def test_probe_and_enlist_mscm_adds_mscm(self):
         nodegroup = factory.make_NodeGroup()
 
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2014-09-11 09:58:18 +0000
+++ src/maasserver/models/nodegroup.py	2014-09-11 12:18:08 +0000
@@ -42,10 +42,10 @@
     AddSeaMicro15k,
     AddVirsh,
     EnlistNodesFromMSCM,
+    EnlistNodesFromUCSM,
     ImportBootImages,
     )
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
-from provisioningserver.tasks import enlist_nodes_from_ucsm
 
 
 class NodeGroupManager(Manager):
@@ -302,9 +302,21 @@
         :param URL: URL of the Cisco UCS Manager HTTP-XML API.
         :param username: username for UCS Manager.
         :param password: password for UCS Manager.
+
+        :raises NoConnectionsAvailable: If no connections to the cluster
+            are available.
         """
-        args = (url, username, password)
-        enlist_nodes_from_ucsm.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(
+                EnlistNodesFromUCSM, url=url, username=username,
+                password=password)
 
     def enlist_nodes_from_mscm(self, host, username, password):
         """ Add the servers from a Moonshot HP iLO Chassis Manager.
 
=== modified file 'src/maasserver/models/tests/test_nodegroup.py'
--- src/maasserver/models/tests/test_nodegroup.py	2014-09-11 09:53:46 +0000
+++ src/maasserver/models/tests/test_nodegroup.py	2014-09-11 12:18:08 +0000
@@ -50,6 +50,7 @@
     AddSeaMicro15k,
     AddVirsh,
     EnlistNodesFromMSCM,
+    EnlistNodesFromUCSM,
     ImportBootImages,
     )
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
@@ -563,3 +564,52 @@
         self.assertRaises(
             NoConnectionsAvailable, nodegroup.enlist_nodes_from_mscm,
             host, username, password)
+
+    def test_enlist_nodes_from_ucsm_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, EnlistNodesFromUCSM)
+        protocol.EnlistNodesFromUCSM.return_value = defer.succeed({})
+
+        url = factory.make_url()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        nodegroup.enlist_nodes_from_ucsm(
+            url, username, password).wait(10)
+
+        self.expectThat(
+            protocol.EnlistNodesFromUCSM,
+            MockCalledOnceWith(
+                ANY, url=url, username=username, password=password))
+
+    def test_enlist_nodes_from_ucsm_calls_client_with_resource_endpoint(self):
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
+
+        url = factory.make_url()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        nodegroup.enlist_nodes_from_ucsm(
+            url, username, password).wait(10)
+
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                EnlistNodesFromUCSM, url=url, username=username,
+                password=password))
+
+    def test_enlist_nodes_from_ucsm_raises_if_no_connection_to_cluster(self):
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        getClientFor.side_effect = NoConnectionsAvailable()
+        nodegroup = factory.make_NodeGroup()
+
+        url = factory.make_url()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        self.assertRaises(
+            NoConnectionsAvailable, nodegroup.enlist_nodes_from_ucsm,
+            url, username, password)
 
=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-09-11 11:16:53 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-09-11 12:18:08 +0000
@@ -391,3 +391,17 @@
     ]
     response = []
     errors = {}
+
+
+class EnlistNodesFromUCSM(amp.Command):
+    """Probe for and enlist ucsm machines attached to the cluster.
+
+    :since: 1.7
+    """
+    arguments = [
+        (b"url", 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-11 09:58:18 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-09-11 12:18:08 +0000
@@ -33,6 +33,7 @@
 from provisioningserver.drivers.hardware.seamicro import (
     probe_seamicro15k_and_enlist,
     )
+from provisioningserver.drivers.hardware.ucsm import probe_and_enlist_ucsm
 from provisioningserver.drivers.hardware.virsh import probe_virsh_and_enlist
 from provisioningserver.logger.log import get_maas_logger
 from provisioningserver.rpc import (
@@ -277,6 +278,16 @@
         probe_and_enlist_mscm(host, username, password)
         return {}
 
+    @cluster.EnlistNodesFromUCSM.responder
+    def enlist_nodes_from_ucsm(self, url, username, password):
+        """enlist_nodes_from_ucsm()
+
+        Implemention of
+        :py:class:`~provisioningserver.rpc.cluster.EnlistNodesFromUCSM`.
+        """
+        probe_and_enlist_ucsm(url, 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-11 09:53:18 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-11 12:18:08 +0000
@@ -1283,3 +1283,30 @@
         self.assertThat(
             probe_and_enlist_mscm, MockCalledOnceWith(
                 host, username, password))
+
+
+class TestClusterProtocol_EnlistNodesFromUCSM(MAASTestCase):
+
+    def test__is_registered(self):
+        protocol = Cluster()
+        responder = protocol.locateResponder(
+            cluster.EnlistNodesFromUCSM.commandName)
+        self.assertIsNot(responder, None)
+
+    def test__calls_probe_and_enlist_ucsm(self):
+        probe_and_enlist_ucsm = self.patch_autospec(
+            clusterservice, 'probe_and_enlist_ucsm')
+
+        url = factory.make_url()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+
+        call_responder(Cluster(), cluster.EnlistNodesFromUCSM, {
+            'url': url,
+            'username': username,
+            'password': password,
+        })
+
+        self.assertThat(
+            probe_and_enlist_ucsm, MockCalledOnceWith(
+                url, username, password))
 
=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2014-09-11 09:54:13 +0000
+++ src/provisioningserver/tasks.py	2014-09-11 12:18:08 +0000
@@ -42,7 +42,6 @@
     set_up_options_conf,
     setup_rndc,
     )
-from provisioningserver.drivers.hardware.ucsm import probe_and_enlist_ucsm
 from provisioningserver.logger import get_maas_logger
 
 # For each item passed to refresh_secrets, a refresh function to give it to.
@@ -296,15 +295,3 @@
                 exc=exc, countdown=UPDATE_NODE_TAGS_RETRY_DELAY)
         else:
             raise
-
-
-# =====================================================================
-# Custom hardware tasks
-# =====================================================================
-
-@task
-@log_task_events()
-@log_exception_text
-def enlist_nodes_from_ucsm(url, username, password):
-    """ See `maasserver.api.NodeGroupHandler.enlist_nodes_from_ucsm`. """
-    probe_and_enlist_ucsm(url, username, password)
 
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2014-09-11 10:56:06 +0000
+++ src/provisioningserver/tests/test_tasks.py	2014-09-11 12:18:08 +0000
@@ -31,7 +31,6 @@
     FakeMethod,
     MultiFakeMethod,
     )
-from maastesting.matchers import MockCalledOnceWith
 from mock import Mock
 from netaddr import IPNetwork
 from provisioningserver import (
@@ -55,7 +54,6 @@
     )
 from provisioningserver.tags import MissingCredentials
 from provisioningserver.tasks import (
-    enlist_nodes_from_ucsm,
     refresh_secrets,
     report_boot_images,
     rndc_command,
@@ -391,14 +389,3 @@
         self.assertRaises(
             MissingCredentials, update_node_tags.delay, tag,
             '//node', tag_nsmap=None, retry=True)
-
-
-class TestAddUCSM(PservTestCase):
-
-    def test_enlist_nodes_from_ucsm(self):
-        url = 'url'
-        username = 'username'
-        password = 'password'
-        mock = self.patch(tasks, 'probe_and_enlist_ucsm')
-        enlist_nodes_from_ucsm(url, username, password)
-        self.assertThat(mock, MockCalledOnceWith(url, username, password))