Merge "Automatically configure enabled_***_interfaces"

devstack/lib/ironic

@@ -217,9 +217,9 @@ IRONIC_ENABLED_BOOT_INTERFACES=${IRONIC_ENABLED_BOOT_INTERFACES:-"fake,ipxe"}
 IRONIC_ENABLED_CONSOLE_INTERFACES=${IRONIC_ENABLED_CONSOLE_INTERFACES:-"fake,no-console"}
 IRONIC_ENABLED_DEPLOY_INTERFACES=${IRONIC_ENABLED_DEPLOY_INTERFACES:-"fake,direct,ramdisk"}
 IRONIC_ENABLED_INSPECT_INTERFACES=${IRONIC_ENABLED_INSPECT_INTERFACES:-"fake,no-inspect"}
-IRONIC_ENABLED_MANAGEMENT_INTERFACES=${IRONIC_ENABLED_MANAGEMENT_INTERFACES:-"fake,ipmitool,noop"}
+IRONIC_ENABLED_MANAGEMENT_INTERFACES=${IRONIC_ENABLED_MANAGEMENT_INTERFACES:-""}
 IRONIC_ENABLED_NETWORK_INTERFACES=${IRONIC_ENABLED_NETWORK_INTERFACES:-"flat,noop"}
-IRONIC_ENABLED_POWER_INTERFACES=${IRONIC_ENABLED_POWER_INTERFACES:-"fake,ipmitool"}
+IRONIC_ENABLED_POWER_INTERFACES=${IRONIC_ENABLED_POWER_INTERFACES:-""}
 IRONIC_ENABLED_RAID_INTERFACES=${IRONIC_ENABLED_RAID_INTERFACES:-"fake,agent,no-raid"}
 IRONIC_ENABLED_RESCUE_INTERFACES=${IRONIC_ENABLED_RESCUE_INTERFACES:-"fake,no-rescue"}
 IRONIC_ENABLED_STORAGE_INTERFACES=${IRONIC_ENABLED_STORAGE_INTERFACES:-"fake,cinder,noop"}
