From 645f9849b6ccf2653ec68e54cf793453af34fe6a 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. Conflicts: neutron/tests/unit/test_dhcp_agent.py Change-Id: I9bdb8c3d37997b22435bca33ec47a67db08efa51 Closes-bug: #1361545 (cherry picked from commit 9569b2fe58d0e836071992f545886ca985d5ace8) --- 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 2cfbe06fb2a..71c970982b4 100644 --- a/neutron/agent/dhcp_agent.py +++ b/neutron/agent/dhcp_agent.py @@ -211,12 +211,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 @@ -226,6 +229,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 749048da8bc..de4f0f895be 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -148,6 +148,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 = [] @@ -514,6 +524,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): @@ -538,7 +549,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) @@ -623,25 +636,36 @@ class Dnsmasq(DhcpLocalProcess): return ','.join((set_tag + tag, '%s' % option) + args) - 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 e54f303db5f..755f37c8375 100644 --- a/neutron/tests/unit/test_dhcp_agent.py +++ b/neutron/tests/unit/test_dhcp_agent.py @@ -88,12 +88,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=[])) @@ -111,6 +113,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', @@ -497,16 +515,17 @@ class TestDhcpAgentEventHandler(base.BaseTestCase): self.mock_init_p.stop() super(TestDhcpAgentEventHandler, self).tearDown() - 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, @@ -518,11 +537,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