Merge ~ltrager/maas:lp1879416 into maas:master

Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: d2cf808b6be4575118d3f0811c5fd2f56c4c5ee6
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:lp1879416
Merge into: maas:master
Diff against target: 124 lines (+33/-18)
4 files modified
src/maasserver/models/tests/test_node.py (+17/-15)
src/metadataserver/builtin_scripts/hooks.py (+5/-0)
src/metadataserver/models/scriptresult.py (+7/-1)
src/metadataserver/models/tests/test_scriptresult.py (+4/-2)
Reviewer Review Type Date Requested Status
Björn Tillenius Approve
MAAS Lander Needs Fixing
Review via email: mp+384152@code.launchpad.net

Commit message

LP: #1879416 - Mark commissioning scripts as failed if processing errored

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1879416 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/7541/console
COMMIT: 6eba2772b7fa785f66838de9e1ec57852602156f

review: Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1879416 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/7542/console
COMMIT: ea8fd27d48cad734a62abae3326d72f3fc80a1b7

review: Needs Fixing
Revision history for this message
Björn Tillenius (bjornt) wrote :

+1

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :
~ltrager/maas:lp1879416 updated
9b9848c... by Lee Trager

Merge branch 'master' into lp1879416

d2cf808... by Lee Trager

Fix failing test

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
2index fb1e3a6..d27487f 100644
3--- a/src/maasserver/models/tests/test_node.py
4+++ b/src/maasserver/models/tests/test_node.py
5@@ -10476,11 +10476,10 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
6
7 def test__new_physical_with_multiple_dhcp_link_with_resource_info(self):
8 controller = self.create_empty_controller(with_empty_script_sets=True)
9- mac_address = factory.make_mac_address()
10 interfaces = {
11 "eth0": {
12 "type": "physical",
13- "mac_address": mac_address,
14+ "mac_address": "00:00:00:00:00:01",
15 "parents": [],
16 "links": [{"mode": "dhcp"}, {"mode": "dhcp"}],
17 "enabled": True,
18@@ -10496,19 +10495,22 @@ class TestUpdateInterfaces(MAASServerTestCase, UpdateInterfacesMixin):
19 lxd_script = controller.current_commissioning_script_set.find_script_result(
20 script_name=LXD_OUTPUT_NAME
21 )
22- lxd_script_output = {
23- "network": {
24- "cards": [
25- {
26- "ports": [
27- {"id": "eth0", "address": mac_address, "port": 0}
28- ],
29- "vendor": vendor,
30- "product": product,
31- "firmware_version": firmware_version,
32- }
33- ]
34- }
35+ lxd_script_output = test_hooks.make_lxd_output()
36+ lxd_script_output["resources"]["network"] = {
37+ "cards": [
38+ {
39+ "ports": [
40+ {
41+ "id": "eth0",
42+ "address": "00:00:00:00:00:01",
43+ "port": 0,
44+ }
45+ ],
46+ "vendor": vendor,
47+ "product": product,
48+ "firmware_version": firmware_version,
49+ }
50+ ]
51 }
52 lxd_script.store_result(
53 0, stdout=json.dumps(lxd_script_output).encode("utf-8")
54diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
55index a65d06a..5f9344a 100644
56--- a/src/metadataserver/builtin_scripts/hooks.py
57+++ b/src/metadataserver/builtin_scripts/hooks.py
58@@ -175,6 +175,11 @@ def parse_interfaces_details(node):
59 return interfaces
60
61 details = json.loads(script_result.stdout)
62+ # MAAS 2.8 added additional information to machine-resources output.
63+ # LXD_OUTPUT_NAME used to only contain resources, now it is one of
64+ # the objects provided.
65+ if "resources" in details:
66+ details = details["resources"]
67 return _parse_interfaces(node, details)
68
69
70diff --git a/src/metadataserver/models/scriptresult.py b/src/metadataserver/models/scriptresult.py
71index 28dfb13..ca4a72a 100644
72--- a/src/metadataserver/models/scriptresult.py
73+++ b/src/metadataserver/models/scriptresult.py
74@@ -381,7 +381,7 @@ class ScriptResult(CleanSave, TimestampedModel):
75 # Circular imports.
76 from metadataserver.api import try_or_log_event
77
78- try_or_log_event(
79+ signal_status = try_or_log_event(
80 self.script_set.node,
81 None,
82 err,
83@@ -390,6 +390,12 @@ class ScriptResult(CleanSave, TimestampedModel):
84 output=self.stdout,
85 exit_status=self.exit_status,
86 )
87+ # If the script failed to process mark the script as failed to
88+ # prevent testing from running and help users identify where
89+ # the error came from. This can happen when a commissioning
90+ # script generated invalid output.
91+ if signal_status is not None:
92+ self.status = SCRIPT_STATUS.FAILED
93
94 if (
95 self.status == SCRIPT_STATUS.PASSED
96diff --git a/src/metadataserver/models/tests/test_scriptresult.py b/src/metadataserver/models/tests/test_scriptresult.py
97index 6683a42..e5a78cb 100644
98--- a/src/metadataserver/models/tests/test_scriptresult.py
99+++ b/src/metadataserver/models/tests/test_scriptresult.py
100@@ -367,7 +367,6 @@ class TestScriptResult(MAASServerTestCase):
101 script_result = factory.make_ScriptResult(
102 script_set=script_set, status=SCRIPT_STATUS.RUNNING
103 )
104- exit_status = random.randint(0, 255)
105
106 def _raise():
107 raise Exception()
108@@ -378,12 +377,15 @@ class TestScriptResult(MAASServerTestCase):
109 self.addCleanup(
110 scriptresult_module.NODE_INFO_SCRIPTS.pop, script_result.name
111 )
112- script_result.store_result(exit_status, stdout=b"")
113+ script_result.store_result(0, stdout=b"")
114 expected_event = Event.objects.first()
115 self.assertThat(
116 expected_event.description,
117 DocTestMatches("...failed during post-processing."),
118 )
119+ self.assertEquals(
120+ reload_object(script_result).status, SCRIPT_STATUS.FAILED
121+ )
122
123 def test_store_result_on_recommission_script_resets_builtin_commiss(self):
124 script_set = factory.make_ScriptSet(

Subscribers

People subscribed via source and target branches