fix bug where network owned resources block delete

bug 1028174

This patch fixes a bug in delete_network and delete_subnet where the InUse
exception was being thrown for resources owned by the network and not
the tenant.  This patch force deletes network resources if they are the
only blocker to deleting the parent resource.

Change-Id: I822cc4a92cb05cdef88679bb628fad4e5063cddd
This commit is contained in:
Mark McClain 2012-08-27 12:04:21 -04:00
parent 11f2a64b47
commit 88e2551c69
7 changed files with 173 additions and 122 deletions

View File

@ -122,7 +122,7 @@ class DhcpAgent(object):
for subnet in network.subnets:
if subnet.enable_dhcp:
self.cache.put(network)
self.call_driver('update_l3', network)
self.call_driver('enable', network)
break
else:
self.disable_dhcp_helper(network.id)
@ -140,19 +140,10 @@ class DhcpAgent(object):
else:
self.disable_dhcp_helper(network_id)
def network_delete_start(self, payload):
"""Handle the network.detete.start notification event."""
def network_delete_end(self, payload):
"""Handle the network.delete.end notification event."""
self.disable_dhcp_helper(payload['network_id'])
def subnet_delete_start(self, payload):
"""Handle the subnet.detete.start notification event."""
subnet_id = payload['subnet_id']
network = self.cache.get_network_by_subnet_id(subnet_id)
if network:
device_id = self.device_manager.get_device_id(network)
self.plugin_rpc.release_port_fixed_ip(network.id, device_id,
subnet_id)
def subnet_update_end(self, payload):
"""Handle the subnet.update.end notification event."""
network_id = payload['subnet']['network_id']
@ -392,20 +383,19 @@ class DeviceManager(object):
self.driver.init_l3(interface_name, ip_cidrs,
namespace=namespace)
def destroy(self, network):
return interface_name
def destroy(self, network, device_name):
"""Destroy the device used for the network's DHCP on this host."""
if self.conf.use_namespaces:
namespace = network.id
else:
namespace = None
self.driver.unplug(self.get_interface_name(network),
namespace=namespace)
self.plugin.release_dhcp_port(network.id, self.get_device_id(network))
self.driver.unplug(device_name, namespace=namespace)
def update_l3(self, network):
"""Update the L3 attributes for the current network's DHCP device."""
self.setup(network, reuse_existing=True)
self.plugin.release_dhcp_port(network.id,
self.get_device_id(network))
class DictModel(object):

View File

