Clean up error handling in the emulator

* Make sure we always return a redfish-style response
* Don't excessively re-raise exceptions
* Use NotFound/BadRequest initially instead of generic FishyError

Change-Id: I313d79b35ad7a09615d6f0ec5764471b50a1609b

sushy_tools/emulator/main.py

@@ -36,7 +36,6 @@ from sushy_tools.emulator.resources.systems import novadriver
 from sushy_tools.emulator.resources import vmedia as vmddriver
 from sushy_tools.emulator.resources import volumes as voldriver
 from sushy_tools import error
-from sushy_tools.error import FishyError
 
 
 def _render_error(message):
@@ -177,7 +176,7 @@ def ensure_instance_access(decorated_func):
     @functools.wraps(decorated_func)
     def decorator(*args, **kwargs):
         if instance_denied(**kwargs):
-            raise FishyError('Error finding instance')
+            raise error.NotFound()
 
         return decorated_func(*args, **kwargs)
 
@@ -340,11 +339,7 @@ def jsonify(obj_type, obj_version, obj):
 def manager_resource(identity):
     app.logger.debug('Serving resources for manager "%s"', identity)
 
-    try:
     manager = app.managers.get_manager(identity)
-    except error.FishyError as exc:
-        return str(exc), 404
-
     systems = app.managers.get_managed_systems(manager)
     chassis = app.managers.get_managed_chassis(manager)
 
@@ -404,7 +399,6 @@ def virtual_media_collection_resource(identity):
            methods=['GET'])
 @returns_json
 def virtual_media_resource(identity, device):
-    try:
     device_name = app.vmedia.get_device_name(
         identity, device)
 
@@ -413,12 +407,6 @@ def virtual_media_resource(identity, device):
 
     device_info = app.vmedia.get_device_image_info(identity, device)
 
-    except error.FishyError as ex:
-        app.logger.warning(
-            'Virtual media %s at manager %s error: '
-            '%s', device, identity, ex)
-        return 'Not found', 404
-
     app.logger.debug('Serving virtual media %s at '
                      'manager "%s"', device, identity)
 
@@ -454,14 +442,7 @@ def virtual_media_patch(identity, device):
         if not isinstance(verify, bool):
             raise error.BadRequest("VerifyCertificate must be a boolean")
 
-        try:
         app.vmedia.update_device_info(identity, device, verify=verify)
-        except error.FishyError as ex:
-            app.logger.warning(
-                'Virtual media %s at manager %s error: '
-                '%s', device, identity, ex)
-            raise error.NotFound("Virtual media device not found")
-
         return '', 204
     else:
         raise error.BadRequest("Empty or malformed patch")
@@ -670,7 +651,7 @@ def ethernet_interface(identity, nic_id):
             return flask.render_template(
                 'ethernet_interface.json', identity=identity, nic=nic)
 
