From a150dc668925ddccc2d4a8fa3001a1ba3bf19c36 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Fri, 31 May 2013 20:44:14 +0000 Subject: [PATCH] Adds default route to DHCP namespace for upstream name resolution. Any time the DHCP server is updated this code will maintain a default route in the DHCP namespace using the gateway_ip attribute of the first DHCP-enabled IPV4 subnet in the list of subnets where gateway_ip is not None. This strategy uses the same gateway ip that the DHCP server hands to the VMs on the network. Fixes: Bug #1181378 Conflicts: quantum/agent/dhcp_agent.py Change-Id: I0807550a848e1b610c7775d215643ad9c83629ed (cherry picked from commit 5dd0cf15463e181b92c8809c2dcc7603bb156a0e) --- quantum/agent/dhcp_agent.py | 55 ++++++++ quantum/tests/unit/test_dhcp_agent.py | 194 ++++++++++++++++++++++++++ 2 files changed, 249 insertions(+) diff --git a/quantum/agent/dhcp_agent.py b/quantum/agent/dhcp_agent.py index 6e4820c4bd2..b252edd54ee 100644 --- a/quantum/agent/dhcp_agent.py +++ b/quantum/agent/dhcp_agent.py @@ -231,6 +231,9 @@ class DhcpAgent(manager.Manager): else: self.disable_dhcp_helper(network.id) + if new_cidrs: + self.device_manager.update(network) + @lockutils.synchronized('agent', 'dhcp-') def network_create_end(self, context, payload): """Handle the network.create.end notification event.""" @@ -526,6 +529,51 @@ class DeviceManager(object): host_uuid = uuid.uuid5(uuid.NAMESPACE_DNS, socket.gethostname()) return 'dhcp%s-%s' % (host_uuid, network.id) + def _get_device(self, network): + """Return DHCP ip_lib device for this host on the network.""" + device_id = self.get_device_id(network) + port = self.plugin.get_dhcp_port(network.id, device_id) + interface_name = self.get_interface_name(network, port) + namespace = NS_PREFIX + network.id + return ip_lib.IPDevice(interface_name, + self.root_helper, + namespace) + + def _set_default_route(self, network): + """Sets the default gateway for this dhcp namespace. + + This method is idempotent and will only adjust the route if adjusting + it would change it from what it already is. This makes it safe to call + and avoids unnecessary perturbation of the system. + """ + device = self._get_device(network) + gateway = device.route.get_gateway() + + for subnet in network.subnets: + skip_subnet = ( + subnet.ip_version != 4 + or not subnet.enable_dhcp + or subnet.gateway_ip is None) + + if skip_subnet: + continue + + if gateway != subnet.gateway_ip: + m = _('Setting gateway for dhcp netns on net %(n)s to %(ip)s') + LOG.debug(m, {'n': network.id, 'ip': subnet.gateway_ip}) + + device.route.add_gateway(subnet.gateway_ip) + + return + + # No subnets on the network have a valid gateway. Clean it up to avoid + # confusion from seeing an invalid gateway here. + if gateway is not None: + msg = _('Removing gateway for dhcp netns on net %s') + LOG.debug(msg, network.id) + + device.route.delete_gateway(gateway) + def setup(self, network, reuse_existing=False): """Create and initialize a device for network's DHCP on this host.""" device_id = self.get_device_id(network) @@ -584,9 +632,16 @@ class DeviceManager(object): # Only 1 subnet on metadata access network gateway_ip = metadata_subnets[0].gateway_ip device.route.add_gateway(gateway_ip) + elif self.conf.use_namespaces: + self._set_default_route(network) return interface_name + def update(self, network): + """Update device settings for the network's DHCP on this host.""" + if self.conf.use_namespaces and not self.conf.enable_metadata_network: + self._set_default_route(network) + def destroy(self, network, device_name): """Destroy the device used for the network's DHCP on this host.""" if self.conf.use_namespaces: diff --git a/quantum/tests/unit/test_dhcp_agent.py b/quantum/tests/unit/test_dhcp_agent.py index b3242e6bf1a..359f1aaad49 100644 --- a/quantum/tests/unit/test_dhcp_agent.py +++ b/quantum/tests/unit/test_dhcp_agent.py @@ -681,12 +681,14 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): payload = dict(subnet=dict(network_id=fake_network.id)) self.cache.get_network_by_id.return_value = fake_network self.plugin.get_network_info.return_value = fake_network + self.dhcp.device_manager.update = mock.Mock() self.dhcp.subnet_update_end(None, payload) self.cache.assert_has_calls([mock.call.put(fake_network)]) self.call_driver.assert_called_once_with('reload_allocations', fake_network) + self.dhcp.device_manager.update.assert_called_once_with(fake_network) def test_subnet_update_end_restart(self): new_state = FakeModel(fake_network.id, @@ -698,12 +700,14 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): payload = dict(subnet=dict(network_id=fake_network.id)) self.cache.get_network_by_id.return_value = fake_network self.plugin.get_network_info.return_value = new_state + self.dhcp.device_manager.update = mock.Mock() self.dhcp.subnet_update_end(None, payload) self.cache.assert_has_calls([mock.call.put(new_state)]) self.call_driver.assert_called_once_with('restart', new_state) + self.dhcp.device_manager.update.assert_called_once_with(new_state) def test_subnet_update_end_delete_payload(self): prev_state = FakeModel(fake_network.id, @@ -716,6 +720,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.cache.get_network_by_subnet_id.return_value = prev_state self.cache.get_network_by_id.return_value = prev_state self.plugin.get_network_info.return_value = fake_network + self.dhcp.device_manager.update = mock.Mock() self.dhcp.subnet_delete_end(None, payload) @@ -726,6 +731,7 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): mock.call.put(fake_network)]) self.call_driver.assert_called_once_with('restart', fake_network) + self.dhcp.device_manager.update.assert_called_once_with(fake_network) def test_port_update_end(self): payload = dict(port=vars(fake_port2)) @@ -938,6 +944,44 @@ class TestNetworkCache(base.BaseTestCase): self.assertEqual(nc.get_port_by_id(fake_port1.id), fake_port1) +class FakePort1: + id = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee' + + +class FakeV4Subnet: + id = 'dddddddd-dddd-dddd-dddd-dddddddddddd' + ip_version = 4 + cidr = '192.168.0.0/24' + gateway_ip = '192.168.0.1' + enable_dhcp = True + + +class FakeV4SubnetNoGateway: + id = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee' + ip_version = 4 + cidr = '192.168.1.0/24' + gateway_ip = None + enable_dhcp = True + + +class FakeV4Network: + id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + subnets = [FakeV4Subnet()] + ports = [FakePort1()] + + +class FakeV4NetworkNoSubnet: + id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + subnets = [] + ports = [] + + +class FakeV4NetworkNoGateway: + id = 'cccccccc-cccc-cccc-cccc-cccccccccccc' + subnets = [FakeV4SubnetNoGateway()] + ports = [FakePort1()] + + class TestDeviceManager(base.BaseTestCase): def setUp(self): super(TestDeviceManager, self).setUp() @@ -982,6 +1026,7 @@ class TestDeviceManager(base.BaseTestCase): self.mock_driver.get_device_name.return_value = 'tap12345678-12' dh = dhcp_agent.DeviceManager(cfg.CONF, plugin) + dh._set_default_route = mock.Mock() interface_name = dh.setup(net, reuse_existing) self.assertEqual(interface_name, 'tap12345678-12') @@ -1005,6 +1050,8 @@ class TestDeviceManager(base.BaseTestCase): namespace=namespace)) self.mock_driver.assert_has_calls(expected) + dh._set_default_route.assert_called_once_with(net) + def test_setup(self): self._test_setup_helper(False) @@ -1119,6 +1166,153 @@ class TestDeviceManager(base.BaseTestCase): uuid5.called_once_with(uuid.NAMESPACE_DNS, 'localhost') self.assertEqual(dh.get_device_id(fake_network), expected) + def _get_device_manager_with_mock_device(self, conf, device): + dh = dhcp_agent.DeviceManager(conf, None) + dh._get_device = mock.Mock(return_value=device) + return dh + + def test_update(self): + # Try with namespaces and no metadata network + cfg.CONF.set_override('use_namespaces', True) + cfg.CONF.set_override('enable_metadata_network', False) + dh = dhcp_agent.DeviceManager(cfg.CONF, None) + dh._set_default_route = mock.Mock() + + dh.update(True) + + dh._set_default_route.assert_called_once_with(True) + + # No namespaces, shouldn't set default route. + cfg.CONF.set_override('use_namespaces', False) + cfg.CONF.set_override('enable_metadata_network', False) + dh = dhcp_agent.DeviceManager(cfg.CONF, None) + dh._set_default_route = mock.Mock() + + dh.update(FakeV4Network()) + + self.assertFalse(dh._set_default_route.called) + + # Meta data network enabled, don't interfere with its gateway. + cfg.CONF.set_override('use_namespaces', True) + cfg.CONF.set_override('enable_metadata_network', True) + dh = dhcp_agent.DeviceManager(cfg.CONF, None) + dh._set_default_route = mock.Mock() + + dh.update(FakeV4Network()) + + self.assertFalse(dh._set_default_route.called) + + # For completeness + cfg.CONF.set_override('use_namespaces', False) + cfg.CONF.set_override('enable_metadata_network', True) + dh = dhcp_agent.DeviceManager(cfg.CONF, None) + dh._set_default_route = mock.Mock() + + dh.update(FakeV4Network()) + + self.assertFalse(dh._set_default_route.called) + + def test_set_default_route(self): + device = mock.Mock() + device.route.get_gateway.return_value = None + + # Basic one subnet with gateway. + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4Network() + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + self.assertFalse(device.route.delete_gateway.called) + device.route.add_gateway.assert_called_once_with('192.168.0.1') + + def test_set_default_route_no_subnet(self): + device = mock.Mock() + device.route.get_gateway.return_value = None + + # Try a namespace but no subnet. + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4NetworkNoSubnet() + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + self.assertFalse(device.route.delete_gateway.called) + self.assertFalse(device.route.add_gateway.called) + + def test_set_default_route_no_subnet_delete_gateway(self): + device = mock.Mock() + device.route.get_gateway.return_value = '192.168.0.1' + + # Try a namespace but no subnet where a gateway needs to be deleted. + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4NetworkNoSubnet() + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + device.route.delete_gateway.assert_called_once_with('192.168.0.1') + self.assertFalse(device.route.add_gateway.called) + + def test_set_default_route_no_gateway(self): + device = mock.Mock() + device.route.get_gateway.return_value = '192.168.0.1' + + # Try a subnet with no gateway + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4NetworkNoGateway() + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + device.route.delete_gateway.assert_called_once_with('192.168.0.1') + self.assertFalse(device.route.add_gateway.called) + + def test_set_default_route_do_nothing(self): + device = mock.Mock() + device.route.get_gateway.return_value = '192.168.0.1' + + # Try a subnet where the gateway doesn't change. Should do nothing. + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4Network() + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + self.assertFalse(device.route.delete_gateway.called) + self.assertFalse(device.route.add_gateway.called) + + def test_set_default_route_change_gateway(self): + device = mock.Mock() + device.route.get_gateway.return_value = '192.168.0.2' + + # Try a subnet with a gateway this is different than the current. + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4Network() + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + self.assertFalse(device.route.delete_gateway.called) + device.route.add_gateway.assert_called_once_with('192.168.0.1') + + def test_set_default_route_two_subnets(self): + device = mock.Mock() + device.route.get_gateway.return_value = None + + # Try two subnets. Should set gateway from the first. + dh = self._get_device_manager_with_mock_device(cfg.CONF, device) + network = FakeV4Network() + subnet2 = FakeV4Subnet() + subnet2.gateway_ip = '192.168.1.1' + network.subnets = [subnet2, FakeV4Subnet()] + + dh._set_default_route(network) + + device.route.get_gateway.assert_called_once() + self.assertFalse(device.route.delete_gateway.called) + device.route.add_gateway.assert_called_once_with('192.168.1.1') + class TestDhcpLeaseRelay(base.BaseTestCase): def setUp(self):