Merge "Raise HTTP 400 rather than 500 error"

ironic/conductor/manager.py

@@ -2389,7 +2389,7 @@ class ConductorManager(base_manager.BaseConductorManager):
     @METRICS.timer('ConductorManager.inspect_hardware')
     @messaging.expected_exceptions(exception.NoFreeConductorWorker,
                                    exception.NodeLocked,
-                                   exception.HardwareInspectionFailure,
+                                   exception.InvalidParameterValue,
                                    exception.InvalidStateRequested,
                                    exception.UnsupportedDriverExtension)
     def inspect_hardware(self, context, node_id):
@@ -2406,8 +2406,10 @@ class ConductorManager(base_manager.BaseConductorManager):
                  support inspect.
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task
-        :raises: HardwareInspectionFailure when unable to get
+        :raises: InvalidParameterValue when unable to get
                  essential scheduling properties from hardware.
+        :raises: MissingParameterValue when required
+                 information is not found.
         :raises: InvalidStateRequested if 'inspect' is not a
                  valid action to do in the current state.
 
@@ -2419,14 +2421,8 @@ class ConductorManager(base_manager.BaseConductorManager):
                 raise exception.UnsupportedDriverExtension(
                     driver=task.node.driver, extension='inspect')
 
-            try:
             task.driver.power.validate(task)
             task.driver.inspect.validate(task)
-            except exception.InvalidParameterValue as e:
-                error = (_("Failed to validate inspection or power info. "
-                           "Error: %(msg)s")
-                         % {'msg': e})
-                raise exception.HardwareInspectionFailure(error=error)
 
             try:
                 task.process_event(

ironic/tests/unit/api/v1/test_nodes.py

@@ -3030,6 +3030,32 @@ class TestPut(test_api_base.BaseApiTest):
                             expect_errors=True)
         self.assertEqual(http_client.CONFLICT, ret.status_code)  # Conflict
 
+    def test_inspect_validation_failed_status_code(self):
+        self.mock_dnih.side_effect = exception.InvalidParameterValue(
+            err='Failed to validate inspection or power info.')
+        node = self.node
+        node.provision_state = states.MANAGEABLE
+        node.reservation = 'fake-host'
+        node.save()
+        ret = self.put_json('/nodes/%s/states/provision' % node.uuid,
+                            {'target': 'inspect'},
+                            headers={api_base.Version.string: "1.6"},
+                            expect_errors=True)
+        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)
+
+    def test_inspect_validation_failed_missing_parameter_value(self):
+        self.mock_dnih.side_effect = exception.MissingParameterValue(
+            err='Failed to validate inspection or power info.')
+        node = self.node
+        node.provision_state = states.MANAGEABLE
+        node.reservation = 'fake-host'
+        node.save()
+        ret = self.put_json('/nodes/%s/states/provision' % node.uuid,
+                            {'target': 'inspect'},
+                            headers={api_base.Version.string: "1.6"},
+                            expect_errors=True)
+        self.assertEqual(http_client.BAD_REQUEST, ret.status_code)
+
     @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
     def test_manage_from_available(self, mock_dpa):
         self.node.provision_state = states.AVAILABLE

ironic/tests/unit/conductor/test_manager.py

@@ -5280,13 +5280,22 @@ class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertIsNone(node.reservation)
 
     def _test_inspect_hardware_validate_fail(self, mock_validate):
-        mock_validate.side_effect = exception.InvalidParameterValue('error')
+        mock_validate.side_effect = exception.InvalidParameterValue(
+            'Fake error message')
         node = obj_utils.create_test_node(self.context, driver='fake')
         exc = self.assertRaises(messaging.rpc.ExpectedException,
                                 self.service.inspect_hardware,
                                 self.context, node.uuid)
         # Compare true exception hidden by @messaging.expected_exceptions
-        self.assertEqual(exception.HardwareInspectionFailure, exc.exc_info[0])
+        self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0])
+
+        mock_validate.side_effect = exception.MissingParameterValue(
+            'Fake error message')
+        exc = self.assertRaises(messaging.rpc.ExpectedException,
+                                self.service.inspect_hardware,
+                                self.context, node.uuid)
+        self.assertEqual(exception.MissingParameterValue, exc.exc_info[0])
+
         # This is a sync operation last_error should be None.
         self.assertIsNone(node.last_error)
         # Verify reservation has been cleared.

releasenotes/notes/raise-bad-request-exception-on-validating-inspection-failure-57d7fd2999cf4ecf.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Raise HTTP 400 Bad request on failure to validate
+    power or inspect interface parameters before inspecting.