Merge lp:~newell-jensen/maas/prefix-filter-virsh-bug-1393423 into lp:~maas-committers/maas/trunk

[Newell Jensen]

This branch gives the user the capability to add an optional `prefix_filter` parameter to the api call for probe_and_enlist_hardware when the model type is `virsh`, which will only enlist the machines with the prefix in their names.

[Newell Jensen (newell-jensen)]

Example usage:

Let us suppose there is a MAAS profile login named `admin`. Let us also suppose that we have three virsh machines with the names node0, node1, and nokk that are on MAAS's network and can PXE boot etc. Additionally let us assume the username for the host is ubuntu and the password for this system is ubuntu as well, with an IP address of 10.0.0.2.

1. Get UUID of the cluster controller for the `admin` profile.

$ uuid=$(maas admin node-groups list | grep uuid | cut -d'"' -f4)

2. probe and enlist ALL virsh nodes.

$ maas admin node-group probe-and-enlist-hardware $uuid model=virsh power_address=qemu+ssh://ubuntu@10.0.0.2/system power_pass=ubuntu

The above command will enlist ALL the virsh nodes, which are the three noted above.
This shows that the probe and enlist still works as it should. Now let's add a prefix_filter parameter to the api command so that we only enlist the nodes that start with `node` (this prefix_filter can obviously be anything that you desire to filter on). Make sure to delete all you current nodes in MAAS so that we can do the probe and enlist again. Once that is complete-

3. probe and enlist nodes with prefix of `node`.

$ maas admin node-group probe-and-enlist-hardware $uuid model=virsh power_address=qemu+ssh://ubuntu@10.0.0.2/system power_pass=ubuntu prefix_filter=node

This will now only enlist the two nodes that begin with the prefix of `node`, node0 and node1.

[Blake Rouse (blake-rouse)]

Looks good.

Thanks for adding the sm15k test in there as well.

review: Approve

=== modified file 'src/maasserver/api/node_groups.py'
--- src/maasserver/api/node_groups.py	2014-11-27 08:05:50 +0000
+++ src/maasserver/api/node_groups.py	2014-12-03 22:30:57 +0000
@@ -395,6 +395,8 @@
         :param power_pass: The password to use, when qemu+ssh is given as a
             connection string and ssh key authentication is not being used.
         :type power_pass: unicode
+        :param prefix_filter: Only import nodes based on supplied prefix.
+        :type prefix_filter: unicode
 
         Returns 404 if the nodegroup (cluster) is not found.
         Returns 403 if the user does not have access to the nodegroup.
@@ -417,8 +419,11 @@
             poweraddr = get_mandatory_param(request.data, 'power_address')
             password = get_optional_param(
                 request.data, 'power_pass', default=None)
+            prefix_filter = get_optional_param(
+                request.data, 'prefix_filter', default=None)
 
-            nodegroup.add_virsh(poweraddr, password=password)
+            nodegroup.add_virsh(
+                poweraddr, password=password, prefix_filter=prefix_filter)
         else:
             return HttpResponse(status=httplib.BAD_REQUEST)
 
 
=== modified file 'src/maasserver/api/tests/test_nodegroup.py'
--- src/maasserver/api/tests/test_nodegroup.py	2014-10-23 18:21:41 +0000
+++ src/maasserver/api/tests/test_nodegroup.py	2014-12-03 22:30:57 +0000
@@ -16,6 +16,7 @@
 
 import httplib
 import json
+import random
 from textwrap import dedent
 from urlparse import urlparse
 
@@ -56,6 +57,8 @@
     )
 from mock import Mock
 from provisioningserver.rpc.cluster import (
+    AddSeaMicro15k,
+    AddVirsh,
     EnlistNodesFromMSCM,
     EnlistNodesFromUCSM,
     ImportBootImages,
@@ -327,6 +330,73 @@
             httplib.BAD_REQUEST, response.status_code,
             explain_unexpected_response(httplib.BAD_REQUEST, response))
 
