Merge lp:~trapnine/maas/events-1 into lp:~maas-committers/maas/trunk

[Jeffrey C Jones]

Relevant node operations add a 'user request' entry into the event log.

An optional comment field is plumbed through the Node model methods and tests but aren't piped through the API in this branch (events-2 contains that change, MP pending).

User requests are displayed inline, hopefully before the events happening to the nodes. This allows one to see who (if anyone) triggered the events.

Relevant node operations add a 'user request' entry into the event log.

An optional comment field is plumbed through the Node model methods and tests but aren't piped through the API in this branch (events-2 contains that change, MP pending).

User requests are displayed inline, hopefully before the events happening to the nodes. This allows one to see who (if anyone) triggered the events.

Examples:

    User aborting node commissioning - (admin)
    User starting node commissioning - (admin)
    User acquiring node - (admin)
    User marking node fixed - (admin)
    User marking node broken - (trapnine) - via web interface
    User starting deployment - (trapnine)

[Blake Rouse (blake-rouse)]

The implementation looks good. You have some coding style issues that need to be fixed and you have an issue with logging the event before permissions are checked in the start and stop methods on the Node model. But overall its really good, just those fixes and it will be ready to land.

Also you are missing a commit message. Setting a commit message is actually more important than setting a description. The commit message is what will be placed in the bzr log when your branch is merged by the lander. I would move the first two paragraphs of your description into the commit message. You can leave the examples in the description. You can duplicate the information if you want. Only the commit message will go into the bzr log the description will just stay on launchpad if this merge ever needs to be looked at again.

review: Needs Fixing

[Jeffrey C Jones (trapnine)]

fixed - please take another look

[Blake Rouse (blake-rouse)]

Looks good. Thanks for all the fixes.

review: Approve

=== modified file 'src/maasserver/api/nodes.py'
--- src/maasserver/api/nodes.py	2015-09-17 18:32:46 +0000
+++ src/maasserver/api/nodes.py	2015-09-17 19:23:21 +0000
@@ -498,7 +498,7 @@
             # postcondition is achieved, so call this success.
             pass
         elif node.status in RELEASABLE_STATUSES:
-            node.release_or_erase()
+            node.release_or_erase(request.user)
         else:
             raise NodeStateViolation(
                 "Node cannot be released in its current state ('%s')."
@@ -677,7 +677,7 @@
             user=request.user, system_id=system_id, perm=NODE_PERMISSION.EDIT)
         error_description = get_optional_param(
             request.POST, 'error_description', '')
-        node.mark_broken(error_description)
+        node.mark_broken(request.user, error_description)
         return node
 
     @operation(idempotent=False)
@@ -690,7 +690,7 @@
         """
         node = Node.nodes.get_node_or_404(
             user=request.user, system_id=system_id, perm=NODE_PERMISSION.ADMIN)
-        node.mark_fixed()
+        node.mark_fixed(request.user)
         maaslog.info(
             "%s: User %s marked node as fixed", node.hostname,
             request.user.username)
@@ -1239,7 +1239,7 @@
                 # Nothing to do.
                 pass
             elif node.status in RELEASABLE_STATUSES:
-                node.release_or_erase()
+                node.release_or_erase(request.user)
                 released_ids.append(node.system_id)
             else:
                 failed.append(

=== modified file 'src/maasserver/fixtures/dev_fixture.yaml'
--- src/maasserver/fixtures/dev_fixture.yaml	2015-08-14 13:48:59 +0000
+++ src/maasserver/fixtures/dev_fixture.yaml	2015-09-17 19:23:21 +0000
@@ -2933,7 +2933,7 @@
     disable_ipv4: false
     distro_series: ''
     error: finished [8/8]
-    error_description: Manually marked as broken by user 'ubuntu'
+    error_description: via web interface
     hostname: eager-needle
     installable: true
     license_key: ''

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2015-09-17 15:37:57 +0000
+++ src/maasserver/models/node.py	2015-09-17 19:23:21 +0000
@@ -120,6 +120,10 @@
 from metadataserver.enum import RESULT_TYPE
 from netaddr import IPAddress
 from piston.models import Token
+from provisioningserver.events import (
+    EVENT_DETAILS,
+    EVENT_TYPES,
+)
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.power import QUERY_POWER_TYPES
 from provisioningserver.power.poweraction import UnknownPowerType
@@ -635,7 +639,22 @@
         """
         return timedelta(minutes=5).total_seconds()
 
-    def start_deployment(self):
+    def _register_request_event(self, user, type_name, comment=None):
+        """Register a node user request event."""
+        # don't register system generated non-user requests
+        if user is not None:
+            event_details = EVENT_DETAILS[type_name]
+            description = "(%s)" % user.username
+            if comment:
+                description = "%s - %s" % (description, comment)
+            # Avoid circular imports.
+            from maasserver.models.event import Event
+            Event.objects.register_event_and_event_type(
+                self.system_id, type_name, type_level=event_details.level,
+                type_description=event_details.description,
+                event_description=description)
+
+    def _start_deployment(self):
         """Mark a node as being deployed."""
         self.status = NODE_STATUS.DEPLOYING
         self.save()
@@ -669,8 +688,8 @@
         failed_status = get_failed_status(self.status)
         if failed_status is not None:
             timeout_timedelta = timedelta(seconds=context['timeout'])
-            self.mark_failed(
-                "Node operation '%s' timed out after %s." % (
+            self._mark_failed(
+                None, "Node operation '%s' timed out after %s." % (
                     (
                         NODE_STATUS_CHOICES_DICT[self.status],
                         timeout_timedelta
@@ -931,6 +950,9 @@
         from metadataserver.user_data.commissioning import generate_user_data
         from metadataserver.models import NodeResult
 
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_START_COMMISSIONING)
+
         # Set the commissioning options on the node.
         self.enable_ssh = enable_ssh
         self.block_poweroff = block_poweroff
@@ -965,7 +987,7 @@
             # Node.start() has synchronous and asynchronous parts, so catch
             # exceptions arising synchronously, and chain callbacks to the
             # Deferred it returns for the asynchronous (post-commit) bits.
-            starting = self.start(user, user_data=commissioning_user_data)
+            starting = self._start(user, commissioning_user_data)
         except Exception as error:
             self.status = old_status
             self.save()
@@ -1045,7 +1067,7 @@
                 "must be started manually", hostname)
 
     @transactional
-    def abort_commissioning(self, user):
+    def abort_commissioning(self, user, comment=None):
         """Power off a commissioning node and set its status to NEW.
 
         :return: a `Deferred` which contains the post-commit tasks that are
@@ -1058,6 +1080,9 @@
                 "node %s is in state %s."
                 % (self.system_id, NODE_STATUS_CHOICES_DICT[self.status]))
 
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_ABORT_COMMISSIONING, comment)
+
         # Prepare a transition monitor for later.
         monitor = TransitionMonitor.fromNode(self)
 
@@ -1065,7 +1090,7 @@
             # Node.stop() has synchronous and asynchronous parts, so catch
             # exceptions arising synchronously, and chain callbacks to the
             # Deferred it returns for the asynchronous (post-commit) bits.
-            stopping = self.stop(user)
+            stopping = self._stop(user)
             if self.owner is not None:
                 self.owner = None
                 self.save()
@@ -1104,7 +1129,7 @@
             return stopping.addErrback(eb_abort, self.hostname)
 
     @transactional
-    def abort_deploying(self, user):
+    def abort_deploying(self, user, comment=None):
         """Power off a deploying node and set its status to ALLOCATED.
 
         :return: a `Deferred` which contains the post-commit tasks that are
@@ -1117,6 +1142,9 @@
                 "node %s is in state %s."
                 % (self.system_id, NODE_STATUS_CHOICES_DICT[self.status]))
 
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_ABORT_DEPLOYMENT, comment)
+
         # Prepare a transition monitor for later.
         monitor = TransitionMonitor.fromNode(self)
 
@@ -1124,7 +1152,7 @@
             # Node.stop() has synchronous and asynchronous parts, so catch
             # exceptions arising synchronously, and chain callbacks to the
             # Deferred it returns for the asynchronous (post-commit) bits.
-            stopping = self.stop(user)
+            stopping = self._stop(user)
         except Exception as error:
             maaslog.error(
                 "%s: Error when aborting deployment: %s",
@@ -1410,7 +1438,7 @@
 
     def acquire(
             self, user, token=None, agent_name='',
-            storage_layout=None, storage_layout_params={}):
+            storage_layout=None, storage_layout_params={}, comment=None):
         """Mark commissioned node as acquired by the given user and token."""
         assert self.owner is None
         assert token is None or token.user == user
@@ -1421,6 +1449,9 @@
                 "default_storage_layout")
         self.set_storage_layout(storage_layout, params=storage_layout_params)
 
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_ACQUIRE, comment)
+
         # Now allocate the node since the storage layout is setup correctly.
         self.status = NODE_STATUS.ALLOCATED
         self.owner = user
