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

Proposed by Newell Jensen on 2014-12-03
Status: Merged
Approved by: Newell Jensen on 2014-12-08
Approved revision: 3379
Merged at revision: 3404
Proposed branch: lp:~newell-jensen/maas/prefix-filter-virsh-bug-1393423
Merge into: lp:maas/trunk
Diff against target: 332 lines (+113/-18)
9 files modified
src/maasserver/api/node_groups.py (+6/-1)
src/maasserver/api/tests/test_nodegroup.py (+70/-0)
src/maasserver/models/nodegroup.py (+4/-2)
src/maasserver/models/tests/test_nodegroup.py (+5/-2)
src/provisioningserver/drivers/hardware/tests/test_virsh.py (+11/-4)
src/provisioningserver/drivers/hardware/virsh.py (+9/-4)
src/provisioningserver/rpc/cluster.py (+1/-0)
src/provisioningserver/rpc/clusterservice.py (+2/-2)
src/provisioningserver/rpc/tests/test_clusterservice.py (+5/-3)
To merge this branch: bzr merge lp:~newell-jensen/maas/prefix-filter-virsh-bug-1393423
Reviewer Review Type Date Requested Status
Blake Rouse 2014-12-03 Approve on 2014-12-08
Review via email: mp+243569@code.launchpad.net

Commit message

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.

To post a comment you must log in.
Newell Jensen (newell-jensen) wrote :

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) wrote :

Looks good.

