openstack/neutron
Code Issues Proposed changes

Auto-delete dhcp ports on segment delete

Browse Source 
Subnet delete triggers dhcp port deletion but asynchronously,
therefore in the condition described in the bug report we may
get a conflict when deleting the segment too fast after the subnet.

Here we follow the example of how we auto-delete ports in net delete.

Please also find a fullstack test in Related-Change below.

Change-Id: Iba02f5a2211b18c2deb9097daad6be5e7d21faf8
Closes-Bug: #1878632
Related-Change: https://review.opendev.org/728904
(cherry picked from commit da45bbbff4)
This commit is contained in:
Bence Romsics 2020-05-19 13:21:52 +02:00 committed by Maciej Józefczyk
parent bcea25351b
commit 96fe5d3509
6 changed files with 116 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
neutron/common/_constants.py
View File

@ -67,3 +67,12 @@ IPTABLES_RANDOM_FULLY_VERSION = '1.6.2'
# Segmentation ID pool; DB select limit to improve the performace.
IDPOOL_SELECT_SIZE = 100
# Ports with the following 'device_owner' values will not prevent
# network deletion. If delete_network() finds that all ports on a
# network have these owners, it will explicitly delete each port
# and allow network deletion to continue. Similarly, if delete_subnet()
# finds out that all existing IP Allocations are associated with ports
# with these owners, it will allow subnet deletion to proceed with the
# IP allocations being cleaned up by cascade.
AUTO_DELETE_PORT_OWNERS = [constants.DEVICE_OWNER_DHCP]

18
neutron/db/db_base_plugin_v2.py
View File

@ -46,6 +46,7 @@ from sqlalchemy import not_
from neutron._i18n import _
from neutron.api.rpc.agentnotifiers import l3_rpc_agent_api
from neutron.common import _constants
from neutron.common import ipv6_utils
from neutron.common import utils
from neutron.db import db_base_plugin_common
@ -67,15 +68,6 @@ from neutron.objects import subnetpool as subnetpool_obj
LOG = logging.getLogger(__name__)
# Ports with the following 'device_owner' values will not prevent
# network deletion. If delete_network() finds that all ports on a
# network have these owners, it will explicitly delete each port
# and allow network deletion to continue. Similarly, if delete_subnet()
# finds out that all existing IP Allocations are associated with ports
# with these owners, it will allow subnet deletion to proceed with the
# IP allocations being cleaned up by cascade.
AUTO_DELETE_PORT_OWNERS = [constants.DEVICE_OWNER_DHCP]
def _check_subnet_not_used(context, subnet_id):
try:
@ -467,7 +459,8 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
def _ensure_network_not_in_use(self, context, net_id):
non_auto_ports = context.session.query(
models_v2.Port.id).filter_by(network_id=net_id).filter(
~models_v2.Port.device_owner.in_(AUTO_DELETE_PORT_OWNERS))
~models_v2.Port.device_owner.in_(
_constants.AUTO_DELETE_PORT_OWNERS))
if non_auto_ports.count():
raise exc.NetworkInUse(net_id=net_id)
@ -480,7 +473,8 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
with db_api.CONTEXT_READER.using(context):
auto_delete_port_ids = [p.id for p in context.session.query(
models_v2.Port.id).filter_by(network_id=id).filter(
models_v2.Port.device_owner.in_(AUTO_DELETE_PORT_OWNERS))]
models_v2.Port.device_owner.in_(
_constants.AUTO_DELETE_PORT_OWNERS))]
for port_id in auto_delete_port_ids:
try:
self.delete_port(context.elevated(), port_id)
@ -998,7 +992,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
def _subnet_get_user_allocation(self, context, subnet_id):
"""Check if there are any user ports on subnet and return first."""
return port_obj.IPAllocation.get_alloc_by_subnet_id(
context, subnet_id, AUTO_DELETE_PORT_OWNERS)
context, subnet_id, _constants.AUTO_DELETE_PORT_OWNERS)
@db_api.CONTEXT_READER
def _subnet_check_ip_allocations_internal_router_ports(self, context,

16
neutron/objects/ports.py
View File

@ -19,6 +19,7 @@ from oslo_log import log as logging
from oslo_utils import versionutils
from oslo_versionedobjects import fields as obj_fields
from neutron.common import _constants
from neutron.db.models import dns as dns_models
from neutron.db.models import l3
from neutron.db.models import securitygroup as sg_models
@ -415,14 +416,25 @@ class Port(base.NeutronDbObject):
**kwargs)
@classmethod
def get_port_ids_filter_by_segment_id(cls, context, segment_id):
def get_auto_deletable_port_ids_and_proper_port_count_by_segment(
cls, context, segment_id):
query = context.session.query(models_v2.Port.id)
query = query.join(
ml2_models.PortBindingLevel,
ml2_models.PortBindingLevel.port_id == models_v2.Port.id)
query = query.filter(
ml2_models.PortBindingLevel.segment_id == segment_id)
return [p.id for p in query]
q_delete = query.filter(
models_v2.Port.device_owner.in_(
_constants.AUTO_DELETE_PORT_OWNERS))
q_proper = query.filter(
~models_v2.Port.device_owner.in_(
_constants.AUTO_DELETE_PORT_OWNERS))
return ([r.id for r in q_delete.all()], q_proper.count())
@classmethod
def modify_fields_to_db(cls, fields):

26
neutron/plugins/ml2/db.py
View File

