Merge lp:~gmb/maas/port-add-seamicro-to-RPC into lp:~maas-committers/maas/trunk

[Graham Binns]

Port NodeGroup.add_seamicro15k() to use RPC rather than celery.

[Gavin Panella (allenap)]

Looks good, but I think not finding the IP address for the given MAC is worthy of an error.

review: Approve

[Graham Binns (gmb)]

> Looks good, but I think not finding the IP address for the given MAC is worthy
> of an error.

Yep. Exception added.

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2014-09-10 14:40:46 +0000
+++ src/maasserver/models/nodegroup.py	2014-09-10 16:44:53 +0000
@@ -39,12 +39,12 @@
     )
 from provisioningserver.dhcp.omshell import generate_omapi_key
 from provisioningserver.rpc.cluster import (
+    AddSeaMicro15k,
     AddVirsh,
     ImportBootImages,
     )
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
 from provisioningserver.tasks import (
-    add_seamicro15k,
     enlist_nodes_from_mscm,
     enlist_nodes_from_ucsm,
     )
@@ -262,15 +262,30 @@
         :param password: password for power controller
         :param power_control: optional specify the power control method,
             either ipmi (default), restapi, or restapi2.
+
+        :raises NoConnectionsAvailable: If no connections to the cluster
+            are available.
         """
-        args = (mac, username, password, power_control)
-        add_seamicro15k.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(
+                AddSeaMicro15k, mac=mac, username=username,
+                password=password, power_control=power_control)
 
     def add_virsh(self, poweraddr, password=None):
         """ Add all of the virtual machines inside a virsh controller.
 
         :param poweraddr: virsh connection string
         :param password: ssh password
+
+        :raises NoConnectionsAvailable: If no connections to the cluster
+            are available.
         """
         try:
             client = getClientFor(self.uuid, timeout=1)

=== modified file 'src/maasserver/models/tests/test_nodegroup.py'
--- src/maasserver/models/tests/test_nodegroup.py	2014-09-10 14:40:46 +0000
+++ src/maasserver/models/tests/test_nodegroup.py	2014-09-10 16:44:53 +0000
@@ -47,6 +47,7 @@
     )
 from provisioningserver.dhcp.omshell import generate_omapi_key
 from provisioningserver.rpc.cluster import (
+    AddSeaMicro15k,
     AddVirsh,
     ImportBootImages,
     )
@@ -459,3 +460,56 @@
         self.assertRaises(
             NoConnectionsAvailable, nodegroup.add_virsh, poweraddr,
             password)
+
+    def test_add_seamicro15k_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, AddSeaMicro15k)
+        protocol.AddSeaMicro15k.return_value = defer.succeed({})
+
+        mac = factory.getRandomMACAddress()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = factory.make_name('power_control')
+        nodegroup.add_seamicro15k(
+            mac, username, password, power_control).wait(10)
+
+        self.expectThat(
+            protocol.AddSeaMicro15k,
+            MockCalledOnceWith(
+                ANY, mac=mac, username=username, password=password,
+                power_control=power_control))
+
+    def test_add_seamicro15k_calls_client_with_resource_endpoint(self):
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
+
+        mac = factory.getRandomMACAddress()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = factory.make_name('power_control')
+        nodegroup.add_seamicro15k(
+            mac, username, password, power_control)
+
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                AddSeaMicro15k, mac=mac, username=username,
+                password=password, power_control=power_control))
+
+    def test_add_seamicro15k_raises_if_no_connection_to_cluster(self):
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        getClientFor.side_effect = NoConnectionsAvailable()
+        nodegroup = factory.make_NodeGroup()
+
+        mac = factory.getRandomMACAddress()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = factory.make_name('power_control')
+        self.assertRaises(
+            NoConnectionsAvailable, nodegroup.add_seamicro15k,
+            mac, username, password, power_control)

=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-09-10 15:06:39 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-09-10 16:44:53 +0000
@@ -360,3 +360,20 @@
     ]
     response = []
     errors = []
+
+
+class AddSeaMicro15k(amp.Command):
+    """Probe for and enlist seamicro15k machines attached to the cluster.
+
+    :since: 1.7
+    """
+    arguments = [
+        (b"mac", amp.Unicode()),
+        (b"username", amp.Unicode()),
+        (b"password", amp.Unicode()),
+        (b"power_control", amp.Unicode(optional=True)),
+    ]
+    response = []
+    errors = {
+        exceptions.NoIPFoundForMACAddress: b"NoIPFoundForMACAddress",
+    }

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-09-10 15:06:39 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-09-10 16:44:53 +0000
@@ -29,7 +29,11 @@
     ArchitectureRegistry,
     PowerTypeRegistry,
     )
+from provisioningserver.drivers.hardware.seamicro import (
+    probe_seamicro15k_and_enlist,
+    )
 from provisioningserver.drivers.hardware.virsh import probe_virsh_and_enlist
+from provisioningserver.logger.log import get_maas_logger
 from provisioningserver.rpc import (
     cluster,
     common,
@@ -61,6 +65,7 @@
     cancel_timer,
     start_timers,
     )
+from provisioningserver.utils.network import find_ip_via_arp
 from twisted.application.internet import TimerService
 from twisted.internet.defer import inlineCallbacks
 from twisted.internet.endpoints import (
@@ -74,6 +79,9 @@
 from zope.interface import implementer
 
 
+maaslog = get_maas_logger("rpc.cluster")
+
+
 class Cluster(RPCProtocol):
     """The RPC protocol supported by a cluster controller.
 