@@ -1464,7 +1495,7 @@
         maaslog.info("%s: moved from %s zone to %s zone." % (
             self.hostname, old_zone_name, self.zone.name))
 
-    def start_disk_erasing(self, user):
+    def start_disk_erasing(self, user, comment=None):
         """Erase the disks on a node.
 
         :return: a `Deferred` which contains the post-commit tasks that are
@@ -1476,6 +1507,10 @@
         from metadataserver.user_data.disk_erasing import generate_user_data
 
         disk_erase_user_data = generate_user_data(node=self)
+
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_ERASE_DISK, comment)
+
         # Change the status of the node now to avoid races when starting
         # nodes in bulk.
         self.status = NODE_STATUS.DISK_ERASING
@@ -1485,7 +1520,7 @@
             # Node.start() has synchronous and asynchronous parts, so catch
             # exceptions arising synchronously, and chain callbacks to the
             # Deferred it returns for the asynchronous (post-commit) bits.
-            starting = self.start(user, user_data=disk_erase_user_data)
+            starting = self._start(user, disk_erase_user_data)
         except Exception as error:
             # We always mark the node as failed here, although we could
             # potentially move it back to the state it was in previously. For
@@ -1545,7 +1580,7 @@
                 "%s: Could not start node for disk erasure; it "
                 "must be started manually", hostname)
 
-    def abort_disk_erasing(self, user):
+    def abort_disk_erasing(self, user, comment=None):
         """Power off disk erasing node and set a failed status.
 
         :return: a `Deferred` which contains the post-commit tasks that are
@@ -1558,11 +1593,14 @@
                 "node %s is in state %s."
                 % (self.system_id, NODE_STATUS_CHOICES_DICT[self.status]))
 
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_ABORT_ERASE_DISK, comment)
+
         try:
             # Node.stop() has synchronous and asynchronous parts, so catch
             # exceptions arising synchronously, and chain callbacks to the
             # Deferred it returns for the asynchronous (post-commit) bits.
-            stopping = self.stop(user)
+            stopping = self._stop(user)
         except Exception as error:
             maaslog.error(
                 "%s: Error when aborting disk erasure: %s",
@@ -1613,26 +1651,29 @@
                 callOut, maaslog.warning, "%s: Could not stop node to abort "
                 "disk erasure; it must be stopped manually", hostname)
 
-    def abort_operation(self, user):
+    def abort_operation(self, user, comment=None):
         """Abort the current operation.
-        This currently only supports aborting Disk Erasing.
         """
         if self.status == NODE_STATUS.DISK_ERASING:
-            self.abort_disk_erasing(user)
+            self.abort_disk_erasing(user, comment)
             return
         if self.status == NODE_STATUS.COMMISSIONING:
-            self.abort_commissioning(user)
+            self.abort_commissioning(user, comment)
             return
         if self.status == NODE_STATUS.DEPLOYING:
-            self.abort_deploying(user)
+            self.abort_deploying(user, comment)
             return
-
         raise NodeStateViolation(
             "Cannot abort in current state: "
             "node %s is in state %s."
             % (self.system_id, NODE_STATUS_CHOICES_DICT[self.status]))
 
-    def release(self):
+    def release(self, user=None, comment=None):
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_RELEASE, comment)
+        self._release(user)
+
+    def _release(self, user=None):
         """Mark allocated or reserved node as available again and power off.
         """
         maaslog.info("%s: Releasing node", self.hostname)
@@ -1643,7 +1684,7 @@
         # the issue this will cause.
         if self.power_state != POWER_STATE.OFF:
             try:
-                self.stop(self.owner)
+                self._stop(self.owner)
             except Exception as ex:
                 maaslog.error(
                     "%s: Unable to shut node down: %s", self.hostname,
@@ -1694,16 +1735,15 @@
         # If this node has non-installable children, remove them.
         self.children.all().delete()
 
-    def release_or_erase(self):
+    def release_or_erase(self, user, comment=None):
         """Either release the node or erase the node then release it, depending
         on settings."""
         erase_on_release = Config.objects.get_config(
             'enable_disk_erasing_on_release')
         if erase_on_release:
-            self.start_disk_erasing(self.owner)
-            return
-
-        self.release()
+            self.start_disk_erasing(user, comment)
+        else:
+            self.release(user, comment)
 
     def set_netboot(self, on=True):
         """Set netboot on or off."""
@@ -1725,7 +1765,12 @@
         arch, subarch = self.architecture.split('/')
         return (arch, subarch)
 
-    def mark_failed(self, error_description):
+    def mark_failed(self, user, comment=None):
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_MARK_FAILED, comment)
+        self._mark_failed(user, comment)
+
+    def _mark_failed(self, user, comment=None):
         """Mark this node as failed.
 
         The actual 'failed' state depends on the current status of the
@@ -1734,11 +1779,10 @@
         new_status = get_failed_status(self.status)
         if new_status is not None:
             self.status = new_status
-            self.error_description = error_description
+            self.error_description = comment if comment else ''
             self.save()
             maaslog.error(
-                "%s: Marking node failed: %s", self.hostname,
-                error_description)
+                "%s: Marking node failed: %s", self.hostname, comment)
         elif self.status == NODE_STATUS.NEW:
             # Silently ignore, failing a new node makes no sense.
             pass
@@ -1751,22 +1795,26 @@
                 "be transitioned to a corresponding failed status." %
                 self.status)
 
-    def mark_broken(self, error_description):
+    def mark_broken(self, user, comment=None):
         """Mark this node as 'BROKEN'.
 
         If the node is allocated, release it first.
         """
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_MARK_BROKEN, comment)
         if self.status in RELEASABLE_STATUSES:
-            self.release()
+            self._release(user)
         # release() normally sets the status to RELEASING and leaves the
         # owner in place, override that here as we're broken.
         self.status = NODE_STATUS.BROKEN
         self.owner = None
-        self.error_description = error_description
+        self.error_description = comment if comment else ''
         self.save()
 
-    def mark_fixed(self):
+    def mark_fixed(self, user, comment=None):
         """Mark a broken node as fixed and change its state to 'READY'."""
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_MARK_FIXED, comment)
         if self.status != NODE_STATUS.BROKEN:
             raise NodeStateViolation(
                 "Can't mark a non-broken node as 'Ready'.")
@@ -2116,11 +2164,23 @@
         return False
 
     @transactional
-    def start(self, by_user, user_data=None):
+    def start(self, user, user_data=None, comment=None):
+        if not user.has_perm(NODE_PERMISSION.EDIT, self):
+            # You can't start a node you don't own unless you're an admin.
+            raise PermissionDenied()
+        event = EVENT_TYPES.REQUEST_NODE_START
+        # if status is ALLOCATED, this start is actually for a deplyment
+        if self.status == NODE_STATUS.ALLOCATED:
+            event = EVENT_TYPES.REQUEST_NODE_START_DEPLOYMENT
+        self._register_request_event(user, event, comment)
+        return self._start(user, user_data)
+
+    @transactional
+    def _start(self, user, user_data=None):
         """Request on given user's behalf that the node be started up.
 
