Merge lp:~andreserl/maas/lp1604962_lp1604987 into lp:maas/trunk

Proposed by Andres Rodriguez
Status: Merged
Approved by: Andres Rodriguez
Approved revision: 5210
Merged at revision: 5204
Proposed branch: lp:~andreserl/maas/lp1604962_lp1604987
Merge into: lp:maas/trunk
Diff against target: 291 lines (+85/-40)
7 files modified
src/maasserver/models/node.py (+33/-20)
src/maasserver/models/tests/test_node.py (+12/-3)
src/maasserver/rpc/nodes.py (+1/-1)
src/maasserver/status_monitor.py (+1/-1)
src/metadataserver/api.py (+18/-12)
src/metadataserver/tests/test_api_status.py (+5/-3)
src/provisioningserver/events.py (+15/-0)
To merge this branch: bzr merge lp:~andreserl/maas/lp1604962_lp1604987
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Approve
Gavin Panella (community) Approve
Review via email: mp+300978@code.launchpad.net

Commit message

Fixes to correctly log cloud-init/curtin FAIL events in the node event log.

Fixes to correctly log "mark_failed" events. We did this by combining the mark_failed with _mark_failed functions, provided that the latter wasn't really storing anything in the event log and it should. It now differentiates between a user driven "Mark Failed" vs a system driven.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Looks good, with a few minor comments.

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (31.3 KiB)

