From aed483412112877563cd4956a39c9a6e5d4aaf05 Mon Sep 17 00:00:00 2001 From: Sam Betts Date: Fri, 19 Jan 2018 15:05:31 +0000 Subject: [PATCH] Only set default network interface flat if enabled in config This patch updates the driver factory logic which sets a default for the network interface when there is not one set in the config file. Before this patch the logic forces "flat" as the default network interface for all hardware types and classic drivers even if flat isn't enabled in the config file, resulting in a misleading error. This patch changes the code to only set "flat" as the default for classic drivers if its enabled in the enabled_network_interfaces configuration option in the config file, and to not override a default for hardware types at all because the default for hardware types should either come from the config file or from the hardware type's supported_network_interfaces list. Change-Id: Ia8676d3483ddc78df8766dc1baaf2db6b5686050 Closes-Bug: #1744332 --- ironic/common/driver_factory.py | 11 ++++++++++- ironic/drivers/fake_hardware.py | 7 +++++++ .../tests/unit/common/test_driver_factory.py | 18 +++++++++++++++--- ...at_network_if_enabled-b5c6ea415239a53c.yaml | 9 +++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/only_default_flat_network_if_enabled-b5c6ea415239a53c.yaml diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index a51e7bc54a..9ab997a8ec 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -175,10 +175,19 @@ def default_interface(driver_or_hw_type, interface_type, hardware_type.AbstractHardwareType) # Explicit interface defaults additional_defaults = { - 'network': 'flat' if CONF.dhcp.dhcp_provider == 'neutron' else 'noop', 'storage': 'noop' } + if not is_hardware_type: + # For non hardware types we need to set a fallback for the network + # interface however hardware_types specify their own defaults if not in + # the config file. + if (CONF.dhcp.dhcp_provider == 'neutron' and + 'flat' in CONF.enabled_network_interfaces): + additional_defaults['network'] = 'flat' + elif 'noop' in CONF.enabled_network_interfaces: + additional_defaults['network'] = 'noop' + # The fallback default from the configuration impl_name = getattr(CONF, 'default_%s_interface' % interface_type) if impl_name is None: diff --git a/ironic/drivers/fake_hardware.py b/ironic/drivers/fake_hardware.py index 6cf6f0d560..5d0776f226 100644 --- a/ironic/drivers/fake_hardware.py +++ b/ironic/drivers/fake_hardware.py @@ -80,3 +80,10 @@ class FakeHardware(hardware_type.AbstractHardwareType): def supported_vendor_interfaces(self): """List of classes of supported rescue interfaces.""" return [fake.FakeVendorB, fake.FakeVendorA] + + @property + def supported_network_interfaces(self): + # import late to avoid circular imports + from ironic.drivers.modules.network import flat + from ironic.drivers.modules.network import noop + return [flat.FlatNetwork, noop.NoopNetwork] diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index 12cb0dbe7c..9d7cbd5dc3 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -480,7 +480,8 @@ class CheckAndUpdateNodeInterfacesTestCase(db_base.DbTestCase): class DefaultInterfaceTestCase(db_base.DbTestCase): def setUp(self): super(DefaultInterfaceTestCase, self).setUp() - self.config(enabled_hardware_types=['manual-management']) + self.config(enabled_hardware_types=['manual-management'], + enabled_drivers=['fake']) self.driver = driver_factory.get_hardware_type('manual-management') def test_from_config(self): @@ -493,16 +494,27 @@ class DefaultInterfaceTestCase(db_base.DbTestCase): iface = driver_factory.default_interface(self.driver, 'storage') self.assertEqual('noop', iface) + def test_network_from_additional_defaults_hardware_type(self): + self.config(default_network_interface=None) + self.config(dhcp_provider='none', group='dhcp') + self.config(enabled_network_interfaces=['neutron']) + iface = driver_factory.default_interface(self.driver, 'network') + self.assertEqual('neutron', 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') + iface = driver_factory.default_interface( + driver_factory.get_driver_or_hardware_type('fake'), + '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') + iface = driver_factory.default_interface( + driver_factory.get_driver_or_hardware_type('fake'), + 'network') self.assertEqual('flat', iface) def test_calculated_with_one(self): diff --git a/releasenotes/notes/only_default_flat_network_if_enabled-b5c6ea415239a53c.yaml b/releasenotes/notes/only_default_flat_network_if_enabled-b5c6ea415239a53c.yaml new file mode 100644 index 0000000000..5a8fd20298 --- /dev/null +++ b/releasenotes/notes/only_default_flat_network_if_enabled-b5c6ea415239a53c.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes a bug seen when no ``default_network_interface`` is set, because the + conductor tries use the ``flat`` network interface instead even if it is + not included in the conductor's ``enabled_network_interfaces`` config + option. Resulting in `Failed to register hardware types` error. See + `bug 1744332 `_ + for more information. \ No newline at end of file