-        :param by_user: Requesting user.
-        :type by_user: User_
+        :param user: Requesting user.
+        :type user: User_
         :param user_data: Optional blob of user-data to be made available to
             the node through the metadata service. If not given, any previous
             user data is used.
@@ -2129,7 +2189,7 @@
         :raise StaticIPAddressExhaustion: if there are not enough IP addresses
             left in the static range for this node to get all the addresses it
             needs.
-        :raise PermissionDenied: If `by_user` does not have permission to
+        :raise PermissionDenied: If `user` does not have permission to
             start this node.
 
         :return: a `Deferred` which contains the post-commit tasks that are
@@ -2142,7 +2202,7 @@
         # Avoid circular imports.
         from metadataserver.models import NodeUserData
 
-        if not by_user.has_perm(NODE_PERMISSION.EDIT, self):
+        if not user.has_perm(NODE_PERMISSION.EDIT, self):
             # You can't start a node you don't own unless you're an admin.
             raise PermissionDenied()
 
@@ -2151,16 +2211,14 @@
         # node; the user may choose to start it manually.
         NodeUserData.objects.set_user_data(self, user_data)
 
-        # Claim AUTO IP addresses for the node if it's ALLOCATED.
         if self.status == NODE_STATUS.ALLOCATED:
+            # Claim AUTO IP addresses for the node if it's ALLOCATED.
             self.claim_auto_ips()
-
-        if self.status == NODE_STATUS.ALLOCATED:
             transition_monitor = (
                 TransitionMonitor.fromNode(self)
                 .within(seconds=self.get_deployment_time())
                 .status_should_be(self.status))
-            self.start_deployment()
+            self._start_deployment()
         else:
             transition_monitor = None
 
@@ -2185,15 +2243,24 @@
             self.nodegroup.uuid, power_info)
 
     @transactional
-    def stop(self, by_user, stop_mode='hard'):
+    def stop(self, user, stop_mode='hard', comment=None):
+        if not user.has_perm(NODE_PERMISSION.EDIT, self):
+            # You can't stop a node you don't own unless you're an admin.
+            raise PermissionDenied()
+        self._register_request_event(
+            user, EVENT_TYPES.REQUEST_NODE_STOP, comment)
+        return self._stop(user, stop_mode)
+
+    @transactional
+    def _stop(self, user, stop_mode='hard'):
         """Request that the node be powered down.
 
-        :param by_user: Requesting user.
-        :type by_user: User_
+        :param user: Requesting user.
+        :type user: User_
         :param stop_mode: Power off mode - usually 'soft' or 'hard'.
         :type stop_mode: unicode
 
-        :raise PermissionDenied: If `by_user` does not have permission to
+        :raise PermissionDenied: If `user` does not have permission to
             stop this node.
 
         :return: a `Deferred` which contains the post-commit tasks that are
@@ -2203,7 +2270,7 @@
             does not support it, `None` will be returned. The node must be
             powered off manually.
         """
-        if not by_user.has_perm(NODE_PERMISSION.EDIT, self):
+        if not user.has_perm(NODE_PERMISSION.EDIT, self):
             # You can't stop a node you don't own unless you're an admin.
             raise PermissionDenied()
 

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2015-09-17 15:37:57 +0000
+++ src/maasserver/models/tests/test_node.py	2015-09-17 19:23:21 +0000
@@ -57,6 +57,7 @@
     PhysicalInterface,
     UnknownInterface,
 )
+from maasserver.models.event import Event
 from maasserver.models.node import PowerInfo
 from maasserver.models.signals import power as node_query
 from maasserver.models.timestampedmodel import now
@@ -107,6 +108,10 @@
     MagicMock,
     sentinel,
 )
+from provisioningserver.events import (
+    EVENT_DETAILS,
+    EVENT_TYPES,
+)
 from provisioningserver.power import QUERY_POWER_TYPES
 from provisioningserver.power.poweraction import UnknownPowerType
 from provisioningserver.power.schema import JSON_POWER_TYPE_PARAMETERS
@@ -628,6 +633,16 @@
             (user, NODE_STATUS.ALLOCATED, agent_name),
             (node.owner, node.status, node.agent_name))
 
+    def test_acquire_logs_user_request(self):
+        node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
+        user = factory.make_User()
+        token = create_auth_token(user)
+        agent_name = factory.make_name('agent-name')
+        register_event = self.patch(node, '_register_request_event')
+        node.acquire(user, token, agent_name)
+        self.assertThat(register_event, MockCalledOnceWith(
+            user, EVENT_TYPES.REQUEST_NODE_ACQUIRE, None))
+
     def test_acquire_calls_set_storage_layout(self):
         node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
         user = factory.make_User()
@@ -731,7 +746,7 @@
         owner = factory.make_User()
         node = factory.make_Node(
             status=NODE_STATUS.ALLOCATED, owner=owner, agent_name=agent_name)
-        node_start = self.patch(node, 'start')
+        node_start = self.patch(node, '_start')
         # Return a post-commit hook from Node.start().
         node_start.side_effect = lambda user, user_data: post_commit()
         with post_commit_hooks:
@@ -740,7 +755,21 @@
         self.expectThat(node.status, Equals(NODE_STATUS.DISK_ERASING))
         self.expectThat(node.agent_name, Equals(agent_name))
         self.assertThat(
-            node_start, MockCalledOnceWith(owner, user_data=ANY))
+            node_start, MockCalledOnceWith(owner, ANY))
+
+    def test_start_disk_erasing_logs_user_request(self):
+        owner = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=owner)
+        node_start = self.patch(node, '_start')
+        # Return a post-commit hook from Node.start().
+        node_start.side_effect = lambda user, user_data: post_commit()
+        register_event = self.patch(node, '_register_request_event')
+        with post_commit_hooks:
+            node.start_disk_erasing(owner)
+        self.assertThat(
+            node_start, MockCalledOnceWith(owner, ANY))
+        self.assertThat(register_event, MockCalledOnceWith(
+            owner, EVENT_TYPES.REQUEST_NODE_ERASE_DISK, None))
 
     def test_abort_disk_erasing_changes_state_and_stops_node(self):
         agent_name = factory.make_name('agent-name')
@@ -748,7 +777,7 @@
         node = factory.make_Node(
             status=NODE_STATUS.DISK_ERASING, owner=owner,
             agent_name=agent_name)
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         # Return a post-commit hook from Node.stop().
         node_stop.side_effect = lambda user: post_commit()
         self.patch(Node, "_set_status")
