From 9569b2fe58d0e836071992f545886ca985d5ace8 Mon Sep 17 00:00:00 2001 From: John Schwarz Date: Tue, 26 Aug 2014 11:43:11 +0300 Subject: [PATCH] Don't spawn metadata-proxy for non-isolated nets If the configuation option "enable_isolated_metadata = True" for the DHCP agent is set, the neutron-ns-metadata-proxy process is spawned for all networks, regardless if they are isolated or not. In case the network is not isolated (ie. connected to a neutron router), the L3 agent also spawns a proxy process, and the DHCP's proxy is left unused. This patch adds a check prior to the spawning of new proxies: if a network is not isolated, no proxy is spawned. Change-Id: I9bdb8c3d37997b22435bca33ec47a67db08efa51 Closes-bug: #1361545 --- neutron/agent/dhcp_agent.py | 13 ++++-- neutron/agent/linux/dhcp.py | 58 +++++++++++++++++------- neutron/tests/unit/test_dhcp_agent.py | 65 ++++++++++++++++++++++----- 3 files changed, 105 insertions(+), 31 deletions(-) diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py index 771d3621d4..fc0fccc3a9 100644 --- a/neutron/agent/dhcp_agent.py +++ b/neutron/agent/dhcp_agent.py @@ -223,12 +223,15 @@ class DhcpAgent(manager.Manager): if not network.admin_state_up: return + enable_metadata = self.dhcp_driver_cls.should_enable_metadata( + self.conf, network) + for subnet in network.subnets: - if subnet.enable_dhcp: + if subnet.enable_dhcp and subnet.ip_version == 4: if self.call_driver('enable', network): - if (self.conf.use_namespaces and - self.conf.enable_isolated_metadata): + if self.conf.use_namespaces and enable_metadata: self.enable_isolated_metadata_proxy(network) + enable_metadata = False # Don't trigger twice self.cache.put(network) break @@ -238,6 +241,10 @@ class DhcpAgent(manager.Manager): if network: if (self.conf.use_namespaces and self.conf.enable_isolated_metadata): + # NOTE(jschwarz): In the case where a network is deleted, all + # the subnets and ports are deleted before this function is + # called, so checking if 'should_enable_metadata' is True + # for any subnet is false logic here. self.disable_isolated_metadata_proxy(network) if self.call_driver('disable', network): self.cache.remove(network) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 59452732ee..cd6ffdb51f 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -175,6 +175,16 @@ class DhcpBase(object): raise NotImplementedError + @classmethod + def get_isolated_subnets(cls, network): + """Returns a dict indicating whether or not a subnet is isolated""" + raise NotImplementedError + + @classmethod + def should_enable_metadata(cls, conf, network): + """True if the metadata-proxy should be enabled for the network.""" + raise NotImplementedError + class DhcpLocalProcess(DhcpBase): PORTS = [] @@ -558,6 +568,7 @@ class Dnsmasq(DhcpLocalProcess): options = [] + isolated_subnets = self.get_isolated_subnets(self.network) dhcp_ips = collections.defaultdict(list) subnet_idx_map = {} for i, subnet in enumerate(self.network.subnets): @@ -593,7 +604,9 @@ class Dnsmasq(DhcpLocalProcess): # Add host routes for isolated network segments - if self._enable_metadata(subnet): + if (isolated_subnets[subnet.id] and + self.conf.enable_isolated_metadata and + subnet.ip_version == 4): subnet_dhcp_ip = subnet_to_interface_ip[subnet.id] host_routes.append( '%s/32,%s' % (METADATA_DEFAULT_IP, subnet_dhcp_ip) @@ -705,25 +718,36 @@ class Dnsmasq(DhcpLocalProcess): return ips return ['[' + ip + ']' for ip in ips] - def _enable_metadata(self, subnet): - '''Determine if the metadata route will be pushed to hosts on subnet. + @classmethod + def get_isolated_subnets(cls, network): + """Returns a dict indicating whether or not a subnet is isolated - If subnet has a Neutron router attached, we want the hosts to get - metadata from the router's proxy via their default route instead. - ''' - if self.conf.enable_isolated_metadata and subnet.ip_version == 4: - if subnet.gateway_ip is None: - return True - else: - for port in self.network.ports: - if port.device_owner == constants.DEVICE_OWNER_ROUTER_INTF: - for alloc in port.fixed_ips: - if alloc.subnet_id == subnet.id: - return False - return True - else: + A subnet is considered non-isolated if there is a port connected to + the subnet, and the port's ip address matches that of the subnet's + gateway. The port must be owned by a nuetron router. + """ + isolated_subnets = collections.defaultdict(lambda: True) + subnets = dict((subnet.id, subnet) for subnet in network.subnets) + + for port in network.ports: + if port.device_owner != constants.DEVICE_OWNER_ROUTER_INTF: + continue + for alloc in port.fixed_ips: + if subnets[alloc.subnet_id].gateway_ip == alloc.ip_address: + isolated_subnets[alloc.subnet_id] = False + + return isolated_subnets + + @classmethod + def should_enable_metadata(cls, conf, network): + """True if there exists a subnet for which a metadata proxy is needed + """ + if not conf.use_namespaces or not conf.enable_isolated_metadata: return False + isolated_subnets = cls.get_isolated_subnets(network) + return any(isolated_subnets[subnet.id] for subnet in network.subnets) + @classmethod def lease_update(cls): network_id = os.environ.get(cls.NEUTRON_NETWORK_ID_KEY) diff --git a/neutron/tests/unit/test_dhcp_agent.py b/neutron/tests/unit/test_dhcp_agent.py index 3740199ae4..d15f0b0a54 100644 --- a/neutron/tests/unit/test_dhcp_agent.py +++ b/neutron/tests/unit/test_dhcp_agent.py @@ -79,12 +79,14 @@ fake_allocation_pool_subnet1 = dhcp.DictModel(dict(id='', start='172.9.9.2', fake_port1 = dhcp.DictModel(dict(id='12345678-1234-aaaa-1234567890ab', device_id='dhcp-12345678-1234-aaaa-1234567890ab', + device_owner='', allocation_pools=fake_subnet1_allocation_pools, mac_address='aa:bb:cc:dd:ee:ff', network_id='12345678-1234-5678-1234567890ab', fixed_ips=[fake_fixed_ip1])) fake_port2 = dhcp.DictModel(dict(id='12345678-1234-aaaa-123456789000', + device_owner='', mac_address='aa:bb:cc:dd:ee:99', network_id='12345678-1234-5678-1234567890ab', fixed_ips=[])) @@ -102,6 +104,22 @@ fake_network = dhcp.NetModel(True, dict(id='12345678-1234-5678-1234567890ab', subnets=[fake_subnet1, fake_subnet2], ports=[fake_port1])) +isolated_network = dhcp.NetModel( + True, dict( + id='12345678-1234-5678-1234567890ab', + tenant_id='aaaaaaaa-aaaa-aaaa-aaaaaaaaaaaa', + admin_state_up=True, + subnets=[fake_subnet1], + ports=[fake_port1])) + +empty_network = dhcp.NetModel( + True, dict( + id='12345678-1234-5678-1234567890ab', + tenant_id='aaaaaaaa-aaaa-aaaa-aaaaaaaaaaaa', + admin_state_up=True, + subnets=[fake_subnet1], + ports=[])) + fake_meta_network = dhcp.NetModel( True, dict(id='12345678-1234-5678-1234567890ab', tenant_id='aaaaaaaa-aaaa-aaaa-aaaaaaaaaaaa', @@ -485,16 +503,17 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): ) self.external_process = self.external_process_p.start() - def _enable_dhcp_helper(self, isolated_metadata=False): - if isolated_metadata: + def _enable_dhcp_helper(self, network, enable_isolated_metadata=False, + is_isolated_network=False): + if enable_isolated_metadata: cfg.CONF.set_override('enable_isolated_metadata', True) - self.plugin.get_network_info.return_value = fake_network - self.dhcp.enable_dhcp_helper(fake_network.id) + self.plugin.get_network_info.return_value = network + self.dhcp.enable_dhcp_helper(network.id) self.plugin.assert_has_calls( - [mock.call.get_network_info(fake_network.id)]) - self.call_driver.assert_called_once_with('enable', fake_network) - self.cache.assert_has_calls([mock.call.put(fake_network)]) - if isolated_metadata: + [mock.call.get_network_info(network.id)]) + self.call_driver.assert_called_once_with('enable', network) + self.cache.assert_has_calls([mock.call.put(network)]) + if is_isolated_network: self.external_process.assert_has_calls([ mock.call( cfg.CONF, @@ -506,11 +525,35 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): else: self.assertFalse(self.external_process.call_count) - def test_enable_dhcp_helper_enable_isolated_metadata(self): - self._enable_dhcp_helper(isolated_metadata=True) + def test_enable_dhcp_helper_enable_metadata_isolated_network(self): + self._enable_dhcp_helper(isolated_network, + enable_isolated_metadata=True, + is_isolated_network=True) + + def test_enable_dhcp_helper_enable_metadata_no_gateway(self): + isolated_network_no_gateway = copy.deepcopy(isolated_network) + isolated_network_no_gateway.subnets[0].gateway_ip = None + + self._enable_dhcp_helper(isolated_network_no_gateway, + enable_isolated_metadata=True, + is_isolated_network=True) + + def test_enable_dhcp_helper_enable_metadata_nonisolated_network(self): + nonisolated_network = copy.deepcopy(isolated_network) + nonisolated_network.ports[0].device_owner = "network:router_interface" + nonisolated_network.ports[0].fixed_ips[0].ip_address = '172.9.9.1' + + self._enable_dhcp_helper(nonisolated_network, + enable_isolated_metadata=True, + is_isolated_network=False) + + def test_enable_dhcp_helper_enable_metadata_empty_network(self): + self._enable_dhcp_helper(empty_network, + enable_isolated_metadata=True, + is_isolated_network=True) def test_enable_dhcp_helper(self): - self._enable_dhcp_helper() + self._enable_dhcp_helper(fake_network) def test_enable_dhcp_helper_down_network(self): self.plugin.get_network_info.return_value = fake_down_network