exception from driver_factory.default_interface()

This changes driver_factory.default_interface() so that instead
of returning None if there is no calculated default interface,
it raises exception.NoValidDefaultForInterface.

This is a follow up to 6206c47720.

Change-Id: I0c3d5d75b5a37af02f3660968cf3f2c669e52019
Partial-Bug: #1524745

ironic/common/driver_factory.py

@@ -142,7 +142,8 @@ def get_interface(driver_or_hw_type, interface_type, interface_name):
     return impl_instance
 
 
-def default_interface(driver_or_hw_type, interface_type):
+def default_interface(driver_or_hw_type, interface_type,
+                      driver_name=None, node=None):
     """Calculate and return the default interface implementation.
 
     Finds the first implementation that is supported by the hardware type
@@ -150,9 +151,13 @@ def default_interface(driver_or_hw_type, interface_type):
 
     :param driver_or_hw_type: classic driver or hardware type instance object.
     :param interface_type: type of the interface (e.g. 'boot').
-    :returns: an entrypoint name of the calculated default implementation
+    :param driver_name: entrypoint name of the driver_or_hw_type object. Is
-              or None if no default implementation can be found.
+                        used for exception message.
+    :param node: the identifier of a node. If specified, is used for exception
+                 message.
+    :returns: an entrypoint name of the calculated default implementation.
     :raises: InterfaceNotFoundInEntrypoint if the entry point was not found.
+    :raises: NoValidDefaultForInterface if no default interface can be found.
     """
     factory = _INTERFACE_LOADERS[interface_type]
     is_hardware_type = isinstance(driver_or_hw_type,
@@ -185,6 +190,21 @@ def default_interface(driver_or_hw_type, interface_type):
             except KeyError:
                 continue
 
+    if impl_name is None:
+        # NOTE(rloo). No i18n on driver_type_str because translating substrings
+        #             on their own may cause the final string to look odd.
+        if is_hardware_type:
+            driver_type_str = 'hardware type'
+        else:
+            driver_type_str = 'driver'
+        driver_name = driver_name or driver_or_hw_type.__class__.__name__
+        node_info = ""
+        if node is not None:
+            node_info = _(' node %s with') % node
+        raise exception.NoValidDefaultForInterface(
+            interface_type=interface_type, driver_type=driver_type_str,
+            driver=driver_name, node_info=node_info)
+
     return impl_name
 
 
@@ -234,11 +254,8 @@ def check_and_update_node_interfaces(node, driver_or_hw_type=None):
                 # Not changing the result, proceeding with the next interface
                 continue
 
-        impl_name = default_interface(driver_or_hw_type, iface)
+        impl_name = default_interface(driver_or_hw_type, iface,
-
+                                      driver_name=node.driver, node=node.uuid)
-        if impl_name is None:
-            raise exception.NoValidDefaultForInterface(
-                interface_type=iface, node=node.uuid, driver=node.driver)
 
         # Set the calculated default and set result to True
         setattr(node, field_name, impl_name)

ironic/common/exception.py

@@ -343,8 +343,11 @@ class IncompatibleInterface(InvalidParameterValue):
 
 
 class NoValidDefaultForInterface(InvalidParameterValue):
-    _msg_fmt = _("No default value found for %(interface_type)s interface "
+    # NOTE(rloo): in the line below, there is no blank space after 'For'
-                 "for node %(node)s with driver or hardware type %(driver)s.")
+    #             because node_info could be an empty string. If node_info
+    #             is not empty, it should start with a space.
+    _msg_fmt = _("For%(node_info)s %(driver_type)s '%(driver)s', no default "
+                 "value found for %(interface_type)s interface.")
 
 
 class ImageNotFound(NotFound):

ironic/conductor/base_manager.py

@@ -274,10 +274,7 @@ class BaseConductorManager(object):
             interface_map = driver_factory.enabled_supported_interfaces(ht)
             for interface_type, interface_names in interface_map.items():
                 default_interface = driver_factory.default_interface(
-                    ht, interface_type)
+                    ht, interface_type, driver_name=ht_name)
-                if default_interface is None:
-                    raise exception.NoValidDefaultForInterface(
-                        interface_type=interface_type, node=None, driver=ht)
                 self.conductor.register_hardware_interfaces(ht_name,
                                                             interface_type,
                                                             interface_names,

ironic/conductor/manager.py

@@ -136,6 +136,8 @@ class ConductorManager(base_manager.BaseConductorManager):
 
         :param context: an admin context
         :param node_obj: a changed (but not saved) node object.
+        :raises: NoValidDefaultForInterface if no default can be calculated
+                 for some interfaces, and explicit values must be provided.
 
         """
         node_id = node_obj.uuid
@@ -334,6 +336,7 @@ class ConductorManager(base_manager.BaseConductorManager):
                                    exception.InvalidParameterValue,
                                    exception.UnsupportedDriverExtension,
                                    exception.DriverNotFound,
+                                   exception.NoValidDefaultForInterface,
                                    exception.InterfaceNotFoundInEntrypoint)
     def driver_vendor_passthru(self, context, driver_name, driver_method,
                                http_method, info):
@@ -362,6 +365,9 @@ class ConductorManager(base_manager.BaseConductorManager):
         :raises: DriverNotFound if the supplied driver is not loaded.
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task.
+        :raises: NoValidDefaultForInterface if no default interface
+                 implementation can be found for this driver's vendor
+                 interface.
         :raises: InterfaceNotFoundInEntrypoint if the default interface for a
                  hardware type is invalid.
         :returns: A dictionary containing:
@@ -381,17 +387,16 @@ class ConductorManager(base_manager.BaseConductorManager):
         driver = driver_factory.get_driver_or_hardware_type(driver_name)
         vendor = None
         if isinstance(driver, hardware_type.AbstractHardwareType):
-            vendor_name = driver_factory.default_interface(driver, 'vendor')
+            vendor_name = driver_factory.default_interface(
-            if vendor_name is not None:
+                driver, 'vendor', driver_name=driver_name)
-                vendor = driver_factory.get_interface(driver, 'vendor',
+            vendor = driver_factory.get_interface(driver, 'vendor',
-                                                      vendor_name)
+                                                  vendor_name)
         else:
             vendor = getattr(driver, 'vendor', None)
-
+            if not vendor:
-        if not vendor:
+                raise exception.UnsupportedDriverExtension(
-            raise exception.UnsupportedDriverExtension(
+                    driver=driver_name,
-                driver=driver_name,
+                    extension='vendor interface')
-                extension='vendor interface')
 
         try:
             vendor_opts = vendor.driver_routes[driver_method]
@@ -449,6 +454,7 @@ class ConductorManager(base_manager.BaseConductorManager):
     @METRICS.timer('ConductorManager.get_driver_vendor_passthru_methods')
     @messaging.expected_exceptions(exception.UnsupportedDriverExtension,
                                    exception.DriverNotFound,
+                                   exception.NoValidDefaultForInterface,
                                    exception.InterfaceNotFoundInEntrypoint)
     def get_driver_vendor_passthru_methods(self, context, driver_name):
         """Retrieve information about vendor methods of the given driver.
@@ -460,6 +466,9 @@ class ConductorManager(base_manager.BaseConductorManager):
         :raises: UnsupportedDriverExtension if current driver does not have
                  vendor interface.
         :raises: DriverNotFound if the supplied driver is not loaded.
+        :raises: NoValidDefaultForInterface if no default interface
+                 implementation can be found for this driver's vendor
+                 interface.
         :raises: InterfaceNotFoundInEntrypoint if the default interface for a
                  hardware type is invalid.
         :returns: dictionary of <method name>:<method metadata> entries.
@@ -472,17 +481,16 @@ class ConductorManager(base_manager.BaseConductorManager):
         driver = driver_factory.get_driver_or_hardware_type(driver_name)
         vendor = None
         if isinstance(driver, hardware_type.AbstractHardwareType):
-            vendor_name = driver_factory.default_interface(driver, 'vendor')
+            vendor_name = driver_factory.default_interface(
-            if vendor_name is not None:
+                driver, 'vendor', driver_name=driver_name)
-                vendor = driver_factory.get_interface(driver, 'vendor',
+            vendor = driver_factory.get_interface(driver, 'vendor',
-                                                      vendor_name)
+                                                  vendor_name)
         else:
             vendor = getattr(driver, 'vendor', None)
-
+            if not vendor:
-        if not vendor:
+                raise exception.UnsupportedDriverExtension(
-            raise exception.UnsupportedDriverExtension(
+                    driver=driver_name,
-                driver=driver_name,
+                    extension='vendor interface')
-                extension='vendor interface')
 
         return get_vendor_passthru_metadata(vendor.driver_routes)
 
@@ -2380,6 +2388,7 @@ class ConductorManager(base_manager.BaseConductorManager):
 
     @METRICS.timer('ConductorManager.get_raid_logical_disk_properties')
     @messaging.expected_exceptions(exception.UnsupportedDriverExtension,
+                                   exception.NoValidDefaultForInterface,
                                    exception.InterfaceNotFoundInEntrypoint)
     def get_raid_logical_disk_properties(self, context, driver_name):
         """Get the logical disk properties for RAID configuration.
@@ -2392,6 +2401,9 @@ class ConductorManager(base_manager.BaseConductorManager):
         :param driver_name: name of the driver
         :raises: UnsupportedDriverExtension, if the driver doesn't
             support RAID configuration.
+        :raises: NoValidDefaultForInterface if no default interface
+                 implementation can be found for this driver's RAID
+                 interface.
         :raises: InterfaceNotFoundInEntrypoint if the default interface for a
                  hardware type is invalid.
         :returns: A dictionary containing the properties and a textual
@@ -2403,16 +2415,15 @@ class ConductorManager(base_manager.BaseConductorManager):
         driver = driver_factory.get_driver_or_hardware_type(driver_name)
         raid_iface = None
         if isinstance(driver, hardware_type.AbstractHardwareType):
-            raid_iface_name = driver_factory.default_interface(driver, 'raid')
+            raid_iface_name = driver_factory.default_interface(
-            if raid_iface_name is not None:
+                driver, 'raid', driver_name=driver_name)
-                raid_iface = driver_factory.get_interface(driver, 'raid',
+            raid_iface = driver_factory.get_interface(driver, 'raid',
-                                                          raid_iface_name)
+                                                      raid_iface_name)
         else:
             raid_iface = getattr(driver, 'raid', None)
-
+            if not raid_iface:
-        if not raid_iface:
+                raise exception.UnsupportedDriverExtension(
-            raise exception.UnsupportedDriverExtension(
+                    driver=driver_name, extension='raid')
-                driver=driver_name, extension='raid')
 
         return raid_iface.get_logical_disk_properties()
 

ironic/conductor/rpcapi.py

@@ -158,6 +158,8 @@ class ConductorAPI(object):
         :returns: created node object.
         :raises: InterfaceNotFoundInEntrypoint if validation fails for any
                  dynamic interfaces (e.g. network_interface).
+        :raises: NoValidDefaultForInterface if no default can be calculated
+                 for some interfaces, and explicit values must be provided.
         """
         cctxt = self.client.prepare(topic=topic or self.topic, version='1.36')
         return cctxt.call(context, 'create_node', node_obj=node_obj)
@@ -178,6 +180,8 @@ class ConductorAPI(object):
         :param node_obj: a changed (but not saved) node object.
         :param topic: RPC topic. Defaults to self.topic.
         :returns: updated node object, including all fields.
+        :raises: NoValidDefaultForInterface if no default can be calculated
+                 for some interfaces, and explicit values must be provided.
 
         """
         cctxt = self.client.prepare(topic=topic or self.topic, version='1.1')
@@ -268,6 +272,9 @@ class ConductorAPI(object):
                  async task.
         :raises: InterfaceNotFoundInEntrypoint if the default interface for a
                  hardware type is invalid.
+        :raises: NoValidDefaultForInterface if no default interface
+                 implementation can be found for this driver's vendor
+                 interface.
         :returns: A dictionary containing:
 
             :return: The response of the invoked vendor method
@@ -311,6 +318,9 @@ class ConductorAPI(object):
         :raises: DriverNotFound if the supplied driver is not loaded.
         :raises: InterfaceNotFoundInEntrypoint if the default interface for a
                  hardware type is invalid.
+        :raises: NoValidDefaultForInterface if no default interface
+                 implementation can be found for this driver's vendor
+                 interface.
         :returns: dictionary of <method name>:<method metadata> entries.
 
         """
@@ -675,6 +685,9 @@ class ConductorAPI(object):
             support RAID configuration.
         :raises: InterfaceNotFoundInEntrypoint if the default interface for a
                  hardware type is invalid.
+        :raises: NoValidDefaultForInterface if no default interface
+                 implementation can be found for this driver's RAID
+                 interface.
         :returns: A dictionary containing the properties that can be mentioned
             for logical disks and a textual description for them.
         """

ironic/drivers/hardware_type.py

@@ -20,6 +20,7 @@ import abc
 
 import six
 
+from ironic.common import exception
 from ironic.drivers import base as driver_base
 from ironic.drivers.modules.network import noop as noop_net
 from ironic.drivers.modules import noop
@@ -106,9 +107,14 @@ class AbstractHardwareType(object):
 
         properties = {}
         for iface_type in driver_base.ALL_INTERFACES:
-            default_iface = driver_factory.default_interface(self, iface_type)
+            try:
-            if default_iface is not None:
+                default_iface = driver_factory.default_interface(self,
-                iface = driver_factory.get_interface(self, iface_type,
+                                                                 iface_type)
-                                                     default_iface)
+            except (exception.InterfaceNotFoundInEntrypoint,
-                properties.update(iface.get_properties())
+                    exception.NoValidDefaultForInterface):
+                continue
+
+            iface = driver_factory.get_interface(self, iface_type,
+                                                 default_iface)
+            properties.update(iface.get_properties())
         return properties

ironic/tests/unit/common/test_driver_factory.py

@@ -310,8 +310,35 @@ class DefaultInterfaceTestCase(db_base.DbTestCase):
         # manual-management supports no power interfaces
         self.config(default_power_interface=None)
         self.config(enabled_power_interfaces=[])
-        iface = driver_factory.default_interface(self.driver, 'power')
+        self.assertRaisesRegex(
-        self.assertIsNone(iface)
+            exception.NoValidDefaultForInterface,
+            "For hardware type 'ManualManagementHardware', no default "
+            "value found for power interface.",
+            driver_factory.default_interface, self.driver, 'power')
+
+    def test_calculated_no_answer_drivername(self):
+        # manual-management instance (of entry-point driver named 'foo')
+        # supports no power interfaces
+        self.config(default_power_interface=None)
+        self.config(enabled_power_interfaces=[])
+        self.assertRaisesRegex(
+            exception.NoValidDefaultForInterface,
+            "For hardware type 'foo', no default value found for power "
+            "interface.",
+            driver_factory.default_interface, self.driver, 'power',
+            driver_name='foo')
+
+    def test_calculated_no_answer_drivername_node(self):
+        # for a node with manual-management instance (of entry-point driver
+        # named 'foo'), no default power interface is supported
+        self.config(default_power_interface=None)
+        self.config(enabled_power_interfaces=[])
+        self.assertRaisesRegex(
+            exception.NoValidDefaultForInterface,
+            "For node bar with hardware type 'foo', no default "
+            "value found for power interface.",
+            driver_factory.default_interface, self.driver, 'power',
+            driver_name='foo', node='bar')
 
 
 class TestFakeHardware(hardware_type.AbstractHardwareType):
@@ -502,6 +529,16 @@ class HardwareTypeLoadTestCase(db_base.DbTestCase):
                           driver_factory.check_and_update_node_interfaces,
                           node)
 
+    def test_no_raid_interface_no_default(self):
+        # NOTE(rloo): It doesn't seem possible to not have a default interface
+        #             for storage, so we'll test this case with raid.
+        self.config(enabled_raid_interfaces=[])
+        node = obj_utils.get_test_node(self.context, driver='fake-hardware')
+        self.assertRaisesRegex(
+            exception.NoValidDefaultForInterface,
+            "raid interface",
+            driver_factory.check_and_update_node_interfaces, node)
+
     def _test_enabled_supported_interfaces(self, enable_storage):
         ht = fake_hardware.FakeHardware()
         expected = {

ironic/tests/unit/conductor/test_base_manager.py

@@ -353,13 +353,16 @@ class RegisterInterfacesTestCase(mgr_utils.ServiceSetUpMixin,
                 ('deploy', ['agent', 'iscsi']),
             )),
         ]
-        default_mock.return_value = None
+        default_mock.side_effect = exception.NoValidDefaultForInterface("boo")
 
         self.assertRaises(
             exception.NoValidDefaultForInterface,
             self.service._register_and_validate_hardware_interfaces,
             hardware_types)
 
+        default_mock.assert_called_once_with(
+            hardware_types['fake-hardware'],
+            mock.ANY, driver_name='fake-hardware')
         unreg_mock.assert_called_once_with(mock.ANY)
         self.assertFalse(reg_mock.called)
 

ironic/tests/unit/conductor/test_manager.py

@@ -946,6 +946,25 @@ class VendorPassthruTestCase(mgr_utils.ServiceSetUpMixin,
                           self.context, 'does_not_exist', 'test_method',
                           'POST', {})
 
+    @mock.patch.object(driver_factory, 'default_interface', autospec=True)
+    def test_driver_vendor_passthru_no_default_interface(self,
+                                                         mock_def_iface):
+        self.service.init_host()
+        # NOTE(rloo): service.init_host() will call
+        #             driver_factory.default_interface() and we want these to
+        #             succeed, so we set the side effect *after* that call.
+        mock_def_iface.reset_mock()
+        mock_def_iface.side_effect = exception.NoValidDefaultForInterface('no')
+        exc = self.assertRaises(messaging.ExpectedException,
+                                self.service.driver_vendor_passthru,
+                                self.context, 'fake-hardware', 'test_method',
+                                'POST', {})
+        mock_def_iface.assert_called_once_with(mock.ANY, 'vendor',
+                                               driver_name='fake-hardware')
+        # Compare true exception hidden by @messaging.expected_exceptions
+        self.assertEqual(exception.NoValidDefaultForInterface,
+                         exc.exc_info[0])
+
     @mock.patch.object(driver_factory, 'get_interface')
     def _test_get_driver_vendor_passthru_methods(self, is_hw_type,
                                                  mock_get_if):
@@ -994,6 +1013,25 @@ class VendorPassthruTestCase(mgr_utils.ServiceSetUpMixin,
         self.assertEqual(exception.UnsupportedDriverExtension,
                          exc.exc_info[0])
 
+    @mock.patch.object(driver_factory, 'default_interface', autospec=True)
+    def test_get_driver_vendor_passthru_methods_no_default_interface(
+            self, mock_def_iface):
+        self.service.init_host()
+        # NOTE(rloo): service.init_host() will call
+        #             driver_factory.default_interface() and we want these to
+        #             succeed, so we set the side effect *after* that call.
+        mock_def_iface.reset_mock()
+        mock_def_iface.side_effect = exception.NoValidDefaultForInterface('no')
+        exc = self.assertRaises(
+            messaging.rpc.ExpectedException,
+            self.service.get_driver_vendor_passthru_methods,
+            self.context, 'fake-hardware')
+        mock_def_iface.assert_called_once_with(mock.ANY, 'vendor',
+                                               driver_name='fake-hardware')
+        # Compare true exception hidden by @messaging.expected_exceptions
+        self.assertEqual(exception.NoValidDefaultForInterface,
+                         exc.exc_info[0])
+
     @mock.patch.object(drivers_base.VendorInterface, 'driver_validate')
     def test_driver_vendor_passthru_validation_failed(self, validate_mock):
         validate_mock.side_effect = exception.MissingParameterValue('error')

ironic/tests/unit/drivers/test_generic.py

@@ -12,7 +12,12 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import mock
+
+from ironic.common import driver_factory
+from ironic.common import exception
 from ironic.conductor import task_manager
+from ironic.drivers import base as driver_base
 from ironic.drivers.modules import agent
 from ironic.drivers.modules import fake
 from ironic.drivers.modules import inspector
@@ -57,3 +62,20 @@ class ManualManagementHardwareTestCase(db_base.DbTestCase):
             self.assertIsInstance(task.driver.deploy, agent.AgentDeploy)
             self.assertIsInstance(task.driver.inspect, inspector.Inspector)
             self.assertIsInstance(task.driver.raid, agent.AgentRAID)
+
+    def test_get_properties(self):
+        # These properties are from vendor (agent) and boot (pxe) interfaces
+        expected_prop_keys = [
+            'deploy_forces_oob_reboot', 'deploy_kernel', 'deploy_ramdisk']
+        hardware_type = driver_factory.get_hardware_type("manual-management")
+        properties = hardware_type.get_properties()
+        self.assertEqual(sorted(expected_prop_keys), sorted(properties.keys()))
+
+    @mock.patch.object(driver_factory, 'default_interface', autospec=True)
+    def test_get_properties_none(self, mock_def_iface):
+        hardware_type = driver_factory.get_hardware_type("manual-management")
+        mock_def_iface.side_effect = exception.NoValidDefaultForInterface("no")
+        properties = hardware_type.get_properties()
+        self.assertEqual({}, properties)
+        self.assertEqual(len(driver_base.ALL_INTERFACES),
+                         mock_def_iface.call_count)