@@ -765,6 +794,19 @@
         self.expectThat(node.owner, Equals(owner))
         self.expectThat(node.agent_name, Equals(agent_name))
 
+    def test_abort_disk_erasing_logs_user_request(self):
+        owner = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.DISK_ERASING, owner=owner)
+        node_stop = self.patch(node, '_stop')
+        # Return a post-commit hook from Node.stop().
+        node_stop.side_effect = lambda user: post_commit()
+        self.patch(Node, "_set_status")
+        register_event = self.patch(node, '_register_request_event')
+        with post_commit_hooks:
+            node.abort_disk_erasing(owner)
+        self.assertThat(register_event, MockCalledOnceWith(
+            owner, EVENT_TYPES.REQUEST_NODE_ABORT_ERASE_DISK, None))
+
     def test_start_disk_erasing_reverts_to_sane_state_on_error(self):
         # If start_disk_erasing encounters an error when calling start(), it
         # will transition the node to a sane state. Failures encountered in
@@ -773,7 +815,7 @@
         admin = factory.make_admin()
         node = factory.make_Node(status=NODE_STATUS.ALLOCATED)
         generate_user_data = self.patch(disk_erasing, 'generate_user_data')
-        node_start = self.patch(node, 'start')
+        node_start = self.patch(node, '_start')
         node_start.side_effect = factory.make_exception()
 
         try:
@@ -786,7 +828,7 @@
 
         self.assertThat(
             node_start, MockCalledOnceWith(
-                admin, user_data=generate_user_data.return_value))
+                admin, generate_user_data.return_value))
         self.assertEqual(NODE_STATUS.FAILED_DISK_ERASING, node.status)
 
     def test_start_disk_erasing_sets_status_on_post_commit_error(self):
@@ -795,7 +837,7 @@
         admin = factory.make_admin()
         node = factory.make_Node(status=NODE_STATUS.ALLOCATED)
         # Patch out some things that we don't want to do right now.
-        self.patch(node, 'start').return_value = None
+        self.patch(node, '_start').return_value = None
         # Fake an error during the post-commit hook.
         error_message = factory.make_name("error")
         error_type = factory.make_exception_type()
@@ -824,7 +866,7 @@
         maaslog = self.patch(node_module, 'maaslog')
         exception_type = factory.make_exception_type()
         exception = exception_type(factory.make_name())
-        self.patch(node, 'start').side_effect = exception
+        self.patch(node, '_start').side_effect = exception
         self.assertRaises(
             exception_type, node.start_disk_erasing, admin)
         self.assertEqual(NODE_STATUS.FAILED_DISK_ERASING, node.status)
@@ -841,7 +883,7 @@
             agent_name=agent_name)
         abort_commissioning = self.patch_autospec(node, 'abort_commissioning')
         node.abort_operation(user)
-        self.assertThat(abort_commissioning, MockCalledOnceWith(user))
+        self.assertThat(abort_commissioning, MockCalledOnceWith(user, None))
 
     def test_abort_operation_aborts_disk_erasing(self):
         agent_name = factory.make_name('agent-name')
@@ -851,7 +893,7 @@
             agent_name=agent_name)
         abort_disk_erasing = self.patch_autospec(node, 'abort_disk_erasing')
         node.abort_operation(owner)
-        self.assertThat(abort_disk_erasing, MockCalledOnceWith(owner))
+        self.assertThat(abort_disk_erasing, MockCalledOnceWith(owner, None))
 
     def test_abort_operation_aborts_deployment(self):
         agent_name = factory.make_name('agent-name')
@@ -861,7 +903,21 @@
             agent_name=agent_name)
         abort_deploying = self.patch_autospec(node, 'abort_deploying')
         node.abort_operation(user)
-        self.assertThat(abort_deploying, MockCalledOnceWith(user))
+        self.assertThat(abort_deploying, MockCalledOnceWith(user, None))
+
+    def test_abort_deployment_logs_user_request(self):
+        agent_name = factory.make_name('agent-name')
+        admin = factory.make_admin()
+        node = factory.make_Node(
+            status=NODE_STATUS.DEPLOYING,
+            agent_name=agent_name)
+        self.patch_autospec(node_module.TransitionMonitor, "stop")
+        self.patch(Node, "_set_status")
+        register_event = self.patch(node, '_register_request_event')
+        with post_commit_hooks:
+            node.abort_deploying(admin)
+        self.assertThat(register_event, MockCalledOnceWith(
+            admin, EVENT_TYPES.REQUEST_NODE_ABORT_DEPLOYMENT, None))
 
     def test_abort_operation_raises_exception_for_unsupported_state(self):
         agent_name = factory.make_name('agent-name')
@@ -878,7 +934,7 @@
         admin = factory.make_admin()
         node = factory.make_Node(
             status=NODE_STATUS.DISK_ERASING, power_type="virsh")
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         node_stop.side_effect = factory.make_exception()
 
         try:
@@ -898,7 +954,7 @@
         maaslog = self.patch(node_module, 'maaslog')
         exception_class = factory.make_exception_type()
         exception = exception_class(factory.make_name())
-        self.patch(node, 'stop').side_effect = exception
+        self.patch(node, '_stop').side_effect = exception
         self.assertRaises(
             exception_class, node.abort_disk_erasing, admin)
         self.assertEqual(NODE_STATUS.DISK_ERASING, node.status)
@@ -991,12 +1047,12 @@
         owner = factory.make_User()
         node = factory.make_Node(
             status=NODE_STATUS.ALLOCATED, owner=owner, agent_name=agent_name)
-        self.patch(node, 'stop')
+        self.patch(node, '_stop')
         self.patch(node, 'start_transition_monitor')
         node.power_state = POWER_STATE.OFF
         with post_commit_hooks:
             node.release()
-        self.expectThat(node.stop, MockNotCalled())
+        self.expectThat(node._stop, MockNotCalled())
         self.expectThat(node.start_transition_monitor, MockNotCalled())
         self.expectThat(node.status, Equals(NODE_STATUS.READY))
         self.expectThat(node.owner, Is(None))
@@ -1176,6 +1232,15 @@
             node.release()
         self.assertTrue(node.netboot)
 
+    def test_release_logs_user_request(self):
+        owner = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=owner)
+        register_event = self.patch(node, '_register_request_event')
+        with post_commit_hooks:
+            node.release(owner)
+        self.assertThat(register_event, MockCalledOnceWith(
+            owner, EVENT_TYPES.REQUEST_NODE_RELEASE, None))
+
     def test_release_clears_osystem_and_distro_series(self):
         node = factory.make_Node(
             status=NODE_STATUS.ALLOCATED, owner=factory.make_User())
@@ -1192,7 +1257,7 @@
             status=NODE_STATUS.ALLOCATED, owner=user, power_type='virsh',
             power_state=POWER_STATE.ON)
         self.patch(node, 'start_transition_monitor')
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         node.release()
         self.assertThat(
             node_stop, MockCalledOnceWith(user))
@@ -1203,7 +1268,7 @@
             status=NODE_STATUS.ALLOCATED, owner=user, power_type='virsh',
             power_state=POWER_STATE.OFF)
         self.patch(node, 'start_transition_monitor')
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         with post_commit_hooks:
             node.release()
         self.assertThat(node_stop, MockNotCalled())
@@ -1239,7 +1304,7 @@
             power_state=POWER_STATE.ON, power_type='virsh')
         release_auto_ips_later = self.patch_autospec(
             node, "release_auto_ips_later")
