diff --git a/ironic/dhcp/base.py b/ironic/dhcp/base.py index e0663186ec..94f61fd091 100644 --- a/ironic/dhcp/base.py +++ b/ironic/dhcp/base.py @@ -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. diff --git a/ironic/dhcp/neutron.py b/ironic/dhcp/neutron.py index 098755ce0d..6f56c39183 100644 --- a/ironic/dhcp/neutron.py +++ b/ironic/dhcp/neutron.py @@ -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. diff --git a/ironic/dhcp/none.py b/ironic/dhcp/none.py index 65b4f52a84..eadf114749 100644 --- a/ironic/dhcp/none.py +++ b/ironic/dhcp/none.py @@ -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 [] diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index f3ea162292..da17bcf8b3 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -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): diff --git a/ironic/tests/unit/dhcp/test_neutron.py b/ironic/tests/unit/dhcp/test_neutron.py index 8d1005ce61..dd088faba9 100644 --- a/ironic/tests/unit/dhcp/test_neutron.py +++ b/ironic/tests/unit/dhcp/test_neutron.py @@ -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): diff --git a/ironic/tests/unit/drivers/test_base.py b/ironic/tests/unit/drivers/test_base.py index 23ecd2e7da..95b6f48b60 100644 --- a/ironic/tests/unit/drivers/test_base.py +++ b/ironic/tests/unit/drivers/test_base.py @@ -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): diff --git a/releasenotes/notes/remove-deprecated-dhcp-provider-method-89926a8f0f4793a4.yaml b/releasenotes/notes/remove-deprecated-dhcp-provider-method-89926a8f0f4793a4.yaml new file mode 100644 index 0000000000..d6e78c7726 --- /dev/null +++ b/releasenotes/notes/remove-deprecated-dhcp-provider-method-89926a8f0f4793a4.yaml @@ -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.