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
This commit is contained in:
Vasyl Saienko
Ruby Loo
parent
6ef7e23aa3
commit
a9a1293312
@ -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)
|
||||
|
||||
@ -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):
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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.
|
||||
Loading…
x
Reference in New Issue
Block a user