-        self.patch_autospec(node, 'stop')
+        self.patch_autospec(node, '_stop')
         self.patch(node, 'start_transition_monitor')
         node.release()
         self.assertThat(
@@ -1251,7 +1316,7 @@
         maaslog = self.patch(node_module, 'maaslog')
         exception_class = factory.make_exception_type()
         exception = exception_class(factory.make_name())
-        self.patch(node, 'stop').side_effect = exception
+        self.patch(node, '_stop').side_effect = exception
         self.assertRaises(exception_class, node.release)
         self.assertEqual(NODE_STATUS.DEPLOYED, node.status)
         self.assertThat(
@@ -1266,7 +1331,7 @@
             status=NODE_STATUS.DEPLOYED, power_type="virsh",
             power_state=POWER_STATE.ON,
             owner=factory.make_User())
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         node_stop.side_effect = factory.make_exception()
 
         try:
@@ -1354,7 +1419,7 @@
     def test_start_commissioning_changes_status_and_starts_node(self):
         node = factory.make_Node(
             interface=True, status=NODE_STATUS.NEW, power_type='ether_wake')
-        node_start = self.patch(node, 'start')
+        node_start = self.patch(node, '_start')
         # Return a post-commit hook from Node.start().
         node_start.side_effect = lambda user, user_data: post_commit()
         admin = factory.make_admin()
@@ -1365,8 +1430,7 @@
             'status': NODE_STATUS.COMMISSIONING,
         }
         self.assertAttributes(node, expected_attrs)
-        self.assertThat(node_start, MockCalledOnceWith(
-            admin, user_data=ANY))
+        self.assertThat(node_start, MockCalledOnceWith(admin, ANY))
 
     def test_start_commissioning_sets_options(self):
         node = factory.make_Node(
@@ -1392,7 +1456,7 @@
 
     def test_start_commissioning_sets_user_data(self):
         node = factory.make_Node(status=NODE_STATUS.NEW)
-        node_start = self.patch(node, 'start')
+        node_start = self.patch(node, '_start')
         node_start.side_effect = lambda user, user_data: post_commit()
         user_data = factory.make_string().encode('ascii')
         generate_user_data = self.patch(
@@ -1401,8 +1465,7 @@
         admin = factory.make_admin()
         node.start_commissioning(admin)
         post_commit_hooks.reset()  # Ignore these for now.
-        self.assertThat(node_start, MockCalledOnceWith(
-            admin, user_data=user_data))
+        self.assertThat(node_start, MockCalledOnceWith(admin, user_data))
 
     def test_start_commissioning_clears_node_commissioning_results(self):
         node = factory.make_Node(status=NODE_STATUS.NEW)
@@ -1458,7 +1521,7 @@
         admin = factory.make_admin()
         node = factory.make_Node(status=NODE_STATUS.NEW)
         generate_user_data = self.patch(commissioning, 'generate_user_data')
-        node_start = self.patch(node, 'start')
+        node_start = self.patch(node, '_start')
         node_start.side_effect = factory.make_exception()
 
         try:
@@ -1471,8 +1534,7 @@
 
         self.assertThat(
             node_start,
-            MockCalledOnceWith(
-                admin, user_data=generate_user_data.return_value))
+            MockCalledOnceWith(admin, generate_user_data.return_value))
         self.assertEqual(NODE_STATUS.NEW, node.status)
 
     def test_start_commissioning_reverts_status_on_post_commit_error(self):
@@ -1485,7 +1547,7 @@
         node = factory.make_Node(status=status)
         # Patch out some things that we don't want to do right now.
         self.patch(node, '_start_transition_monitor_async')
-        self.patch(node, 'start').return_value = None
+        self.patch(node, '_start').return_value = None
         # Fake an error during the post-commit hook.
         error_message = factory.make_name("error")
         error_type = factory.make_exception_type()
@@ -1512,7 +1574,7 @@
         node = factory.make_Node(status=NODE_STATUS.NEW)
         maaslog = self.patch(node_module, 'maaslog')
         exception = NoConnectionsAvailable(factory.make_name())
-        self.patch(node, 'start').side_effect = exception
+        self.patch(node, '_start').side_effect = exception
         self.assertRaises(
             NoConnectionsAvailable, node.start_commissioning, admin)
         self.assertEqual(NODE_STATUS.NEW, node.status)
@@ -1521,6 +1583,20 @@
                 "%s: Could not start node for commissioning: %s",
                 node.hostname, exception))
 
+    def test_start_commissioning_logs_user_request(self):
+        node = factory.make_Node(
+            interface=True, status=NODE_STATUS.NEW, power_type='ether_wake')
+        register_event = self.patch(node, '_register_request_event')
+        node_start = self.patch(node, '_start')
+        # Return a post-commit hook from Node.start().
+        node_start.side_effect = lambda user, user_data: post_commit()
+        admin = factory.make_admin()
+        node.start_commissioning(admin)
+        post_commit_hooks.reset()  # Ignore these for now.
+        node = reload_object(node)
+        self.assertThat(register_event, MockCalledOnceWith(
+            admin, EVENT_TYPES.REQUEST_NODE_START_COMMISSIONING))
+
     def test_abort_commissioning_reverts_to_sane_state_on_error(self):
         # If abort commissioning hits an error when trying to stop the
         # node, it will revert the node to the state it was in before
@@ -1528,7 +1604,7 @@
         admin = factory.make_admin()
         node = factory.make_Node(
             status=NODE_STATUS.COMMISSIONING, power_type="virsh")
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         node_stop.side_effect = factory.make_exception()
 
         try:
@@ -1576,13 +1652,24 @@
         [monitor], _ = monitor_stop.call_args  # Extract `self`.
         self.assertAttributes(monitor, {"system_id": node.system_id})
 
+    def test_abort_commissioning_logs_user_request(self):
+        node = factory.make_Node(status=NODE_STATUS.COMMISSIONING)
+        admin = factory.make_admin()
+        self.patch_autospec(node_module.TransitionMonitor, "stop")
+        self.patch(Node, "_set_status")
+        register_event = self.patch(node, '_register_request_event')
+        with post_commit_hooks:
+            node.abort_commissioning(admin)
+        self.assertThat(register_event, MockCalledOnceWith(
+            admin, EVENT_TYPES.REQUEST_NODE_ABORT_COMMISSIONING, None))
+
     def test_abort_commissioning_logs_and_raises_errors_in_stopping(self):
         admin = factory.make_admin()
         node = factory.make_Node(status=NODE_STATUS.COMMISSIONING)
         maaslog = self.patch(node_module, 'maaslog')
         exception_class = factory.make_exception_type()
         exception = exception_class(factory.make_name())
-        self.patch(node, 'stop').side_effect = exception
+        self.patch(node, '_stop').side_effect = exception
         self.assertRaises(
             exception_class, node.abort_commissioning, admin)
         self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
@@ -1596,7 +1683,7 @@
             status=NODE_STATUS.COMMISSIONING, power_type='virsh')
         admin = factory.make_admin()
 
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         # Return a post-commit hook from Node.stop().
         node_stop.side_effect = lambda user: post_commit()
         self.patch(Node, "_set_status")
@@ -1642,7 +1729,7 @@
             enable_ssh=True)
         admin = factory.make_admin()
 
-        node_stop = self.patch(node, 'stop')
+        node_stop = self.patch(node, '_stop')
         # Return a post-commit hook from Node.stop().
         node_stop.side_effect = lambda user: post_commit()
         self.patch(Node, "_set_status")