The attempt to merge lp:~andreserl/maas/lp1604962_lp1604987 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://security.ubuntu.com/ubuntu xenial-security InRelease
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [95.7 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Fetched 95.7 kB in 0s (193 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6.03+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5.0-1build1).
python-netaddr is already the newest version (0.7.18-1).
python-netifaces is already the newes...

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (31.6 KiB)

The attempt to merge lp:~andreserl/maas/lp1604962_lp1604987 into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [95.7 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://security.ubuntu.com/ubuntu xenial-security/universe Sources [8,948 B]
Get:6 http://security.ubuntu.com/ubuntu xenial-security/universe amd64 Packages [36.4 kB]
Get:7 http://security.ubuntu.com/ubuntu xenial-security/universe Translation-en [22.0 kB]
Fetched 257 kB in 0s (529 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is...

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

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Revision history for this message
Andres Rodriguez (andreserl) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/node.py'
2--- src/maasserver/models/node.py 2016-07-21 23:33:35 +0000
3+++ src/maasserver/models/node.py 2016-07-25 22:18:24 +0000
4@@ -1132,20 +1132,29 @@
5
6 def _register_request_event(
7 self, user, type_name, action='', comment=None):
8- """Register a node user request event."""
9- # don't register system generated non-user requests
10+ """Register a node request event.
11+
12+ It registers events like start_commission (started by a user),
13+ or mark_failed (started by the system)"""
14+
15+ # the description will be the comment, if any.
16+ description = comment if comment else ''
17+ # if the user exists, we need to construct the description with
18+ # the user. as it would be a user-driven request.
19 if user is not None:
20- event_details = EVENT_DETAILS[type_name]
21- description = "(%s)" % user.username
22- if comment:
23- description = "%s - %s" % (description, comment)
24- # Avoid circular imports.
25- from maasserver.models.event import Event
26- Event.objects.register_event_and_event_type(
27- self.system_id, type_name, type_level=event_details.level,
28- type_description=event_details.description,
29- event_action=action,
30- event_description=description)
31+ if len(description) == 0:
32+ description = "(%s)" % user
33+ else:
34+ description = "(%s) - %s" % (user, description)
35+ event_details = EVENT_DETAILS[type_name]
36+
37+ # Avoid circular imports.
38+ from maasserver.models.event import Event
39+ Event.objects.register_event_and_event_type(
40+ self.system_id, type_name, type_level=event_details.level,
41+ type_description=event_details.description,
42+ event_action=action,
43+ event_description=description)
44
45 def storage_layout_issues(self):
46 """Return any errors with the storage layout.
47@@ -2408,18 +2417,22 @@
48 arch, subarch = self.architecture.split('/')
49 return (arch, subarch)
50
51- def mark_failed(self, user, comment=None):
52- self._register_request_event(
53- user, EVENT_TYPES.REQUEST_NODE_MARK_FAILED, action='mark_failed',
54- comment=comment)
55- self._mark_failed(user, comment)
56-
57- def _mark_failed(self, user, comment=None, commit=True):
58+ def mark_failed(self, user=None, comment=None, commit=True):
59 """Mark this node as failed.
60
61 The actual 'failed' state depends on the current status of the
62 node.
63 """
64+ if user is None:
65+ # This is a system-driven event. Log level is ERROR.
66+ event_type = EVENT_TYPES.REQUEST_NODE_MARK_FAILED_SYSTEM
67+ else:
68+ # This is a user-driven event. Log level is INFO.
69+ event_type = EVENT_TYPES.REQUEST_NODE_MARK_FAILED
70+ self._register_request_event(
71+ user, event_type, action='mark_failed',
72+ comment=comment)
73+
74 new_status = get_failed_status(self.status)
75 if new_status is not None:
76 self.status = new_status
77
78=== modified file 'src/maasserver/models/tests/test_node.py'
79--- src/maasserver/models/tests/test_node.py 2016-07-18 20:06:21 +0000
80+++ src/maasserver/models/tests/test_node.py 2016-07-25 22:18:24 +0000
81@@ -3148,14 +3148,23 @@
82 event_action=event_action,
83 event_description=event_description))
84
85- def test__register_request_event_with_none_user_saves_no_event(self):
86+ def test__register_request_event_none_user_saves_comment_not_user(self):
87 node = factory.make_Node()
88 log_mock = self.patch_autospec(
89 Event.objects, 'register_event_and_event_type')
90 event_name = EVENT_TYPES.REQUEST_NODE_START
91+ event_action = factory.make_name('action')
92+ event_details = EVENT_DETAILS[event_name]
93 comment = factory.make_name("comment")
94- node._register_request_event(None, event_name, comment)
95- self.assertThat(log_mock, MockNotCalled())
96+ event_description = "%s" % (comment)
97+ node._register_request_event(None, event_name, event_action, comment)
98+ self.assertThat(log_mock, MockCalledOnceWith(
99+ node.system_id,
100+ EVENT_TYPES.REQUEST_NODE_START,
101+ type_level=event_details.level,
102+ type_description=event_details.description,
103+ event_action=event_action,
104+ event_description=event_description))
105
106 def test__status_message_returns_most_recent_event(self):
107 # The first event won't be returned.
108
109=== modified file 'src/maasserver/rpc/nodes.py'
110--- src/maasserver/rpc/nodes.py 2016-05-11 00:25:14 +0000
111+++ src/maasserver/rpc/nodes.py 2016-07-25 22:18:24 +0000
112@@ -49,7 +49,7 @@
113 except Node.DoesNotExist:
114 raise NoSuchNode.from_system_id(system_id)
115 try:
116- node.mark_failed(None, error_description)
117+ node.mark_failed(comment=error_description)
118 except exceptions.NodeStateViolation as e:
119 raise NodeStateViolation(e)
120
121
122=== modified file 'src/maasserver/status_monitor.py'
123--- src/maasserver/status_monitor.py 2016-07-22 14:41:58 +0000
124+++ src/maasserver/status_monitor.py 2016-07-25 22:18:24 +0000
125@@ -43,7 +43,7 @@
126 NODE_STATUS_CHOICES_DICT[node.status],
127 NODE_FAILURE_MONITORED_STATUS_TIMEOUTS[node.status],
128 )
129- node._mark_failed(None, commit=False, comment=comment)
130+ node.mark_failed(commit=False, comment=comment)
131 node.status_expires = None
132 node.save()
133
134
135=== modified file 'src/metadataserver/api.py'
136--- src/metadataserver/api.py 2016-07-22 00:41:50 +0000
137+++ src/metadataserver/api.py 2016-07-25 22:18:24 +0000
138@@ -157,12 +157,19 @@
139 raise UnknownMetadataVersion("Unknown metadata version: %s" % version)
140
141
142-def add_event_to_node_event_log(node, origin, action, description):
143+def add_event_to_node_event_log(
144+ node, origin, action, description, result=None):
145 """Add an entry to the node's event log."""
146 if node.status == NODE_STATUS.COMMISSIONING:
147- type_name = EVENT_TYPES.NODE_COMMISSIONING_EVENT
148+ if result in ['SUCCESS', None]:
149+ type_name = EVENT_TYPES.NODE_COMMISSIONING_EVENT
150+ else:
151+ type_name = EVENT_TYPES.NODE_COMMISSIONING_EVENT_FAILED
152 elif node.status == NODE_STATUS.DEPLOYING:
153- type_name = EVENT_TYPES.NODE_INSTALL_EVENT
154+ if result in ['SUCCESS', None]:
155+ type_name = EVENT_TYPES.NODE_INSTALL_EVENT
156+ else:
157+ type_name = EVENT_TYPES.NODE_INSTALL_EVENT_FAILED
158 elif node.node_type in [
159 NODE_TYPE.RACK_CONTROLLER,
160 NODE_TYPE.REGION_AND_RACK_CONTROLLER]:
161@@ -327,7 +334,8 @@
162 result = message.get('result')
163
164 # Add this event to the node event log.
165- add_event_to_node_event_log(node, origin, activity_name, description)
166+ add_event_to_node_event_log(
167+ node, origin, activity_name, description, result)
168
169 # Save attached files, if any.
170 for sent_file in message.get('files', []):
171@@ -358,12 +366,11 @@
172 elif node.status == NODE_STATUS.DEPLOYING:
173 if result in ['FAIL', 'FAILURE']:
174 node.mark_failed(
175- None,
176- "Installation failed (refer to the "
177- "installation log for more information).")
178+ comment="Installation failed (refer to the "
179+ "installation log for more information).")
180 elif node.status == NODE_STATUS.DISK_ERASING:
181 if result in ['FAIL', 'FAILURE']:
182- node.mark_failed(None, "Failed to erase disks.")
183+ node.mark_failed(comment="Failed to erase disks.")
184
185 # Deallocate the node if we enter any terminal state.
186 if node.node_type == NODE_TYPE.MACHINE and node.status in [
187@@ -530,16 +537,15 @@
188 self._store_installation_results(node, request)
189 if status == SIGNAL_STATUS.FAILED:
190 node.mark_failed(
191- None,
192- "Installation failed (refer to the "
193- "installation log for more information).")
194+ comment="Installation failed (refer to the "
195+ "installation log for more information).")
196 target_status = None
197 elif node.status == NODE_STATUS.DISK_ERASING:
198 if status == SIGNAL_STATUS.OK:
199 # disk erasing complete, release node
200 node.release()
201 elif status == SIGNAL_STATUS.FAILED:
202- node.mark_failed(None, "Failed to erase disks.")
203+ node.mark_failed(comment="Failed to erase disks.")
204 target_status = None
205
206 if target_status in (None, node.status):
207
208=== modified file 'src/metadataserver/tests/test_api_status.py'
209--- src/metadataserver/tests/test_api_status.py 2016-06-11 23:58:38 +0000
210+++ src/metadataserver/tests/test_api_status.py 2016-07-25 22:18:24 +0000
211@@ -193,7 +193,8 @@
212 NODE_STATUS.FAILED_DEPLOYMENT, reload_object(node).status)
213 # Check last node event.
214 self.assertEqual(
215- "'curtin' Command Install",
216+ "Installation failed (refer to the installation"
217+ " log for more information).",
218 Event.objects.filter(node=node).last().description)
219
220 def test_status_installation_fail_leaves_node_failed(self):
221@@ -212,7 +213,8 @@
222 NODE_STATUS.FAILED_DEPLOYMENT, reload_object(node).status)
223 # Check last node event.
224 self.assertEqual(
225- "'curtin' Command Install",
226+ "Installation failed (refer to the installation"
227+ " log for more information).",
228 Event.objects.filter(node=node).last().description)
229
230 def test_status_installation_failure_doesnt_clear_owner(self):
231@@ -270,7 +272,7 @@
232 NODE_STATUS.FAILED_DISK_ERASING, reload_object(node).status)
233 # Check last node event.
234 self.assertEqual(
235- "'curtin' Erasing disk",
236+ "Failed to erase disks.",
237 Event.objects.filter(node=node).last().description)
238
239 def test_status_erasure_failure_does_not_populate_tags(self):
240
241=== modified file 'src/provisioningserver/events.py'
242--- src/provisioningserver/events.py 2016-07-13 19:03:15 +0000
243+++ src/provisioningserver/events.py 2016-07-25 22:18:24 +0000
244@@ -66,7 +66,9 @@
245 # Node status events
246 NODE_STATUS_EVENT = "NODE_STATUS_EVENT"
247 NODE_COMMISSIONING_EVENT = "NODE_COMMISSIONING_EVENT"
248+ NODE_COMMISSIONING_EVENT_FAILED = "NODE_COMMISSIONING_EVENT_FAILED"
249 NODE_INSTALL_EVENT = "NODE_INSTALL_EVENT"
250+ NODE_INSTALL_EVENT_FAILED = "NODE_INSTALL_EVENT_FAILED"
251 # Node user request events
252 REQUEST_NODE_START_COMMISSIONING = "REQUEST_NODE_START_COMMISSIONING"
253 REQUEST_NODE_ABORT_COMMISSIONING = "REQUEST_NODE_ABORT_COMMISSIONING"
254@@ -76,6 +78,7 @@
255 REQUEST_NODE_ABORT_ERASE_DISK = "REQUEST_NODE_ABORT_ERASE_DISK"
256 REQUEST_NODE_RELEASE = "REQUEST_NODE_RELEASE"
257 REQUEST_NODE_MARK_FAILED = "REQUEST_NODE_MARK_FAILED"
258+ REQUEST_NODE_MARK_FAILED_SYSTEM = "REQUEST_NODE_MARK_FAILED_SYSTEM"
259 REQUEST_NODE_MARK_BROKEN = "REQUEST_NODE_MARK_BROKEN"
260 REQUEST_NODE_MARK_FIXED = "REQUEST_NODE_MARK_FIXED"
261 REQUEST_NODE_START_DEPLOYMENT = "REQUEST_NODE_START_DEPLOYMENT"
262@@ -151,10 +154,18 @@
263 description="Node commissioning",
264 level=DEBUG,
265 ),
266+ EVENT_TYPES.NODE_COMMISSIONING_EVENT_FAILED: EventDetail(
267+ description="Node commissioning failure",
268+ level=ERROR,
269+ ),
270 EVENT_TYPES.NODE_INSTALL_EVENT: EventDetail(
271 description="Node installation",
272 level=DEBUG,
273 ),
274+ EVENT_TYPES.NODE_INSTALL_EVENT_FAILED: EventDetail(
275+ description="Node installation failure",
276+ level=ERROR,
277+ ),
278 EVENT_TYPES.REQUEST_NODE_START_COMMISSIONING: EventDetail(
279 description="User starting node commissioning",
280 level=INFO,
281@@ -187,6 +198,10 @@
282 description="User marking node failed",
283 level=INFO,
284 ),
285+ EVENT_TYPES.REQUEST_NODE_MARK_FAILED_SYSTEM: EventDetail(
286+ description="Marking node failed",
287+ level=ERROR,
288+ ),
289 EVENT_TYPES.REQUEST_NODE_MARK_BROKEN: EventDetail(
290 description="User marking node broken",
291 level=INFO,