Merge lp:~allenap/maas/rpc-tags-cluster into lp:~maas-committers/maas/trunk
- rpc-tags-cluster
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Gavin Panella |
Approved revision: | no longer in the source branch. |
Merged at revision: | 3054 |
Proposed branch: | lp:~allenap/maas/rpc-tags-cluster |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
814 lines (+263/-207) 14 files modified
src/maasserver/api/tests/test_tag.py (+10/-0) src/maasserver/models/tag.py (+3/-2) src/maasserver/models/tests/test_tag.py (+10/-0) src/maasserver/populate_tags.py (+2/-2) src/maasserver/tests/test_populate_tags.py (+1/-39) src/provisioningserver/rpc/cluster.py (+20/-0) src/provisioningserver/rpc/clusterservice.py (+19/-0) src/provisioningserver/rpc/tags.py (+46/-0) src/provisioningserver/rpc/tests/test_clusterservice.py (+66/-0) src/provisioningserver/rpc/tests/test_tags.py (+73/-0) src/provisioningserver/tags.py (+6/-42) src/provisioningserver/tasks.py (+1/-36) src/provisioningserver/tests/test_tags.py (+6/-41) src/provisioningserver/tests/test_tasks.py (+0/-45) |
To merge this branch: | bzr merge lp:~allenap/maas/rpc-tags-cluster |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | Approve | ||
Review via email:
|
Commit message
New EvaluateTag RPC command on the cluster.
Description of the change
This hobbles tag evaluation in the region, but I won't land this as-is. It's a merge proposal for review purposes only.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Gavin Panella (allenap) wrote : | # |
Thanks for the review :)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~allenap/maas/rpc-tags-cluster into lp:maas failed. Below is the output from the failed tests.
Ign http://
Ign http://
Get:1 http://
Ign http://
Get:2 http://
Hit http://
Get:3 http://
Hit http://
Get:4 http://
Get:5 http://
Hit http://
Get:6 http://
Hit http://
Get:7 http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:8 http://
Hit http://
Hit http://
Ign http://
Ign http://
Get:9 http://
Get:10 http://
Get:11 http://
Get:12 http://
Hit http://
Hit http://
Fetched 1,101 kB in 0s (1,663 kB/s)
Reading package lists...
sudo DEBIAN_
--
Preview Diff
1 | === modified file 'src/maasserver/api/tests/test_tag.py' |
2 | --- src/maasserver/api/tests/test_tag.py 2014-09-10 16:20:31 +0000 |
3 | +++ src/maasserver/api/tests/test_tag.py 2014-09-23 09:14:01 +0000 |
4 | @@ -104,6 +104,8 @@ |
5 | self.assertTrue(Tag.objects.filter(name='new-tag-name').exists()) |
6 | |
7 | def test_PUT_updates_node_associations(self): |
8 | + self.skip("Tag evaluation is being ported to RPC.") |
9 | + |
10 | node1 = factory.make_Node() |
11 | inject_lshw_result(node1, b'<node><foo/></node>') |
12 | node2 = factory.make_Node() |
13 | @@ -141,6 +143,8 @@ |
14 | [r['system_id'] for r in parsed_result]) |
15 | |
16 | def test_GET_nodes_hides_invisible_nodes(self): |
17 | + self.skip("Tag evaluation is being ported to RPC.") |
18 | + |
19 | user2 = factory.make_User() |
20 | node1 = factory.make_Node() |
21 | inject_lshw_result(node1, b'<node><foo/></node>') |
22 | @@ -162,6 +166,8 @@ |
23 | [r['system_id'] for r in parsed_result]) |
24 | |
25 | def test_PUT_invalid_definition(self): |
26 | + self.skip("Tag evaluation is being ported to RPC.") |
27 | + |
28 | self.become_admin() |
29 | node = factory.make_Node() |
30 | inject_lshw_result(node, b'<node ><child/></node>') |
31 | @@ -332,6 +338,8 @@ |
32 | self.assertItemsEqual([], node.tags.all()) |
33 | |
34 | def test_POST_rebuild_rebuilds_node_mapping(self): |
35 | + self.skip("Tag evaluation is being ported to RPC.") |
36 | + |
37 | tag = factory.make_Tag(definition='//foo/bar') |
38 | # Only one node matches the tag definition, rebuilding should notice |
39 | node_matching = factory.make_Node() |
40 | @@ -483,6 +491,8 @@ |
41 | extra_kernel_opts, Tag.objects.filter(name=name)[0].kernel_opts) |
42 | |
43 | def test_POST_new_populates_nodes(self): |
44 | + self.skip("Tag evaluation is being ported to RPC.") |
45 | + |
46 | self.become_admin() |
47 | node1 = factory.make_Node() |
48 | inject_lshw_result(node1, b'<node><child/></node>') |
49 | |
50 | === modified file 'src/maasserver/models/tag.py' |
51 | --- src/maasserver/models/tag.py 2013-12-02 14:29:41 +0000 |
52 | +++ src/maasserver/models/tag.py 2014-09-23 09:14:01 +0000 |
53 | @@ -101,7 +101,6 @@ |
54 | |
55 | def populate_nodes(self): |
56 | """Find all nodes that match this tag, and update them.""" |
57 | - from maasserver.populate_tags import populate_tags |
58 | if not self.is_defined: |
59 | return |
60 | # before we pass off any work, ensure the definition is valid XPATH |
61 | @@ -112,7 +111,9 @@ |
62 | raise ValidationError({'definition': [msg]}) |
63 | # Now delete the existing tags |
64 | self.node_set.clear() |
65 | - populate_tags(self) |
66 | + # Being ported to RPC. |
67 | + # from maasserver.populate_tags import populate_tags |
68 | + # populate_tags(self) |
69 | |
70 | def save(self, *args, **kwargs): |
71 | super(Tag, self).save(*args, **kwargs) |
72 | |
73 | === modified file 'src/maasserver/models/tests/test_tag.py' |
74 | --- src/maasserver/models/tests/test_tag.py 2014-09-19 13:27:37 +0000 |
75 | +++ src/maasserver/models/tests/test_tag.py 2014-09-23 09:14:01 +0000 |
76 | @@ -67,6 +67,8 @@ |
77 | self.assertRaises(ValidationError, factory.make_Tag, name=invalid) |
78 | |
79 | def test_applies_tags_to_nodes(self): |
80 | + self.skip("Tag evaluation is being ported to RPC.") |
81 | + |
82 | node1 = factory.make_Node() |
83 | inject_lshw_result(node1, b'<node><child /></node>') |
84 | node2 = factory.make_Node() |
85 | @@ -76,6 +78,8 @@ |
86 | self.assertItemsEqual([], node2.tag_names()) |
87 | |
88 | def test_removes_old_values(self): |
89 | + self.skip("Tag evaluation is being ported to RPC.") |
90 | + |
91 | node1 = factory.make_Node() |
92 | inject_lshw_result(node1, b'<node><foo /></node>') |
93 | node2 = factory.make_Node() |
94 | @@ -94,6 +98,8 @@ |
95 | self.assertItemsEqual([], node2.tag_names()) |
96 | |
97 | def test_doesnt_touch_other_tags(self): |
98 | + self.skip("Tag evaluation is being ported to RPC.") |
99 | + |
100 | node1 = factory.make_Node() |
101 | inject_lshw_result(node1, b'<node><foo /></node>') |
102 | node2 = factory.make_Node() |
103 | @@ -106,6 +112,8 @@ |
104 | self.assertItemsEqual([tag2.name], node2.tag_names()) |
105 | |
106 | def test_rollsback_invalid_xpath(self): |
107 | + self.skip("Tag evaluation is being ported to RPC.") |
108 | + |
109 | node = factory.make_Node() |
110 | inject_lshw_result(node, b'<node><foo /></node>') |
111 | tag = factory.make_Tag(definition='//node/foo') |
112 | @@ -159,6 +167,8 @@ |
113 | self.assertItemsEqual([], tag.node_set.all()) |
114 | |
115 | def test__calls_populate_tags(self): |
116 | + self.skip("Tag evaluation is being ported to RPC.") |
117 | + |
118 | populate_tags = self.patch_autospec( |
119 | populate_tags_module, "populate_tags") |
120 | |
121 | |
122 | === modified file 'src/maasserver/populate_tags.py' |
123 | --- src/maasserver/populate_tags.py 2014-08-13 21:49:35 +0000 |
124 | +++ src/maasserver/populate_tags.py 2014-09-23 09:14:01 +0000 |
125 | @@ -28,7 +28,6 @@ |
126 | ) |
127 | from maasserver.refresh_worker import refresh_worker |
128 | from provisioningserver.tags import merge_details |
129 | -from provisioningserver.tasks import update_node_tags |
130 | from provisioningserver.utils import classify |
131 | from provisioningserver.utils.xpath import try_match_xpath |
132 | |
133 | @@ -56,7 +55,8 @@ |
134 | logger.debug('Refreshing tag definition for %s' % (items,)) |
135 | for nodegroup in NodeGroup.objects.all(): |
136 | refresh_worker(nodegroup) |
137 | - update_node_tags.apply_async(queue=nodegroup.work_queue, kwargs=items) |
138 | + raise NotImplementedError( |
139 | + "Tag evaluation is being ported to RPC.") |
140 | |
141 | |
142 | def populate_tags_for_single_node(tags, node): |
143 | |
144 | === modified file 'src/maasserver/tests/test_populate_tags.py' |
145 | --- src/maasserver/tests/test_populate_tags.py 2014-09-15 14:28:28 +0000 |
146 | +++ src/maasserver/tests/test_populate_tags.py 2014-09-23 09:14:01 +0000 |
147 | @@ -14,49 +14,11 @@ |
148 | __metaclass__ = type |
149 | __all__ = [] |
150 | |
151 | -from maasserver import populate_tags as populate_tags_module |
152 | from maasserver.models import Tag |
153 | -from maasserver.populate_tags import ( |
154 | - populate_tags, |
155 | - populate_tags_for_single_node, |
156 | - tag_nsmap, |
157 | - ) |
158 | +from maasserver.populate_tags import populate_tags_for_single_node |
159 | from maasserver.testing.factory import factory |
160 | from maasserver.testing.testcase import MAASServerTestCase |
161 | from metadataserver.models import commissioningscript |
162 | -import mock |
163 | - |
164 | - |
165 | -class TestPopulateTags(MAASServerTestCase): |
166 | - |
167 | - def test_populate_tags_task_routed_to_nodegroup_worker(self): |
168 | - nodegroup = factory.make_NodeGroup() |
169 | - tag = factory.make_Tag() |
170 | - task = self.patch(populate_tags_module, 'update_node_tags') |
171 | - populate_tags(tag) |
172 | - args, kwargs = task.apply_async.call_args |
173 | - self.assertEqual(nodegroup.work_queue, kwargs['queue']) |
174 | - |
175 | - def test_populate_tags_task_routed_to_all_nodegroup_workers(self): |
176 | - nodegroups = [factory.make_NodeGroup() for _ in range(5)] |
177 | - tag = factory.make_Tag() |
178 | - refresh = self.patch(populate_tags_module, 'refresh_worker') |
179 | - task = self.patch(populate_tags_module, 'update_node_tags') |
180 | - populate_tags(tag) |
181 | - refresh_calls = [mock.call(nodegroup) for nodegroup in nodegroups] |
182 | - refresh.assert_has_calls(refresh_calls, any_order=True) |
183 | - task_calls = [ |
184 | - mock.call( |
185 | - queue=nodegroup.work_queue, |
186 | - kwargs={ |
187 | - 'tag_name': tag.name, |
188 | - 'tag_definition': tag.definition, |
189 | - 'tag_nsmap': tag_nsmap, |
190 | - }, |
191 | - ) |
192 | - for nodegroup in nodegroups |
193 | - ] |
194 | - task.apply_async.assert_has_calls(task_calls, any_order=True) |
195 | |
196 | |
197 | class TestPopulateTagsForSingleNode(MAASServerTestCase): |
198 | |
199 | === modified file 'src/provisioningserver/rpc/cluster.py' |
200 | --- src/provisioningserver/rpc/cluster.py 2014-09-18 21:21:18 +0000 |
201 | +++ src/provisioningserver/rpc/cluster.py 2014-09-23 09:14:01 +0000 |
202 | @@ -345,6 +345,26 @@ |
203 | error = [] |
204 | |
205 | |
206 | +class EvaluateTag(amp.Command): |
207 | + """Evaluate a tag against all of the cluster's nodes. |
208 | + |
209 | + :since: 1.7 |
210 | + """ |
211 | + |
212 | + arguments = [ |
213 | + (b"tag_name", amp.Unicode()), |
214 | + (b"tag_definition", amp.Unicode()), |
215 | + (b"tag_nsmap", amp.AmpList([ |
216 | + (b"prefix", amp.Unicode()), |
217 | + (b"uri", amp.Unicode()), |
218 | + ])), |
219 | + # A 3-part credential string for the web API. |
220 | + (b"credentials", amp.Unicode()), |
221 | + ] |
222 | + response = [] |
223 | + errors = [] |
224 | + |
225 | + |
226 | class AddVirsh(amp.Command): |
227 | """Probe for and enlist virsh VMs attached to the cluster. |
228 | |
229 | |
230 | === modified file 'src/provisioningserver/rpc/clusterservice.py' |
231 | --- src/provisioningserver/rpc/clusterservice.py 2014-09-22 11:31:14 +0000 |
232 | +++ src/provisioningserver/rpc/clusterservice.py 2014-09-23 09:14:01 +0000 |
233 | @@ -21,6 +21,7 @@ |
234 | import random |
235 | from urlparse import urlparse |
236 | |
237 | +from apiclient.creds import convert_string_to_tuple |
238 | from apiclient.utils import ascii_url |
239 | from provisioningserver.cluster_config import ( |
240 | get_cluster_uuid, |
241 | @@ -68,6 +69,7 @@ |
242 | change_power_state, |
243 | get_power_state, |
244 | ) |
245 | +from provisioningserver.rpc.tags import evaluate_tag |
246 | from provisioningserver.utils.network import find_ip_via_arp |
247 | from twisted.application.internet import TimerService |
248 | from twisted.internet.defer import inlineCallbacks |
249 | @@ -76,6 +78,7 @@ |
250 | TCP4ClientEndpoint, |
251 | ) |
252 | from twisted.internet.error import ConnectError |
253 | +from twisted.internet.threads import deferToThread |
254 | from twisted.protocols import amp |
255 | from twisted.python import log |
256 | from twisted.web.client import getPage |
257 | @@ -245,6 +248,22 @@ |
258 | else: |
259 | return tls.get_tls_parameters_for_cluster() |
260 | |
261 | + @cluster.EvaluateTag.responder |
262 | + def evaluate_tag(self, tag_name, tag_definition, tag_nsmap, credentials): |
263 | + """evaluate_tag() |
264 | + |
265 | + Implementation of |
266 | + :py:class:`~provisioningserver.rpc.cluster.EvaluateTag`. |
267 | + """ |
268 | + # It's got to run in a thread because it does blocking IO. |
269 | + d = deferToThread( |
270 | + evaluate_tag, tag_name, tag_definition, |
271 | + # Transform tag_nsmap into a format that LXML likes. |
272 | + {entry["prefix"]: entry["uri"] for entry in tag_nsmap}, |
273 | + # Parse the credential string into a 3-tuple. |
274 | + convert_string_to_tuple(credentials)) |
275 | + return d.addCallback(lambda _: {}) |
276 | + |
277 | @cluster.AddVirsh.responder |
278 | def add_virsh(self, poweraddr, password): |
279 | """add_virsh() |
280 | |
281 | === added file 'src/provisioningserver/rpc/tags.py' |
282 | --- src/provisioningserver/rpc/tags.py 1970-01-01 00:00:00 +0000 |
283 | +++ src/provisioningserver/rpc/tags.py 2014-09-23 09:14:01 +0000 |
284 | @@ -0,0 +1,46 @@ |
285 | +# Copyright 2014 Canonical Ltd. This software is licensed under the |
286 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
287 | + |
288 | +"""RPC helpers for dealing with tags.""" |
289 | + |
290 | +from __future__ import ( |
291 | + absolute_import, |
292 | + print_function, |
293 | + unicode_literals, |
294 | + ) |
295 | + |
296 | +str = None |
297 | + |
298 | +__metaclass__ = type |
299 | +__all__ = [ |
300 | + "evaluate_tag", |
301 | +] |
302 | + |
303 | +from apiclient.maas_client import ( |
304 | + MAASClient, |
305 | + MAASDispatcher, |
306 | + MAASOAuth, |
307 | + ) |
308 | +from provisioningserver.cluster_config import ( |
309 | + get_cluster_uuid, |
310 | + get_maas_url, |
311 | + ) |
312 | +from provisioningserver.tags import process_node_tags |
313 | +from provisioningserver.utils.twisted import synchronous |
314 | + |
315 | + |
316 | +@synchronous |
317 | +def evaluate_tag(tag_name, tag_definition, tag_nsmap, credentials): |
318 | + """Evaluate `tag_definition` against this cluster's nodes' details. |
319 | + |
320 | + :param tag_name: The name of the tag, used for logging. |
321 | + :param tag_definition: The XPath expression of the tag. |
322 | + :param tag_nsmap: The namespace map as used by LXML's ETree library. |
323 | + :param credentials: A 3-tuple of OAuth credentials. |
324 | + """ |
325 | + client = MAASClient( |
326 | + auth=MAASOAuth(*credentials), dispatcher=MAASDispatcher(), |
327 | + base_url=get_maas_url()) |
328 | + process_node_tags( |
329 | + tag_name=tag_name, tag_definition=tag_definition, tag_nsmap=tag_nsmap, |
330 | + client=client, nodegroup_uuid=get_cluster_uuid()) |
331 | |
332 | === modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py' |
333 | --- src/provisioningserver/rpc/tests/test_clusterservice.py 2014-09-22 21:34:33 +0000 |
334 | +++ src/provisioningserver/rpc/tests/test_clusterservice.py 2014-09-23 09:14:01 +0000 |
335 | @@ -25,6 +25,7 @@ |
336 | from random import randint |
337 | from urlparse import urlparse |
338 | |
339 | +from apiclient.creds import convert_tuple_to_string |
340 | from fixtures import EnvironmentVariable |
341 | from maastesting.factory import factory |
342 | from maastesting.matchers import ( |
343 | @@ -64,6 +65,7 @@ |
344 | osystems as osystems_rpc_module, |
345 | power as power_module, |
346 | region, |
347 | + tags, |
348 | ) |
349 | from provisioningserver.rpc.clusterservice import ( |
350 | Cluster, |
351 | @@ -1185,6 +1187,70 @@ |
352 | self.assertThat(running_monitors, Not(Contains(monitors[0]["id"]))) |
353 | |
354 | |
355 | +class TestClusterProtocol_EvaluateTag(MAASTestCase): |
356 | + |
357 | + run_tests_with = MAASTwistedRunTest.make_factory(timeout=5) |
358 | + |
359 | + def test__is_registered(self): |
360 | + protocol = Cluster() |
361 | + responder = protocol.locateResponder( |
362 | + cluster.EvaluateTag.commandName) |
363 | + self.assertIsNot(responder, None) |
364 | + |
365 | + @inlineCallbacks |
366 | + def test_happy_path(self): |
367 | + get_maas_url = self.patch_autospec(tags, "get_maas_url") |
368 | + get_maas_url.return_value = sentinel.maas_url |
369 | + get_cluster_uuid = self.patch_autospec(tags, "get_cluster_uuid") |
370 | + get_cluster_uuid.return_value = sentinel.cluster_uuid |
371 | + |
372 | + # Prevent real work being done, which would involve HTTP calls. |
373 | + self.patch_autospec(tags, "process_node_tags") |
374 | + |
375 | + response = yield call_responder( |
376 | + Cluster(), cluster.EvaluateTag, { |
377 | + "tag_name": "all-nodes", |
378 | + "tag_definition": "//*", |
379 | + "tag_nsmap": [ |
380 | + {"prefix": "foo", |
381 | + "uri": "http://foo.example.com/"}, |
382 | + ], |
383 | + "credentials": "abc:def:ghi", |
384 | + }) |
385 | + |
386 | + self.assertEqual({}, response) |
387 | + |
388 | + @inlineCallbacks |
389 | + def test__calls_through_to_evaluate_tag_helper(self): |
390 | + evaluate_tag = self.patch_autospec(clusterservice, "evaluate_tag") |
391 | + |
392 | + tag_name = factory.make_name("tag-name") |
393 | + tag_definition = factory.make_name("tag-definition") |
394 | + tag_ns_prefix = factory.make_name("tag-ns-prefix") |
395 | + tag_ns_uri = factory.make_name("tag-ns-uri") |
396 | + |
397 | + consumer_key = factory.make_name("ckey") |
398 | + resource_token = factory.make_name("rtok") |
399 | + resource_secret = factory.make_name("rsec") |
400 | + credentials = convert_tuple_to_string( |
401 | + (consumer_key, resource_token, resource_secret)) |
402 | + |
403 | + yield call_responder( |
404 | + Cluster(), cluster.EvaluateTag, { |
405 | + "tag_name": tag_name, |
406 | + "tag_definition": tag_definition, |
407 | + "tag_nsmap": [ |
408 | + {"prefix": tag_ns_prefix, "uri": tag_ns_uri}, |
409 | + ], |
410 | + "credentials": credentials, |
411 | + }) |
412 | + |
413 | + self.assertThat(evaluate_tag, MockCalledOnceWith( |
414 | + tag_name, tag_definition, {tag_ns_prefix: tag_ns_uri}, |
415 | + (consumer_key, resource_token, resource_secret), |
416 | + )) |
417 | + |
418 | + |
419 | class TestClusterProtocol_AddVirsh(MAASTestCase): |
420 | |
421 | def test__is_registered(self): |
422 | |
423 | === added file 'src/provisioningserver/rpc/tests/test_tags.py' |
424 | --- src/provisioningserver/rpc/tests/test_tags.py 1970-01-01 00:00:00 +0000 |
425 | +++ src/provisioningserver/rpc/tests/test_tags.py 2014-09-23 09:14:01 +0000 |
426 | @@ -0,0 +1,73 @@ |
427 | +# Copyright 2014 Canonical Ltd. This software is licensed under the |
428 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
429 | + |
430 | +"""Tests for :py:module:`~provisioningserver.rpc.dhcp`.""" |
431 | + |
432 | +from __future__ import ( |
433 | + absolute_import, |
434 | + print_function, |
435 | + unicode_literals, |
436 | + ) |
437 | + |
438 | +str = None |
439 | + |
440 | +__metaclass__ = type |
441 | +__all__ = [] |
442 | + |
443 | +from apiclient.maas_client import ( |
444 | + MAASClient, |
445 | + MAASDispatcher, |
446 | + MAASOAuth, |
447 | + ) |
448 | +from maastesting.factory import factory |
449 | +from maastesting.matchers import MockCalledOnceWith |
450 | +from maastesting.testcase import MAASTestCase |
451 | +from mock import ( |
452 | + ANY, |
453 | + sentinel, |
454 | + ) |
455 | +from provisioningserver.rpc import tags |
456 | + |
457 | + |
458 | +class TestEvaluateTag(MAASTestCase): |
459 | + |
460 | + def setUp(self): |
461 | + super(TestEvaluateTag, self).setUp() |
462 | + get_maas_url = self.patch_autospec(tags, "get_maas_url") |
463 | + get_maas_url.return_value = sentinel.maas_url |
464 | + get_cluster_uuid = self.patch_autospec(tags, "get_cluster_uuid") |
465 | + get_cluster_uuid.return_value = sentinel.cluster_uuid |
466 | + |
467 | + def test__calls_process_node_tags(self): |
468 | + credentials = "aaa", "bbb", "ccc" |
469 | + process_node_tags = self.patch_autospec(tags, "process_node_tags") |
470 | + tags.evaluate_tag( |
471 | + sentinel.tag_name, sentinel.tag_definition, sentinel.tag_nsmap, |
472 | + credentials) |
473 | + self.assertThat( |
474 | + process_node_tags, MockCalledOnceWith( |
475 | + tag_name=sentinel.tag_name, |
476 | + tag_definition=sentinel.tag_definition, |
477 | + tag_nsmap=sentinel.tag_nsmap, client=ANY, |
478 | + nodegroup_uuid=sentinel.cluster_uuid)) |
479 | + |
480 | + def test__constructs_client_with_credentials(self): |
481 | + consumer_key = factory.make_name("ckey") |
482 | + resource_token = factory.make_name("rtok") |
483 | + resource_secret = factory.make_name("rsec") |
484 | + credentials = consumer_key, resource_token, resource_secret |
485 | + |
486 | + self.patch_autospec(tags, "process_node_tags") |
487 | + self.patch_autospec(tags, "MAASOAuth").side_effect = MAASOAuth |
488 | + |
489 | + tags.evaluate_tag( |
490 | + sentinel.tag_name, sentinel.tag_definition, sentinel.tag_nsmap, |
491 | + credentials) |
492 | + |
493 | + client = tags.process_node_tags.call_args[1]["client"] |
494 | + self.assertIsInstance(client, MAASClient) |
495 | + self.assertEqual(sentinel.maas_url, client.url) |
496 | + self.assertIsInstance(client.dispatcher, MAASDispatcher) |
497 | + self.assertIsInstance(client.auth, MAASOAuth) |
498 | + self.assertThat(tags.MAASOAuth, MockCalledOnceWith( |
499 | + consumer_key, resource_token, resource_secret)) |
500 | |
501 | === modified file 'src/provisioningserver/tags.py' |
502 | --- src/provisioningserver/tags.py 2014-08-13 21:49:35 +0000 |
503 | +++ src/provisioningserver/tags.py 2014-09-23 09:14:01 +0000 |
504 | @@ -17,7 +17,6 @@ |
505 | __all__ = [ |
506 | 'merge_details', |
507 | 'merge_details_cleanly', |
508 | - 'MissingCredentials', |
509 | 'process_node_tags', |
510 | ] |
511 | |
512 | @@ -27,18 +26,8 @@ |
513 | import httplib |
514 | import urllib2 |
515 | |
516 | -from apiclient.maas_client import ( |
517 | - MAASClient, |
518 | - MAASDispatcher, |
519 | - MAASOAuth, |
520 | - ) |
521 | import bson |
522 | from lxml import etree |
523 | -from provisioningserver.auth import ( |
524 | - get_recorded_api_credentials, |
525 | - get_recorded_nodegroup_uuid, |
526 | - ) |
527 | -from provisioningserver.cluster_config import get_maas_url |
528 | from provisioningserver.logger import get_maas_logger |
529 | from provisioningserver.utils import classify |
530 | from provisioningserver.utils.xpath import try_match_xpath |
531 | @@ -48,10 +37,6 @@ |
532 | maaslog = get_maas_logger("tag_processing") |
533 | |
534 | |
535 | -class MissingCredentials(Exception): |
536 | - """The MAAS URL or credentials are not yet set.""" |
537 | - |
538 | - |
539 | # An example laptop's lshw XML dump was 135kB. An example lab's LLDP |
540 | # XML dump was 1.6kB. A batch size of 100 would mean downloading ~14MB |
541 | # from the region controller, which seems workable. The previous batch |
542 | @@ -60,25 +45,6 @@ |
543 | DEFAULT_BATCH_SIZE = 100 |
544 | |
545 | |
546 | -def get_cached_knowledge(): |
547 | - """Get all the information that we need to know, or raise an error. |
548 | - |
549 | - :return: (client, nodegroup_uuid) |
550 | - """ |
551 | - api_credentials = get_recorded_api_credentials() |
552 | - if api_credentials is None: |
553 | - maaslog.error("Not updating tags: don't have API key yet.") |
554 | - return None, None |
555 | - nodegroup_uuid = get_recorded_nodegroup_uuid() |
556 | - if nodegroup_uuid is None: |
557 | - maaslog.error("Not updating tags: don't have UUID yet.") |
558 | - return None, None |
559 | - client = MAASClient( |
560 | - MAASOAuth(*api_credentials), MAASDispatcher(), |
561 | - get_maas_url()) |
562 | - return client, nodegroup_uuid |
563 | - |
564 | - |
565 | # A content-type: function mapping that can decode data of that type. |
566 | decoders = { |
567 | "application/json": lambda data: json.loads(data), |
568 | @@ -346,20 +312,18 @@ |
569 | nodes_matched, nodes_unmatched) |
570 | |
571 | |
572 | -def process_node_tags(tag_name, tag_definition, tag_nsmap, batch_size=None): |
573 | +def process_node_tags( |
574 | + tag_name, tag_definition, tag_nsmap, |
575 | + client, nodegroup_uuid, batch_size=None): |
576 | """Update the nodes for a new/changed tag definition. |
577 | |
578 | + :param client: A `MAASClient` used to fetch the node's details via |
579 | + calls to the web API. |
580 | + :param nodegroup_uuid: The UUID for this cluster. |
581 | :param tag_name: Name of the tag to update nodes for |
582 | :param tag_definition: Tag definition |
583 | :param batch_size: Size of batch |
584 | """ |
585 | - client, nodegroup_uuid = get_cached_knowledge() |
586 | - if not all([client, nodegroup_uuid]): |
587 | - maaslog.error( |
588 | - "Unable to update tag: %s for definition %r. " |
589 | - "Please refresh secrets, then rebuild this tag." |
590 | - % (tag_name, tag_definition)) |
591 | - raise MissingCredentials() |
592 | # We evaluate this early, so we can fail before sending a bunch of data to |
593 | # the server |
594 | xpath = etree.XPath(tag_definition, namespaces=tag_nsmap) |
595 | |
596 | === modified file 'src/provisioningserver/tasks.py' |
597 | --- src/provisioningserver/tasks.py 2014-09-17 08:51:31 +0000 |
598 | +++ src/provisioningserver/tasks.py 2014-09-23 09:14:01 +0000 |
599 | @@ -28,10 +28,7 @@ |
600 | |
601 | from celery.app import app_or_default |
602 | from celery.task import task |
603 | -from provisioningserver import ( |
604 | - boot_images, |
605 | - tags, |
606 | - ) |
607 | +from provisioningserver import boot_images |
608 | from provisioningserver.auth import ( |
609 | record_api_credentials, |
610 | record_nodegroup_uuid, |
611 | @@ -242,35 +239,3 @@ |
612 | def report_boot_images(): |
613 | """For master worker only: report available netboot images.""" |
614 | boot_images.report_to_server() |
615 | - |
616 | - |
617 | -# How many times should a update node tags task be retried? |
618 | -UPDATE_NODE_TAGS_MAX_RETRY = 10 |
619 | - |
620 | -# How long to wait between update node tags task retries (in seconds)? |
621 | -UPDATE_NODE_TAGS_RETRY_DELAY = 2 |
622 | - |
623 | - |
624 | -# ===================================================================== |
625 | -# Tags-related tasks |
626 | -# ===================================================================== |
627 | - |
628 | - |
629 | -@task(max_retries=UPDATE_NODE_TAGS_MAX_RETRY) |
630 | -@log_call() |
631 | -@log_exception_text |
632 | -def update_node_tags(tag_name, tag_definition, tag_nsmap, retry=True): |
633 | - """Update the nodes for a new/changed tag definition. |
634 | - |
635 | - :param tag_name: Name of the tag to update nodes for |
636 | - :param tag_definition: Tag definition |
637 | - :param retry: Whether to retry on failure |
638 | - """ |
639 | - try: |
640 | - tags.process_node_tags(tag_name, tag_definition, tag_nsmap) |
641 | - except tags.MissingCredentials, exc: |
642 | - if retry: |
643 | - return update_node_tags.retry( |
644 | - exc=exc, countdown=UPDATE_NODE_TAGS_RETRY_DELAY) |
645 | - else: |
646 | - raise |
647 | |
648 | === modified file 'src/provisioningserver/tests/test_tags.py' |
649 | --- src/provisioningserver/tests/test_tags.py 2014-09-11 09:19:44 +0000 |
650 | +++ src/provisioningserver/tests/test_tags.py 2014-09-23 09:14:01 +0000 |
651 | @@ -37,7 +37,6 @@ |
652 | sentinel, |
653 | ) |
654 | from provisioningserver import tags |
655 | -from provisioningserver.auth import get_recorded_nodegroup_uuid |
656 | from provisioningserver.testing.testcase import PservTestCase |
657 | from testtools.matchers import ( |
658 | DocTestMatches, |
659 | @@ -484,29 +483,6 @@ |
660 | super(TestTagUpdating, self).setUp() |
661 | self.useFixture(FakeLogger()) |
662 | |
663 | - def test_get_cached_knowledge_knows_nothing(self): |
664 | - # If we haven't given it any secrets, we should get back nothing |
665 | - self.assertEqual((None, None), tags.get_cached_knowledge()) |
666 | - |
667 | - def test_get_cached_knowledge_with_only_url(self): |
668 | - self.set_maas_url() |
669 | - self.assertEqual((None, None), tags.get_cached_knowledge()) |
670 | - |
671 | - def test_get_cached_knowledge_with_only_url_creds(self): |
672 | - self.set_maas_url() |
673 | - self.set_api_credentials() |
674 | - self.assertEqual((None, None), tags.get_cached_knowledge()) |
675 | - |
676 | - def test_get_cached_knowledge_with_all_info(self): |
677 | - self.set_maas_url() |
678 | - self.set_api_credentials() |
679 | - self.set_node_group_uuid() |
680 | - client, uuid = tags.get_cached_knowledge() |
681 | - self.assertIsNot(None, client) |
682 | - self.assertIsInstance(client, MAASClient) |
683 | - self.assertIsNot(None, uuid) |
684 | - self.assertEqual(get_recorded_nodegroup_uuid(), uuid) |
685 | - |
686 | def fake_client(self): |
687 | return MAASClient(None, None, self.make_maas_url()) |
688 | |
689 | @@ -615,16 +591,6 @@ |
690 | (['a', 'c'], ['b']), |
691 | tags.classify(xpath, node_details)) |
692 | |
693 | - def test_process_node_tags_no_secrets(self): |
694 | - self.patch(MAASClient, 'get') |
695 | - self.patch(MAASClient, 'post') |
696 | - tag_name = factory.make_name('tag') |
697 | - self.assertRaises( |
698 | - tags.MissingCredentials, |
699 | - tags.process_node_tags, tag_name, '//node', None) |
700 | - self.assertFalse(MAASClient.get.called) |
701 | - self.assertFalse(MAASClient.post.called) |
702 | - |
703 | def test_process_node_tags_integration(self): |
704 | self.set_secrets() |
705 | get_nodes = FakeMethod( |
706 | @@ -653,10 +619,12 @@ |
707 | self.patch(MAASClient, 'get', get_fake) |
708 | self.patch(MAASClient, 'post', post_fake) |
709 | tag_name = factory.make_name('tag') |
710 | - nodegroup_uuid = get_recorded_nodegroup_uuid() |
711 | + nodegroup_uuid = factory.make_name("nodegroup-uuid") |
712 | tag_definition = '//lshw:node' |
713 | tag_nsmap = {"lshw": "lshw"} |
714 | - tags.process_node_tags(tag_name, tag_definition, tag_nsmap=tag_nsmap) |
715 | + tags.process_node_tags( |
716 | + tag_name, tag_definition, tag_nsmap, |
717 | + self.fake_client(), nodegroup_uuid) |
718 | nodegroup_url = '/api/1.0/nodegroups/%s/' % (nodegroup_uuid,) |
719 | tag_url = '/api/1.0/tags/%s/' % (tag_name,) |
720 | self.assertEqual( |
721 | @@ -687,9 +655,6 @@ |
722 | client = object() |
723 | uuid = factory.make_name('nodegroupuuid') |
724 | self.patch( |
725 | - tags, 'get_cached_knowledge', |
726 | - MagicMock(return_value=(client, uuid))) |
727 | - self.patch( |
728 | tags, 'get_nodes_for_node_group', |
729 | MagicMock(return_value=['a', 'b', 'c'])) |
730 | fake_first = FakeMethod(result={ |
731 | @@ -706,8 +671,8 @@ |
732 | tag_name = factory.make_name('tag') |
733 | tag_definition = '//node' |
734 | tags.process_node_tags( |
735 | - tag_name, tag_definition, tag_nsmap=None, batch_size=2) |
736 | - tags.get_cached_knowledge.assert_called_once_with() |
737 | + tag_name, tag_definition, tag_nsmap=None, client=client, |
738 | + nodegroup_uuid=uuid, batch_size=2) |
739 | tags.get_nodes_for_node_group.assert_called_once_with(client, uuid) |
740 | self.assertEqual([((client, uuid, ['a', 'c']), {})], fake_first.calls) |
741 | self.assertEqual([((client, uuid, ['b']), {})], fake_second.calls) |
742 | |
743 | === modified file 'src/provisioningserver/tests/test_tasks.py' |
744 | --- src/provisioningserver/tests/test_tasks.py 2014-09-15 14:28:28 +0000 |
745 | +++ src/provisioningserver/tests/test_tasks.py 2014-09-23 09:14:01 +0000 |
746 | @@ -37,7 +37,6 @@ |
747 | auth, |
748 | boot_images, |
749 | cache, |
750 | - tags, |
751 | tasks, |
752 | ) |
753 | from provisioningserver.boot import tftppath |
754 | @@ -52,15 +51,12 @@ |
755 | DNSForwardZoneConfig, |
756 | DNSReverseZoneConfig, |
757 | ) |
758 | -from provisioningserver.tags import MissingCredentials |
759 | from provisioningserver.tasks import ( |
760 | refresh_secrets, |
761 | report_boot_images, |
762 | rndc_command, |
763 | RNDC_COMMAND_MAX_RETRY, |
764 | setup_rndc_configuration, |
765 | - update_node_tags, |
766 | - UPDATE_NODE_TAGS_MAX_RETRY, |
767 | write_dns_config, |
768 | write_dns_zone_config, |
769 | write_full_dns_config, |
770 | @@ -348,44 +344,3 @@ |
771 | |
772 | args, kwargs = MAASClient.post.call_args |
773 | self.assertItemsEqual([image], json.loads(kwargs['images'])) |
774 | - |
775 | - |
776 | -class TestTagTasks(PservTestCase): |
777 | - |
778 | - def setUp(self): |
779 | - super(TestTagTasks, self).setUp() |
780 | - self.celery = self.useFixture(CeleryFixture()) |
781 | - |
782 | - def test_update_node_tags_can_be_retried(self): |
783 | - self.set_secrets() |
784 | - # The update_node_tags task can be retried. |
785 | - # Simulate a temporary failure. |
786 | - number_of_failures = UPDATE_NODE_TAGS_MAX_RETRY |
787 | - raised_exception = MissingCredentials( |
788 | - factory.make_name('exception'), random.randint(100, 200)) |
789 | - simulate_failures = MultiFakeMethod( |
790 | - [FakeMethod(failure=raised_exception)] * number_of_failures + |
791 | - [FakeMethod()]) |
792 | - self.patch(tags, 'process_node_tags', simulate_failures) |
793 | - tag = factory.make_string() |
794 | - result = update_node_tags.delay( |
795 | - tag, '//node', tag_nsmap=None, retry=True) |
796 | - assertTaskRetried( |
797 | - self, result, UPDATE_NODE_TAGS_MAX_RETRY + 1, |
798 | - 'provisioningserver.tasks.update_node_tags') |
799 | - |
800 | - def test_update_node_tags_is_retried_a_limited_number_of_times(self): |
801 | - self.set_secrets() |
802 | - # If we simulate UPDATE_NODE_TAGS_MAX_RETRY + 1 failures, the |
803 | - # task fails. |
804 | - number_of_failures = UPDATE_NODE_TAGS_MAX_RETRY + 1 |
805 | - raised_exception = MissingCredentials( |
806 | - factory.make_name('exception'), random.randint(100, 200)) |
807 | - simulate_failures = MultiFakeMethod( |
808 | - [FakeMethod(failure=raised_exception)] * number_of_failures + |
809 | - [FakeMethod()]) |
810 | - self.patch(tags, 'process_node_tags', simulate_failures) |
811 | - tag = factory.make_string() |
812 | - self.assertRaises( |
813 | - MissingCredentials, update_node_tags.delay, tag, |
814 | - '//node', tag_nsmap=None, retry=True) |
Looking good so far. One minor nitpick, but otherwise it's cool beans.