@@ -1780,16 +1867,26 @@
             for status in NODE_FAILURE_STATUS_TRANSITIONS
         }
         for node in nodes_mapping.values():
-            node.mark_failed(factory.make_name('error-description'))
+            node.mark_failed(None, factory.make_name('error-description'))
         self.assertEqual(
             NODE_FAILURE_STATUS_TRANSITIONS,
             {status: node.status for status, node in nodes_mapping.items()})
 
+    def test_mark_failed_logs_user_request(self):
+        owner = factory.make_User()
+        self.disable_node_query()
+        node = factory.make_Node(status=NODE_STATUS.COMMISSIONING, owner=owner)
+        description = factory.make_name('error-description')
+        register_event = self.patch(node, '_register_request_event')
+        node.mark_failed(owner, description)
+        self.assertThat(register_event, MockCalledOnceWith(
+            owner, EVENT_TYPES.REQUEST_NODE_MARK_FAILED, description))
+
     def test_mark_failed_updates_error_description(self):
         self.disable_node_query()
         node = factory.make_Node(status=NODE_STATUS.COMMISSIONING)
         description = factory.make_name('error-description')
-        node.mark_failed(description)
+        node.mark_failed(None, description)
         self.assertEqual(description, reload_object(node).error_description)
 
     def test_mark_failed_raises_for_unauthorized_node_status(self):
@@ -1799,63 +1896,82 @@
         status = factory.pick_choice(NODE_STATUS_CHOICES, but_not=but_not)
         node = factory.make_Node(status=status)
         description = factory.make_name('error-description')
-        self.assertRaises(NodeStateViolation, node.mark_failed, description)
+        self.assertRaises(
+            NodeStateViolation, node.mark_failed, None, description)
 
     def test_mark_failed_ignores_if_already_failed(self):
         status = random.choice([
             NODE_STATUS.FAILED_DEPLOYMENT, NODE_STATUS.FAILED_COMMISSIONING])
         node = factory.make_Node(status=status)
         description = factory.make_name('error-description')
-        node.mark_failed(description)
+        node.mark_failed(None, description)
         self.assertEqual(status, node.status)
 
     def test_mark_failed_ignores_if_status_is_NEW(self):
         node = factory.make_Node(status=NODE_STATUS.NEW)
         description = factory.make_name('error-description')
-        node.mark_failed(description)
+        node.mark_failed(None, description)
         self.assertEqual(NODE_STATUS.NEW, node.status)
 
     def test_mark_broken_changes_status_to_broken(self):
-        node = factory.make_Node(
-            status=NODE_STATUS.NEW, owner=factory.make_User())
-        node.mark_broken(factory.make_name('error-description'))
+        user = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.NEW, owner=user)
+        node.mark_broken(user, factory.make_name('error-description'))
         self.assertEqual(NODE_STATUS.BROKEN, reload_object(node).status)
 
+    def test_mark_broken_logs_user_request(self):
+        owner = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.NEW, owner=owner)
+        description = factory.make_name('error-description')
+        register_event = self.patch(node, '_register_request_event')
+        node.mark_broken(owner, description)
+        self.assertThat(register_event, MockCalledOnceWith(
+            owner, EVENT_TYPES.REQUEST_NODE_MARK_BROKEN, description))
+
     def test_mark_broken_releases_allocated_node(self):
-        node = factory.make_Node(
-            status=NODE_STATUS.ALLOCATED, owner=factory.make_User())
+        user = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=user)
         err_desc = factory.make_name('error-description')
-        release = self.patch(node, 'release')
-        node.mark_broken(err_desc)
+        release = self.patch(node, '_release')
+        node.mark_broken(user, err_desc)
         self.expectThat(node.owner, Is(None))
-        self.assertThat(release, MockCalledOnceWith())
+        self.assertThat(release, MockCalledOnceWith(user))
 
     def test_mark_fixed_sets_default_osystem_and_distro_series(self):
         node = factory.make_Node(status=NODE_STATUS.BROKEN)
         node.osystem = factory.make_name('osystem')
         node.distro_series = factory.make_name('distro_series')
-        node.mark_fixed()
+        node.mark_fixed(factory.make_User())
         expected_osystem = expected_distro_series = ''
         self.expectThat(expected_osystem, Equals(node.osystem))
         self.expectThat(expected_distro_series, Equals(node.distro_series))
 
     def test_mark_fixed_changes_status(self):
         node = factory.make_Node(status=NODE_STATUS.BROKEN)
-        node.mark_fixed()
+        node.mark_fixed(factory.make_User())
         self.assertEqual(NODE_STATUS.READY, reload_object(node).status)
 
+    def test_mark_fixed_logs_user_request(self):
+        owner = factory.make_User()
+        node = factory.make_Node(status=NODE_STATUS.BROKEN, owner=owner)
+        register_event = self.patch(node, '_register_request_event')
+        node.mark_fixed(owner)
+        self.assertThat(register_event, MockCalledOnceWith(
+            owner, EVENT_TYPES.REQUEST_NODE_MARK_FIXED, None))
+
     def test_mark_fixed_updates_error_description(self):
         description = factory.make_name('error-description')
         node = factory.make_Node(
             status=NODE_STATUS.BROKEN, error_description=description)
-        node.mark_fixed()
+        node.mark_fixed(factory.make_User())
         self.assertEqual('', reload_object(node).error_description)
 
     def test_mark_fixed_fails_if_node_isnt_broken(self):
         status = factory.pick_choice(
             NODE_STATUS_CHOICES, but_not=[NODE_STATUS.BROKEN])
         node = factory.make_Node(status=status)
-        self.assertRaises(NodeStateViolation, node.mark_fixed)
+        self.assertRaises(
+            NodeStateViolation, node.mark_fixed, factory.make_User())
 
     def test_mark_fixed_clears_installation_results(self):
         node = factory.make_Node(status=NODE_STATUS.BROKEN)
@@ -1863,7 +1979,7 @@
         self.assertEquals(
             [node_result], list(NodeResult.objects.filter(
                 node=node, result_type=RESULT_TYPE.INSTALLATION)))
-        node.mark_fixed()
+        node.mark_fixed(factory.make_User())
         self.assertEquals(
             [], list(NodeResult.objects.filter(
                 node=node, result_type=RESULT_TYPE.INSTALLATION)))
@@ -1946,9 +2062,17 @@
 
     def test_start_deployment_changes_state(self):
         node = factory.make_Node(status=NODE_STATUS.ALLOCATED)
-        node.start_deployment()
+        node._start_deployment()
         self.assertEqual(NODE_STATUS.DEPLOYING, reload_object(node).status)
 
+    def test_start_deployment_logs_user_request(self):
+        admin = factory.make_admin()
+        node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=admin)
+        register_event = self.patch(node, '_register_request_event')
+        node.start(admin)
+        self.assertThat(register_event, MockCalledOnceWith(
+            admin, EVENT_TYPES.REQUEST_NODE_START_DEPLOYMENT, None))
+
     def test_handle_monitor_expired_marks_node_as_failed(self):
         self.disable_node_query()
         status = random.choice(MONITORED_STATUSES)
@@ -2160,6 +2284,32 @@
         node.boot_interface = interface
         node.save()
 
