From d2b82168cd35adc02599e4fe226f25cfc9b786a8 Mon Sep 17 00:00:00 2001 From: Harald Jensas Date: Sun, 28 May 2017 21:49:59 +0200 Subject: [PATCH] DHCP Agent: Separate local from non-local subnets In order to allow the DHCP agent to service other subnets on the network in other segments via DHCP relay, we need to use the 'non_local_subnets' network attribute returned by rpc to set up dhcp for off-link subnets. Change-Id: I88e1c574bc429dc599ad7c956c03fa0688338186 Closes-Bug: 1692486 --- neutron/agent/dhcp/agent.py | 19 ++++++++--- neutron/agent/linux/dhcp.py | 30 ++++++++++------ .../tests/functional/agent/test_dhcp_agent.py | 5 ++- neutron/tests/unit/agent/linux/test_dhcp.py | 34 +++++++++++++++++++ 4 files changed, 72 insertions(+), 16 deletions(-) diff --git a/neutron/agent/dhcp/agent.py b/neutron/agent/dhcp/agent.py index 45ab6d23cf2..515e35e4501 100644 --- a/neutron/agent/dhcp/agent.py +++ b/neutron/agent/dhcp/agent.py @@ -109,7 +109,8 @@ class DhcpAgent(manager.Manager): self.conf ) for net_id in existing_networks: - net = dhcp.NetModel({"id": net_id, "subnets": [], "ports": []}) + net = dhcp.NetModel({"id": net_id, "subnets": [], + "non_local_subnets": [], "ports": []}) self.cache.put(net) except NotImplementedError: # just go ahead with an empty networks cache @@ -334,8 +335,12 @@ class DhcpAgent(manager.Manager): return # NOTE(kevinbenton): we don't exclude dhcp disabled subnets because # they still change the indexes used for tags - old_cidrs = [s.cidr for s in old_network.subnets] - new_cidrs = [s.cidr for s in network.subnets] + old_non_local_subnets = getattr(old_network, 'non_local_subnets', []) + new_non_local_subnets = getattr(network, 'non_local_subnets', []) + old_cidrs = [s.cidr for s in (old_network.subnets + + old_non_local_subnets)] + new_cidrs = [s.cidr for s in (network.subnets + + new_non_local_subnets)] if old_cidrs == new_cidrs: self.call_driver('reload_allocations', network) self.cache.put(network) @@ -639,7 +644,8 @@ class NetworkCache(object): self.cache[network.id] = network - for subnet in network.subnets: + non_local_subnets = getattr(network, 'non_local_subnets', []) + for subnet in (network.subnets + non_local_subnets): self.subnet_lookup[subnet.id] = network.id for port in network.ports: @@ -648,7 +654,8 @@ class NetworkCache(object): def remove(self, network): del self.cache[network.id] - for subnet in network.subnets: + non_local_subnets = getattr(network, 'non_local_subnets', []) + for subnet in (network.subnets + non_local_subnets): del self.subnet_lookup[subnet.id] for port in network.ports: @@ -688,7 +695,9 @@ class NetworkCache(object): num_ports = 0 for net_id in net_ids: network = self.get_network_by_id(net_id) + non_local_subnets = getattr(network, 'non_local_subnets', []) num_subnets += len(network.subnets) + num_subnets += len(non_local_subnets) num_ports += len(network.ports) return {'networks': num_nets, 'subnets': num_subnets, diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 07cb83509cd..ea7e00446fe 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -195,6 +195,11 @@ class DhcpLocalProcess(DhcpBase): def _remove_config_files(self): shutil.rmtree(self.network_conf_dir, ignore_errors=True) + @staticmethod + def _get_all_subnets(network): + non_local_subnets = getattr(network, 'non_local_subnets', []) + return network.subnets + non_local_subnets + def _enable_dhcp(self): """check if there is a subnet within the network with dhcp enabled.""" for subnet in self.network.subnets: @@ -346,7 +351,7 @@ class Dnsmasq(DhcpLocalProcess): ] possible_leases = 0 - for i, subnet in enumerate(self.network.subnets): + for i, subnet in enumerate(self._get_all_subnets(self.network)): mode = None # if a subnet is specified to have dhcp disabled if not subnet.enable_dhcp: @@ -562,7 +567,8 @@ class Dnsmasq(DhcpLocalProcess): ) """ v6_nets = dict((subnet.id, subnet) for subnet in - self.network.subnets if subnet.ip_version == 6) + self._get_all_subnets(self.network) + if subnet.ip_version == 6) for port in self.network.ports: fixed_ips = self._sort_fixed_ips_for_dnsmasq(port.fixed_ips, @@ -625,7 +631,8 @@ class Dnsmasq(DhcpLocalProcess): timestamp = 0 else: timestamp = int(time.time()) + self.conf.dhcp_lease_duration - dhcp_enabled_subnet_ids = [s.id for s in self.network.subnets + dhcp_enabled_subnet_ids = [s.id for s in + self._get_all_subnets(self.network) if s.enable_dhcp] for host_tuple in self._iter_hosts(): port, alloc, hostname, name, no_dhcp, no_opts = host_tuple @@ -674,7 +681,8 @@ class Dnsmasq(DhcpLocalProcess): filename = self.get_conf_file_name('host') LOG.debug('Building host file: %s', filename) - dhcp_enabled_subnet_ids = [s.id for s in self.network.subnets + dhcp_enabled_subnet_ids = [s.id for s in + self._get_all_subnets(self.network) if s.enable_dhcp] # NOTE(ihrachyshka): the loop should not log anything inside it, to # avoid potential performance drop when lots of hosts are dumped @@ -867,7 +875,7 @@ class Dnsmasq(DhcpLocalProcess): if self.conf.enable_isolated_metadata or self.conf.force_metadata: subnet_to_interface_ip = self._make_subnet_interface_ip_map() isolated_subnets = self.get_isolated_subnets(self.network) - for i, subnet in enumerate(self.network.subnets): + for i, subnet in enumerate(self._get_all_subnets(self.network)): addr_mode = getattr(subnet, 'ipv6_address_mode', None) segment_id = getattr(subnet, 'segment_id', None) if (not subnet.enable_dhcp or @@ -915,7 +923,7 @@ class Dnsmasq(DhcpLocalProcess): ) if subnet.ip_version == 4: - for s in self.network.subnets: + for s in self._get_all_subnets(self.network): sub_segment_id = getattr(s, 'segment_id', None) if (s.ip_version == 4 and s.cidr != subnet.cidr and @@ -1046,7 +1054,8 @@ class Dnsmasq(DhcpLocalProcess): gateway. The port must be owned by a neutron router. """ isolated_subnets = collections.defaultdict(lambda: True) - subnets = dict((subnet.id, subnet) for subnet in network.subnets) + all_subnets = cls._get_all_subnets(network) + subnets = dict((subnet.id, subnet) for subnet in all_subnets) for port in network.ports: if port.device_owner not in constants.ROUTER_INTERFACE_OWNERS: @@ -1074,7 +1083,8 @@ class Dnsmasq(DhcpLocalProcess): with 3rd party backends. """ # Only IPv4 subnets, with dhcp enabled, will use the metadata proxy. - v4_dhcp_subnets = [s for s in network.subnets + all_subnets = cls._get_all_subnets(network) + v4_dhcp_subnets = [s for s in all_subnets if s.ip_version == 4 and s.enable_dhcp] if not v4_dhcp_subnets: return False @@ -1089,7 +1099,7 @@ class Dnsmasq(DhcpLocalProcess): # check if the network has a metadata subnet meta_cidr = netaddr.IPNetwork(METADATA_DEFAULT_CIDR) if any(netaddr.IPNetwork(s.cidr) in meta_cidr - for s in network.subnets): + for s in all_subnets): return True isolated_subnets = cls.get_isolated_subnets(network) @@ -1294,7 +1304,7 @@ class DeviceManager(object): # The ID that the DHCP port will have (or already has). device_id = self.get_device_id(network) - # Get the set of DHCP-enabled subnets on this network. + # Get the set of DHCP-enabled local subnets on this network. dhcp_subnets = {subnet.id: subnet for subnet in network.subnets if subnet.enable_dhcp} diff --git a/neutron/tests/functional/agent/test_dhcp_agent.py b/neutron/tests/functional/agent/test_dhcp_agent.py index 83e35f49dfc..f242b511b6f 100644 --- a/neutron/tests/functional/agent/test_dhcp_agent.py +++ b/neutron/tests/functional/agent/test_dhcp_agent.py @@ -136,12 +136,15 @@ class DHCPAgentOVSTestFramework(base.BaseSudoTestCase): "ip_address": ip_address}], }) return port_dict - def create_network_dict(self, net_id, subnets=None, ports=None): + def create_network_dict(self, net_id, subnets=None, ports=None, + non_local_subnets=None): subnets = [] if not subnets else subnets ports = [] if not ports else ports + non_local_subnets = [] if not non_local_subnets else non_local_subnets net_dict = dhcp.NetModel(d={ "id": net_id, "subnets": subnets, + "non_local_subnets": non_local_subnets, "ports": ports, "admin_state_up": True, "tenant_id": uuidutils.generate_uuid(), }) diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 1de836a071e..dda3e9407ed 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -405,6 +405,14 @@ class FakeV4SubnetSegmentID(FakeV4Subnet): self.segment_id = 1 +class FakeV4SubnetSegmentID2(FakeV4Subnet): + def __init__(self): + super(FakeV4SubnetSegmentID2, self).__init__() + self.id = 'jjjjjjjj-jjjj-jjjj-jjjj-jjjjjjjjjjjj' + self.host_routes = [] + self.segment_id = 2 + + class FakeV4MetadataSubnet(FakeV4Subnet): def __init__(self): super(FakeV4MetadataSubnet, self).__init__() @@ -685,6 +693,15 @@ class FakeDualNetworkDualDHCPOnLinkSubnetRoutesDisabled(object): self.namespace = 'qdhcp-ns' +class FakeNonLocalSubnets(object): + def __init__(self): + self.id = 'cccccccc-cccc-cccc-cccc-cccccccccccc' + self.subnets = [FakeV4SubnetSegmentID2()] + self.non_local_subnets = [FakeV4SubnetSegmentID()] + self.ports = [FakePort1(), FakeRouterPort(), FakeRouterPortSegmentID()] + self.namespace = 'qdhcp-ns' + + class FakeDualNetworkTriDHCPOneOnLinkSubnetRoute(object): def __init__(self): self.id = 'cccccccc-cccc-cccc-cccc-cccccccccccc' @@ -1522,6 +1539,23 @@ class TestDnsmasq(TestBase): self._test_output_opts_file(expected, FakeV4NoGatewayNetwork(), ipm_retval=ipm_retval) + def test_non_local_subnets(self): + expected = ( + 'tag:tag0,option:dns-server,8.8.8.8\n' + 'tag:tag0,option:classless-static-route,' + '169.254.169.254/32,192.168.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,169.254.169.254/32,192.168.0.1,' + '0.0.0.0/0,192.168.0.1\ntag:tag0,option:router,192.168.0.1\n' + 'tag:tag1,option:dns-server,8.8.8.8\n' + 'tag:tag1,option:classless-static-route,' + '169.254.169.254/32,192.168.2.1,0.0.0.0/0,192.168.2.1\n' + 'tag:tag1,249,169.254.169.254/32,192.168.2.1,' + '0.0.0.0/0,192.168.2.1\n' + 'tag:tag1,option:router,192.168.2.1').lstrip() + ipm_retval = {FakeV4SubnetSegmentID2().id: '192.168.0.1'} + self._test_output_opts_file(expected, FakeNonLocalSubnets(), + ipm_retval=ipm_retval) + def test_output_opts_file_no_neutron_router_on_subnet(self): expected = ( 'tag:tag0,option:classless-static-route,'