Thanks for adding the sm15k test in there as well.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/node_groups.py'
2--- src/maasserver/api/node_groups.py 2014-11-27 08:05:50 +0000
3+++ src/maasserver/api/node_groups.py 2014-12-03 22:30:57 +0000
4@@ -395,6 +395,8 @@
5 :param power_pass: The password to use, when qemu+ssh is given as a
6 connection string and ssh key authentication is not being used.
7 :type power_pass: unicode
8+ :param prefix_filter: Only import nodes based on supplied prefix.
9+ :type prefix_filter: unicode
10
11 Returns 404 if the nodegroup (cluster) is not found.
12 Returns 403 if the user does not have access to the nodegroup.
13@@ -417,8 +419,11 @@
14 poweraddr = get_mandatory_param(request.data, 'power_address')
15 password = get_optional_param(
16 request.data, 'power_pass', default=None)
17+ prefix_filter = get_optional_param(
18+ request.data, 'prefix_filter', default=None)
19
20- nodegroup.add_virsh(poweraddr, password=password)
21+ nodegroup.add_virsh(
22+ poweraddr, password=password, prefix_filter=prefix_filter)
23 else:
24 return HttpResponse(status=httplib.BAD_REQUEST)
25
26
27=== modified file 'src/maasserver/api/tests/test_nodegroup.py'
28--- src/maasserver/api/tests/test_nodegroup.py 2014-10-23 18:21:41 +0000
29+++ src/maasserver/api/tests/test_nodegroup.py 2014-12-03 22:30:57 +0000
30@@ -16,6 +16,7 @@
31
32 import httplib
33 import json
34+import random
35 from textwrap import dedent
36 from urlparse import urlparse
37
38@@ -56,6 +57,8 @@
39 )
40 from mock import Mock
41 from provisioningserver.rpc.cluster import (
42+ AddSeaMicro15k,
43+ AddVirsh,
44 EnlistNodesFromMSCM,
45 EnlistNodesFromUCSM,
46 ImportBootImages,
47@@ -327,6 +330,73 @@
48 httplib.BAD_REQUEST, response.status_code,
49 explain_unexpected_response(httplib.BAD_REQUEST, response))
50
51+ def test_probe_and_enlist_hardware_adds_seamicro(self):
52+ nodegroup = factory.make_NodeGroup()
53+ model = 'seamicro15k'
54+ mac = factory.make_mac_address()
55+ username = factory.make_name('user')
56+ password = factory.make_name('password')
57+ power_control = random.choice(
58+ ['ipmi', 'restapi', 'restapi2'])
59+ self.become_admin()
60+
61+ getClientFor = self.patch(nodegroup_module, 'getClientFor')
62+ client = getClientFor.return_value
63+ nodegroup = factory.make_NodeGroup()
64+
65+ response = self.client.post(
66+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
67+ {
68+ 'op': 'probe_and_enlist_hardware',
69+ 'model': model,
70+ 'mac': mac,
71+ 'username': username,
72+ 'password': password,
73+ 'power_control': power_control,
74+ })
75+
76+ self.assertEqual(
77+ httplib.OK, response.status_code,
78+ explain_unexpected_response(httplib.OK, response))
79+
80+ self.expectThat(
81+ client,
82+ MockCalledOnceWith(
83+ AddSeaMicro15k, mac=mac, username=username,
84+ password=password, power_control=power_control))
85+
86+ def test_probe_and_enlist_hardware_adds_virsh(self):
87+ nodegroup = factory.make_NodeGroup()
88+ model = 'virsh'
89+ poweraddr = factory.make_ipv4_address()
90+ password = factory.make_name('password')
91+ prefix_filter = factory.make_name('filter')
92+ self.become_admin()
93+
94+ getClientFor = self.patch(nodegroup_module, 'getClientFor')
95+ client = getClientFor.return_value
96+ nodegroup = factory.make_NodeGroup()
97+
98+ response = self.client.post(
99+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
100+ {
101+ 'op': 'probe_and_enlist_hardware',
102+ 'model': model,
103+ 'power_address': poweraddr,
104+ 'power_pass': password,
105+ 'prefix_filter': prefix_filter,
106+ })
107+
108+ self.assertEqual(
109+ httplib.OK, response.status_code,
110+ explain_unexpected_response(httplib.OK, response))
111+
112+ self.expectThat(
113+ client,
114+ MockCalledOnceWith(
115+ AddVirsh, poweraddr=poweraddr,
116+ password=password, prefix_filter=prefix_filter))
117+
118 def test_probe_and_enlist_ucsm_adds_ucsm(self):
119 nodegroup = factory.make_NodeGroup()
120 url = 'http://url'
121
122=== modified file 'src/maasserver/models/nodegroup.py'
123--- src/maasserver/models/nodegroup.py 2014-10-23 18:21:41 +0000
124+++ src/maasserver/models/nodegroup.py 2014-12-03 22:30:57 +0000
125@@ -337,11 +337,12 @@
126 AddSeaMicro15k, mac=mac, username=username,
127 password=password, power_control=power_control)
128
129- def add_virsh(self, poweraddr, password=None):
130+ def add_virsh(self, poweraddr, password=None, prefix_filter=None):
131 """ Add all of the virtual machines inside a virsh controller.
132
133 :param poweraddr: virsh connection string
134 :param password: ssh password
135+ :param prefix_filter: import based on prefix
136
137 :raises NoConnectionsAvailable: If no connections to the cluster
138 are available.
139@@ -355,7 +356,8 @@
140 raise
141 else:
142 return client(
143- AddVirsh, poweraddr=poweraddr, password=password)
144+ AddVirsh, poweraddr=poweraddr,
145+ password=password, prefix_filter=prefix_filter)
146
147 def enlist_nodes_from_ucsm(self, url, username, password):
148 """ Add the servers from a Cicso UCS Manager.
149
150=== modified file 'src/maasserver/models/tests/test_nodegroup.py'
151--- src/maasserver/models/tests/test_nodegroup.py 2014-10-23 18:21:41 +0000
152+++ src/maasserver/models/tests/test_nodegroup.py 2014-12-03 22:30:57 +0000
153@@ -506,7 +506,9 @@
154
155 self.expectThat(
156 protocol.AddVirsh,
157- MockCalledOnceWith(ANY, poweraddr=poweraddr, password=password))
158+ MockCalledOnceWith(
159+ ANY, poweraddr=poweraddr,
160+ password=password, prefix_filter=None))
161
162 def test_add_virsh_calls_client_with_resource_endpoint(self):
163 getClientFor = self.patch(nodegroup_module, 'getClientFor')
164@@ -520,7 +522,8 @@
165 self.expectThat(
166 client,
167 MockCalledOnceWith(
168- AddVirsh, poweraddr=poweraddr, password=password))
169+ AddVirsh, poweraddr=poweraddr,
170+ password=password, prefix_filter=None))
171
172 def test_add_virsh_raises_if_no_connection_to_cluster(self):
173 getClientFor = self.patch(nodegroup_module, 'getClientFor')
174
175=== modified file 'src/provisioningserver/drivers/hardware/tests/test_virsh.py'
176--- src/provisioningserver/drivers/hardware/tests/test_virsh.py 2014-09-18 12:44:38 +0000
177+++ src/provisioningserver/drivers/hardware/tests/test_virsh.py 2014-12-03 22:30:57 +0000
178@@ -53,10 +53,10 @@
179 class TestVirshSSH(MAASTestCase):
180 """Tests for `VirshSSH`."""
181
182- def configure_virshssh_pexpect(self, inputs=None):
183+ def configure_virshssh_pexpect(self, inputs=None, dom_prefix=None):
184 """Configures the VirshSSH class to use 'cat' process
185 for testing instead of the actual virsh."""
186- conn = virsh.VirshSSH(timeout=0.1)
187+ conn = virsh.VirshSSH(timeout=0.1, dom_prefix=dom_prefix)
188 self.addCleanup(conn.close)
189 self.patch(conn, '_execute')
190 conn._spawn('cat')
191@@ -65,9 +65,9 @@
192 conn.sendline(line)
193 return conn
194
195- def configure_virshssh(self, output):
196+ def configure_virshssh(self, output, dom_prefix=None):
197 self.patch(virsh.VirshSSH, 'run').return_value = output
198- return virsh.VirshSSH()
199+ return virsh.VirshSSH(dom_prefix=dom_prefix)
200
201 def test_login_prompt(self):
202 virsh_outputs = [
203@@ -157,6 +157,13 @@
204 expected = conn.list()
205 self.assertItemsEqual(names, expected)
206
207+ def test_list_dom_prefix(self):
208+ prefix = 'dom_prefix'
209+ names = [prefix + factory.make_name('machine') for _ in range(3)]
210+ conn = self.configure_virshssh('\n'.join(names), dom_prefix=prefix)
211+ expected = conn.list()
212+ self.assertItemsEqual(names, expected)
213+
214 def test_get_state(self):
215 state = factory.make_name('state')
216 conn = self.configure_virshssh(state)
217
218=== modified file 'src/provisioningserver/drivers/hardware/virsh.py'
219--- src/provisioningserver/drivers/hardware/virsh.py 2014-12-02 05:56:30 +0000
220+++ src/provisioningserver/drivers/hardware/virsh.py 2014-12-03 22:30:57 +0000
221@@ -79,10 +79,14 @@
222 I_PROMPT_SSHKEY = PROMPTS.index(PROMPT_SSHKEY)
223 I_PROMPT_PASSWORD = PROMPTS.index(PROMPT_PASSWORD)
224
225- def __init__(self, timeout=30, maxread=2000):
226+ def __init__(self, timeout=30, maxread=2000, dom_prefix=None):
227 super(VirshSSH, self).__init__(
228 None, timeout=timeout, maxread=maxread)
229 self.name = '<virssh>'
230+ if dom_prefix is None:
231+ self.dom_prefix = ''
232+ else:
233+ self.dom_prefix = dom_prefix
234
235 def _execute(self, poweraddr):
236 """Spawns the pexpect command."""
237@@ -136,7 +140,8 @@
238 def list(self):
239 """Lists all virtual machines by name."""
240 machines = self.run(['list', '--all', '--name'])
241- return machines.strip().splitlines()
242+ machines = machines.strip().splitlines()
243+ return [m for m in machines if m.startswith(self.dom_prefix)]
244
245 def get_state(self, machine):
246 """Gets the virtual machine state."""
247@@ -183,13 +188,13 @@
248 return True
249
250
251-def probe_virsh_and_enlist(poweraddr, password=None):
252+def probe_virsh_and_enlist(poweraddr, password=None, prefix_filter=None):
253 """Extracts all of the virtual machines from virsh and enlists them
254 into MAAS.
255
256 :param poweraddr: virsh connection string
257 """
258- conn = VirshSSH()
259+ conn = VirshSSH(dom_prefix=prefix_filter)
260 if not conn.login(poweraddr, password):
261 raise VirshError('Failed to login to virsh console.')
262
263
264=== modified file 'src/provisioningserver/rpc/cluster.py'
265--- src/provisioningserver/rpc/cluster.py 2014-10-23 18:21:41 +0000
266+++ src/provisioningserver/rpc/cluster.py 2014-12-03 22:30:57 +0000
267@@ -419,6 +419,7 @@
268 arguments = [
269 (b"poweraddr", amp.Unicode()),
270 (b"password", amp.Unicode(optional=True)),
271+ (b"prefix_filter", amp.Unicode(optional=True)),
272 ]
273 response = []
274 errors = []
275
276=== modified file 'src/provisioningserver/rpc/clusterservice.py'
277--- src/provisioningserver/rpc/clusterservice.py 2014-11-10 15:11:58 +0000
278+++ src/provisioningserver/rpc/clusterservice.py 2014-12-03 22:30:57 +0000
279@@ -344,12 +344,12 @@
280 return d.addCallback(lambda _: {})
281
282 @cluster.AddVirsh.responder
283- def add_virsh(self, poweraddr, password):
284+ def add_virsh(self, poweraddr, password, prefix_filter):
285 """add_virsh()
286
287 Implementation of :py:class:`~provisioningserver.rpc.cluster.AddVirsh`.
288 """
289- probe_virsh_and_enlist(poweraddr, password)
290+ probe_virsh_and_enlist(poweraddr, password, prefix_filter)
291 return {}
292
293 @cluster.AddSeaMicro15k.responder
294
295=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
296--- src/provisioningserver/rpc/tests/test_clusterservice.py 2014-11-10 15:11:58 +0000
297+++ src/provisioningserver/rpc/tests/test_clusterservice.py 2014-12-03 22:30:57 +0000
298@@ -1889,13 +1889,15 @@
299 clusterservice, 'probe_virsh_and_enlist')
300 poweraddr = factory.make_name('poweraddr')
301 password = factory.make_name('password')
302+ prefix_filter = factory.make_name('prefix_filter')
303 call_responder(Cluster(), cluster.AddVirsh, {
304 "poweraddr": poweraddr,
305 "password": password,
306+ "prefix_filter": prefix_filter,
307 })
308 self.assertThat(
309 probe_virsh_and_enlist, MockCalledOnceWith(
310- poweraddr, password))
311+ poweraddr, password, prefix_filter))
312
313 def test__password_is_optional(self):
314 probe_virsh_and_enlist = self.patch_autospec(
315@@ -1907,7 +1909,7 @@
316 })
317 self.assertThat(
318 probe_virsh_and_enlist, MockCalledOnceWith(
319- poweraddr, None))
320+ poweraddr, None, None))
321
322 def test__can_be_called_without_password_key(self):
323 probe_virsh_and_enlist = self.patch_autospec(
324@@ -1918,7 +1920,7 @@
325 })
326 self.assertThat(
327 probe_virsh_and_enlist, MockCalledOnceWith(
328- poweraddr, None))
329+ poweraddr, None, None))
330
331
332 class TestClusterProtocol_AddSeaMicro15k(MAASTestCase):