+    def test__register_request_event_saves_event(self):
+        node = factory.make_Node()
+        user = factory.make_User()
+        log_mock = self.patch_autospec(
+            Event.objects, 'register_event_and_event_type')
+        event_name = EVENT_TYPES.REQUEST_NODE_START
+        event_details = EVENT_DETAILS[event_name]
+        comment = factory.make_name("comment")
+        event_description = "(%s) - %s" % (user.username, comment)
+        node._register_request_event(user, event_name, comment)
+        self.assertThat(log_mock, MockCalledOnceWith(
+            node.system_id,
+            EVENT_TYPES.REQUEST_NODE_START,
+            type_level=event_details.level,
+            type_description=event_details.description,
+            event_description=event_description))
+
+    def test__register_request_event_with_none_user_saves_no_event(self):
+        node = factory.make_Node()
+        log_mock = self.patch_autospec(
+            Event.objects, 'register_event_and_event_type')
+        event_name = EVENT_TYPES.REQUEST_NODE_START
+        comment = factory.make_name("comment")
+        node._register_request_event(None, event_name, comment)
+        self.assertThat(log_mock, MockNotCalled())
+
 
 class TestNodeIsBootInterfaceOnManagedInterface(MAASServerTestCase):
 
@@ -2433,8 +2583,8 @@
             'enable_disk_erasing_on_release', True)
         erase_mock = self.patch_autospec(node, 'start_disk_erasing')
         release_mock = self.patch_autospec(node, 'release')
-        node.release_or_erase()
-        self.assertThat(erase_mock, MockCalledOnceWith(owner))
+        node.release_or_erase(owner)
+        self.assertThat(erase_mock, MockCalledOnceWith(owner, None))
         self.assertThat(release_mock, MockNotCalled())
 
     def test_release_or_erase_releases_when_disabled(self):
@@ -2444,8 +2594,8 @@
             'enable_disk_erasing_on_release', False)
         erase_mock = self.patch_autospec(node, 'start_disk_erasing')
         release_mock = self.patch_autospec(node, 'release')
-        node.release_or_erase()
-        self.assertThat(release_mock, MockCalledOnceWith())
+        node.release_or_erase(owner)
+        self.assertThat(release_mock, MockCalledOnceWith(owner, None))
         self.assertThat(erase_mock, MockNotCalled())
 
 
@@ -3234,6 +3384,15 @@
             node._start_transition_monitor_async,
             MockCalledOnceWith(ANY, node.hostname))
 
+    def test_start_logs_user_request(self):
+        admin = factory.make_admin()
+        node = factory.make_Node(status=NODE_STATUS.NEW, owner=admin)
+        self.patch(node, '_start')
+        register_event = self.patch(node, '_register_request_event')
+        node.start(admin)
+        self.assertThat(register_event, MockCalledOnceWith(
+            admin, EVENT_TYPES.REQUEST_NODE_START, None))
+
     def test__raises_failures_when_power_action_fails(self):
         class PraiseBeToJTVException(Exception):
             """A nonsense exception for this test.
@@ -3373,6 +3532,16 @@
         self.assertRaises(PermissionDenied, node.stop, user)
         self.assertThat(power_off_node, MockNotCalled())
 
+    def test_stop_logs_user_request(self):
+        admin = factory.make_admin()
+        node = self.make_node_with_interface(admin)
+        self.patch_autospec(node_module, "power_off_node")
+        register_event = self.patch(node, '_register_request_event')
+        with post_commit_hooks:
+            node.stop(admin)
+        self.assertThat(register_event, MockCalledOnceWith(
+            admin, EVENT_TYPES.REQUEST_NODE_STOP, None))
+
     def test__allows_admin_to_stop_any_node(self):
         power_off_node = self.patch_autospec(node_module, "power_off_node")
         owner = factory.make_User()

=== modified file 'src/maasserver/node_action.py'
--- src/maasserver/node_action.py	2015-09-17 15:37:57 +0000
+++ src/maasserver/node_action.py	2015-09-17 19:23:21 +0000
@@ -410,7 +410,7 @@
     def execute(self):
         """See `NodeAction.execute`."""
         try:
-            self.node.release_or_erase()
+            self.node.release_or_erase(self.user)
             self.node.hwe_kernel = ""
             self.node.save()
         except RPC_EXCEPTIONS + (ExternalProcessError,) as exception:
@@ -437,8 +437,7 @@
 
     def execute(self):
         """See `NodeAction.execute`."""
-        self.node.mark_broken(
-            "Manually marked as broken by user '%s'" % self.user.username)
+        self.node.mark_broken(self.user, "via web interface")
 
 
 class MarkFixed(NodeAction):
@@ -459,7 +458,7 @@
             raise NodeActionError(
                 "Unable to be mark fixed because it has not been commissioned "
                 "successfully.")
-        self.node.mark_fixed()
+        self.node.mark_fixed(self.user)
 
     def has_commissioning_data(self):
         """Return True when the node is missing the required commissioning

=== modified file 'src/maasserver/rpc/nodes.py'
--- src/maasserver/rpc/nodes.py	2015-08-28 14:01:35 +0000
+++ src/maasserver/rpc/nodes.py	2015-09-17 19:23:21 +0000
@@ -58,7 +58,7 @@
     except Node.DoesNotExist:
         raise NoSuchNode.from_system_id(system_id)
     try:
-        node.mark_failed(error_description)
+        node.mark_failed(None, error_description)
     except exceptions.NodeStateViolation as e:
         raise NodeStateViolation(e)
 

=== modified file 'src/maasserver/tests/test_node_action.py'
--- src/maasserver/tests/test_node_action.py	2015-08-28 01:56:18 +0000
+++ src/maasserver/tests/test_node_action.py	2015-09-17 19:23:21 +0000
@@ -282,7 +282,7 @@
             interface=True, status=self.status,
             power_type='ether_wake', power_state=POWER_STATE.OFF)
         self.patch_autospec(node, 'start_transition_monitor')
-        node_start = self.patch(node, 'start')
+        node_start = self.patch(node, '_start')
         node_start.side_effect = lambda user, user_data: post_commit()
         admin = factory.make_admin()
         action = Commission(node, admin)
@@ -290,7 +290,7 @@
             action.execute()
         self.assertEqual(NODE_STATUS.COMMISSIONING, node.status)
         self.assertThat(
-            node_start, MockCalledOnceWith(admin, user_data=ANY))
+            node_start, MockCalledOnceWith(admin, ANY))
 
 
 class TestAbortAction(MAASTransactionServerTestCase):
@@ -301,7 +301,7 @@
             node = factory.make_Node(
                 status=NODE_STATUS.DISK_ERASING, owner=owner)
 
-        node_stop = self.patch_autospec(node, 'stop')
+        node_stop = self.patch_autospec(node, '_stop')
         # Return a post-commit hook from Node.stop().
         node_stop.side_effect = lambda user: post_commit()
 
@@ -326,7 +326,7 @@
             admin = factory.make_admin()
 
         self.patch_autospec(node, 'stop_transition_monitor')
-        node_stop = self.patch_autospec(node, 'stop')
+        node_stop = self.patch_autospec(node, '_stop')
         # Return a post-commit hook from Node.stop().
         node_stop.side_effect = lambda user: post_commit()
 
@@ -351,7 +351,7 @@
             admin = factory.make_admin()
 
         self.patch_autospec(node, 'stop_transition_monitor')
-        node_stop = self.patch_autospec(node, 'stop')
+        node_stop = self.patch_autospec(node, '_stop')
         # Return a post-commit hook from Node.stop().
         node_stop.side_effect = lambda user: post_commit()
 
@@ -701,7 +701,7 @@
             power_type='ipmi', power_state=POWER_STATE.ON,
             owner=user, power_parameters=params)
         self.patch(node, 'start_transition_monitor')
-        node_stop = self.patch_autospec(node, 'stop')
+        node_stop = self.patch_autospec(node, '_stop')
 
         Release(node, user).execute()
 
