From a165fe3264fd8f3a4bd7af9d7a958b9b3abf099f Mon Sep 17 00:00:00 2001 From: Tzu-Mainn Chen Date: Tue, 23 Feb 2021 21:41:14 +0000 Subject: [PATCH] Allow instance_info to override node interface This change allows instance_info values to override node interface definitions, so non-admins can make temporary changes to various interfaces. Story: #2008652 Task: #41918 Change-Id: I6c3dc74705bde02bd02882d14838f184f8d4a5e3 --- doc/source/admin/index.rst | 1 + doc/source/admin/node-interface-override.rst | 22 +++++++++++++ ironic/common/driver_factory.py | 27 +++++++++++----- .../tests/unit/common/test_driver_factory.py | 31 +++++++++++++++++++ ...o-interface-override-287c7fcff1081469.yaml | 5 +++ 5 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 doc/source/admin/node-interface-override.rst create mode 100644 releasenotes/notes/instance-info-interface-override-287c7fcff1081469.yaml diff --git a/doc/source/admin/index.rst b/doc/source/admin/index.rst index dd3a17bf67..7a06dc4dbb 100644 --- a/doc/source/admin/index.rst +++ b/doc/source/admin/index.rst @@ -30,6 +30,7 @@ the services. Node Multi-Tenancy Fast-Track Deployment Booting a Ramdisk or an ISO + Node Interface Override Drivers, Hardware Types and Hardware Interfaces ----------------------------------------------- diff --git a/doc/source/admin/node-interface-override.rst b/doc/source/admin/node-interface-override.rst new file mode 100644 index 0000000000..907c8a00cf --- /dev/null +++ b/doc/source/admin/node-interface-override.rst @@ -0,0 +1,22 @@ +======================= +Node Interface Override +======================= + +Non-admins with temporary access to a node, may wish to specify different +node interfaces. However, allowing them to set these interface values is +problematic, as there is no automated way to ensure that the original +interface values are restored. + +This guide details a method for temporarily overriding a node interface +value. + +Overriding a Node Interface +=========================== + +In order to temporarily override a node interface, simply set the +appropriate value in `instance_info`. For example, if you'd like to +override a node's storage interface, run the following:: + + baremetal node set --instance-info storage_interface=cinder node-1 + +`instance_info` values persist until after a node is cleaned. diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index 0775980192..37ce123fee 100644 --- a/ironic/common/driver_factory.py +++ b/ironic/common/driver_factory.py @@ -69,7 +69,9 @@ def _attach_interfaces_to_driver(bare_driver, node, hw_type): the requested implementation is not compatible with it. """ for iface in _INTERFACE_LOADERS: - impl_name = getattr(node, '%s_interface' % iface) + iface_name = '%s_interface' % iface + impl_name = node.instance_info.get(iface_name, + getattr(node, iface_name)) impl = get_interface(hw_type, iface, impl_name) setattr(bare_driver, iface, impl) @@ -204,20 +206,29 @@ def check_and_update_node_interfaces(node, hw_type=None): # 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. + set_default = True + if 'instance_info' in node and field_name in node.instance_info: + impl_name = node.instance_info.get(field_name) + if impl_name is not None: + # Check that the provided value is correct for this type + get_interface(hw_type, iface, impl_name) + set_default = False + if field_name in node: impl_name = getattr(node, field_name) if impl_name is not None: # Check that the provided value is correct for this type get_interface(hw_type, iface, impl_name) - # Not changing the result, proceeding with the next interface - continue + set_default = False - impl_name = default_interface(hw_type, iface, - driver_name=node.driver, node=node.uuid) + if set_default: + impl_name = default_interface(hw_type, iface, + driver_name=node.driver, + node=node.uuid) - # Set the calculated default and set result to True - setattr(node, field_name, impl_name) - result = True + # Set the calculated default and set result to True + setattr(node, field_name, impl_name) + result = True return result diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index 3d9fb3e906..77626a0842 100644 --- a/ironic/tests/unit/common/test_driver_factory.py +++ b/ironic/tests/unit/common/test_driver_factory.py @@ -214,6 +214,26 @@ class CheckAndUpdateNodeInterfacesTestCase(db_base.DbTestCase): driver_factory.check_and_update_node_interfaces, node) + def test_create_node_valid_network_interface_instance_info_override(self): + instance_info = {'network_interface': 'noop', + 'storage_interface': 'noop'} + node = obj_utils.get_test_node(self.context, + instance_info=instance_info) + self.assertTrue(driver_factory.check_and_update_node_interfaces(node)) + self.assertIsNone(node.network_interface) + self.assertIsNone(node.storage_interface) + self.assertEqual('noop', node.instance_info.get('network_interface')) + self.assertEqual('noop', node.instance_info.get('storage_interface')) + + def test_create_node_invalid_network_interface_instance_info_override( + self): + instance_info = {'network_interface': 'banana'} + node = obj_utils.get_test_node(self.context, + instance_info=instance_info) + self.assertRaises(exception.InterfaceNotFoundInEntrypoint, + driver_factory.check_and_update_node_interfaces, + node) + def _get_valid_default_interface_name(self, iface): i_name = 'fake' # there is no 'fake' network interface @@ -506,6 +526,17 @@ class HardwareTypeLoadTestCase(db_base.DbTestCase): self.assertRaises(exception.InterfaceNotFoundInEntrypoint, task_manager.acquire, self.context, node.id) + def test_build_driver_for_task_instance_info_override(self): + self.config(enabled_network_interfaces=['noop', 'neutron']) + instance_info = {'network_interface': 'neutron'} + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + instance_info=instance_info, + **self.node_kwargs) + with task_manager.acquire(self.context, node.id) as task: + self.assertEqual( + getattr(task.driver, 'network').__class__.__name__, + 'NeutronNetwork') + def test_no_storage_interface(self): node = obj_utils.get_test_node(self.context) self.assertTrue(driver_factory.check_and_update_node_interfaces(node)) diff --git a/releasenotes/notes/instance-info-interface-override-287c7fcff1081469.yaml b/releasenotes/notes/instance-info-interface-override-287c7fcff1081469.yaml new file mode 100644 index 0000000000..56af6c8c18 --- /dev/null +++ b/releasenotes/notes/instance-info-interface-override-287c7fcff1081469.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Allows node _interface values to be overridden by values in instance_info. + This gives non-admins a temporary method of setting interface values. \ No newline at end of file