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 9569b2fe58)

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
+                    if self.conf.use_namespaces and enable_metadata:
-                        self.conf.enable_isolated_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)

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):
+    @classmethod
-        '''Determine if the metadata route will be pushed to hosts on subnet.
+    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
+        A subnet is considered non-isolated if there is a port connected to
-        metadata from the router's proxy via their default route instead.
+        the subnet, and the port's ip address matches that of the subnet's
-        '''
+        gateway. The port must be owned by a nuetron router.
-        if self.conf.enable_isolated_metadata and subnet.ip_version == 4:
+        """
-            if subnet.gateway_ip is None:
+        isolated_subnets = collections.defaultdict(lambda: True)
-                return True
+        subnets = dict((subnet.id, subnet) for subnet in network.subnets)
-            else:
+
-                for port in self.network.ports:
+        for port in network.ports:
-                    if port.device_owner == constants.DEVICE_OWNER_ROUTER_INTF:
+            if port.device_owner != constants.DEVICE_OWNER_ROUTER_INTF:
-                        for alloc in port.fixed_ips:
+                continue
-                            if alloc.subnet_id == subnet.id:
+            for alloc in port.fixed_ips:
-                                return False
+                if subnets[alloc.subnet_id].gateway_ip == alloc.ip_address:
-                return True
+                    isolated_subnets[alloc.subnet_id] = False
-        else:
+
+        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)

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):
+    def _enable_dhcp_helper(self, network, enable_isolated_metadata=False,
-        if isolated_metadata:
+                            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.plugin.get_network_info.return_value = network
-        self.dhcp.enable_dhcp_helper(fake_network.id)
+        self.dhcp.enable_dhcp_helper(network.id)
         self.plugin.assert_has_calls(
-            [mock.call.get_network_info(fake_network.id)])
+            [mock.call.get_network_info(network.id)])
-        self.call_driver.assert_called_once_with('enable', fake_network)
+        self.call_driver.assert_called_once_with('enable', network)
-        self.cache.assert_has_calls([mock.call.put(fake_network)])
+        self.cache.assert_has_calls([mock.call.put(network)])
-        if isolated_metadata:
+        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):
+    def test_enable_dhcp_helper_enable_metadata_isolated_network(self):
-        self._enable_dhcp_helper(isolated_metadata=True)
+        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