From a9a1293312ca67c5e5dbb258146832516a41e939 Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Thu, 17 Nov 2016 11:03:00 +0200 Subject: [PATCH] Remove deprecated Neutron DHCP provider methods This patch removes deprecated methods from Neutron DHCP provider: * create_cleaning_ports * delete_cleaning_ports Also drop related methods from deploy_utils in ironic.drivers.modules * prepare_cleaning_ports * tear_down_cleaning_ports If you have your own custom ironic DHCP provider that implements cleaning methods, you may need to update your code to use the add_cleaning_network() and remove_cleaning_network() network interface methods. They were deprecated in I0c26582b6b6e9d32650ff3e2b9a3269c3c2d5454 Change-Id: I758e5b21028a4dfcca9c907c63020d0cfca4e37d Closes-Bug: #1642512 --- ironic/dhcp/neutron.py | 38 ------ ironic/drivers/modules/deploy_utils.py | 121 +----------------- ironic/tests/unit/dhcp/test_neutron.py | 32 ----- .../unit/drivers/modules/test_deploy_utils.py | 105 --------------- ...hcp-provider-methods-582742f3000be3c7.yaml | 17 +++ 5 files changed, 19 insertions(+), 294 deletions(-) create mode 100644 releasenotes/notes/remove-deprecated-dhcp-provider-methods-582742f3000be3c7.yaml diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index af44e0bb47..eccb8ec1ba 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -31,9 +31,6 @@ from ironic import objects LOG = logging.getLogger(__name__) -create_cleaning_ports_deprecation = False -delete_cleaning_ports_deprecation = False - class NeutronDHCPApi(base.BaseDHCP): """API for communicating to neutron 2.x API.""" @@ -298,38 +295,3 @@ class NeutronDHCPApi(base.BaseDHCP): task, task.portgroups, client) return port_ip_addresses + portgroup_ip_addresses - - # TODO(vsaienko) Remove this method when deprecation period is passed - # in Ocata. - def create_cleaning_ports(self, task): - """Create neutron ports for each port on task.node to boot the ramdisk. - - :param task: a TaskManager instance. - :raises: NetworkError, InvalidParameterValue - :returns: a dictionary in the form {port.uuid: neutron_port['id']} - """ - global create_cleaning_ports_deprecation - if not create_cleaning_ports_deprecation: - LOG.warning(_LW('create_cleaning_ports via dhcp provider is ' - 'deprecated. The node.network_interface setting ' - 'should be used instead.')) - create_cleaning_ports_deprecation = True - - return task.driver.network.add_cleaning_network(task) - - # TODO(vsaienko) Remove this method when deprecation period is passed - # in Ocata. - def delete_cleaning_ports(self, task): - """Deletes the neutron port created for booting the ramdisk. - - :param task: a TaskManager instance. - :raises: NetworkError, InvalidParameterValue - """ - global delete_cleaning_ports_deprecation - if not delete_cleaning_ports_deprecation: - LOG.warning(_LW('delete_cleaning_ports via dhcp provider is ' - 'deprecated. The node.network_interface setting ' - 'should be used instead.')) - delete_cleaning_ports_deprecation = True - - task.driver.network.remove_cleaning_network(task) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 91fd776260..5f1550e4cc 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -29,7 +29,6 @@ from oslo_utils import excutils from oslo_utils import strutils import six -from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.glance_service import service_utils from ironic.common.i18n import _, _LE, _LW @@ -931,122 +930,6 @@ def get_boot_option(node): return capabilities.get('boot_option', get_default_boot_option()).lower() -# TODO(vdrok): This method is left here for backwards compatibility with out of -# tree DHCP providers implementing cleaning methods. Remove it in Ocata -def prepare_cleaning_ports(task): - """Prepare the Ironic ports of the node for cleaning. - - This method deletes the cleaning ports currently existing - for all the ports of the node and then creates a new one - for each one of them. It also adds 'cleaning_vif_port_id' to internal_info - of each Ironic port, after creating the cleaning ports. - - :param task: a TaskManager object containing the node - :raises: NodeCleaningFailure, NetworkError if the previous cleaning ports - cannot be removed or if new cleaning ports cannot be created. - :raises: InvalidParameterValue if cleaning network UUID config option has - an invalid value. - """ - provider = dhcp_factory.DHCPFactory() - provider_manages_delete_cleaning = hasattr(provider.provider, - 'delete_cleaning_ports') - provider_manages_create_cleaning = hasattr(provider.provider, - 'create_cleaning_ports') - # NOTE(vdrok): The neutron DHCP provider was changed to call network - # interface's add_cleaning_network anyway, so call it directly to avoid - # duplication of some actions - if (CONF.dhcp.dhcp_provider == 'neutron' or - (not provider_manages_delete_cleaning and - not provider_manages_create_cleaning)): - task.driver.network.add_cleaning_network(task) - return - - LOG.warning(_LW("delete_cleaning_ports and create_cleaning_ports " - "functions in DHCP providers are deprecated, please move " - "this logic to the network interface's " - "remove_cleaning_network or add_cleaning_network methods " - "respectively and remove the old DHCP provider methods. " - "Possibility to do the cleaning via DHCP providers will " - "be removed in Ocata release.")) - # If we have left over ports from a previous cleaning, remove them - if provider_manages_delete_cleaning: - # Allow to raise if it fails, is caught and handled in conductor - provider.provider.delete_cleaning_ports(task) - - # Create cleaning ports if necessary - if provider_manages_create_cleaning: - # Allow to raise if it fails, is caught and handled in conductor - ports = provider.provider.create_cleaning_ports(task) - - # Add cleaning_vif_port_id for each of the ports because some boot - # interfaces expects these to prepare for booting ramdisk. - for port in task.ports: - internal_info = port.internal_info - try: - internal_info['cleaning_vif_port_id'] = ports[port.uuid] - except KeyError: - # This is an internal error in Ironic. All DHCP providers - # implementing create_cleaning_ports are supposed to - # return a VIF port ID for all Ironic ports. But - # that doesn't seem to be true here. - error = (_("When creating cleaning ports, DHCP provider " - "didn't return VIF port ID for %s") % port.uuid) - raise exception.NodeCleaningFailure( - node=task.node.uuid, reason=error) - else: - port.internal_info = internal_info - port.save() - - -# TODO(vdrok): This method is left here for backwards compatibility with out of -# tree DHCP providers implementing cleaning methods. Remove it in Ocata -def tear_down_cleaning_ports(task): - """Deletes the cleaning ports created for each of the Ironic ports. - - This method deletes the cleaning port created before cleaning - was started. - - :param task: a TaskManager object containing the node - :raises: NodeCleaningFailure, NetworkError if the cleaning ports cannot be - removed. - """ - # If we created cleaning ports, delete them - provider = dhcp_factory.DHCPFactory() - provider_manages_delete_cleaning = hasattr(provider.provider, - 'delete_cleaning_ports') - try: - # NOTE(vdrok): The neutron DHCP provider was changed to call network - # interface's remove_cleaning_network anyway, so call it directly to - # avoid duplication of some actions - if (CONF.dhcp.dhcp_provider == 'neutron' or - not provider_manages_delete_cleaning): - task.driver.network.remove_cleaning_network(task) - return - - # NOTE(vdrok): No need for another deprecation warning here, if - # delete_cleaning_ports is in the DHCP provider the warning was - # printed in prepare_cleaning_ports - # Allow to raise if it fails, is caught and handled in conductor - provider.provider.delete_cleaning_ports(task) - for port in task.ports: - if 'cleaning_vif_port_id' in port.internal_info: - internal_info = port.internal_info - del internal_info['cleaning_vif_port_id'] - port.internal_info = internal_info - port.save() - finally: - for port in task.ports: - if 'vif_port_id' in port.extra: - # TODO(vdrok): This piece is left for backwards compatibility, - # if ironic was upgraded during cleaning, vif_port_id - # containing cleaning neutron port UUID should be cleared, - # remove in Ocata - extra_dict = port.extra - del extra_dict['vif_port_id'] - port.extra = extra_dict - port.save() - - def build_agent_options(node): """Build the options to be passed to the agent ramdisk. @@ -1085,7 +968,7 @@ def prepare_inband_cleaning(task, manage_boot=True): :raises: InvalidParameterValue if cleaning network UUID config option has an invalid value. """ - prepare_cleaning_ports(task) + task.driver.network.add_cleaning_network(task) # Append required config parameters to node's driver_internal_info # to pass to IPA. @@ -1123,7 +1006,7 @@ def tear_down_inband_cleaning(task, manage_boot=True): if manage_boot: task.driver.boot.clean_up_ramdisk(task) - tear_down_cleaning_ports(task) + task.driver.network.remove_cleaning_network(task) def get_image_instance_info(node): diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index e9e867d6e3..00ef5f874f 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -507,35 +507,3 @@ class TestNeutron(db_base.DbTestCase): get_ip_mock.assert_has_calls( [mock.call(task, task.ports[0], mock.ANY), mock.call(task, task.portgroups[0], mock.ANY)]) - - @mock.patch.object(neutron, 'create_cleaning_ports_deprecation', False) - @mock.patch.object(neutron, 'LOG', autospec=True) - def test_create_cleaning_ports(self, log_mock): - self.config(cleaning_network_uuid=uuidutils.generate_uuid(), - group='neutron') - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - with mock.patch.object( - task.driver.network, 'add_cleaning_network', - autospec=True) as add_net_mock: - api.create_cleaning_ports(task) - add_net_mock.assert_called_once_with(task) - - api.create_cleaning_ports(task) - self.assertEqual(1, log_mock.warning.call_count) - - @mock.patch.object(neutron, 'delete_cleaning_ports_deprecation', False) - @mock.patch.object(neutron, 'LOG', autospec=True) - def test_delete_cleaning_ports(self, log_mock): - api = dhcp_factory.DHCPFactory().provider - - with task_manager.acquire(self.context, self.node.uuid) as task: - with mock.patch.object( - task.driver.network, 'remove_cleaning_network', - autospec=True) as rm_net_mock: - api.delete_cleaning_ports(task) - rm_net_mock.assert_called_once_with(task) - - api.delete_cleaning_ports(task) - self.assertEqual(1, log_mock.warning.call_count) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 939480965b..b1765219c2 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -28,7 +28,6 @@ import testtools from testtools import matchers from ironic.common import boot_devices -from ironic.common import dhcp_factory from ironic.common import exception from ironic.common import image_service from ironic.common import states @@ -1960,110 +1959,6 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.assertEqual(True, task.node.driver_internal_info[ 'agent_continue_if_ata_erase_failed']) - @mock.patch.object(utils.LOG, 'warning', autospec=True) - @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) - def _test_prepare_inband_cleaning_ports_out_of_tree( - self, dhcp_factory_mock, log_mock, return_vif_port_id=True): - self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') - dhcp_provider = dhcp_factory_mock.return_value.provider - create = dhcp_provider.create_cleaning_ports - delete = dhcp_provider.delete_cleaning_ports - if return_vif_port_id: - create.return_value = {self.ports[0].uuid: 'vif-port-id'} - else: - create.return_value = {} - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - utils.prepare_cleaning_ports(task) - create.assert_called_once_with(task) - delete.assert_called_once_with(task) - self.assertTrue(log_mock.called) - - self.ports[0].refresh() - self.assertEqual('vif-port-id', - self.ports[0].internal_info['cleaning_vif_port_id']) - - def test_prepare_inband_cleaning_ports_out_of_tree(self): - self._test_prepare_inband_cleaning_ports_out_of_tree() - - def test_prepare_inband_cleaning_ports_out_of_tree_no_vif_port_id(self): - self.assertRaises( - exception.NodeCleaningFailure, - self._test_prepare_inband_cleaning_ports_out_of_tree, - return_vif_port_id=False) - - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'add_cleaning_network') - def test_prepare_inband_cleaning_ports_neutron(self, add_clean_net_mock): - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - utils.prepare_cleaning_ports(task) - add_clean_net_mock.assert_called_once_with(task) - - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'add_cleaning_network') - @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) - def test_prepare_inband_cleaning_ports_provider_does_not_create( - self, dhcp_factory_mock, add_clean_net_mock): - self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') - self.node.network_interface = 'noop' - self.node.save() - dhcp_provider = dhcp_factory_mock.return_value.provider - del dhcp_provider.delete_cleaning_ports - del dhcp_provider.create_cleaning_ports - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - utils.prepare_cleaning_ports(task) - add_clean_net_mock.assert_called_once_with(task) - - @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) - def test_tear_down_inband_cleaning_ports_out_of_tree(self, - dhcp_factory_mock): - self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') - dhcp_provider = dhcp_factory_mock.return_value.provider - delete = dhcp_provider.delete_cleaning_ports - internal_info = self.ports[0].internal_info - internal_info['cleaning_vif_port_id'] = 'vif-port-id-1' - self.ports[0].internal_info = internal_info - self.ports[0].save() - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - utils.tear_down_cleaning_ports(task) - delete.assert_called_once_with(task) - - self.ports[0].refresh() - self.assertNotIn('cleaning_vif_port_id', self.ports[0].internal_info) - self.assertNotIn('vif_port_id', self.ports[0].extra) - - @mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.' - 'remove_cleaning_network') - def test_tear_down_inband_cleaning_ports_neutron(self, rm_clean_net_mock): - extra_port = obj_utils.create_test_port( - self.context, node_id=self.node.id, address='10:00:00:00:00:01', - extra={'vif_port_id': 'vif-port'}, uuid=uuidutils.generate_uuid() - ) - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - utils.tear_down_cleaning_ports(task) - rm_clean_net_mock.assert_called_once_with(task) - extra_port.refresh() - self.assertNotIn('vif_port_id', extra_port.extra) - - @mock.patch('ironic.drivers.modules.network.noop.NoopNetwork.' - 'remove_cleaning_network') - @mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True) - def test_tear_down_inband_cleaning_ports_provider_does_not_delete( - self, dhcp_factory_mock, rm_clean_net_mock): - self.config(group='dhcp', dhcp_provider='my_shiny_dhcp_provider') - self.node.network_interface = 'noop' - self.node.save() - dhcp_provider = dhcp_factory_mock.return_value.provider - del dhcp_provider.delete_cleaning_ports - with task_manager.acquire( - self.context, self.node.uuid, shared=False) as task: - utils.tear_down_cleaning_ports(task) - rm_clean_net_mock.assert_called_once_with(task) - @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) @mock.patch.object(utils, 'build_agent_options', autospec=True) diff --git a/releasenotes/notes/remove-deprecated-dhcp-provider-methods-582742f3000be3c7.yaml b/releasenotes/notes/remove-deprecated-dhcp-provider-methods-582742f3000be3c7.yaml new file mode 100644 index 0000000000..d39728cf5c --- /dev/null +++ b/releasenotes/notes/remove-deprecated-dhcp-provider-methods-582742f3000be3c7.yaml @@ -0,0 +1,17 @@ +--- +upgrade: + - | + Removes these deprecated methods from the neutron provider built into ironic: + + * create_cleaning_ports + * delete_cleaning_ports + + Removes these related methods from ironic.drivers.modules.deploy_utils: + + * prepare_cleaning_ports + * tear_down_cleaning_ports + + If you have your own custom ironic DHCP provider that implements + cleaning methods, you may need to update your code to use the + add_cleaning_network() and remove_cleaning_network() network + interface methods.