From 472abf43b03ee5ce8aaf25a89732a318cd9cf680 Mon Sep 17 00:00:00 2001 From: Jim Rollenhagen Date: Tue, 10 Jan 2017 21:20:03 +0000 Subject: [PATCH] Expose default interface calculation from driver_factory The API for this feature will need to be able to calculate the default interfaces for a given hardware interface. The code was already there to do it, but just needed a bit of refactoring and a public method. Move things around as we need them, and add a few tests to test the default_interface method directly. Change-Id: I993b76b7a2e6219cacc86558a612ecf0ba10e685 Partial-Bug: #1524745 --- ironic/common/driver_factory.py | 70 ++++++++++--------- .../tests/unit/common/test_driver_factory.py | 55 +++++++++++++++ 2 files changed, 92 insertions(+), 33 deletions(-) diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index 2465c60951..60a6b7905c 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -142,30 +142,50 @@ def _get_interface(driver_or_hw_type, interface_type, interface_name): return impl_instance -def _default_interface(hardware_type, interface_type, factory): +def default_interface(driver_or_hw_type, interface_type): """Calculate and return the default interface implementation. Finds the first implementation that is supported by the hardware type and is enabled in the configuration. - :param hardware_type: hardware type instance. + :param driver_or_hw_type: classic driver or hardware type instance object. :param interface_type: type of the interface (e.g. 'boot'). - :param factory: interface factory class to use for loading implementations. :returns: an entrypoint name of the calculated default implementation or None if no default implementation can be found. :raises: InterfaceNotFoundInEntrypoint if the entry point was not found. """ - supported = getattr(hardware_type, - 'supported_%s_interfaces' % interface_type) - # Mapping of classes to entry points - enabled = {obj.__class__: name for (name, obj) in factory().items()} + factory = _INTERFACE_LOADERS[interface_type] + is_hardware_type = isinstance(driver_or_hw_type, + hardware_type.AbstractHardwareType) + # Explicit interface defaults + additional_defaults = { + 'network': 'flat' if CONF.dhcp.dhcp_provider == 'neutron' else 'noop', + 'storage': 'noop' + } - # Order of the supported list matters - for impl_class in supported: - try: - return enabled[impl_class] - except KeyError: - pass + # The fallback default from the configuration + impl_name = getattr(CONF, 'default_%s_interface' % interface_type) + if impl_name is None: + impl_name = additional_defaults.get(interface_type) + + if impl_name is not None: + # Check that the default is correct for this type + _get_interface(driver_or_hw_type, interface_type, impl_name) + elif is_hardware_type: + supported = getattr(driver_or_hw_type, + 'supported_%s_interfaces' % interface_type) + # Mapping of classes to entry points + enabled = {obj.__class__: name for (name, obj) in factory().items()} + + # Order of the supported list matters + for impl_class in supported: + try: + impl_name = enabled[impl_class] + break + except KeyError: + continue + + return impl_name def check_and_update_node_interfaces(node, driver_or_hw_type=None): @@ -191,24 +211,17 @@ def check_and_update_node_interfaces(node, driver_or_hw_type=None): is_hardware_type = isinstance(driver_or_hw_type, hardware_type.AbstractHardwareType) - # Explicit interface defaults - additional_defaults = { - 'network': 'flat' if CONF.dhcp.dhcp_provider == 'neutron' else 'noop', - 'storage': 'noop' - } - if is_hardware_type: - factories = _INTERFACE_LOADERS + factories = _INTERFACE_LOADERS.keys() else: # Only network and storage interfaces are dynamic for classic drivers - factories = {'network': _INTERFACE_LOADERS['network'], - 'storage': _INTERFACE_LOADERS['storage']} + factories = ['network', 'storage'] # Result - whether the node object was modified result = False # Walk through all dynamic interfaces and check/update them - for iface, factory in factories.items(): + for iface in factories: field_name = '%s_interface' % iface # NOTE(dtantsur): objects raise NotImplementedError on accessing fields # that are known, but missing from an object. Thus, we cannot just use @@ -221,16 +234,7 @@ def check_and_update_node_interfaces(node, driver_or_hw_type=None): # Not changing the result, proceeding with the next interface continue - # The fallback default from the configuration - impl_name = getattr(CONF, 'default_%s_interface' % iface) - if impl_name is None: - impl_name = additional_defaults.get(iface) - - if impl_name is not None: - # Check that the default is correct for this type - _get_interface(driver_or_hw_type, iface, impl_name) - elif is_hardware_type: - impl_name = _default_interface(driver_or_hw_type, iface, factory) + impl_name = default_interface(driver_or_hw_type, iface) if impl_name is None: raise exception.NoValidDefaultForInterface( diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index 54f241c8a1..10cf3ed244 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -259,6 +259,61 @@ class CheckAndUpdateNodeInterfacesTestCase(db_base.DbTestCase): node) +class DefaultInterfaceTestCase(db_base.DbTestCase): + def setUp(self): + super(DefaultInterfaceTestCase, self).setUp() + self.config(enabled_hardware_types=['manual-management']) + self.driver = driver_factory.get_hardware_type('manual-management') + + def test_from_config(self): + self.config(default_deploy_interface='direct') + iface = driver_factory.default_interface(self.driver, 'deploy') + self.assertEqual('direct', iface) + + def test_from_additional_defaults(self): + self.config(default_storage_interface=None) + iface = driver_factory.default_interface(self.driver, 'storage') + self.assertEqual('noop', iface) + + def test_network_from_additional_defaults(self): + self.config(default_network_interface=None) + self.config(dhcp_provider='none', group='dhcp') + iface = driver_factory.default_interface(self.driver, 'network') + self.assertEqual('noop', iface) + + def test_network_from_additional_defaults_neutron_dhcp(self): + self.config(default_network_interface=None) + self.config(dhcp_provider='neutron', group='dhcp') + iface = driver_factory.default_interface(self.driver, 'network') + self.assertEqual('flat', iface) + + def test_calculated_with_one(self): + self.config(default_deploy_interface=None) + self.config(enabled_deploy_interfaces=['direct']) + iface = driver_factory.default_interface(self.driver, 'deploy') + self.assertEqual('direct', iface) + + def test_calculated_with_two(self): + self.config(default_deploy_interface=None) + self.config(enabled_deploy_interfaces=['iscsi', 'direct']) + iface = driver_factory.default_interface(self.driver, 'deploy') + self.assertEqual('iscsi', iface) + + def test_calculated_with_unsupported(self): + self.config(default_deploy_interface=None) + # manual-management doesn't support fake deploy + self.config(enabled_deploy_interfaces=['fake', 'direct']) + iface = driver_factory.default_interface(self.driver, 'deploy') + self.assertEqual('direct', iface) + + def test_calculated_no_answer(self): + # 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.assertIsNone(iface) + + class TestFakeHardware(hardware_type.AbstractHardwareType): @property def supported_boot_interfaces(self):