From b9d6bb031f4679af58c33667bbbff1164f830ce0 Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Tue, 18 Nov 2014 15:59:40 -0800 Subject: [PATCH] Fix enable_metadata_network flag The following patch: 9569b2fe broke the desired functionality of the enable_metadata_network flag, by not allowing the metadata proxy to be spawn for 'metadata networks', which are used for accessing the metadata service when the logical router is not implemented through the l3 agent. This patch enables spawning of the metadata proxy for metadata networks when the appropriate flag is set to True. The patch also adds rather pedant unit test coverage for the should_enable_metadata method which previously had no unit test. Change-Id: I8dca1fce9fbc83e75ba7e4ce948531427bf7e88b Closes-bug: 1394020 --- neutron/agent/dhcp_agent.py | 8 ++--- neutron/agent/linux/dhcp.py | 19 +++++++++- neutron/tests/unit/test_linux_dhcp.py | 50 ++++++++++++++++++++++++--- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py index 58c5eb4c8b..1fb29a3964 100644 --- a/neutron/agent/dhcp_agent.py +++ b/neutron/agent/dhcp_agent.py @@ -20,7 +20,6 @@ import sys import eventlet eventlet.monkey_patch() -import netaddr from oslo.config import cfg from oslo import messaging from oslo.utils import importutils @@ -355,10 +354,9 @@ class DhcpAgent(manager.Manager): # or all the networks connected via a router # to the one passed as a parameter neutron_lookup_param = '--network_id=%s' % network.id - meta_cidr = netaddr.IPNetwork(dhcp.METADATA_DEFAULT_CIDR) - has_metadata_subnet = any(netaddr.IPNetwork(s.cidr) in meta_cidr - for s in network.subnets) - if (self.conf.enable_metadata_network and has_metadata_subnet): + # When the metadata network is enabled, the proxy might + # be started for the router attached to the network + if self.conf.enable_metadata_network: router_ports = [port for port in network.ports if (port.device_owner == constants.DEVICE_OWNER_ROUTER_INTF)] diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index cc57c78061..f5008f7bbe 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -747,8 +747,25 @@ class Dnsmasq(DhcpLocalProcess): @classmethod def should_enable_metadata(cls, conf, network): - """True if there exists a subnet for which a metadata proxy is needed + """Determine whether the metadata proxy is needed for a network + + This method returns True for truly isolated networks (ie: not attached + to a router), when the enable_isolated_metadata flag is True. + + This method also returns True when enable_metadata_network is True, + and the network passed as a parameter has a subnet in the link-local + CIDR, thus characterizing it as a "metadata" network. The metadata + network is used by solutions which do not leverage the l3 agent for + providing access to the metadata service via logical routers built + with 3rd party backends. """ + if conf.enable_metadata_network and conf.enable_isolated_metadata: + # 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): + return True + if not conf.use_namespaces or not conf.enable_isolated_metadata: return False diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 39edd30a8f..c90b69f9cb 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -126,13 +126,14 @@ class FakeRouterPort: id = 'rrrrrrrr-rrrr-rrrr-rrrr-rrrrrrrrrrrr' admin_state_up = True device_owner = constants.DEVICE_OWNER_ROUTER_INTF - fixed_ips = [FakeIPAllocation('192.168.0.1', - 'dddddddd-dddd-dddd-dddd-dddddddddddd')] mac_address = '00:00:0f:rr:rr:rr' - def __init__(self, dev_owner=constants.DEVICE_OWNER_ROUTER_INTF): + def __init__(self, dev_owner=constants.DEVICE_OWNER_ROUTER_INTF, + ip_address='192.168.0.1'): self.extra_dhcp_opts = [] self.device_owner = dev_owner + self.fixed_ips = [FakeIPAllocation( + ip_address, 'dddddddd-dddd-dddd-dddd-dddddddddddd')] class FakePortMultipleAgents1: @@ -184,6 +185,16 @@ class FakeV4Subnet: dns_nameservers = ['8.8.8.8'] +class FakeV4MetadataSubnet: + id = 'dddddddd-dddd-dddd-dddd-dddddddddddd' + ip_version = 4 + cidr = '169.254.169.254/30' + gateway_ip = '169.254.169.253' + enable_dhcp = True + host_routes = [] + dns_nameservers = [] + + class FakeV4SubnetGatewayRoute: id = 'dddddddd-dddd-dddd-dddd-dddddddddddd' ip_version = 4 @@ -342,6 +353,12 @@ class FakeV4NetworkNoRouter: ports = [FakePort1()] +class FakeV4MetadataNetwork: + id = 'cccccccc-cccc-cccc-cccc-cccccccccccc' + subnets = [FakeV4MetadataSubnet()] + ports = [FakeRouterPort(ip_address='169.254.169.253')] + + class FakeV4NetworkDistRouter: id = 'cccccccc-cccc-cccc-cccc-cccccccccccc' subnets = [FakeV4Subnet()] @@ -477,13 +494,15 @@ class TestBase(base.BaseTestCase): self.conf.register_opts(base_config.core_opts) self.conf.register_opts(dhcp.OPTS) config.register_interface_driver_opts_helper(self.conf) + config.register_use_namespaces_opts_helper(self.conf) instance = mock.patch("neutron.agent.linux.dhcp.DeviceManager") self.mock_mgr = instance.start() self.conf.register_opt(cfg.BoolOpt('enable_isolated_metadata', default=True)) + self.conf.register_opt(cfg.BoolOpt('enable_metadata_network', + default=False)) self.config_parse(self.conf) self.conf.set_override('state_path', '') - self.conf.use_namespaces = True self.replace_p = mock.patch('neutron.agent.linux.utils.replace_file') self.execute_p = mock.patch('neutron.agent.linux.utils.execute') @@ -1371,3 +1390,26 @@ tag:tag0,option:router""".lstrip() dm._output_hosts_file() self.safe.assert_has_calls([mock.call(exp_host_name, exp_host_data)]) + + def test_should_enable_metadata_namespaces_disabled_returns_false(self): + self.conf.set_override('use_namespaces', False) + self.assertFalse(dhcp.Dnsmasq.should_enable_metadata(self.conf, + mock.ANY)) + + def test_should_enable_metadata_isolated_network_returns_true(self): + self.assertTrue(dhcp.Dnsmasq.should_enable_metadata( + self.conf, FakeV4NetworkNoRouter())) + + def test_should_enable_metadata_non_isolated_network_returns_false(self): + self.assertFalse(dhcp.Dnsmasq.should_enable_metadata( + self.conf, FakeV4NetworkDistRouter())) + + def test_should_enable_metadata_isolated_meta_disabled_returns_false(self): + self.conf.set_override('enable_isolated_metadata', False) + self.assertFalse(dhcp.Dnsmasq.should_enable_metadata(self.conf, + mock.ANY)) + + def test_should_enable_metadata_with_metadata_network_returns_true(self): + self.conf.set_override('enable_metadata_network', True) + self.assertTrue(dhcp.Dnsmasq.should_enable_metadata( + self.conf, FakeV4MetadataNetwork()))