diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index 6b3e30bfc7..815f78a54d 100644 --- a/quantum/agent/dhcp_agent.py +++ b/quantum/agent/dhcp_agent.py @@ -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): diff --git a/quantum/agent/linux/dhcp.py b/quantum/agent/linux/dhcp.py index 720e732de7..7d8b3c32d7 100644 --- a/quantum/agent/linux/dhcp.py +++ b/quantum/agent/linux/dhcp.py @@ -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', diff --git a/quantum/db/db_base_plugin_v2.py b/quantum/db/db_base_plugin_v2.py index 6ea8339e28..9388ba7c07 100644 --- a/quantum/db/db_base_plugin_v2.py +++ b/quantum/db/db_base_plugin_v2.py @@ -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) diff --git a/quantum/plugins/cisco/network_plugin.py b/quantum/plugins/cisco/network_plugin.py index b3ce907082..ddfdb62434 100644 --- a/quantum/plugins/cisco/network_plugin.py +++ b/quantum/plugins/cisco/network_plugin.py @@ -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: diff --git a/quantum/tests/unit/test_db_plugin.py b/quantum/tests/unit/test_db_plugin.py index c541a540ca..15ffbf5032 100644 --- a/quantum/tests/unit/test_db_plugin.py +++ b/quantum/tests/unit/test_db_plugin.py @@ -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' diff --git a/quantum/tests/unit/test_dhcp_agent.py b/quantum/tests/unit/test_dhcp_agent.py index c6cd495b90..cb1d20cd64 100644 --- a/quantum/tests/unit/test_dhcp_agent.py +++ b/quantum/tests/unit/test_dhcp_agent.py @@ -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', diff --git a/quantum/tests/unit/test_linux_dhcp.py b/quantum/tests/unit/test_linux_dhcp.py index 20ca18d7c2..0dbf6416c9 100644 --- a/quantum/tests/unit/test_linux_dhcp.py +++ b/quantum/tests/unit/test_linux_dhcp.py @@ -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