openstack/ironic
Code Issues Proposed changes

Remove deprecated DHCP provider methods

Browse Source 
This patch removes the deprecated DHCP provider method
``update_port_address``. For users who created their own network
interfaces or DHCP providers the logic should be moved to a custom
network interface's ``port_changed`` and ``portgroup_changed``
methods. The following methods should be implemented by custom
network interfaces:

    * ``vif_list``: List attached VIF IDs for a node.
    * ``vif_attach``: Attach a virtual network interface to a node.
    * ``vif_detach``: Detach a virtual network interface from a node.
    * ``port_changed``: Handle any actions required when a port
       changes.
    * ``portgroup_changed``: Handle any actions required when a
      port group changes.
    * ``get_current_vif``: Return VIF ID attached to port or port group
      object.removes deprecated by [0] DHCP provider update_port_address().

[0] 9088891ce7

Change-Id: I887f31937c576ff8933e7c1454e0b2cb0962e340
This commit is contained in:
Vasyl Saienko 2017-04-05 18:17:13 +03:00
parent 2be449f03a
commit 06e8930727
7 changed files with 24 additions and 208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
ironic/dhcp/base.py
View File

@ -47,19 +47,6 @@ class BaseDHCP(object):
:raises: FailedToUpdateDHCPOptOnPort
"""
# TODO(vsaienko) Remove this method when deprecation period is passed
# in Pike.
def update_port_address(self, port_id, address, token=None):
"""Update a port's MAC address.
:param port_id: port id.
:param address: new MAC address.
:param token: An optional authentication token.
:raises: FailedToUpdateMacOnPort
"""
pass
@abc.abstractmethod
def update_dhcp_opts(self, task, options, vifs=None):
"""Send or update the DHCP BOOT options for this node.

21
ironic/dhcp/neutron.py
View File