@@ -727,7 +727,7 @@
         self.assertTrue(action.is_permitted())
         action.execute()
         self.assertEqual(
-            "Manually marked as broken by user '%s'" % user.username,
+            "via web interface",
             reload_object(node).error_description
         )
 
@@ -831,8 +831,8 @@
 
     def patch_rpc_methods(self, node):
         exception = self.make_exception()
-        self.patch(node, 'start').side_effect = exception
-        self.patch(node, 'stop').side_effect = exception
+        self.patch(node, '_start').side_effect = exception
+        self.patch(node, '_stop').side_effect = exception
         self.patch_autospec(node, 'start_transition_monitor')
         self.patch_autospec(node, 'stop_transition_monitor')
 
@@ -852,8 +852,7 @@
         self.patch_rpc_methods(action.node)
         exception = self.assertRaises(NodeActionError, action.execute)
         self.assertEqual(
-            get_error_message_for_exception(
-                action.node.start.side_effect),
+            get_error_message_for_exception(action.node._start.side_effect),
             unicode(exception))
 
     def test_Abort_handles_rpc_errors(self):
@@ -862,8 +861,7 @@
         self.patch_rpc_methods(action.node)
         exception = self.assertRaises(NodeActionError, action.execute)
         self.assertEqual(
-            get_error_message_for_exception(
-                action.node.stop.side_effect),
+            get_error_message_for_exception(action.node._stop.side_effect),
             unicode(exception))
 
     def test_PowerOn_handles_rpc_errors(self):
@@ -871,8 +869,7 @@
         self.patch_rpc_methods(action.node)
         exception = self.assertRaises(NodeActionError, action.execute)
         self.assertEqual(
-            get_error_message_for_exception(
-                action.node.start.side_effect),
+            get_error_message_for_exception(action.node._start.side_effect),
             unicode(exception))
 
     def test_PowerOff_handles_rpc_errors(self):
@@ -880,8 +877,7 @@
         self.patch_rpc_methods(action.node)
         exception = self.assertRaises(NodeActionError, action.execute)
         self.assertEqual(
-            get_error_message_for_exception(
-                action.node.stop.side_effect),
+            get_error_message_for_exception(action.node._stop.side_effect),
             unicode(exception))
 
     def test_Release_handles_rpc_errors(self):
@@ -890,6 +886,5 @@
         self.patch_rpc_methods(action.node)
         exception = self.assertRaises(NodeActionError, action.execute)
         self.assertEqual(
-            get_error_message_for_exception(
-                action.node.stop.side_effect),
+            get_error_message_for_exception(action.node._stop.side_effect),
             unicode(exception))

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2015-09-17 14:39:49 +0000
+++ src/metadataserver/api.py	2015-09-17 19:23:21 +0000
@@ -355,14 +355,15 @@
             elif node.status == NODE_STATUS.DEPLOYING:
                 if result in ['FAIL', 'FAILURE']:
                     node.mark_failed(
-                        "Installation failed (refer to the installation log "
-                        "for more information).")
+                        None,
+                        "Installation failed (refer to the "
+                        "installation log for more information).")
             elif node.status == NODE_STATUS.DISK_ERASING:
                 if result == 'SUCCESS':
                     # disk erasing complete, release node.
                     node.release()
                 elif result in ['FAIL', 'FAILURE']:
-                    node.mark_failed("Failed to erase disks.")
+                    node.mark_failed(None, "Failed to erase disks.")
 
             # Deallocate the node if we enter any terminal state.
             if node.status in [
@@ -518,15 +519,16 @@
             self._store_installation_results(node, request)
             if status == SIGNAL_STATUS.FAILED:
                 node.mark_failed(
-                    "Installation failed (refer to the installation log "
-                    "for more information).")
+                    None,
+                    "Installation failed (refer to the "
+                    "installation log for more information).")
             target_status = None
         elif node.status == NODE_STATUS.DISK_ERASING:
             if status == SIGNAL_STATUS.OK:
                 # disk erasing complete, release node
                 node.release()
             elif status == SIGNAL_STATUS.FAILED:
-                node.mark_failed("Failed to erase disks.")
+                node.mark_failed(None, "Failed to erase disks.")
             target_status = None
 
         if target_status in (None, node.status):

=== modified file 'src/provisioningserver/events.py'
--- src/provisioningserver/events.py	2015-09-02 16:20:10 +0000
+++ src/provisioningserver/events.py	2015-09-17 19:23:21 +0000
@@ -64,6 +64,20 @@
     NODE_STATUS_EVENT = "NODE_STATUS_EVENT"
     NODE_COMMISSIONING_EVENT = "NODE_COMMISSIONING_EVENT"
     NODE_INSTALL_EVENT = "NODE_INSTALL_EVENT"
+    # Node user request events
+    REQUEST_NODE_START_COMMISSIONING = "REQUEST_NODE_START_COMMISSIONING"
+    REQUEST_NODE_ABORT_COMMISSIONING = "REQUEST_NODE_ABORT_COMMISSIONING"
+    REQUEST_NODE_ABORT_DEPLOYMENT = "REQUEST_NODE_ABORT_DEPLOYMENT"
+    REQUEST_NODE_ACQUIRE = "REQUEST_NODE_ACQUIRE"
+    REQUEST_NODE_ERASE_DISK = "REQUEST_NODE_ERASE_DISK"
+    REQUEST_NODE_ABORT_ERASE_DISK = "REQUEST_NODE_ABORT_ERASE_DISK"
+    REQUEST_NODE_RELEASE = "REQUEST_NODE_RELEASE"
+    REQUEST_NODE_MARK_FAILED = "REQUEST_NODE_MARK_FAILED"
+    REQUEST_NODE_MARK_BROKEN = "REQUEST_NODE_MARK_BROKEN"
+    REQUEST_NODE_MARK_FIXED = "REQUEST_NODE_MARK_FIXED"
+    REQUEST_NODE_START_DEPLOYMENT = "REQUEST_NODE_START_DEPLOYMENT"
+    REQUEST_NODE_START = "REQUEST_NODE_START"
+    REQUEST_NODE_STOP = "REQUEST_NODE_STOP"
 
 
 EventDetail = namedtuple("EventDetail", ("description", "level"))
@@ -127,6 +141,58 @@
         description="Node installation",
         level=DEBUG,
     ),
+    EVENT_TYPES.REQUEST_NODE_START_COMMISSIONING: EventDetail(
+        description="User starting node commissioning",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_ABORT_COMMISSIONING: EventDetail(
+        description="User aborting node commissioning",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_ABORT_DEPLOYMENT: EventDetail(
+        description="User aborting deployment",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_ACQUIRE: EventDetail(
+        description="User acquiring node",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_ERASE_DISK: EventDetail(
+        description="User erasing disk",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_ABORT_ERASE_DISK: EventDetail(
+        description="User aborting disk erase",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_RELEASE: EventDetail(
+        description="User releasing node",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_MARK_FAILED: EventDetail(
+        description="User marking node failed",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_MARK_BROKEN: EventDetail(
+        description="User marking node broken",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_MARK_FIXED: EventDetail(
+        description="User marking node fixed",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_START_DEPLOYMENT: EventDetail(
+        description="User starting deployment",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_START: EventDetail(
+        description="User powering up node",
+        level=INFO,
+    ),
+    EVENT_TYPES.REQUEST_NODE_STOP: EventDetail(
+        description="User powering down node",
+        level=INFO,
+    ),
 }