Merge ~ltrager/maas:2.3_1722607_1 into maas:2.3

Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: 5064e863ebca6c8165311f32dc6f370e6913a48a
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:2.3_1722607_1
Merge into: maas:2.3
Diff against target: 512 lines (+104/-236)
5 files modified
src/maasserver/websockets/handlers/controller.py (+3/-1)
src/maasserver/websockets/handlers/machine.py (+3/-1)
src/maasserver/websockets/handlers/node.py (+53/-93)
src/maasserver/websockets/handlers/switch.py (+4/-1)
src/maasserver/websockets/handlers/tests/test_machine.py (+41/-140)
Reviewer Review Type Date Requested Status
Blake Rouse (community) Approve
Alberto Donato (community) Approve
Review via email: mp+368109@code.launchpad.net

Commit message

LP: #1722607 - 1/2 Stop sending logs with node object over the websocket.

Removes the summary YAML, XML, and installation log from being sent with the
node object over the websocket. This removes 4 database lookups and with a
libvirt machine 79KB of data from being sent with the node object.

Two new functions have been added to the node handler and child classes,
get_summary_yaml and get_summary_xml. Both read summary data from the cache
and return the formatted data.

Backport of 54ae961 for LP: #1830365

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

+1

review: Approve
Revision history for this message
Blake Rouse (blake-rouse) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/websockets/handlers/controller.py b/src/maasserver/websockets/handlers/controller.py
2index 44b79a9..84ccdd8 100644
3--- a/src/maasserver/websockets/handlers/controller.py
4+++ b/src/maasserver/websockets/handlers/controller.py
5@@ -1,4 +1,4 @@
6-# Copyright 2016 Canonical Ltd. This software is licensed under the
7+# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
8 # GNU Affero General Public License version 3 (see the file LICENSE).
9
10 """The controller handler for the WebSocket connection."""
11@@ -43,6 +43,8 @@ class ControllerHandler(MachineHandler):
12 'delete_interface',
13 'link_subnet',
14 'unlink_subnet',
15+ 'get_summary_xml',
16+ 'get_summary_yaml',
17 ]
18 form = ControllerForm
19 exclude = [
20diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
21index b3126d3..4f16b5f 100644
22--- a/src/maasserver/websockets/handlers/machine.py
23+++ b/src/maasserver/websockets/handlers/machine.py
24@@ -130,7 +130,9 @@ class MachineHandler(NodeHandler):
25 'create_volume_group',
26 'create_logical_volume',
27 'set_boot_disk',
28- 'default_user'
29+ 'default_user',
30+ 'get_summary_xml',
31+ 'get_summary_yaml',
32 ]
33 form = AdminMachineWithMACAddressesForm
34 exclude = [
35diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
36index 112d602..a2bc88d 100644
37--- a/src/maasserver/websockets/handlers/node.py
38+++ b/src/maasserver/websockets/handlers/node.py
39@@ -25,7 +25,7 @@ from maasserver.models.cacheset import CacheSet
40 from maasserver.models.config import Config
41 from maasserver.models.event import Event
42 from maasserver.models.filesystemgroup import VolumeGroup
43-from maasserver.models.nodeprobeddetails import get_single_probed_details
44+from maasserver.models.nodeprobeddetails import script_output_nsmap
45 from maasserver.models.physicalblockdevice import PhysicalBlockDevice
46 from maasserver.models.tag import Tag
47 from maasserver.models.virtualblockdevice import VirtualBlockDevice
48@@ -219,6 +219,7 @@ class NodeHandler(TimestampedModelHandler):
49 if obj.node_type != NODE_TYPE.DEVICE:
50 commissioning_script_results = []
51 testing_script_results = []
52+ log_results = set()
53 for hw_type in self._script_results.get(obj.id, {}).values():
54 for script_result in hw_type:
55 if (script_result.script_set.result_type ==
56@@ -232,6 +233,9 @@ class NodeHandler(TimestampedModelHandler):
57 elif (script_result.script_set.result_type ==
58 RESULT_TYPE.COMMISSIONING):
59 commissioning_script_results.append(script_result)
60+ if (script_result.name in script_output_nsmap and
61+ script_result.status == SCRIPT_STATUS.PASSED):
62+ log_results.add(script_result.name)
63 elif (script_result.script_set.result_type ==
64 RESULT_TYPE.TESTING):
65 testing_script_results.append(script_result)
66@@ -248,6 +252,8 @@ class NodeHandler(TimestampedModelHandler):
67 data["testing_status"] = get_status_from_qs(testing_script_results)
68 data["testing_status_tooltip"] = (
69 self.dehydrate_hardware_status_tooltip(testing_script_results))
70+ data["has_logs"] = (
71+ log_results.difference(script_output_nsmap.keys()) == set())
72
73 if not for_list:
74 data["on_network"] = obj.on_network()
75@@ -292,11 +298,8 @@ class NodeHandler(TimestampedModelHandler):
76 data["events"] = self.dehydrate_events(obj)
77
78 # Machine logs
79- data = self.dehydrate_summary_output(obj, data)
80 data["installation_status"] = self.dehydrate_script_set_status(
81 obj.current_installation_script_set)
82- data["installation_results"] = self.dehydrate_script_set(
83- obj.current_installation_script_set)
84
85 # Third party drivers
86 if Config.objects.get_config('enable_third_party_drivers'):
87@@ -552,95 +555,6 @@ class NodeHandler(TimestampedModelHandler):
88
89 return data
90
91- def dehydrate_summary_output(self, obj, data):
92- """Dehydrate the machine summary output."""
93- # Produce a "clean" composite details document.
94- probed_details = merge_details_cleanly(
95- get_single_probed_details(obj))
96-
97- # We check here if there's something to show instead of after
98- # the call to get_single_probed_details() because here the
99- # details will be guaranteed well-formed.
100- if len(probed_details.xpath('/*/*')) == 0:
101- data['summary_xml'] = None
102- data['summary_yaml'] = None
103- else:
104- data['summary_xml'] = etree.tostring(
105- probed_details, encoding=str, pretty_print=True)
106- data['summary_yaml'] = XMLToYAML(
107- etree.tostring(
108- probed_details, encoding=str,
109- pretty_print=True)).convert()
110- return data
111-
112- def dehydrate_script_set(self, script_set):
113- """Dehydrate ScriptResults."""
114- if script_set is None:
115- return []
116- ret = []
117- for script_result in script_set:
118- # MAAS stores stdout, stderr, and the combined output. The
119- # metadata API determine which field uploaded data should go
120- # into based on the extention of the uploaded file. .out goes
121- # to stdout, .err goes to stderr, otherwise its assumed the
122- # data is combined. Curtin uploads the installation log as
123- # install.log so its stored as a combined result. This ensures
124- # a result is always returned. Always return the combined result
125- # for testing.
126- if (script_result.stdout == b'' or
127- script_set.result_type == RESULT_TYPE.TESTING):
128- output = script_result.output
129- else:
130- output = script_result.stdout
131-
132- if script_result.script is not None:
133- tags = ', '.join(script_result.script.tags)
134- title = script_result.script.title
135- else:
136- tags = ''
137- title = ''
138- if title == '':
139- ui_name = script_result.name
140- else:
141- ui_name = '%s (%s)' % (title, script_result.name)
142- ret.append({
143- 'id': script_result.id,
144- 'name': script_result.name,
145- 'ui_name': ui_name,
146- 'title': title,
147- 'status': script_result.status,
148- 'status_name': script_result.status_name,
149- 'tags': tags,
150- 'output': output,
151- 'updated': dehydrate_datetime(script_result.updated),
152- 'started': dehydrate_datetime(script_result.started),
153- 'ended': dehydrate_datetime(script_result.ended),
154- 'runtime': script_result.runtime,
155- 'starttime': script_result.starttime,
156- 'endtime': script_result.endtime,
157- 'estimated_runtime': script_result.estimated_runtime,
158- })
159- if (script_result.stderr != b'' and
160- script_set.result_type != RESULT_TYPE.TESTING):
161- ret.append({
162- 'id': script_result.id,
163- 'name': '%s.err' % script_result.name,
164- 'ui_name': ui_name,
165- 'title': title,
166- 'status': script_result.status,
167- 'status_name': script_result.status_name,
168- 'tags': tags,
169- 'output': script_result.stderr,
170- 'updated': dehydrate_datetime(script_result.updated),
171- 'started': dehydrate_datetime(script_result.started),
172- 'ended': dehydrate_datetime(script_result.ended),
173- 'runtime': script_result.runtime,
174- 'starttime': script_result.starttime,
175- 'endtime': script_result.endtime,
176- 'estimated_runtime': script_result.estimated_runtime,
177- })
178- return sorted(ret, key=lambda i: i['name'])
179-
180 def dehydrate_script_set_status(self, obj):
181 """Dehydrate the script set status."""
182 if obj is None:
183@@ -786,3 +700,49 @@ class NodeHandler(TimestampedModelHandler):
184 "disk_type": disk_type
185 })
186 return grouped_storages
187+
188+ def get_summary_xml(self, params):
189+ """Return the node summary XML formatted."""
190+ node = self.get_object(params)
191+ # Produce a "clean" composite details document.
192+ details_template = dict.fromkeys(script_output_nsmap.values())
193+ for hw_type in self._script_results.get(node.id, {}).values():
194+ for script_result in hw_type:
195+ if (script_result.name in script_output_nsmap and
196+ script_result.status == SCRIPT_STATUS.PASSED):
197+ namespace = script_output_nsmap[script_result.name]
198+ details_template[namespace] = script_result.stdout
199+ probed_details = merge_details_cleanly(details_template)
200+
201+ # We check here if there's something to show instead of after
202+ # the call to get_single_probed_details() because here the
203+ # details will be guaranteed well-formed.
204+ if len(probed_details.xpath('/*/*')) == 0:
205+ return ''
206+ else:
207+ return etree.tostring(
208+ probed_details, encoding=str, pretty_print=True)
209+
210+ def get_summary_yaml(self, params):
211+ """Return the node summary YAML formatted."""
212+ node = self.get_object(params)
213+ # Produce a "clean" composite details document.
214+ details_template = dict.fromkeys(script_output_nsmap.values())
215+ for hw_type in self._script_results.get(node.id, {}).values():
216+ for script_result in hw_type:
217+ if (script_result.name in script_output_nsmap and
218+ script_result.status == SCRIPT_STATUS.PASSED):
219+ namespace = script_output_nsmap[script_result.name]
220+ details_template[namespace] = script_result.stdout
221+ probed_details = merge_details_cleanly(details_template)
222+
223+ # We check here if there's something to show instead of after
224+ # the call to get_single_probed_details() because here the
225+ # details will be guaranteed well-formed.
226+ if len(probed_details.xpath('/*/*')) == 0:
227+ return ''
228+ else:
229+ return XMLToYAML(
230+ etree.tostring(
231+ probed_details, encoding=str,
232+ pretty_print=True)).convert()
233diff --git a/src/maasserver/websockets/handlers/switch.py b/src/maasserver/websockets/handlers/switch.py
234index ceae6e1..e40f779 100644
235--- a/src/maasserver/websockets/handlers/switch.py
236+++ b/src/maasserver/websockets/handlers/switch.py
237@@ -32,7 +32,10 @@ class SwitchHandler(NodeHandler):
238 'list',
239 'get',
240 'update',
241- 'action']
242+ 'action',
243+ 'get_summary_xml',
244+ 'get_summary_yaml',
245+ ]
246 exclude = MachineHandler.Meta.exclude
247 list_fields = MachineHandler.Meta.list_fields
248 listen_channels = [
249diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
250index e0b91a8..299a5e8 100644
251--- a/src/maasserver/websockets/handlers/tests/test_machine.py
252+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
253@@ -50,7 +50,10 @@ from maasserver.models.node import (
254 Machine,
255 Node,
256 )
257-from maasserver.models.nodeprobeddetails import get_single_probed_details
258+from maasserver.models.nodeprobeddetails import (
259+ get_single_probed_details,
260+ script_output_nsmap,
261+)
262 from maasserver.models.partition import (
263 Partition,
264 PARTITION_ALIGNMENT_SIZE,
265@@ -140,8 +143,7 @@ class TestMachineHandler(MAASServerTestCase):
266 ]
267 return get_status_from_qs(blockdevice_script_results)
268
269- def dehydrate_node(
270- self, node, handler, for_list=False, include_summary=False):
271+ def dehydrate_node(self, node, handler, for_list=False):
272 # Prime handler._script_results
273 handler._script_results = {}
274 handler._refresh_script_result_cache(node.get_latest_script_results)
275@@ -172,6 +174,11 @@ class TestMachineHandler(MAASServerTestCase):
276 testing_scripts = node.get_latest_testing_script_results
277 testing_scripts = testing_scripts.exclude(
278 status=SCRIPT_STATUS.ABORTED)
279+ log_results = set()
280+ for script_result in commissioning_scripts:
281+ if (script_result.name in script_output_nsmap and
282+ script_result.status == SCRIPT_STATUS.PASSED):
283+ log_results.add(script_result.name)
284 data = {
285 "actions": list(compile_node_actions(node, handler.user).keys()),
286 "architecture": node.architecture,
287@@ -191,13 +198,13 @@ class TestMachineHandler(MAASServerTestCase):
288 "testing_status_tooltip": (
289 handler.dehydrate_hardware_status_tooltip(testing_scripts)),
290 "current_testing_script_set": node.current_testing_script_set_id,
291- "installation_results": handler.dehydrate_script_set(
292- node.current_installation_script_set),
293 "current_installation_script_set": (
294 node.current_installation_script_set_id),
295 "installation_status": (
296 handler.dehydrate_script_set_status(
297 node.current_installation_script_set)),
298+ "has_logs": (
299+ log_results.difference(script_output_nsmap.keys()) == set()),
300 "cpu_count": node.cpu_count,
301 "cpu_speed": node.cpu_speed,
302 "created": dehydrate_datetime(node.created),
303@@ -318,6 +325,7 @@ class TestMachineHandler(MAASServerTestCase):
304 "testing_script_count",
305 "testing_status",
306 "testing_status_tooltip",
307+ "has_logs",
308 ]
309 for key in list(data):
310 if key not in allowed_fields:
311@@ -386,8 +394,6 @@ class TestMachineHandler(MAASServerTestCase):
312
313 data["grouped_storages"] = handler.get_grouped_storages(blockdevices)
314
315- if include_summary:
316- data = handler.dehydrate_summary_output(node, data)
317 return data
318
319 def make_nodes(self, number):
320@@ -1259,17 +1265,14 @@ class TestMachineHandler(MAASServerTestCase):
321 [{"subnet_id": bond_subnet.id, "ip_address": bond_ip}],
322 dehydrated_interface["discovered"])
323
324- def test_dehydrate_summary_output_returns_None(self):
325+ def test_get_summary_xml_returns_empty_string(self):
326 owner = factory.make_User()
327 node = factory.make_Node(owner=owner)
328 handler = MachineHandler(owner, {})
329- observed = handler.dehydrate_summary_output(node, {})
330- self.assertEqual({
331- "summary_xml": None,
332- "summary_yaml": None,
333- }, observed)
334+ observed = handler.get_summary_xml({'system_id': node.system_id})
335+ self.assertEquals('', observed)
336
337- def test_dehydrate_summary_output_returns_data(self):
338+ def test_dehydrate_summary_xml_returns_data(self):
339 owner = factory.make_User()
340 node = factory.make_Node(owner=owner, with_empty_script_sets=True)
341 handler = MachineHandler(owner, {})
342@@ -1278,138 +1281,36 @@ class TestMachineHandler(MAASServerTestCase):
343 script_result = script_set.find_script_result(
344 script_name=LLDP_OUTPUT_NAME)
345 script_result.store_result(exit_status=0, stdout=lldp_data)
346- observed = handler.dehydrate_summary_output(node, {})
347+ observed = handler.get_summary_xml({'system_id': node.system_id})
348 probed_details = merge_details_cleanly(
349 get_single_probed_details(node))
350- self.assertEqual({
351- "summary_xml": etree.tostring(
352- probed_details, encoding=str, pretty_print=True),
353- "summary_yaml": XMLToYAML(
354- etree.tostring(
355- probed_details, encoding=str,
356- pretty_print=True)).convert(),
357- }, observed)
358-
359- def test_dehydrate_script_set(self):
360- owner = factory.make_User()
361- handler = MachineHandler(owner, {})
362- script_set = factory.make_ScriptSet(
363- result_type=RESULT_TYPE.COMMISSIONING)
364- script_result = factory.make_ScriptResult(
365- status=SCRIPT_STATUS.PASSED, script_set=script_set)
366- self.assertEqual([
367- {
368- 'id': script_result.id,
369- 'name': script_result.name,
370- 'ui_name': '%s (%s)' % (
371- script_result.script.title, script_result.name),
372- 'title': script_result.script.title,
373- 'status': script_result.status,
374- 'status_name': script_result.status_name,
375- 'tags': ', '.join(script_result.script.tags),
376- 'output': script_result.stdout,
377- 'updated': dehydrate_datetime(script_result.updated),
378- 'started': dehydrate_datetime(script_result.started),
379- 'ended': dehydrate_datetime(script_result.ended),
380- 'runtime': script_result.runtime,
381- 'starttime': script_result.starttime,
382- 'endtime': script_result.endtime,
383- 'estimated_runtime': script_result.estimated_runtime,
384- },
385- {
386- 'id': script_result.id,
387- 'name': '%s.err' % script_result.name,
388- 'ui_name': '%s (%s)' % (
389- script_result.script.title, script_result.name),
390- 'title': script_result.script.title,
391- 'status': script_result.status,
392- 'status_name': script_result.status_name,
393- 'tags': ', '.join(script_result.script.tags),
394- 'output': script_result.stderr,
395- 'updated': dehydrate_datetime(script_result.updated),
396- 'started': dehydrate_datetime(script_result.started),
397- 'ended': dehydrate_datetime(script_result.ended),
398- 'runtime': script_result.runtime,
399- 'starttime': script_result.starttime,
400- 'endtime': script_result.endtime,
401- 'estimated_runtime': script_result.estimated_runtime,
402- }], handler.dehydrate_script_set(script_set))
403-
404- def test_dehydrate_script_set_returns_output_if_stdout_empty(self):
405- owner = factory.make_User()
406- handler = MachineHandler(owner, {})
407- script_set = factory.make_ScriptSet(
408- result_type=RESULT_TYPE.COMMISSIONING)
409- script_result = factory.make_ScriptResult(
410- status=SCRIPT_STATUS.PASSED, stdout=''.encode(),
411- stderr=''.encode(), output=factory.make_string().encode(),
412- script_set=script_set)
413- self.assertDictEqual(
414- {
415- 'id': script_result.id,
416- 'name': script_result.name,
417- 'ui_name': '%s (%s)' % (
418- script_result.script.title, script_result.name),
419- 'title': script_result.script.title,
420- 'status': script_result.status,
421- 'status_name': script_result.status_name,
422- 'tags': ', '.join(script_result.script.tags),
423- 'output': script_result.output,
424- 'updated': dehydrate_datetime(script_result.updated),
425- 'started': dehydrate_datetime(script_result.started),
426- 'ended': dehydrate_datetime(script_result.ended),
427- 'runtime': script_result.runtime,
428- 'starttime': script_result.starttime,
429- 'endtime': script_result.endtime,
430- 'estimated_runtime': script_result.estimated_runtime,
431- }, handler.dehydrate_script_set(script_result.script_set)[0])
432-
433- def test_dehydrate_script_set_returns_combined_for_testing(self):
434- owner = factory.make_User()
435- handler = MachineHandler(owner, {})
436- script_set = factory.make_ScriptSet(result_type=RESULT_TYPE.TESTING)
437- script_result = factory.make_ScriptResult(
438- status=SCRIPT_STATUS.PASSED, script_set=script_set)
439- self.assertDictEqual(
440- {
441- 'id': script_result.id,
442- 'name': script_result.name,
443- 'ui_name': '%s (%s)' % (
444- script_result.script.title, script_result.name),
445- 'title': script_result.script.title,
446- 'status': script_result.status,
447- 'status_name': script_result.status_name,
448- 'tags': ', '.join(script_result.script.tags),
449- 'output': script_result.output,
450- 'updated': dehydrate_datetime(script_result.updated),
451- 'started': dehydrate_datetime(script_result.started),
452- 'ended': dehydrate_datetime(script_result.ended),
453- 'runtime': script_result.runtime,
454- 'starttime': script_result.starttime,
455- 'endtime': script_result.endtime,
456- 'estimated_runtime': script_result.estimated_runtime,
457- }, handler.dehydrate_script_set(script_result.script_set)[0])
458-
459- def test_dehydrate_script_set_status(self):
460+ self.assertEquals(
461+ etree.tostring(probed_details, encoding=str, pretty_print=True),
462+ observed)
463+
464+ def test_get_summary_yaml_returns_empty_string(self):
465 owner = factory.make_User()
466+ node = factory.make_Node(owner=owner)
467 handler = MachineHandler(owner, {})
468- script_result = factory.make_ScriptResult()
469- # See ScriptSet.status
470- if script_result.status == SCRIPT_STATUS.INSTALLING:
471- expected_script_status = SCRIPT_STATUS.RUNNING
472- elif script_result.status in (
473- SCRIPT_STATUS.TIMEDOUT, SCRIPT_STATUS.FAILED_INSTALLING):
474- expected_script_status = SCRIPT_STATUS.FAILED
475- else:
476- expected_script_status = script_result.status
477- self.assertEquals(
478- expected_script_status,
479- handler.dehydrate_script_set_status(script_result.script_set))
480+ observed = handler.get_summary_yaml({'system_id': node.system_id})
481+ self.assertEquals('', observed)
482
483- def test_dehydrate_script_set_status_returns_neg_when_none(self):
484+ def test_dehydrate_summary_yaml_returns_data(self):
485 owner = factory.make_User()
486+ node = factory.make_Node(owner=owner, with_empty_script_sets=True)
487 handler = MachineHandler(owner, {})
488- self.assertEquals(-1, handler.dehydrate_script_set_status(None))
489+ lldp_data = "<foo>bar</foo>".encode("utf-8")
490+ script_set = node.current_commissioning_script_set
491+ script_result = script_set.find_script_result(
492+ script_name=LLDP_OUTPUT_NAME)
493+ script_result.store_result(exit_status=0, stdout=lldp_data)
494+ observed = handler.get_summary_yaml({'system_id': node.system_id})
495+ probed_details = merge_details_cleanly(
496+ get_single_probed_details(node))
497+ self.assertEqual(
498+ XMLToYAML(etree.tostring(
499+ probed_details, encoding=str, pretty_print=True)).convert(),
500+ observed)
501
502 def test_dehydrate_events_only_includes_lastest_50(self):
503 owner = factory.make_User()
504@@ -1575,7 +1476,7 @@ class TestMachineHandler(MAASServerTestCase):
505 node.save()
506
507 observed = handler.get({"system_id": node.system_id})
508- expected = self.dehydrate_node(node, handler, include_summary=True)
509+ expected = self.dehydrate_node(node, handler)
510 self.assertThat(observed, MatchesDict({
511 name: Equals(value) for name, value in expected.items()
512 }))

Subscribers

People subscribed via source and target branches