@@ -1712,18 +1712,7 @@ function configure_ironic_conductor {
         fi
     done
 
-    if is_deployed_by_redfish; then
-        # TODO(lucasagomes): We need to make it easier to configure
-        # specific driver interfaces in DevStack
-        iniset $IRONIC_CONF_FILE DEFAULT enabled_power_interfaces "redfish"
-        iniset $IRONIC_CONF_FILE DEFAULT enabled_management_interfaces "redfish"
-    fi
-
     if is_deployed_by_snmp; then
-        # TODO(lucasagomes): We need to make it easier to configure
-        # specific driver interfaces in DevStack
-        iniset $IRONIC_CONF_FILE DEFAULT enabled_power_interfaces "snmp"
-        iniset $IRONIC_CONF_FILE DEFAULT enabled_management_interfaces "noop"
         iniset $IRONIC_CONF_FILE pxe enable_netboot_fallback True
     fi
 

ironic/common/driver_factory.py

@@ -17,7 +17,7 @@ import collections
 
 from oslo_concurrency import lockutils
 from oslo_log import log
-from stevedore import named
+import stevedore
 
 from ironic.common import exception
 from ironic.common.i18n import _
@@ -404,7 +404,7 @@ class BaseDriverFactory(object):
             cls._set_enabled_drivers()
 
             cls._extension_manager = (
-                named.NamedExtensionManager(
+                stevedore.NamedExtensionManager(
                     cls._entrypoint_name,
                     cls._enabled_driver_list,
                     invoke_on_load=True,
@@ -429,6 +429,34 @@ class BaseDriverFactory(object):
         return ((ext.name, ext.obj) for ext in self._extension_manager)
 
 
+class InterfaceFactory(BaseDriverFactory):
+
+    # Name of a HardwareType attribute with a list of supported interfaces
+    _supported_driver_list_field = ''
+
+    @classmethod
+    def _set_enabled_drivers(cls):
+        super()._set_enabled_drivers()
+        if cls._enabled_driver_list:
+            return
+
+        tmp_ext_mgr = stevedore.ExtensionManager(
+            cls._entrypoint_name,
+            invoke_on_load=False,  # do not create interfaces
+            on_load_failure_callback=cls._catch_driver_not_found)
+        cls_names = {v.plugin: k for (k, v) in tmp_ext_mgr.items()}
+
+        # Fallback: calculate based on hardware type defaults
+        for hw_type in hardware_types().values():
+            supported = getattr(hw_type, cls._supported_driver_list_field)[0]
+            try:
+                name = cls_names[supported]
+            except KeyError:
+                raise KeyError("%s not in %s" % (supported, cls_names))
+            if name not in cls._enabled_driver_list:
+                cls._enabled_driver_list.append(name)
+
+
 def _warn_if_unsupported(ext):
     if not ext.obj.supported:
         LOG.warning('Driver "%s" is UNSUPPORTED. It has been deprecated '
@@ -443,10 +471,12 @@ class HardwareTypesFactory(BaseDriverFactory):
 
 _INTERFACE_LOADERS = {
     name: type('%sInterfaceFactory' % name.capitalize(),
-               (BaseDriverFactory,),
+               (InterfaceFactory,),
                {'_entrypoint_name': 'ironic.hardware.interfaces.%s' % name,
                 '_enabled_driver_list_config_option':
                 'enabled_%s_interfaces' % name,
+                '_supported_driver_list_field':
+                'supported_%s_interfaces' % name,
                 '_logging_template':
                 "Loaded the following %s interfaces: %%s" % name})
     for name in driver_base.ALL_INTERFACES

ironic/conductor/base_manager.py

@@ -40,7 +40,6 @@ from ironic.conductor import task_manager
 from ironic.conductor import utils
 from ironic.conf import CONF
 from ironic.db import api as dbapi
-from ironic.drivers import base as driver_base
 from ironic.drivers.modules import deploy_utils
 from ironic import objects
 from ironic.objects import fields as obj_fields
@@ -49,27 +48,6 @@ from ironic.objects import fields as obj_fields
 LOG = log.getLogger(__name__)
 
 
-def _check_enabled_interfaces():
-    """Sanity-check enabled_*_interfaces configs.
-
-    We do this before we even bother to try to load up drivers. If we have any
-    dynamic drivers enabled, then we need interfaces enabled as well.
-
-    :raises: ConfigInvalid if an enabled interfaces config option is empty.
-    """
-    empty_confs = []
-    iface_types = ['enabled_%s_interfaces' % i
-                   for i in driver_base.ALL_INTERFACES]
-    for iface_type in iface_types:
-        conf_value = getattr(CONF, iface_type)
-        if not conf_value:
-            empty_confs.append(iface_type)
-    if empty_confs:
-        msg = (_('Configuration options %s cannot be an empty list.') %
-               ', '.join(empty_confs))
-        raise exception.ConfigInvalid(error_msg=msg)
-
-
 class BaseConductorManager(object):
 
     def __init__(self, host, topic):
@@ -146,8 +124,6 @@ class BaseConductorManager(object):
             use_groups=self._use_groups())
         """Consistent hash ring which maps drivers to conductors."""
 
-        _check_enabled_interfaces()
-
         # NOTE(tenbrae): these calls may raise DriverLoadError or
         # DriverNotFound
         # NOTE(vdrok): Instantiate network and storage interface factory on

ironic/conf/default.py

@@ -121,7 +121,7 @@ driver_opts = [
     cfg.StrOpt('default_inspect_interface',
                help=_DEFAULT_IFACE_HELP.format('inspect')),
     cfg.ListOpt('enabled_management_interfaces',
-                default=['ipmitool', 'redfish'],
+                default=None,  # automatically calculate
                 help=_ENABLED_IFACE_HELP.format('management')),
     cfg.StrOpt('default_management_interface',
                help=_DEFAULT_IFACE_HELP.format('management')),
@@ -131,7 +131,7 @@ driver_opts = [
     cfg.StrOpt('default_network_interface',
                help=_DEFAULT_IFACE_HELP.format('network')),
     cfg.ListOpt('enabled_power_interfaces',
-                default=['ipmitool', 'redfish'],
+                default=None,  # automatically calculate
                 help=_ENABLED_IFACE_HELP.format('power')),
     cfg.StrOpt('default_power_interface',
                help=_DEFAULT_IFACE_HELP.format('power')),

ironic/tests/unit/common/test_driver_factory.py

@@ -319,39 +319,31 @@ class DefaultInterfaceTestCase(db_base.DbTestCase):
         iface = driver_factory.default_interface(self.driver, 'deploy')
         self.assertEqual('ansible', iface)
 
-    def test_calculated_no_answer(self):
+    def test_calculated_fallback(self):
-        # manual-management supports no power interfaces
         self.config(default_power_interface=None)
         self.config(enabled_power_interfaces=[])
-        self.assertRaisesRegex(
+        iface = driver_factory.default_interface(self.driver, 'power')
-            exception.NoValidDefaultForInterface,
+        self.assertEqual('agent', iface)
-            "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(
+        self.assertEqual(
-            exception.NoValidDefaultForInterface,
+            "agent",
-            "For hardware type 'foo', no default value found for power "
+            driver_factory.default_interface(self.driver, 'power',
-            "interface.",
+                                             driver_name='foo'))
-            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(
+        self.assertEqual(
-            exception.NoValidDefaultForInterface,
+            "agent",
-            "For node bar with hardware type 'foo', no default "
+            driver_factory.default_interface(self.driver, 'power',
-            "value found for power interface.",
+                                             driver_name='foo', node='bar'))
-            driver_factory.default_interface, self.driver, 'power',
-            driver_name='foo', node='bar')
 
     @mock.patch.object(driver_factory, 'get_interface', autospec=True)
     def test_check_exception_IncompatibleInterface(self, mock_get_interface):
@@ -490,15 +482,18 @@ class HardwareTypeLoadTestCase(db_base.DbTestCase):
                           task_manager.acquire, self.context, node.id)
         mock_get_hw_type.assert_called_once_with('fake-2')
 
-    def test_build_driver_for_task_no_defaults(self):
+    def test_build_driver_for_task_fallback_defaults(self):
         self.config(dhcp_provider=None, group='dhcp')
+        self.config(enabled_hardware_types=['fake-hardware'])
         for iface in drivers_base.ALL_INTERFACES:
             if iface not in ['network', 'storage']:
                 self.config(**{'enabled_%s_interfaces' % iface: []})
                 self.config(**{'default_%s_interface' % iface: None})
         node = obj_utils.create_test_node(self.context, driver='fake-hardware')
-        self.assertRaises(exception.NoValidDefaultForInterface,
+        with task_manager.acquire(self.context, node.id) as task:
-                          task_manager.acquire, self.context, node.id)
+            for iface in drivers_base.ALL_INTERFACES:
+                impl = getattr(task.driver, iface)
+                self.assertIsNotNone(impl)
 
     def test_build_driver_for_task_calculated_defaults(self):
         self.config(dhcp_provider=None, group='dhcp')
@@ -581,10 +576,8 @@ class HardwareTypeLoadTestCase(db_base.DbTestCase):
         #             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(
+        self.assertTrue(driver_factory.check_and_update_node_interfaces(node))
-            exception.NoValidDefaultForInterface,
+        self.assertEqual('fake', node.raid_interface)
-            "raid interface",
-            driver_factory.check_and_update_node_interfaces, node)
 
     def _test_enabled_supported_interfaces(self, enable_storage):
         ht = fake_hardware.FakeHardware()

ironic/tests/unit/conductor/test_base_manager.py

@@ -193,11 +193,11 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
             self.assertTrue(mock_df.called)
             self.assertFalse(mock_reg.called)
 
-    def test_start_fails_on_no_enabled_interfaces(self):
+    def test_start_with_no_enabled_interfaces(self):
-        self.config(enabled_boot_interfaces=[])
+        self.config(enabled_boot_interfaces=[],
-        self.assertRaisesRegex(exception.ConfigInvalid,
+                    enabled_deploy_interfaces=[],
-                               'options enabled_boot_interfaces',
+                    enabled_hardware_types=['fake-hardware'])
-                               self.service.init_host)
+        self._start_service()
 
     @mock.patch.object(base_manager, 'LOG', autospec=True)
     @mock.patch.object(driver_factory, 'HardwareTypesFactory', autospec=True)
@@ -311,17 +311,6 @@ class StartStopTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
         self.assertEqual(2, mock_dbapi.call_count)
 
 
-class CheckInterfacesTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
-    def test__check_enabled_interfaces_success(self):
-        base_manager._check_enabled_interfaces()
-
-    def test__check_enabled_interfaces_failure(self):
-        self.config(enabled_boot_interfaces=[])
-        self.assertRaisesRegex(exception.ConfigInvalid,
-                               'options enabled_boot_interfaces',
-                               base_manager._check_enabled_interfaces)
-
-
 class KeepAliveTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test__conductor_service_record_keepalive(self):
         self._start_service()

releasenotes/notes/auto-ifaces-fdb8c680eab711f4.yaml

@@ -0,0 +1,9 @@
+---
+features:
+  - |
+    The configuration options ``enabled_power_interfaces`` and
+    ``enabled_management_interfaces`` are now empty by default. If left empty,
+    their values will be calculated based on ``enabled_hardware_types``.
+  - |
+    The Bare Metal service is now capable of calculating the default value
+    for any ``enabled_***_interfaces`` based on ``enabled_hardware_types``.