-    return 'Not found', 404
+    raise error.NotFound()
 
 
 @app.route('/redfish/v1/Systems/<identity>/Processors',
@@ -697,7 +678,7 @@ def processor(identity, processor_id):
             return flask.render_template(
                 'processor.json', identity=identity, processor=proc)
 
-    return 'Not found', 404
+    raise error.NotFound()
 
 
 @app.route('/redfish/v1/Systems/<identity>/Actions/ComputerSystem.Reset',
@@ -791,7 +772,7 @@ def simple_storage(identity, simple_storage_id):
         storage_controller = simple_storage_controllers[simple_storage_id]
     except KeyError:
         app.logger.debug('"%s" Simple Storage resource was not found')
-        return 'Not found', 404
+        raise error.NotFound()
     return flask.render_template('simple_storage.json', identity=identity,
                                  simple_storage=storage_controller)
 
@@ -823,7 +804,7 @@ def storage(identity, storage_id):
             return flask.render_template(
                 'storage.json', identity=identity, storage=stg)
 
-    return 'Not found', 404
+    raise error.NotFound()
 
 
 @app.route('/redfish/v1/Systems/<identity>/Storage/<stg_id>/Drives/<drv_id>',
@@ -839,7 +820,7 @@ def drive_resource(identity, stg_id, drv_id):
             return flask.render_template(
                 'drive.json', identity=identity, storage_id=stg_id, drive=drv)
 
-    return 'Not found', 404
+    raise error.NotFound()
 
 
 @app.route('/redfish/v1/Systems/<identity>/Storage/<storage_id>/Volumes',
@@ -902,7 +883,7 @@ def volume(identity, stg_id, vol_id):
                     'volume.json', identity=identity, storage_id=stg_id,
                     volume=vol)
 
-    return 'Not Found', 404
+    raise error.NotFound()
 
 
 @app.route('/redfish/v1/Registries')

sushy_tools/emulator/resources/managers.py

@@ -26,16 +26,18 @@ class FakeDriver(base.DriverBase):
         """Get a manager by its identity
 
         :returns: Redfish manager UUID.
+        :raises: NotFound if the manager cannot be found
         """
         try:
             system_uuid = self._systems.uuid(identity)
             system_name = self._systems.name(identity)
         except error.AliasAccessError:
             raise
-        except error.FishyError:
+        except error.NotFound:
+            # Re-raise hiding the fact that managers are backed by systems
             msg = 'Manager with UUID %s was not found' % identity
             self._logger.error(msg)
-            raise error.FishyError(msg)
+            raise error.NotFound(msg)
         else:
             result = {'Id': system_uuid,
                       'UUID': system_uuid,

sushy_tools/emulator/resources/systems/libvirtdriver.py

@@ -185,7 +185,7 @@ class LibvirtDriver(AbstractSystemsDriver):
 
                     self._logger.debug(msg)
 
-                    raise error.FishyError(msg)
+                    raise error.NotFound(msg)
 
             raise error.AliasAccessError(domain.UUIDString())
 
@@ -213,7 +213,7 @@ class LibvirtDriver(AbstractSystemsDriver):
         in place of system name if there are duplicates.
 
         :param identity: libvirt domain name or UUID
-
+        :raises: NotFound if the system cannot be found
         :returns: computer system UUID
         """
         domain = self._get_domain(identity, readonly=True)
@@ -223,7 +223,7 @@ class LibvirtDriver(AbstractSystemsDriver):
         """Get computer system name by name
 
         :param identity: libvirt domain name or UUID
-
+        :raises: NotFound if the system cannot be found
         :returns: computer system name
         """
         domain = self._get_domain(identity, readonly=True)
@@ -501,7 +501,7 @@ class LibvirtDriver(AbstractSystemsDriver):
             msg = ('Unknown boot mode requested: '
                    '%(boot_mode)s' % {'boot_mode': boot_mode})
 
-            raise error.FishyError(msg)
+            raise error.BadRequest(msg)
 
         os_elements = tree.findall('os')
         if len(os_elements) != 1:
@@ -939,7 +939,7 @@ class LibvirtDriver(AbstractSystemsDriver):
                 lv_device = self.BOOT_DEVICE_MAP[device]
 
             except KeyError:
-                raise error.FishyError(
+                raise error.BadRequest(
                     'Unknown device %s at %s' % (device, identity))
 
             disk_elements = device_element.findall('disk')
@@ -1026,7 +1026,7 @@ class LibvirtDriver(AbstractSystemsDriver):
             lv_device = self.BOOT_DEVICE_MAP[device]
 
         except KeyError:
-            raise error.FishyError(
+            raise error.BadRequest(
                 'Unknown device %s at %s' % (device, identity))
 
         device_element = domain_tree.find('devices')

sushy_tools/emulator/resources/systems/novadriver.py

@@ -76,7 +76,7 @@ class OpenStackDriver(AbstractSystemsDriver):
 
         self._logger.debug(msg)
 
-        raise error.FishyError(msg)
+        raise error.NotFound(msg)
 
     @memoize.memoize(permanent_cache=PERMANENT_CACHE)
     def _get_flavor(self, identity):
@@ -188,7 +188,7 @@ class OpenStackDriver(AbstractSystemsDriver):
         # NOTE(etingof) can't support `state == "Nmi"` as
         # openstacksdk does not seem to support that
         else:
-            raise error.FishyError(
+            raise error.BadRequest(
                 'Unknown ResetType "%(state)s"' % {'state': state})
 
     def get_boot_device(self, identity):
@@ -234,7 +234,7 @@ class OpenStackDriver(AbstractSystemsDriver):
             msg = ('Unknown power state requested: '
                    '%(boot_source)s' % {'boot_source': boot_source})
 
-            raise error.FishyError(msg)
+            raise error.BadRequest(msg)
 
         # NOTE(etingof): the following probably only works with
         # libvirt-backed compute nodes
@@ -271,7 +271,7 @@ class OpenStackDriver(AbstractSystemsDriver):
         msg = ('The cloud driver %(driver)s does not allow changing boot '
                'mode through Redfish' % {'driver': self.driver})
 
-        raise error.FishyError(msg)
+        raise error.NotSupportedError(msg)
 
     def get_total_memory(self, identity):
         """Get computer system total memory
@@ -307,17 +307,17 @@ class OpenStackDriver(AbstractSystemsDriver):
 
     def get_bios(self, identity):
         """Not supported as Openstack SDK does not expose API for BIOS"""
-        raise error.FishyError(
+        raise error.NotSupportedError(
             'Operation not supported by the virtualization driver')
 
     def set_bios(self, identity, attributes):
         """Not supported as Openstack SDK does not expose API for BIOS"""
-        raise error.FishyError(
+        raise error.NotSupportedError(
             'Operation not supported by the virtualization driver')
 
     def reset_bios(self, identity):
         """Not supported as Openstack SDK does not expose API for BIOS"""
-        raise error.FishyError(
+        raise error.NotSupportedError(
             'Operation not supported by the virtualization driver')
 
     def get_nics(self, identity):

sushy_tools/emulator/resources/vmedia.py

@@ -76,7 +76,7 @@ class StaticDriver(base.DriverBase):
             return self._devices[(identity, device)]
 
         except KeyError:
-            raise error.FishyError(
+            raise error.NotFound(
                 'No such virtual media device %s owned by resource '
                 '%s' % (device, identity))
 

sushy_tools/error.py

@@ -33,7 +33,7 @@ class NotSupportedError(FishyError):
 class NotFound(FishyError):
     """Entity not found."""
 
-    def __init__(self, msg, code=404):
+    def __init__(self, msg='Not found', code=404):
         super().__init__(msg, code)
 
 

sushy_tools/tests/unit/emulator/test_main.py

@@ -536,7 +536,7 @@ class VirtualMediaTestCase(EmulatorTestCase):
         self.assertFalse(response.json['VerifyCertificate'])
 
     def test_virtual_media_not_found(self, managers_mock, vmedia_mock):
-        vmedia_mock.return_value.get_device_name.side_effect = error.FishyError
+        vmedia_mock.return_value.get_device_name.side_effect = error.NotFound
 
         response = self.app.get(
             '/redfish/v1/Managers/%s/VirtualMedia/DVD-ROM' % self.uuid)
@@ -555,7 +555,7 @@ class VirtualMediaTestCase(EmulatorTestCase):
 
     def test_virtual_media_update_not_found(self, managers_mock, vmedia_mock):
         vmedia_mock = vmedia_mock.return_value
-        vmedia_mock.update_device_info.side_effect = error.FishyError
+        vmedia_mock.update_device_info.side_effect = error.NotFound
 
         response = self.app.patch(
             '/redfish/v1/Managers/%s/VirtualMedia/DVD-ROM' % self.uuid,