@ -20,6 +20,7 @@ from neutron_lib.callbacks import registry
from neutron_lib.callbacks import resources
from neutron_lib import constants as n_const
from neutron_lib.db import api as db_api
from neutron_lib import exceptions as nlib_exc
from neutron_lib.plugins import directory
from oslo_db import exception as db_exc
from oslo_log import log
@ -342,17 +343,28 @@ def _prevent_segment_delete_with_port_bound(resource, event, trigger,
return
with db_api.CONTEXT_READER.using(payload.context):
port_ids = port_obj.Port.get_port_ids_filter_by_segment_id(
payload.context, segment_id=payload.resource_id)
auto_delete_port_ids, proper_port_count = port_obj.Port.\
get_auto_deletable_port_ids_and_proper_port_count_by_segment(
payload.context, segment_id=payload.resource_id)
# There are still some ports in the segment, segment should not be deleted
# TODO(xiaohhui): Should we delete the dhcp port automatically here?
if port_ids:
reason = _("The segment is still bound with port(s) "
"%s") % ", ".join(port_ids)
if proper_port_count:
reason = (_("The segment is still bound with %s port(s)") %
(proper_port_count + len(auto_delete_port_ids)))
raise seg_exc.SegmentInUse(segment_id=payload.resource_id,
reason=reason)
if auto_delete_port_ids:
LOG.debug("Auto-deleting dhcp port(s) on segment %s: %s",
payload.resource_id, ", ".join(auto_delete_port_ids))
plugin = directory.get_plugin()
for port_id in auto_delete_port_ids:
try:
plugin.delete_port(payload.context.elevated(), port_id)
except nlib_exc.PortNotFound:
# Don't raise if something else concurrently deleted the port
LOG.debug("Ignoring PortNotFound when deleting port '%s'. "
"The port has already been deleted.", port_id)
def subscribe():
registry.subscribe(_prevent_segment_delete_with_port_bound,

29
neutron/tests/unit/objects/test_ports.py
View File

@ -573,3 +573,32 @@ class PortDbObjectTestCase(obj_test_base.BaseDbObjectTestCase,
subnet_id)
self.assertEqual(1, len(ports_alloc))
self.assertEqual(objs[0].id, ports_alloc[0].id)
def _test_get_auto_deletable_ports(self, device_owner):
network_id = self._create_test_network_id()
segment_id = self._create_test_segment_id(network_id)
port = self._create_test_port(device_owner=device_owner)
binding = ports.PortBindingLevel(
self.context, port_id=port.id,
host='host1', level=0, segment_id=segment_id)
binding.create()
return (
ports.Port.
get_auto_deletable_port_ids_and_proper_port_count_by_segment(
self.context, segment_id))
def test_get_auto_deletable_ports_dhcp(self):
dhcp_ports, count = self._test_get_auto_deletable_ports(
'network:dhcp')
self.assertEqual(
(1, 0),
(len(dhcp_ports), count),
)
def test_get_auto_deletable_ports_not_dhcp(self):
dhcp_ports, count = self._test_get_auto_deletable_ports(
'not_network_dhcp')
self.assertEqual(
(0, 1),
(len(dhcp_ports), count),
)

39
neutron/tests/unit/plugins/ml2/test_db.py
View File

@ -22,6 +22,7 @@ from neutron_lib.api.definitions import portbindings
from neutron_lib import constants
from neutron_lib import context
from neutron_lib.db import api as db_api
from neutron_lib.plugins import directory
from neutron_lib.plugins.ml2 import api
from oslo_utils import uuidutils
from sqlalchemy.orm import exc
@ -34,6 +35,7 @@ from neutron.objects import network as network_obj
from neutron.objects import ports as port_obj
from neutron.plugins.ml2 import db as ml2_db
from neutron.plugins.ml2 import models
from neutron.services.segments import exceptions as seg_exc
from neutron.tests.unit import testlib_api
@ -300,6 +302,43 @@ class Ml2DBTestCase(testlib_api.SqlTestCase):
for mac in macs:
self.assertIsNotNone(re.search(mac_regex, mac))
def test__prevent_segment_delete_with_port_bound_raise(self):
payload_mock = mock.Mock()
payload_mock.metadata.get.return_value = False
payload_mock.context = self.ctx
with mock.patch.object(
port_obj.Port,
'get_auto_deletable_port_ids_and_proper_port_count_by_segment'
) as mock_get:
mock_get.return_value = ([], 1)
self.assertRaises(
seg_exc.SegmentInUse,
ml2_db._prevent_segment_delete_with_port_bound,
resource=mock.Mock(),
event=mock.Mock(),
trigger=mock.Mock(),
payload=payload_mock,
)
def test__prevent_segment_delete_with_port_bound_auto_delete(self):
payload_mock = mock.Mock()
payload_mock.metadata.get.return_value = False
payload_mock.context = self.ctx
plugin = directory.get_plugin()
with mock.patch.object(
port_obj.Port,
'get_auto_deletable_port_ids_and_proper_port_count_by_segment'
) as mock_get, \
mock.patch.object(plugin, 'delete_port') as mock_delete_port:
mock_get.return_value = (['fake-port'], 0)
ml2_db._prevent_segment_delete_with_port_bound(
resource=mock.Mock(),
event=mock.Mock(),
trigger=mock.Mock(),
payload=payload_mock,
)
mock_delete_port.assert_called_with(mock.ANY, 'fake-port')
class Ml2DvrDBTestCase(testlib_api.SqlTestCase):