diff options
| -rw-r--r-- | devstack/lib/ironic | 2 | ||||
| -rw-r--r-- | ironic/common/driver_factory.py | 15 | ||||
| -rw-r--r-- | ironic/drivers/modules/deploy_utils.py | 3 | ||||
| -rw-r--r-- | ironic/tests/unit/common/test_driver_factory.py | 166 | ||||
| -rw-r--r-- | releasenotes/notes/fix-disk-identifier-overwrite-42b33a5a0f7742d8.yaml | 5 | ||||
| -rw-r--r-- | releasenotes/notes/fix-updating-node-driver-to-classic-16b0d5ba47e74d10.yaml | 4 | ||||
| -rw-r--r-- | requirements.txt | 2 |
7 files changed, 188 insertions, 9 deletions
diff --git a/devstack/lib/ironic b/devstack/lib/ironic index 2292da6e3..4474f6372 100644 --- a/devstack/lib/ironic +++ b/devstack/lib/ironic @@ -1612,6 +1612,8 @@ function configure_iptables { # nodes boot from TFTP and callback to the API server listening on $HOST_IP sudo iptables -I INPUT -d $IRONIC_TFTPSERVER_IP -p udp --dport 69 -j ACCEPT || true sudo iptables -I INPUT -d $HOST_IP -p tcp --dport $IRONIC_SERVICE_PORT -j ACCEPT || true + sudo iptables -I INPUT -d $IRONIC_HTTP_SERVER -p tcp --dport $IRONIC_SERVICE_PORT -j ACCEPT || true + sudo iptables -I FORWARD -p tcp --dport $IRONIC_SERVICE_PORT -j ACCEPT || true if is_deployed_by_agent; then # agent ramdisk gets instance image from swift sudo iptables -I INPUT -d $HOST_IP -p tcp --dport ${SWIFT_DEFAULT_BIND_PORT:-8080} -j ACCEPT || true diff --git a/ironic/common/driver_factory.py b/ironic/common/driver_factory.py index a45da9386..c4ba83aef 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/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index ce7da1971..4cf11d1a6 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -401,12 +401,13 @@ def deploy_disk_image(address, port, iqn, lun, with _iscsi_setup_and_handle_errors(address, port, iqn, lun) as dev: disk_utils.populate_image(image_path, dev) - disk_identifier = disk_utils.get_disk_identifier(dev) if configdrive: disk_utils.create_config_drive_partition(node_uuid, dev, configdrive) + disk_identifier = disk_utils.get_disk_identifier(dev) + return {'disk identifier': disk_identifier} diff --git a/ironic/tests/unit/common/test_driver_factory.py b/ironic/tests/unit/common/test_driver_factory.py index b4e370513..53e5ed0ea 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.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-disk-identifier-overwrite-42b33a5a0f7742d8.yaml b/releasenotes/notes/fix-disk-identifier-overwrite-42b33a5a0f7742d8.yaml new file mode 100644 index 000000000..6404bc15e --- /dev/null +++ b/releasenotes/notes/fix-disk-identifier-overwrite-42b33a5a0f7742d8.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - Fix handling of whole disk images with 0x00000000 disk identifier. + Instances failed to boot as the identifier in boot config was overwritten + during config drive creation. See https://bugs.launchpad.net/ironic/+bug/1685093 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 000000000..f1cf8cd88 --- /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. diff --git a/requirements.txt b/requirements.txt index 54cb24f1c..849c9e8ec 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ # The order of packages is significant, because pip processes them in the order # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -pbr<2.0.0,>=1.8 # Apache-2.0 +pbr>=1.8 # Apache-2.0 SQLAlchemy<1.1.0,>=1.0.10 # MIT alembic>=0.8.10 # MIT automaton>=0.5.0 # Apache-2.0 |