Merge lp:~ltrager/maas/lp1600328 into lp:~maas-committers/maas/trunk

[Lee Trager]

Add a node event when no rack controller can access the BMC and handle failed defers.

When no rack controller can be found to access the nodes BMC an exception is raised. This exception is being thrown in a call back. The 500 error was being shown because nothing caught the error. The API and websocket now silently ignore that exception as this is now logged as a node power event. The power event is explicitly shown in the UI and the node is kept in a new state.

[Gavin Panella (allenap)]

review: Approve

=== modified file 'src/maasserver/api/machines.py'
--- src/maasserver/api/machines.py	2016-10-20 08:41:30 +0000
+++ src/maasserver/api/machines.py	2016-10-22 05:50:38 +0000
@@ -941,7 +941,10 @@
         """
         machine = create_machine(request)
         if request.user.is_superuser:
-            machine.accept_enlistment(request.user)
+            d = machine.start_commissioning(request.user)
+            # Silently ignore errors to prevent 500 errors. The commissioning
+            # callbacks have their own logging. This fixes LP1600328.
+            d.addErrback(lambda _: None)
         return machine
 
     def _check_system_ids_exist(self, system_ids):

=== modified file 'src/maasserver/api/tests/test_machines.py'
--- src/maasserver/api/tests/test_machines.py	2016-10-03 22:13:11 +0000
+++ src/maasserver/api/tests/test_machines.py	2016-10-22 05:50:38 +0000
@@ -204,6 +204,28 @@
             "Select a valid choice. %s is not one of the "
             "available choices." % power_type, validation_errors[0])
 
+    def test_POST_handles_error_when_unable_to_access_bmc(self):
+        # Regression test for LP1600328
+        self.become_admin()
+        power_address = factory.make_ip_address()
+        power_id = factory.make_name('power_id')
+        response = self.client.post(
+            reverse('machines_handler'),
+            {
+                'architecture': make_usable_architecture(self),
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+                'power_type': 'virsh',
+                'power_parameters_power_address': power_address,
+                'power_parameters_power_id': power_id,
+            })
+        parsed_result = json.loads(
+            response.content.decode(settings.DEFAULT_CHARSET))
+        machine = Machine.objects.get(system_id=parsed_result['system_id'])
+        self.assertEqual('virsh', parsed_result['power_type'])
+        self.assertEqual(
+            power_address, machine.power_parameters['power_address'])
+        self.assertEqual(power_id, machine.power_parameters['power_id'])
+
     def test_GET_lists_machines(self):
         # The api allows for fetching the list of Machines.
         machine1 = factory.make_Node()

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-10-13 00:11:49 +0000
+++ src/maasserver/models/node.py	2016-10-22 05:50:38 +0000
@@ -3084,9 +3084,12 @@
             for rack in self.get_boot_rack_controllers()
         ]
         if len(client_idents) == 0 and len(fallback_idents) == 0:
-            raise PowerProblem(
-                "No rack controllers can access the BMC of node: %s" % (
-                    self.hostname))
+            err_msg = "No rack controllers can access the BMC of node: %s" % (
+                self.hostname)
+            self._register_request_event(
+                self.owner, EVENT_TYPES.NODE_POWER_QUERY_FAILED,
+                "Failed to query node's BMC", err_msg)
+            raise PowerProblem(err_msg)
         return client_idents, fallback_idents
 
     @transactional

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-10-18 17:35:20 +0000
+++ src/maasserver/models/tests/test_node.py	2016-10-22 05:50:38 +0000
@@ -51,7 +51,10 @@
     POWER_STATE,
     SERVICE_STATUS,
 )
-from maasserver.exceptions import NodeStateViolation
+from maasserver.exceptions import (
+    NodeStateViolation,
+    PowerProblem,
+)
 from maasserver.models import (
     bmc as bmc_module,
     BondInterface,
@@ -61,6 +64,7 @@
     Controller,
     Device,
     Domain,
+    EventType,
     Fabric,
     Interface,
     LicenseKey,
@@ -5026,6 +5030,37 @@
              "deploy this node."], node.storage_layout_issues())
 
 
+class TestGetBMCClientConnectionInfo(MAASServerTestCase):
+
+    def test__returns_bmc_identifiers(self):
+        node = factory.make_Node()
+
+        mock_bmcs = self.patch(node.bmc, 'get_client_identifiers')
+        fake_bmc_id = factory.make_name('system_id')
+        mock_bmcs.return_value = [fake_bmc_id]
+
+        mock_fallbacks = self.patch(node, 'get_boot_rack_controllers')
+        fake_fallback_id = factory.make_name('system_id')
+        fallback = MagicMock()
+        fallback.system_id = fake_fallback_id
+        mock_fallbacks.return_value = [fallback]
+
+        self.assertEquals(
+            ([fake_bmc_id], [fake_fallback_id]),
+            node._get_bmc_client_connection_info())
+
+    def test__creates_event_on_error(self):
+        node = factory.make_Node()
+
+        self.assertRaises(PowerProblem, node._get_bmc_client_connection_info)
+        event_type = EventType.objects.get(
+            name=EVENT_TYPES.NODE_POWER_QUERY_FAILED)
+        event = node.event_set.get(type=event_type)
+        self.assertEquals(
+            ('No rack controllers can access the BMC of node: %s' %
+             node.hostname), event.description)
+
+
 class TestNode_Stop(MAASServerTestCase):
     """Tests for Node.stop()."""
 

=== modified file 'src/maasserver/websockets/handlers/machine.py'
--- src/maasserver/websockets/handlers/machine.py	2016-10-21 01:19:51 +0000
+++ src/maasserver/websockets/handlers/machine.py	2016-10-22 05:50:38 +0000
@@ -265,7 +265,10 @@
 
         # Start the commissioning process right away, which has the
         # desired side effect of initializing the node's power state.
-        node_obj.start_commissioning(self.user)
+        d = node_obj.start_commissioning(self.user)
+        # Silently ignore errors to prevent tracebacks. The commissioning
+        # callbacks have their own logging. This fixes LP1600328.
+        d.addErrback(lambda _: None)
 
         return self.full_dehydrate(node_obj)