+    def test_probe_and_enlist_hardware_adds_seamicro(self):
+        nodegroup = factory.make_NodeGroup()
+        model = 'seamicro15k'
+        mac = factory.make_mac_address()
+        username = factory.make_name('user')
+        password = factory.make_name('password')
+        power_control = random.choice(
+            ['ipmi', 'restapi', 'restapi2'])
+        self.become_admin()
+
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
+
+        response = self.client.post(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {
+                'op': 'probe_and_enlist_hardware',
+                'model': model,
+                'mac': mac,
+                'username': username,
+                'password': password,
+                'power_control': power_control,
+            })
+
+        self.assertEqual(
+            httplib.OK, response.status_code,
+            explain_unexpected_response(httplib.OK, response))
+
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                AddSeaMicro15k, mac=mac, username=username,
+                password=password, power_control=power_control))
+
+    def test_probe_and_enlist_hardware_adds_virsh(self):
+        nodegroup = factory.make_NodeGroup()
+        model = 'virsh'
+        poweraddr = factory.make_ipv4_address()
+        password = factory.make_name('password')
+        prefix_filter = factory.make_name('filter')
+        self.become_admin()
+
+        getClientFor = self.patch(nodegroup_module, 'getClientFor')
+        client = getClientFor.return_value
+        nodegroup = factory.make_NodeGroup()
+
+        response = self.client.post(
+            reverse('nodegroup_handler', args=[nodegroup.uuid]),
+            {
+                'op': 'probe_and_enlist_hardware',
+                'model': model,
+                'power_address': poweraddr,
+                'power_pass': password,
+                'prefix_filter': prefix_filter,
+            })
+
+        self.assertEqual(
+            httplib.OK, response.status_code,
+            explain_unexpected_response(httplib.OK, response))
+
+        self.expectThat(
+            client,
+            MockCalledOnceWith(
+                AddVirsh, poweraddr=poweraddr,
+                password=password, prefix_filter=prefix_filter))
+
     def test_probe_and_enlist_ucsm_adds_ucsm(self):
         nodegroup = factory.make_NodeGroup()
         url = 'http://url'
 
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2014-10-23 18:21:41 +0000
+++ src/maasserver/models/nodegroup.py	2014-12-03 22:30:57 +0000
@@ -337,11 +337,12 @@
                 AddSeaMicro15k, mac=mac, username=username,
                 password=password, power_control=power_control)
 
-    def add_virsh(self, poweraddr, password=None):
+    def add_virsh(self, poweraddr, password=None, prefix_filter=None):
         """ Add all of the virtual machines inside a virsh controller.
 
         :param poweraddr: virsh connection string
         :param password: ssh password
+        :param prefix_filter: import based on prefix
 
         :raises NoConnectionsAvailable: If no connections to the cluster
             are available.
@@ -355,7 +356,8 @@
             raise
         else:
             return client(
-                AddVirsh, poweraddr=poweraddr, password=password)
+                AddVirsh, poweraddr=poweraddr,
+                password=password, prefix_filter=prefix_filter)
 
     def enlist_nodes_from_ucsm(self, url, username, password):
         """ Add the servers from a Cicso UCS Manager.
 
=== modified file 'src/maasserver/models/tests/test_nodegroup.py'
--- src/maasserver/models/tests/test_nodegroup.py	2014-10-23 18:21:41 +0000
+++ src/maasserver/models/tests/test_nodegroup.py	2014-12-03 22:30:57 +0000
@@ -506,7 +506,9 @@
 
         self.expectThat(
             protocol.AddVirsh,
-            MockCalledOnceWith(ANY, poweraddr=poweraddr, password=password))
+            MockCalledOnceWith(
+                ANY, poweraddr=poweraddr,
+                password=password, prefix_filter=None))
 
     def test_add_virsh_calls_client_with_resource_endpoint(self):
         getClientFor = self.patch(nodegroup_module, 'getClientFor')
@@ -520,7 +522,8 @@
         self.expectThat(
             client,
             MockCalledOnceWith(
-                AddVirsh, poweraddr=poweraddr, password=password))
+                AddVirsh, poweraddr=poweraddr,
+                password=password, prefix_filter=None))
 
     def test_add_virsh_raises_if_no_connection_to_cluster(self):
         getClientFor = self.patch(nodegroup_module, 'getClientFor')
 
=== modified file 'src/provisioningserver/drivers/hardware/tests/test_virsh.py'
--- src/provisioningserver/drivers/hardware/tests/test_virsh.py	2014-09-18 12:44:38 +0000
+++ src/provisioningserver/drivers/hardware/tests/test_virsh.py	2014-12-03 22:30:57 +0000
@@ -53,10 +53,10 @@
 class TestVirshSSH(MAASTestCase):
     """Tests for `VirshSSH`."""
 
-    def configure_virshssh_pexpect(self, inputs=None):
+    def configure_virshssh_pexpect(self, inputs=None, dom_prefix=None):
         """Configures the VirshSSH class to use 'cat' process
         for testing instead of the actual virsh."""
-        conn = virsh.VirshSSH(timeout=0.1)
+        conn = virsh.VirshSSH(timeout=0.1, dom_prefix=dom_prefix)
         self.addCleanup(conn.close)
         self.patch(conn, '_execute')
         conn._spawn('cat')
@@ -65,9 +65,9 @@
                 conn.sendline(line)
         return conn
 
-    def configure_virshssh(self, output):
+    def configure_virshssh(self, output, dom_prefix=None):
         self.patch(virsh.VirshSSH, 'run').return_value = output