@ -30,8 +30,6 @@ from ironic import objects
LOG = logging.getLogger(__name__)
update_port_address_deprecation = False
class NeutronDHCPApi(base.BaseDHCP):
"""API for communicating to neutron 2.x API."""
@ -66,25 +64,6 @@ class NeutronDHCPApi(base.BaseDHCP):
LOG.exception("Failed to update Neutron port %s.", port_id)
raise exception.FailedToUpdateDHCPOptOnPort(port_id=port_id)
# TODO(vsaienko) Remove this method when deprecation period is passed
# in Pike.
def update_port_address(self, port_id, address, token=None):
"""Update a port's mac address.
:param port_id: Neutron port id.
:param address: new MAC address.
:param token: optional auth token.
:raises: FailedToUpdateMacOnPort
"""
global update_port_address_deprecation
if not update_port_address_deprecation:
LOG.warning('update_port_address via DHCP provider is '
'deprecated. The node.network_interface '
'port_changed() should be used instead.')
update_port_address_deprecation = True
neutron.update_port_address(port_id, address, token)
def update_dhcp_opts(self, task, options, vifs=None):
"""Send or update the DHCP BOOT options for this node.

5
ironic/dhcp/none.py
View File

@ -25,10 +25,5 @@ class NoneDHCPApi(base.BaseDHCP):
def update_dhcp_opts(self, task, options, vifs=None):
pass
# TODO(vsaienko) Remove this method when deprecation period is passed
# in Pike.
def update_port_address(self, port_id, address, token=None):
pass
def get_ip_addresses(self, task):
return []

76
ironic/drivers/base.py
View File

@ -32,7 +32,6 @@ from ironic.common import exception
from ironic.common.i18n import _, _LE, _LW
from ironic.common import raid
from ironic.common import states
from ironic.drivers.modules.network import common as net_common
LOG = logging.getLogger(__name__)
@ -951,15 +950,6 @@ class NetworkInterface(BaseInterface):
interface_type = 'network'
# Use these booleans to prevent conductor logs being spammed by deprecation
# warnings.
_deprecated_port_change_shown = False
_deprecated_portgroup_change_shown = False
_deprecated_vif_attach_shown = False
_deprecated_vif_detach_shown = False
_deprecated_vif_list_shown = False
_deprecated_get_current_vif_shown = False
def get_properties(self):
"""Return the properties of the interface.
@ -976,7 +966,7 @@ class NetworkInterface(BaseInterface):
:raises: MissingParameterValue, if some parameters are missing.
"""
# TODO(vsaienko) Make this method abstract in Pike.
@abc.abstractmethod
def port_changed(self, task, port_obj):
"""Handle any actions required when a port changes
@ -984,17 +974,8 @@ class NetworkInterface(BaseInterface):
:param port_obj: a changed Port object.
:raises: Conflict, FailedToUpdateDHCPOptOnPort
"""
default_impl = net_common.VIFPortIDMixin()
if not self._deprecated_port_change_shown:
self.__class__._deprecated_port_change_shown = True
LOG.warning(_LW('The network interface %s should be updated to '
'implement the port_changed function. Falling '
'back to default implementation, this behaviour '
'will be removed in Pike'),
self.__class__.__name__)
return default_impl.port_changed(task, port_obj)
# TODO(vsaienko) Make this method abstract in Pike.
@abc.abstractmethod
def portgroup_changed(self, task, portgroup_obj):
"""Handle any actions required when a port changes
@ -1002,17 +983,8 @@ class NetworkInterface(BaseInterface):
:param portgroup_obj: a changed Port object.
:raises: Conflict, FailedToUpdateDHCPOptOnPort
"""
default_impl = net_common.VIFPortIDMixin()
if not self._deprecated_portgroup_change_shown:
self.__class__._deprecated_port_change_shown = True
LOG.warning(_LW('The network interface %s should be updated to '
'implement the portgroup_changed function. '
'Falling back to default implementation, this '
'behaviour will be removed in Pike'),
self.__class__.__name__)
return default_impl.portgroup_changed(task, portgroup_obj)
# TODO(vsaienko) Make this method abstract in Pike.
@abc.abstractmethod
def vif_attach(self, task, vif_info):
"""Attach a virtual network interface to a node
@ -1022,17 +994,8 @@ class NetworkInterface(BaseInterface):
for that VIF.
:raises: NetworkError, VifAlreadyAttached, NoFreePhysicalPorts
"""
default_impl = net_common.VIFPortIDMixin()
if not self._deprecated_vif_attach_shown:
self.__class__._deprecated_vif_attach_shown = True
LOG.warning(_LW('The network interface %s should be updated to '
'implement the vif_attach function. Falling '
'back to default implementation, this behaviour '
'will be removed in Pike'),
self.__class__.__name__)
return default_impl.vif_attach(task, vif_info)
# TODO(vsaienko) Make this method abstract in Pike.
@abc.abstractmethod
def vif_detach(self, task, vif_id):
"""Detach a virtual network interface from a node
@ -1040,17 +1003,8 @@ class NetworkInterface(BaseInterface):
:param vif_id: A VIF ID to detach
:raises: NetworkError, VifNotAttached
"""
default_impl = net_common.VIFPortIDMixin()
if not self._deprecated_vif_detach_shown:
self.__class__._deprecated_vif_detach_shown = True
LOG.warning(_LW('The network interface %s should be updated to '
'implement the vif_detach function. Falling '
'back to default implementation, this behaviour '
'will be removed in Pike'),
self.__class__.__name__)
return default_impl.vif_detach(task, vif_id)
# TODO(vsaienko) Make this method abstract in Pike.
@abc.abstractmethod
def vif_list(self, task):
"""List attached VIF IDs for a node
@ -1058,17 +1012,8 @@ class NetworkInterface(BaseInterface):
:returns: List of VIF dictionaries, each dictionary will have an 'id'
entry with the ID of the VIF.
"""
default_impl = net_common.VIFPortIDMixin()
if not self._deprecated_vif_list_shown:
self.__class__._deprecated_vif_list_shown = True
LOG.warning(_LW('The network interface %s should be updated to '
'implement the vif_list function. Falling '
'back to default implementation, this behaviour '
'will be removed in Pike'),
self.__class__.__name__)
return default_impl.vif_list(task)
# TODO(vsaienko) Make this method abstract in Pike.
@abc.abstractmethod
def get_current_vif(self, task, p_obj):
"""Returns the currently used VIF associated with port or portgroup
@ -1081,15 +1026,6 @@ class NetworkInterface(BaseInterface):
:param p_obj: Ironic port or portgroup object.
:returns: VIF ID associated with p_obj or None.
"""
default_impl = net_common.VIFPortIDMixin()
if not self._deprecated_get_current_vif_shown:
self.__class__._deprecated_get_current_vif_shown = True
LOG.warning(_LW('The network interface %s should be updated to '
'implement the get_current_vif function. Falling '
'back to default implementation, this behaviour '
'will be removed in Pike'),
self.__class__.__name__)
return default_impl.get_current_vif(task, p_obj)
@abc.abstractmethod
def add_provisioning_network(self, task):

9
ironic/tests/unit/dhcp/test_neutron.py
View File

@ -97,15 +97,6 @@ class TestNeutron(db_base.DbTestCase):
api.provider.update_port_dhcp_opts,
port_id, opts)
@mock.patch('ironic.common.neutron.update_port_address')
def test_update_port_address(self, mock_update_port_addr):
address = 'fe:54:00:77:07:d9'
port_id = 'fake-port-id'
api = dhcp_factory.DHCPFactory()
api.provider.update_port_address(port_id, address)
mock_update_port_addr.assert_called_once_with(port_id, address, None)
@mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.update_port_dhcp_opts')
@mock.patch('ironic.common.network.get_node_vif_ids')
def test_update_dhcp(self, mock_gnvi, mock_updo):

90
ironic/tests/unit/drivers/test_base.py
View File

@ -21,7 +21,6 @@ from ironic.common import exception
from ironic.common import raid
from ironic.drivers import base as driver_base
from ironic.drivers.modules import fake
from ironic.drivers.modules.network import common as net_common
from ironic.tests import base
@ -411,95 +410,6 @@ class TestDeployInterface(base.TestCase):
self.assertTrue(mock_log.called)
class TestNetwork(driver_base.NetworkInterface):
def add_provisioning_network(self, task):
pass
def remove_provisioning_network(self, task):
pass
def add_cleaning_network(self, task):
pass
def remove_cleaning_network(self, task):
pass
def configure_tenant_networks(self, task):
pass
def unconfigure_tenant_networks(self, task):
pass
class NetworkInterfaceTestCase(base.TestCase):
@mock.patch.object(driver_base.LOG, 'warning', autospec=True)
@mock.patch.object(net_common.VIFPortIDMixin, 'vif_list')
def test_vif_list(self, mock_vif, mock_warn):
mock_task = mock.MagicMock()
network = TestNetwork()
network.vif_list(mock_task)
mock_vif.assert_called_once_with(mock_task)
mock_warn.assert_called_once_with(mock.ANY, 'TestNetwork')
# Test if the log is only called once even if we recreate a new
# NetworkInterface and call it again.
# NOTE(sambetts): This must be done inside this test instead of a
# separate test because otherwise we end up race conditions between the
# test cases regarding the class variables that are set.
network = TestNetwork()
network.vif_list(mock_task)
mock_warn.assert_called_once_with(mock.ANY, 'TestNetwork')
@mock.patch.object(driver_base.LOG, 'warning', autospec=True)
@mock.patch.object(net_common.VIFPortIDMixin, 'vif_attach')
def test_vif_attach(self, mock_attach, mock_warn):
mock_task = mock.MagicMock()
network = TestNetwork()
network.vif_attach(mock_task, {'id': 'fake'})
mock_attach.assert_called_once_with(mock_task, {'id': 'fake'})
self.assertTrue(mock_warn.called)
@mock.patch.object(driver_base.LOG, 'warning', autospec=True)
@mock.patch.object(net_common.VIFPortIDMixin, 'vif_detach')
def test_vif_detach(self, mock_detach, mock_warn):
mock_task = mock.MagicMock()
network = TestNetwork()
network.vif_detach(mock_task, 'fake-vif-id')
mock_detach.assert_called_once_with(mock_task, 'fake-vif-id')
self.assertTrue(mock_warn.called)
@mock.patch.object(driver_base.LOG, 'warning', autospec=True)
@mock.patch.object(net_common.VIFPortIDMixin, 'port_changed')
def test_port_changed(self, mock_pc, mock_warn):
port = mock.MagicMock()
mock_task = mock.MagicMock()
network = TestNetwork()
network.port_changed(mock_task, port)
mock_pc.assert_called_once_with(mock_task, port)
self.assertTrue(mock_warn.called)
@mock.patch.object(driver_base.LOG, 'warning', autospec=True)
@mock.patch.object(net_common.VIFPortIDMixin, 'portgroup_changed')
def test_portgroup_changed(self, mock_pgc, mock_warn):
port = mock.MagicMock()
mock_task = mock.MagicMock()
network = TestNetwork()
network.portgroup_changed(mock_task, port)
mock_pgc.assert_called_once_with(mock_task, port)
self.assertTrue(mock_warn.called)
@mock.patch.object(driver_base.LOG, 'warning', autospec=True)
@mock.patch.object(net_common.VIFPortIDMixin, 'get_current_vif')
def test_get_current_vif(self, mock_gcv, mock_warn):
port = mock.MagicMock()
mock_task = mock.MagicMock()
network = TestNetwork()
network.get_current_vif(mock_task, port)
mock_gcv.assert_called_once_with(mock_task, port)
self.assertTrue(mock_warn.called)
class TestManagementInterface(base.TestCase):
def test_inject_nmi_default_impl(self):

18
releasenotes/notes/remove-deprecated-dhcp-provider-method-89926a8f0f4793a4.yaml Normal file
View File

@ -0,0 +1,18 @@
---
upgrade:
- |
Removes the deprecated DHCP provider method ``update_port_address``.
For users who created their own network interfaces or DHCP providers
the logic should be moved to a custom network interface's
``port_changed`` and ``portgroup_changed`` methods. The following
methods should be implemented by custom network interfaces:
* ``vif_list``: List attached VIF IDs for a node.
* ``vif_attach``: Attach a virtual network interface to a node.
* ``vif_detach``: Detach a virtual network interface from a node.
* ``port_changed``: Handle any actions required when a port
changes.
* ``portgroup_changed``: Handle any actions required when a
port group changes.
* ``get_current_vif``: Return VIF ID attached to port or port group
object.