@ -78,10 +78,6 @@ class DhcpBase(object):
def disable(self):
"""Disable dhcp for this network."""
@abc.abstractmethod
def update_l3(self, subnet, reason):
"""Alert the driver that a subnet has changed."""
def restart(self):
"""Restart the dhcp service for the network."""
self.disable()
@ -108,10 +104,12 @@ class DhcpLocalProcess(DhcpBase):
def enable(self):
"""Enables DHCP for this network by spawning a local process."""
interface_name = self.device_delegate.setup(self.network,
reuse_existing=True)
if self.active:
self.reload_allocations()
elif self._enable_dhcp():
self.device_delegate.setup(self.network, reuse_existing=True)
self.interface_name = interface_name
self.spawn_process()
def disable(self):
@ -126,18 +124,15 @@ class DhcpLocalProcess(DhcpBase):
ip_wrapper.netns.execute(cmd)
else:
utils.execute(cmd, self.root_helper)
self.device_delegate.destroy(self.network)
self.device_delegate.destroy(self.network, self.interface_name)
elif pid:
LOG.debug(_('DHCP for %s pid %d is stale, ignoring command') %
(self.network.id, pid))
else:
LOG.debug(_('No DHCP started for %s') % self.network.id)
def update_l3(self):
"""Update the L3 settings for the interface and reload settings."""
self.device_delegate.update_l3(self.network)
self.reload_allocations()
def get_conf_file_name(self, kind, ensure_conf_dir=False):
"""Returns the file name for a given kind of config file."""
confs_dir = os.path.abspath(os.path.normpath(self.conf.dhcp_confs))
@ -179,6 +174,16 @@ class DhcpLocalProcess(DhcpBase):
except RuntimeError, e:
return False
@property
def interface_name(self):
return self._get_value_from_conf_file('interface')
@interface_name.setter
def interface_name(self, value):
interface_file_path = self.get_conf_file_name('interface',
ensure_conf_dir=True)
replace_file(interface_file_path, value)
@abc.abstractmethod
def spawn_process(self):
pass
@ -199,8 +204,6 @@ class Dnsmasq(DhcpLocalProcess):
def spawn_process(self):
"""Spawns a Dnsmasq process for the network."""
interface_name = self.device_delegate.get_interface_name(self.network)
env = {
self.QUANTUM_NETWORK_ID_KEY: self.network.id,
self.QUANTUM_RELAY_SOCKET_PATH_KEY:
@ -213,7 +216,7 @@ class Dnsmasq(DhcpLocalProcess):
'--no-resolv',
'--strict-order',
'--bind-interfaces',
'--interface=%s' % interface_name,
'--interface=%s' % self.interface_name,
'--except-interface=lo',
'--domain=%s' % self.conf.dhcp_domain,
'--pid-file=%s' % self.get_conf_file_name('pid',

View File

@ -34,6 +34,8 @@ from quantum import quantum_plugin_base_v2
LOG = logging.getLogger(__name__)
AGENT_OWNER_PREFIX = 'network:'
class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
""" A class that implements the v2 Quantum plugin interface
@ -794,9 +796,19 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
filter = {'network_id': [id]}
ports = self.get_ports(context, filters=filter)
if ports:
# check if there are any tenant owned ports in-use
only_svc = all(p['device_owner'].startswith(AGENT_OWNER_PREFIX)
for p in ports)
if not only_svc:
raise q_exc.NetworkInUse(net_id=id)
# clean up network owned ports
for port in ports:
self._delete_port(context, port['id'])
# clean up subnets
subnets_qry = context.session.query(models_v2.Subnet)
subnets_qry.filter_by(network_id=id).delete()
context.session.delete(network)
@ -951,11 +963,19 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
def delete_subnet(self, context, id):
with context.session.begin(subtransactions=True):
subnet = self._get_subnet(context, id)
# Check if ports are using this subnet
# Check if any tenant owned ports are using this subnet
allocated_qry = context.session.query(models_v2.IPAllocation)
allocated_qry = allocated_qry.options(orm.joinedload('ports'))
allocated = allocated_qry.filter_by(subnet_id=id).all()
if allocated:
raise q_exc.SubnetInUse(subnet_id=id)
only_svc = all(a.ports.device_owner.startswith(AGENT_OWNER_PREFIX)
for a in allocated)
if not only_svc:
raise q_exc.NetworkInUse(subnet_id=id)
# remove network owned ports
for allocation in allocated:
context.session.delete(allocation)
context.session.delete(subnet)
@ -1054,33 +1074,36 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
def delete_port(self, context, id):
with context.session.begin(subtransactions=True):
port = self._get_port(context, id)
self._delete_port(context, id)
allocated_qry = context.session.query(models_v2.IPAllocation)
# recycle all of the IP's
# NOTE(garyk) this may be have to be addressed differently when
# working with a DHCP server.
allocated = allocated_qry.filter_by(port_id=id).all()
if allocated:
for a in allocated:
subnet = self._get_subnet(context, a['subnet_id'])
if a['ip_address'] == subnet['gateway_ip']:
# Gateway address will not be recycled, but we do
# need to delete the allocation from the DB
QuantumDbPluginV2._delete_ip_allocation(
context,
a['network_id'], a['subnet_id'],
id, a['ip_address'])
LOG.debug("Gateway address (%s/%s) is not recycled",
a['ip_address'], a['subnet_id'])
continue
def _delete_port(self, context, id):
port = self._get_port(context, id)
QuantumDbPluginV2._recycle_ip(context,
network_id=a['network_id'],
subnet_id=a['subnet_id'],
ip_address=a['ip_address'],
port_id=id)
context.session.delete(port)
allocated_qry = context.session.query(models_v2.IPAllocation)
# recycle all of the IP's
# NOTE(garyk) this may be have to be addressed differently when
# working with a DHCP server.
allocated = allocated_qry.filter_by(port_id=id).all()
if allocated:
for a in allocated:
subnet = self._get_subnet(context, a['subnet_id'])
if a['ip_address'] == subnet['gateway_ip']:
# Gateway address will not be recycled, but we do
# need to delete the allocation from the DB
QuantumDbPluginV2._delete_ip_allocation(
context,
a['network_id'], a['subnet_id'],
id, a['ip_address'])
LOG.debug("Gateway address (%s/%s) is not recycled",
a['ip_address'], a['subnet_id'])
continue
QuantumDbPluginV2._recycle_ip(context,
network_id=a['network_id'],
subnet_id=a['subnet_id'],
ip_address=a['ip_address'],
port_id=id)
context.session.delete(port)
def get_port(self, context, id, fields=None):
port = self._get_port(context, id)

View File

@ -19,6 +19,8 @@
import inspect
import logging
from sqlalchemy import orm
from quantum.common import exceptions as exc
from quantum.db import db_base_plugin_v2
from quantum.db import models_v2
@ -136,7 +138,11 @@ class PluginV2(db_base_plugin_v2.QuantumDbPluginV2):
network = self._get_network(context, id)
filter = {'network_id': [id]}
ports = self.get_ports(context, filters=filter)
if ports:
# check if there are any tenant owned ports in-use
prefix = db_base_plugin_v2.AGENT_OWNER_PREFIX
only_svc = all(p['device_owner'].startswith(prefix) for p in ports)
if not only_svc:
raise exc.NetworkInUse(net_id=id)
context.session.close()
#Network does not have any ports, we can proceed to delete
@ -248,8 +254,12 @@ class PluginV2(db_base_plugin_v2.QuantumDbPluginV2):
subnet = self._get_subnet(context, id)
# Check if ports are using this subnet
allocated_qry = context.session.query(models_v2.IPAllocation)
allocated_qry = allocated_qry.options(orm.joinedload('ports'))
allocated = allocated_qry.filter_by(subnet_id=id).all()
if allocated:
prefix = db_base_plugin_v2.AGENT_OWNER_PREFIX
if not all(a.ports.device_owner.startswith(prefix) for a in
allocated):
raise exc.SubnetInUse(subnet_id=id)
context.session.close()
try:

View File

@ -780,12 +780,26 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
def test_delete_network_if_port_exists(self):
fmt = 'json'
with self.port() as port:
net_id = port['port']['network_id']
req = self.new_delete_request('networks',
port['port']['network_id'])
res = req.get_response(self.api)
self.assertEquals(res.status_int, 409)
def test_delete_network_port_exists_owned_by_network(self):
gateway_ip = '10.0.0.1'
cidr = '10.0.0.0/24'
fmt = 'json'
# Create new network
res = self._create_network(fmt=fmt, name='net',
admin_status_up=True)
network = self.deserialize(fmt, res)
network_id = network['network']['id']
port = self._make_port(fmt, network_id, device_owner='network:dhcp')
req = self.new_delete_request('networks', network_id)
res = req.get_response(self.api)
self.assertEquals(res.status_int, 204)
def test_update_port_delete_ip(self):
with self.subnet() as subnet:
with self.port(subnet=subnet) as port:
@ -1701,6 +1715,31 @@ class TestSubnetsV2(QuantumDbPluginV2TestCase):
res = req.get_response(self.api)
self.assertEquals(res.status_int, 204)
def test_delete_subnet_port_exists_owned_by_network(self):
gateway_ip = '10.0.0.1'
cidr = '10.0.0.0/24'
fmt = 'json'
# Create new network
res = self._create_network(fmt=fmt, name='net',
admin_status_up=True)
network = self.deserialize(fmt, res)
network_id = network['network']['id']
subnet = self._make_subnet(fmt, network, gateway_ip,
cidr, ip_version=4)
port = self._make_port(fmt, network['network']['id'],
device_owner='network:dhcp')
req = self.new_delete_request('subnets', subnet['subnet']['id'])
res = req.get_response(self.api)
self.assertEquals(res.status_int, 204)
def test_delete_subnet_port_exists_owned_by_other(self):
with self.subnet() as subnet:
with self.port(subnet=subnet) as port:
req = self.new_delete_request('subnets',
subnet['subnet']['id'])
res = req.get_response(self.api)
self.assertEquals(res.status_int, 409)
def test_delete_network(self):
gateway_ip = '10.0.0.1'
cidr = '10.0.0.0/24'

View File

@ -207,28 +207,13 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
self.dhcp.network_update_end(payload)
disable.assertCalledOnceWith(fake_network.id)
def test_network_delete_start(self):
def test_network_delete_end(self):
payload = dict(network_id=fake_network.id)
with mock.patch.object(self.dhcp, 'disable_dhcp_helper') as disable:
self.dhcp.network_delete_start(payload)
self.dhcp.network_delete_end(payload)
disable.assertCalledOnceWith(fake_network.id)
def test_subnet_delete_start(self):
payload = dict(subnet_id=fake_subnet1.id)
self.cache.get_network_by_subnet_id.return_value = fake_network
self.dhcp.subnet_delete_start(payload)
self.cache.assert_has_calls(
[mock.call.get_network_by_subnet_id(fake_subnet1.id)])
self.plugin.assert_has_calls(
[mock.call.release_port_fixed_ip(fake_network.id,
mock.ANY,
fake_subnet1.id)])
self.assertEqual(self.call_driver.call_count, 0)
def test_refresh_dhcp_helper_no_dhcp_enabled_networks(self):
network = FakeModel('12345678-1234-5678-1234567890ab',
tenant_id='aaaaaaaa-aaaa-aaaa-aaaaaaaaaaaa',
@ -251,7 +236,7 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
self.dhcp.subnet_update_end(payload)
self.cache.assert_has_calls([mock.call.put(fake_network)])
self.call_driver.assert_called_once_with('update_l3', fake_network)
self.call_driver.assert_called_once_with('enable', fake_network)
def test_subnet_update_end_delete_payload(self):
payload = dict(subnet_id=fake_subnet1.id)
@ -261,7 +246,7 @@ class TestDhcpAgentEventHandler(unittest.TestCase):
self.dhcp.subnet_delete_end(payload)
self.cache.assert_has_calls([mock.call.put(fake_network)])
self.call_driver.assert_called_once_with('update_l3', fake_network)
self.call_driver.assert_called_once_with('enable', fake_network)
def test_port_update_end(self):
payload = dict(port=vars(fake_port2))
@ -548,24 +533,14 @@ class TestDeviceManager(unittest.TestCase):
plugin.get_dhcp_port.return_value = fake_port
dh = dhcp_agent.DeviceManager(cfg.CONF, plugin)
dh.destroy(fake_network)
dh.destroy(fake_network, 'tap12345678-12')
dvr_cls.assert_called_once_with(cfg.CONF)
mock_driver.assert_has_calls(
[mock.call.get_device_name(mock.ANY),
mock.call.unplug('tap12345678-12',
[mock.call.unplug('tap12345678-12',
namespace=fake_network.id)])
plugin.assert_has_calls(
[mock.call.get_dhcp_port(fake_network.id, mock.ANY),
mock.call.release_dhcp_port(fake_network.id, mock.ANY)])
def test_update_l3(self):
fake_network = mock.Mock()
dh = dhcp_agent.DeviceManager(cfg.CONF, None)
with mock.patch.object(dh, 'setup') as setup:
dh.update_l3(fake_network)
setup.called_once_with(fake_network, True)
[mock.call.release_dhcp_port(fake_network.id, mock.ANY)])
def test_get_interface_name(self):
fake_network = FakeModel('12345678-1234-5678-1234567890ab',

View File

@ -135,9 +135,6 @@ class TestDhcpBase(unittest.TestCase):
def disable(self):
self.called.append('disable')
def update_l3(self):
pass
def reload_allocations(self):
pass
@ -227,9 +224,12 @@ class TestDhcpLocalProcess(TestBase):
self.assertTrue(makedirs.called)
def test_enable_already_active(self):
delegate = mock.Mock()
delegate.setup.return_value = 'tap0'
with mock.patch.object(LocalChild, 'active') as patched:
patched.__get__ = mock.Mock(return_value=True)
lp = LocalChild(self.conf, FakeV4Network())
lp = LocalChild(self.conf, FakeV4Network(),
device_delegate=delegate)
lp.enable()
self.assertEqual(lp.called, ['reload'])
@ -238,12 +238,13 @@ class TestDhcpLocalProcess(TestBase):
delegate = mock.Mock(return_value='tap0')
attrs_to_mock = dict(
[(a, mock.DEFAULT) for a in
['active', 'get_conf_file_name']]
['active', 'get_conf_file_name', 'interface_name']]
)
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
mocks['active'].__get__ = mock.Mock(return_value=False)
mocks['get_conf_file_name'].return_value = '/dir'
mocks['interface_name'].__set__ = mock.Mock()
lp = LocalChild(self.conf,
FakeDualNetwork(),
device_delegate=delegate)
@ -252,12 +253,15 @@ class TestDhcpLocalProcess(TestBase):
delegate.assert_has_calls(
[mock.call.setup(mock.ANY, reuse_existing=True)])
self.assertEqual(lp.called, ['spawn'])
self.assertTrue(mocks['interface_name'].__set__.called)
def test_disable_not_active(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in ['active', 'pid']])
attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'interface_name', 'pid']])
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
mocks['active'].__get__ = mock.Mock(return_value=False)
mocks['pid'].__get__ = mock.Mock(return_value=5)
mocks['interface_name'].__get__ = mock.Mock(return_value='tap0')
with mock.patch.object(dhcp.LOG, 'debug') as log:
lp = LocalChild(self.conf, FakeDualNetwork())
lp.disable()
@ -265,10 +269,12 @@ class TestDhcpLocalProcess(TestBase):
self.assertIn('stale', msg)
def test_disable_unknown_network(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in ['active', 'pid']])
attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'interface_name', 'pid']])
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
mocks['active'].__get__ = mock.Mock(return_value=False)
mocks['pid'].__get__ = mock.Mock(return_value=None)
mocks['interface_name'].__get__ = mock.Mock(return_value='tap0')
with mock.patch.object(dhcp.LOG, 'debug') as log:
lp = LocalChild(self.conf, FakeDualNetwork())
lp.disable()
@ -277,35 +283,21 @@ class TestDhcpLocalProcess(TestBase):
def test_disable(self):
attrs_to_mock = dict([(a, mock.DEFAULT) for a in
['active', 'pid']])
['active', 'interface_name', 'pid']])
delegate = mock.Mock()
delegate.intreface_name = 'tap0'
network = FakeDualNetwork()
with mock.patch.multiple(LocalChild, **attrs_to_mock) as mocks:
mocks['active'].__get__ = mock.Mock(return_value=True)
mocks['pid'].__get__ = mock.Mock(return_value=5)
mocks['interface_name'].__get__ = mock.Mock(return_value='tap0')
lp = LocalChild(self.conf, network, device_delegate=delegate)
lp.disable()
delegate.assert_has_calls([mock.call.destroy(network)])
delegate.assert_has_calls([mock.call.destroy(network, 'tap0')])
exp_args = ['ip', 'netns', 'exec',
'cccccccc-cccc-cccc-cccc-cccccccccccc', 'kill', '-9', 5]
self.execute.assert_called_once_with(exp_args, root_helper='sudo')
def test_update_l3(self):
delegate = mock.Mock()
fake_net = FakeDualNetwork()
with mock.patch.object(LocalChild, 'active') as active:
active.__get__ = mock.Mock(return_value=False)
lp = LocalChild(self.conf,
fake_net,
device_delegate=delegate)
lp.update_l3()
delegate.assert_has_calls(
[mock.call.update_l3(fake_net)])
self.assertEqual(lp.called, ['reload'])
def test_pid(self):
with mock.patch('__builtin__.open') as mock_open:
mock_open.return_value.__enter__ = lambda s: s
@ -328,6 +320,24 @@ class TestDhcpLocalProcess(TestBase):
lp = LocalChild(self.conf, FakeDualNetwork())
self.assertIsNone(lp.pid)
def test_get_interface_name(self):
with mock.patch('__builtin__.open') as mock_open:
mock_open.return_value.__enter__ = lambda s: s
mock_open.return_value.__exit__ = mock.Mock()
mock_open.return_value.read.return_value = 'tap0'
lp = LocalChild(self.conf, FakeDualNetwork())
self.assertEqual(lp.interface_name, 'tap0')
def test_set_interface_name(self):
with mock.patch('quantum.agent.linux.dhcp.replace_file') as replace:
lp = LocalChild(self.conf, FakeDualNetwork())
with mock.patch.object(lp, 'get_conf_file_name') as conf_file:
conf_file.return_value = '/interface'
lp.interface_name = 'tap0'
conf_file.assert_called_once_with('interface',
ensure_conf_dir=True)
replace.assert_called_once_with(mock.ANY, 'tap0')
class TestDnsmasq(TestBase):
def _test_spawn(self, extra_options):
@ -372,7 +382,7 @@ class TestDnsmasq(TestBase):
attrs_to_mock = dict(
[(a, mock.DEFAULT) for a in
['_output_opts_file', 'get_conf_file_name']]
['_output_opts_file', 'get_conf_file_name', 'interface_name']]
)
with mock.patch.multiple(dhcp.Dnsmasq, **attrs_to_mock) as mocks:
@ -380,6 +390,7 @@ class TestDnsmasq(TestBase):
mocks['_output_opts_file'].return_value = (
'/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/opts'
)
mocks['interface_name'].__get__ = mock.Mock(return_value='tap0')
with mock.patch.object(dhcp.sys, 'argv') as argv:
argv.__getitem__.side_effect = fake_argv