@@ -241,6 +249,24 @@
         probe_virsh_and_enlist(poweraddr, password)
         return {}
 
+    @cluster.AddSeaMicro15k.responder
+    def add_seamicro15k(self, mac, username, password, power_control=None):
+        """add_virsh()
+
+        Implementation of
+        :py:class:`~provisioningserver.rpc.cluster.AddSeaMicro15k`.
+        """
+        ip = find_ip_via_arp(mac)
+        if ip is not None:
+            probe_seamicro15k_and_enlist(
+                ip, username, password,
+                power_control=power_control)
+        else:
+            message = "Couldn't find IP address for MAC %s" % mac
+            maaslog.warning(message)
+            raise exceptions.NoIPFoundForMACAddress(message)
+        return {}
+
 
 @implementer(IConnection)
 class ClusterClient(Cluster):

=== modified file 'src/provisioningserver/rpc/exceptions.py'
--- src/provisioningserver/rpc/exceptions.py	2014-09-10 12:47:59 +0000
+++ src/provisioningserver/rpc/exceptions.py	2014-09-10 16:44:53 +0000
@@ -97,3 +97,7 @@
 
 class NodeAlreadyExists(Exception):
     """A node already exists with a given MAC address."""
+
+
+class NoIPFoundForMACAddress(Exception):
+    """No IP was found for a given MAC address."""

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-10 15:06:39 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-10 16:44:53 +0000
@@ -1175,3 +1175,82 @@
         self.assertThat(
             probe_virsh_and_enlist, MockCalledOnceWith(
                 poweraddr, None))
+
+
+class TestClusterProtocol_AddSeaMicro15k(MAASTestCase):
+
+    def test__is_registered(self):
+        protocol = Cluster()
+        responder = protocol.locateResponder(
+            cluster.AddSeaMicro15k.commandName)
+        self.assertIsNot(responder, None)
+
+    def test__calls_find_ip_via_arp(self):
+        # Prevent any actual probing from happing.
+        self.patch_autospec(
+            clusterservice, 'probe_seamicro15k_and_enlist')
+        find_ip_via_arp = self.patch_autospec(
+            clusterservice, 'find_ip_via_arp')
+        find_ip_via_arp.return_value = factory.getRandomIPAddress()
+
+        mac = factory.getRandomMACAddress()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = factory.make_name('power_control')
+        call_responder(Cluster(), cluster.AddSeaMicro15k, {
+            "mac": mac,
+            "username": username,
+            "password": password,
+            "power_control": power_control
+            })
+
+        self.assertThat(
+            find_ip_via_arp, MockCalledOnceWith(mac))
+
+    @inlineCallbacks
+    def test__raises_and_logs_warning_if_no_ip_found_for_mac(self):
+        maaslog = self.patch(clusterservice, 'maaslog')
+        find_ip_via_arp = self.patch_autospec(
+            clusterservice, 'find_ip_via_arp')
+        find_ip_via_arp.return_value = None
+
+        mac = factory.getRandomMACAddress()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = factory.make_name('power_control')
+
+        with ExpectedException(exceptions.NoIPFoundForMACAddress):
+            yield call_responder(Cluster(), cluster.AddSeaMicro15k, {
+                "mac": mac,
+                "username": username,
+                "password": password,
+                "power_control": power_control
+                })
+
+        self.assertThat(
+            maaslog.warning,
+            MockCalledOnceWith(
+                "Couldn't find IP address for MAC %s" % mac))
+
+    def test__calls_probe_seamicro15k_and_enlist(self):
+        probe_seamicro15k_and_enlist = self.patch_autospec(
+            clusterservice, 'probe_seamicro15k_and_enlist')
+        find_ip_via_arp = self.patch_autospec(
+            clusterservice, 'find_ip_via_arp')
+        find_ip_via_arp.return_value = factory.getRandomIPAddress()
+
+        mac = factory.getRandomMACAddress()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = factory.make_name('power_control')
+        call_responder(Cluster(), cluster.AddSeaMicro15k, {
+            "mac": mac,
+            "username": username,
+            "password": password,
+            "power_control": power_control
+            })
+
+        self.assertThat(
+            probe_seamicro15k_and_enlist, MockCalledOnceWith(
+                find_ip_via_arp.return_value, username, password,
+                power_control=power_control))

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2014-09-10 15:06:39 +0000
+++ src/provisioningserver/tasks.py	2014-09-10 16:44:53 +0000
@@ -52,13 +52,9 @@
     setup_rndc,
     )
 from provisioningserver.drivers.hardware.mscm import probe_and_enlist_mscm
-from provisioningserver.drivers.hardware.seamicro import (
-    probe_seamicro15k_and_enlist,
-    )
 from provisioningserver.drivers.hardware.ucsm import probe_and_enlist_ucsm
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.utils.fs import sudo_write_file
-from provisioningserver.utils.network import find_ip_via_arp
 
 # For each item passed to refresh_secrets, a refresh function to give it to.
 refresh_functions = {
@@ -392,20 +388,6 @@
 @task
 @log_task_events()
 @log_exception_text
-def add_seamicro15k(mac, username, password, power_control=None):
-    """ See `maasserver.api.NodeGroup.add_seamicro15k`. """
-    ip = find_ip_via_arp(mac)
-    if ip is not None:
-        probe_seamicro15k_and_enlist(
-            ip, username, password,
-            power_control=power_control)
-    else:
-        maaslog.warning("Couldn't find IP address for MAC %s" % mac)
-
-
-@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)