From 3d02084559e91c0a5c156b259e615eac42a5d882 Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Tue, 28 Feb 2017 17:42:52 +0200 Subject: [PATCH] Fix updating node.driver to classic An issue when it was impossible to update ``node.driver`` to classic if the current driver is one of hardware types. Change-Id: I348a27a13d3025fe7e8ddfa31992b895a3809730 Closes-Bug: #1668579 --- ironic/common/driver_factory.py | 15 +- .../tests/unit/common/test_driver_factory.py | 170 +++++++++++++++++- ...de-driver-to-classic-16b0d5ba47e74d10.yaml | 4 + 3 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/fix-updating-node-driver-to-classic-16b0d5ba47e74d10.yaml diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index a45da9386c..c4ba83aeff 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -253,13 +253,23 @@ def check_and_update_node_interfaces(node, driver_or_hw_type=None): # for classic drivers, none are allowed except for network & storage. not_allowed_ifaces = driver_base.ALL_INTERFACES - set(factories) + updates = node.obj_what_changed() + # Result - whether the node object was modified + result = False + bad_interface_fields = [] for iface in not_allowed_ifaces: field_name = '%s_interface' % iface + # NOTE(vsaienko): reset *_interface fields that shouldn't exist for + # classic driver, only when driver was changed and field not set + # explicitly + if 'driver' in updates and field_name not in updates: + setattr(node, field_name, None) + result = True # NOTE(dtantsur): objects raise NotImplementedError on accessing fields # that are known, but missing from an object. Thus, we cannot just use # getattr(node, field_name, None) here. - if field_name in node: + elif field_name in node: impl_name = getattr(node, field_name) if impl_name is not None: bad_interface_fields.append(field_name) @@ -268,9 +278,6 @@ def check_and_update_node_interfaces(node, driver_or_hw_type=None): raise exception.MustBeNone(node=node.uuid, driver=node.driver, node_fields=','.join(bad_interface_fields)) - # Result - whether the node object was modified - result = False - # Walk through all dynamic interfaces and check/update them for iface in factories: field_name = '%s_interface' % iface diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index b4e3705139..53e5ed0ea2 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -273,20 +273,22 @@ class CheckAndUpdateNodeInterfacesTestCase(db_base.DbTestCase): # "none" dhcp provider corresponds to "noop" network_interface self.assertEqual('noop', node.network_interface) - def test_valid_interfaces(self): + def test_create_node_classic_driver_valid_interfaces(self): node = obj_utils.get_test_node(self.context, driver='fake', network_interface='noop', storage_interface='noop') self.assertFalse(driver_factory.check_and_update_node_interfaces(node)) + self.assertEqual('noop', node.network_interface) + self.assertEqual('noop', node.storage_interface) - def test_invalid_network_interface(self): + def test_create_node_classic_driver_invalid_network_interface(self): node = obj_utils.get_test_node(self.context, driver='fake', network_interface='banana') self.assertRaises(exception.InterfaceNotFoundInEntrypoint, driver_factory.check_and_update_node_interfaces, node) - def test_classic_setting_interfaces(self): + def test_create_node_classic_driver_not_allowed_interfaces_set(self): # Cannot set these node interfaces for classic driver no_set_interfaces = (drivers_base.ALL_INTERFACES - set(['network', 'storage'])) @@ -294,13 +296,171 @@ class CheckAndUpdateNodeInterfacesTestCase(db_base.DbTestCase): iface_name = '%s_interface' % iface node_kwargs = {'uuid': uuidutils.generate_uuid(), iface_name: 'fake'} - node = obj_utils.create_test_node(self.context, driver='fake', - **node_kwargs) + node = obj_utils.get_test_node(self.context, driver='fake', + **node_kwargs) self.assertRaisesRegex( exception.InvalidParameterValue, 'driver fake.*%s' % iface_name, driver_factory.check_and_update_node_interfaces, node) + def test_create_node_classic_driver_no_interfaces_set(self): + no_set_interfaces = (drivers_base.ALL_INTERFACES - + set(['network', 'storage'])) + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.get_test_node(self.context, driver='fake', + **node_kwargs) + driver_factory.check_and_update_node_interfaces(node) + + for iface in no_set_interfaces: + iface_name = '%s_interface' % iface + self.assertIsNone(getattr(node, iface_name)) + + def _get_valid_default_interface_name(self, iface): + i_name = 'fake' + # there is no 'fake' network interface + if iface == 'network': + i_name = 'noop' + return i_name + + def _set_config_interface_options_hardware_type(self): + for iface in drivers_base.ALL_INTERFACES: + i_name = self._get_valid_default_interface_name(iface) + config_kwarg = {'enabled_%s_interfaces' % iface: [i_name], + 'default_%s_interface' % iface: i_name} + self.config(**config_kwarg) + + def test_create_node_dynamic_driver_invalid_network_interface(self): + self._set_config_interface_options_hardware_type() + + node = obj_utils.get_test_node(self.context, driver='fake-hardware', + network_interface='banana') + self.assertRaises(exception.InterfaceNotFoundInEntrypoint, + driver_factory.check_and_update_node_interfaces, + node) + + def test_create_node_dynamic_driver_interfaces_set(self): + self._set_config_interface_options_hardware_type() + + for iface in drivers_base.ALL_INTERFACES: + iface_name = '%s_interface' % iface + i_name = self._get_valid_default_interface_name(iface) + node_kwargs = {'uuid': uuidutils.generate_uuid(), + iface_name: i_name} + node = obj_utils.get_test_node( + self.context, driver='fake-hardware', **node_kwargs) + driver_factory.check_and_update_node_interfaces(node) + self.assertEqual(i_name, getattr(node, iface_name)) + + def test_update_node_set_classic_driver_and_not_allowed_interfaces(self): + """Update driver to classic and interfaces specified""" + not_allowed_interfaces = (drivers_base.ALL_INTERFACES - + set(['network', 'storage'])) + self.config(enabled_drivers=['fake', 'fake_agent']) + for iface in not_allowed_interfaces: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.create_test_node(self.context, driver='fake', + **node_kwargs) + setattr(node, iface_name, 'fake') + node.driver = 'fake_agent' + self.assertRaisesRegex( + exception.InvalidParameterValue, + 'driver fake.*%s' % iface_name, + driver_factory.check_and_update_node_interfaces, node) + + def test_update_node_set_classic_driver_and_allowed_interfaces(self): + """Update driver to classic and interfaces specified""" + self._set_config_interface_options_hardware_type() + self.config(enabled_drivers=['fake', 'fake_agent']) + for iface in ['network', 'storage']: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.create_test_node(self.context, driver='fake', + **node_kwargs) + i_name = self._get_valid_default_interface_name(iface) + setattr(node, iface_name, i_name) + node.driver = 'fake_agent' + driver_factory.check_and_update_node_interfaces(node) + self.assertEqual(i_name, getattr(node, iface_name)) + + def test_update_node_set_classic_driver_unset_interfaces(self): + """Update driver to classic and set interfaces to None""" + no_set_interfaces = (drivers_base.ALL_INTERFACES - + set(['network', 'storage'])) + self.config(enabled_drivers=['fake', 'fake_agent']) + for iface in no_set_interfaces: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.create_test_node(self.context, driver='fake', + **node_kwargs) + setattr(node, iface_name, None) + node.driver = 'fake_agent' + driver_factory.check_and_update_node_interfaces(node) + self.assertEqual('fake_agent', node.driver) + self.assertIsNone(getattr(node, iface_name)) + + def test_update_node_classic_driver_unset_interfaces(self): + """Update interfaces to None for node with classic driver""" + no_set_interfaces = (drivers_base.ALL_INTERFACES - + set(['network', 'storage'])) + self.config(enabled_drivers=['fake', 'fake_agent']) + for iface in no_set_interfaces: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.create_test_node(self.context, driver='fake', + **node_kwargs) + setattr(node, iface_name, None) + driver_factory.check_and_update_node_interfaces(node) + self.assertIsNone(getattr(node, iface_name)) + + def test_update_node_set_classic_driver_no_interfaces(self): + """Update driver to classic no interfaces specified""" + self._set_config_interface_options_hardware_type() + no_set_interfaces = (drivers_base.ALL_INTERFACES - + set(['network', 'storage'])) + for iface in no_set_interfaces: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node_kwargs[iface_name] = 'fake' + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + **node_kwargs) + node.driver = 'fake' + driver_factory.check_and_update_node_interfaces(node) + self.assertEqual('fake', node.driver) + self.assertIsNone(getattr(node, iface_name)) + self.assertEqual('noop', node.network_interface) + + def test_update_node_set_dynamic_driver_and_interfaces(self): + """Update driver to dynamic and interfaces specified""" + self._set_config_interface_options_hardware_type() + + for iface in drivers_base.ALL_INTERFACES: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.create_test_node(self.context, driver='fake', + **node_kwargs) + i_name = self._get_valid_default_interface_name(iface) + setattr(node, iface_name, i_name) + node.driver = 'fake-hardware' + driver_factory.check_and_update_node_interfaces(node) + self.assertEqual(i_name, getattr(node, iface_name)) + + def test_node_update_dynamic_driver_set_interfaces(self): + """Update interfaces for node with dynamic driver""" + self._set_config_interface_options_hardware_type() + for iface in drivers_base.ALL_INTERFACES: + iface_name = '%s_interface' % iface + node_kwargs = {'uuid': uuidutils.generate_uuid()} + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + **node_kwargs) + + i_name = self._get_valid_default_interface_name(iface) + setattr(node, iface_name, i_name) + driver_factory.check_and_update_node_interfaces(node) + self.assertEqual(i_name, getattr(node, iface_name)) + class DefaultInterfaceTestCase(db_base.DbTestCase): def setUp(self): diff --git a/releasenotes/notes/fix-updating-node-driver-to-classic-16b0d5ba47e74d10.yaml b/releasenotes/notes/fix-updating-node-driver-to-classic-16b0d5ba47e74d10.yaml new file mode 100644 index 0000000000..f1cf8cd885 --- /dev/null +++ b/releasenotes/notes/fix-updating-node-driver-to-classic-16b0d5ba47e74d10.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixes failure to update a node's driver from a hardware type to + a classic driver.