-        return virsh.VirshSSH()
+        return virsh.VirshSSH(dom_prefix=dom_prefix)
 
     def test_login_prompt(self):
         virsh_outputs = [
@@ -157,6 +157,13 @@
         expected = conn.list()
         self.assertItemsEqual(names, expected)
 
+    def test_list_dom_prefix(self):
+        prefix = 'dom_prefix'
+        names = [prefix + factory.make_name('machine') for _ in range(3)]
+        conn = self.configure_virshssh('\n'.join(names), dom_prefix=prefix)
+        expected = conn.list()
+        self.assertItemsEqual(names, expected)
+
     def test_get_state(self):
         state = factory.make_name('state')
         conn = self.configure_virshssh(state)
 
=== modified file 'src/provisioningserver/drivers/hardware/virsh.py'
--- src/provisioningserver/drivers/hardware/virsh.py	2014-12-02 05:56:30 +0000
+++ src/provisioningserver/drivers/hardware/virsh.py	2014-12-03 22:30:57 +0000
@@ -79,10 +79,14 @@
     I_PROMPT_SSHKEY = PROMPTS.index(PROMPT_SSHKEY)
     I_PROMPT_PASSWORD = PROMPTS.index(PROMPT_PASSWORD)
 
-    def __init__(self, timeout=30, maxread=2000):
+    def __init__(self, timeout=30, maxread=2000, dom_prefix=None):
         super(VirshSSH, self).__init__(
             None, timeout=timeout, maxread=maxread)
         self.name = '<virssh>'
+        if dom_prefix is None:
+            self.dom_prefix = ''
+        else:
+            self.dom_prefix = dom_prefix
 
     def _execute(self, poweraddr):
         """Spawns the pexpect command."""
@@ -136,7 +140,8 @@
     def list(self):
         """Lists all virtual machines by name."""
         machines = self.run(['list', '--all', '--name'])
-        return machines.strip().splitlines()
+        machines = machines.strip().splitlines()
+        return [m for m in machines if m.startswith(self.dom_prefix)]
 
     def get_state(self, machine):
         """Gets the virtual machine state."""
@@ -183,13 +188,13 @@
         return True
 
 
-def probe_virsh_and_enlist(poweraddr, password=None):
+def probe_virsh_and_enlist(poweraddr, password=None, prefix_filter=None):
     """Extracts all of the virtual machines from virsh and enlists them
     into MAAS.
 
     :param poweraddr: virsh connection string
     """
-    conn = VirshSSH()
+    conn = VirshSSH(dom_prefix=prefix_filter)
     if not conn.login(poweraddr, password):
         raise VirshError('Failed to login to virsh console.')
 
 
=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-10-23 18:21:41 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-12-03 22:30:57 +0000
@@ -419,6 +419,7 @@
     arguments = [
         (b"poweraddr", amp.Unicode()),
         (b"password", amp.Unicode(optional=True)),
+        (b"prefix_filter", amp.Unicode(optional=True)),
     ]
     response = []
     errors = []
 
=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-11-10 15:11:58 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-12-03 22:30:57 +0000
@@ -344,12 +344,12 @@
         return d.addCallback(lambda _: {})
 
     @cluster.AddVirsh.responder
-    def add_virsh(self, poweraddr, password):
+    def add_virsh(self, poweraddr, password, prefix_filter):
         """add_virsh()
 
         Implementation of :py:class:`~provisioningserver.rpc.cluster.AddVirsh`.
         """
-        probe_virsh_and_enlist(poweraddr, password)
+        probe_virsh_and_enlist(poweraddr, password, prefix_filter)
         return {}
 
     @cluster.AddSeaMicro15k.responder
 
=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-11-10 15:11:58 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-12-03 22:30:57 +0000
@@ -1889,13 +1889,15 @@
             clusterservice, 'probe_virsh_and_enlist')
         poweraddr = factory.make_name('poweraddr')
         password = factory.make_name('password')
+        prefix_filter = factory.make_name('prefix_filter')
         call_responder(Cluster(), cluster.AddVirsh, {
             "poweraddr": poweraddr,
             "password": password,
+            "prefix_filter": prefix_filter,
             })
         self.assertThat(
             probe_virsh_and_enlist, MockCalledOnceWith(
-                poweraddr, password))
+                poweraddr, password, prefix_filter))
 
     def test__password_is_optional(self):
         probe_virsh_and_enlist = self.patch_autospec(
@@ -1907,7 +1909,7 @@
             })
         self.assertThat(
             probe_virsh_and_enlist, MockCalledOnceWith(
-                poweraddr, None))
+                poweraddr, None, None))
 
     def test__can_be_called_without_password_key(self):
         probe_virsh_and_enlist = self.patch_autospec(
@@ -1918,7 +1920,7 @@
             })
         self.assertThat(
             probe_virsh_and_enlist, MockCalledOnceWith(
-                poweraddr, None))
+                poweraddr, None, None))
 
 
 class TestClusterProtocol_AddSeaMicro15k(MAASTestCase):