DHCP agent: clarify logic of setup_dhcp_port
When the DHCP port already exists, the code for finding it is unhelpfully mixed up with the code for updating its subnet IDs and fixed IP addresses. Clarify that area by splitting setup_dhcp_port into 3 subroutines, for each of the existing, reserved and new port cases. Related-Bug: #1486649 Change-Id: I2a537560dc7a37299f4b7b4cd508d9309bbe1209
This commit is contained in:
Neil Jerram
parent
f5344dec5b
commit
43d62c62a8
|
|
@ -996,77 +996,111 @@ class DeviceManager(object):
|
|||
|
||||
device.route.delete_gateway(gateway)
|
||||
|
||||
def setup_dhcp_port(self, network):
|
||||
"""Create/update DHCP port for the host if needed and return port."""
|
||||
def _setup_existing_dhcp_port(self, network, device_id, dhcp_subnets):
|
||||
"""Set up the existing DHCP port, if there is one."""
|
||||
|
||||
device_id = self.get_device_id(network)
|
||||
subnets = {subnet.id: subnet for subnet in network.subnets
|
||||
if subnet.enable_dhcp}
|
||||
# To avoid pylint thinking that port might be undefined after
|
||||
# the following loop...
|
||||
port = None
|
||||
|
||||
dhcp_port = None
|
||||
# Look for an existing DHCP for this network.
|
||||
for port in network.ports:
|
||||
port_device_id = getattr(port, 'device_id', None)
|
||||
if port_device_id == device_id:
|
||||
dhcp_enabled_subnet_ids = set(subnets)
|
||||
port_fixed_ips = []
|
||||
for fixed_ip in port.fixed_ips:
|
||||
if fixed_ip.subnet_id in dhcp_enabled_subnet_ids:
|
||||
port_fixed_ips.append(
|
||||
{'subnet_id': fixed_ip.subnet_id,
|
||||
'ip_address': fixed_ip.ip_address})
|
||||
|
||||
port_subnet_ids = set(ip.subnet_id for ip in port.fixed_ips)
|
||||
# If there is a new dhcp enabled subnet or a port that is no
|
||||
# longer on a dhcp enabled subnet, we need to call update.
|
||||
if dhcp_enabled_subnet_ids != port_subnet_ids:
|
||||
port_fixed_ips.extend(
|
||||
dict(subnet_id=s)
|
||||
for s in dhcp_enabled_subnet_ids - port_subnet_ids)
|
||||
dhcp_port = self.plugin.update_dhcp_port(
|
||||
port.id, {'port': {'network_id': network.id,
|
||||
'fixed_ips': port_fixed_ips}})
|
||||
if not dhcp_port:
|
||||
raise exceptions.Conflict()
|
||||
else:
|
||||
dhcp_port = port
|
||||
# break since we found port that matches device_id
|
||||
break
|
||||
else:
|
||||
return None
|
||||
|
||||
# check for a reserved DHCP port
|
||||
if dhcp_port is None:
|
||||
LOG.debug('DHCP port %(device_id)s on network %(network_id)s'
|
||||
' does not yet exist. Checking for a reserved port.',
|
||||
{'device_id': device_id, 'network_id': network.id})
|
||||
for port in network.ports:
|
||||
port_device_id = getattr(port, 'device_id', None)
|
||||
if port_device_id == constants.DEVICE_ID_RESERVED_DHCP_PORT:
|
||||
dhcp_port = self.plugin.update_dhcp_port(
|
||||
port.id, {'port': {'network_id': network.id,
|
||||
'device_id': device_id}})
|
||||
if dhcp_port:
|
||||
break
|
||||
# Compare what the subnets should be against what is already
|
||||
# on the port.
|
||||
dhcp_enabled_subnet_ids = set(dhcp_subnets)
|
||||
port_subnet_ids = set(ip.subnet_id for ip in port.fixed_ips)
|
||||
|
||||
# DHCP port has not yet been created.
|
||||
if dhcp_port is None:
|
||||
LOG.debug('DHCP port %(device_id)s on network %(network_id)s'
|
||||
' does not yet exist.', {'device_id': device_id,
|
||||
'network_id': network.id})
|
||||
port_dict = dict(
|
||||
name='',
|
||||
admin_state_up=True,
|
||||
device_id=device_id,
|
||||
network_id=network.id,
|
||||
tenant_id=network.tenant_id,
|
||||
fixed_ips=[dict(subnet_id=s) for s in subnets])
|
||||
dhcp_port = self.plugin.create_dhcp_port({'port': port_dict})
|
||||
# If those differ, we need to call update.
|
||||
if dhcp_enabled_subnet_ids != port_subnet_ids:
|
||||
# Collect the subnets and fixed IPs that the port already
|
||||
# has, for subnets that are still in the DHCP-enabled set.
|
||||
wanted_fixed_ips = []
|
||||
for fixed_ip in port.fixed_ips:
|
||||
if fixed_ip.subnet_id in dhcp_enabled_subnet_ids:
|
||||
wanted_fixed_ips.append(
|
||||
{'subnet_id': fixed_ip.subnet_id,
|
||||
'ip_address': fixed_ip.ip_address})
|
||||
|
||||
if not dhcp_port:
|
||||
# Add subnet IDs for new DHCP-enabled subnets.
|
||||
wanted_fixed_ips.extend(
|
||||
dict(subnet_id=s)
|
||||
for s in dhcp_enabled_subnet_ids - port_subnet_ids)
|
||||
|
||||
# Update the port to have the calculated subnets and fixed
|
||||
# IPs. The Neutron server will allocate a fresh IP for
|
||||
# each subnet that doesn't already have one.
|
||||
port = self.plugin.update_dhcp_port(
|
||||
port.id,
|
||||
{'port': {'network_id': network.id,
|
||||
'fixed_ips': wanted_fixed_ips}})
|
||||
if not port:
|
||||
raise exceptions.Conflict()
|
||||
|
||||
return port
|
||||
|
||||
def _setup_reserved_dhcp_port(self, network, device_id, dhcp_subnets):
|
||||
"""Setup the reserved DHCP port, if there is one."""
|
||||
LOG.debug('DHCP port %(device_id)s on network %(network_id)s'
|
||||
' does not yet exist. Checking for a reserved port.',
|
||||
{'device_id': device_id, 'network_id': network.id})
|
||||
for port in network.ports:
|
||||
port_device_id = getattr(port, 'device_id', None)
|
||||
if port_device_id == constants.DEVICE_ID_RESERVED_DHCP_PORT:
|
||||
port = self.plugin.update_dhcp_port(
|
||||
port.id, {'port': {'network_id': network.id,
|
||||
'device_id': device_id}})
|
||||
if port:
|
||||
return port
|
||||
|
||||
def _setup_new_dhcp_port(self, network, device_id, dhcp_subnets):
|
||||
"""Create and set up new DHCP port for the specified network."""
|
||||
LOG.debug('DHCP port %(device_id)s on network %(network_id)s'
|
||||
' does not yet exist. Creating new one.',
|
||||
{'device_id': device_id, 'network_id': network.id})
|
||||
port_dict = dict(
|
||||
name='',
|
||||
admin_state_up=True,
|
||||
device_id=device_id,
|
||||
network_id=network.id,
|
||||
tenant_id=network.tenant_id,
|
||||
fixed_ips=[dict(subnet_id=s) for s in dhcp_subnets])
|
||||
return self.plugin.create_dhcp_port({'port': port_dict})
|
||||
|
||||
def setup_dhcp_port(self, network):
|
||||
"""Create/update DHCP port for the host if needed and return port."""
|
||||
|
||||
# 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.
|
||||
dhcp_subnets = {subnet.id: subnet for subnet in network.subnets
|
||||
if subnet.enable_dhcp}
|
||||
|
||||
# There are 3 cases: either the DHCP port already exists (but
|
||||
# might need to be updated for a changed set of subnets); or
|
||||
# some other code has already prepared a 'reserved' DHCP port,
|
||||
# and we just need to adopt that; or we need to create a new
|
||||
# DHCP port. Try each of those in turn until we have a DHCP
|
||||
# port.
|
||||
for setup_method in (self._setup_existing_dhcp_port,
|
||||
self._setup_reserved_dhcp_port,
|
||||
self._setup_new_dhcp_port):
|
||||
dhcp_port = setup_method(network, device_id, dhcp_subnets)
|
||||
if dhcp_port:
|
||||
break
|
||||
else:
|
||||
raise exceptions.Conflict()
|
||||
|
||||
# Convert subnet_id to subnet dict
|
||||
fixed_ips = [dict(subnet_id=fixed_ip.subnet_id,
|
||||
ip_address=fixed_ip.ip_address,
|
||||
subnet=subnets[fixed_ip.subnet_id])
|
||||
subnet=dhcp_subnets[fixed_ip.subnet_id])
|
||||
for fixed_ip in dhcp_port.fixed_ips]
|
||||
|
||||
ips = [DictModel(item) if isinstance(item, dict) else item
|
||||
|
|
|
|||
|
|
@ -48,6 +48,13 @@ class DhcpOpt(object):
|
|||
return str(self.__dict__)
|
||||
|
||||
|
||||
# A base class where class attributes can also be accessed by treating
|
||||
# an instance as a dict.
|
||||
class Dictable(object):
|
||||
def __getitem__(self, k):
|
||||
return self.__class__.__dict__.get(k)
|
||||
|
||||
|
||||
class FakeDhcpPort(object):
|
||||
id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaa'
|
||||
admin_state_up = True
|
||||
|
|
@ -61,6 +68,19 @@ class FakeDhcpPort(object):
|
|||
self.extra_dhcp_opts = []
|
||||
|
||||
|
||||
class FakeReservedPort(object):
|
||||
admin_state_up = True
|
||||
device_owner = 'network:dhcp'
|
||||
fixed_ips = [FakeIPAllocation('192.168.0.6',
|
||||
'dddddddd-dddd-dddd-dddd-dddddddddddd')]
|
||||
mac_address = '00:00:80:aa:bb:ee'
|
||||
device_id = constants.DEVICE_ID_RESERVED_DHCP_PORT
|
||||
|
||||
def __init__(self, id='reserved-aaaa-aaaa-aaaa-aaaaaaaaaaa'):
|
||||
self.extra_dhcp_opts = []
|
||||
self.id = id
|
||||
|
||||
|
||||
class FakePort1(object):
|
||||
id = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee'
|
||||
admin_state_up = True
|
||||
|
|
@ -283,7 +303,7 @@ class FakeV6HostRoute(object):
|
|||
nexthop = '2001:0200:feed:7ac0::1'
|
||||
|
||||
|
||||
class FakeV4Subnet(object):
|
||||
class FakeV4Subnet(Dictable):
|
||||
id = 'dddddddd-dddd-dddd-dddd-dddddddddddd'
|
||||
ip_version = 4
|
||||
cidr = '192.168.0.0/24'
|
||||
|
|
@ -400,7 +420,7 @@ class FakeV4SubnetNoDHCP(object):
|
|||
dns_nameservers = []
|
||||
|
||||
|
||||
class FakeV6SubnetDHCPStateful(object):
|
||||
class FakeV6SubnetDHCPStateful(Dictable):
|
||||
id = 'ffffffff-ffff-ffff-ffff-ffffffffffff'
|
||||
ip_version = 6
|
||||
cidr = 'fdca:3ba5:a17a:4ba3::/64'
|
||||
|
|
@ -483,6 +503,29 @@ class FakeDualNetwork(object):
|
|||
namespace = 'qdhcp-ns'
|
||||
|
||||
|
||||
class FakeDeviceManagerNetwork(object):
|
||||
id = 'cccccccc-cccc-cccc-cccc-cccccccccccc'
|
||||
subnets = [FakeV4Subnet(), FakeV6SubnetDHCPStateful()]
|
||||
ports = [FakePort1(), FakeV6Port(), FakeDualPort(), FakeRouterPort()]
|
||||
namespace = 'qdhcp-ns'
|
||||
|
||||
|
||||
class FakeDualNetworkReserved(object):
|
||||
id = 'cccccccc-cccc-cccc-cccc-cccccccccccc'
|
||||
subnets = [FakeV4Subnet(), FakeV6SubnetDHCPStateful()]
|
||||
ports = [FakePort1(), FakeV6Port(), FakeDualPort(), FakeRouterPort(),
|
||||
FakeReservedPort()]
|
||||
namespace = 'qdhcp-ns'
|
||||
|
||||
|
||||
class FakeDualNetworkReserved2(object):
|
||||
id = 'cccccccc-cccc-cccc-cccc-cccccccccccc'
|
||||
subnets = [FakeV4Subnet(), FakeV6SubnetDHCPStateful()]
|
||||
ports = [FakePort1(), FakeV6Port(), FakeDualPort(), FakeRouterPort(),
|
||||
FakeReservedPort(), FakeReservedPort(id='reserved-2')]
|
||||
namespace = 'qdhcp-ns'
|
||||
|
||||
|
||||
class FakeNetworkDhcpPort(object):
|
||||
id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'
|
||||
subnets = [FakeV4Subnet()]
|
||||
|
|
@ -714,9 +757,9 @@ class LocalChild(dhcp.DhcpLocalProcess):
|
|||
self.called.append('spawn')
|
||||
|
||||
|
||||
class TestBase(base.BaseTestCase):
|
||||
class TestConfBase(base.BaseTestCase):
|
||||
def setUp(self):
|
||||
super(TestBase, self).setUp()
|
||||
super(TestConfBase, self).setUp()
|
||||
self.conf = config.setup_conf()
|
||||
self.conf.register_opts(base_config.core_opts)
|
||||
self.conf.register_opts(dhcp_config.DHCP_OPTS)
|
||||
|
|
@ -724,6 +767,11 @@ class TestBase(base.BaseTestCase):
|
|||
self.conf.register_opts(external_process.OPTS)
|
||||
config.register_interface_driver_opts_helper(self.conf)
|
||||
config.register_use_namespaces_opts_helper(self.conf)
|
||||
|
||||
|
||||
class TestBase(TestConfBase):
|
||||
def setUp(self):
|
||||
super(TestBase, self).setUp()
|
||||
instance = mock.patch("neutron.agent.linux.dhcp.DeviceManager")
|
||||
self.mock_mgr = instance.start()
|
||||
self.conf.register_opt(cfg.BoolOpt('enable_isolated_metadata',
|
||||
|
|
@ -1829,3 +1877,138 @@ class TestDnsmasq(TestBase):
|
|||
self.conf.set_override('enable_metadata_network', True)
|
||||
self.assertTrue(dhcp.Dnsmasq.should_enable_metadata(
|
||||
self.conf, FakeV4MetadataNetwork()))
|
||||
|
||||
|
||||
class TestDeviceManager(TestConfBase):
|
||||
|
||||
@mock.patch('neutron.agent.linux.dhcp.ip_lib')
|
||||
@mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
|
||||
def test_setup(self, load_interface_driver, ip_lib):
|
||||
"""Test new and existing cases of DeviceManager's DHCP port setup
|
||||
logic.
|
||||
"""
|
||||
|
||||
# Create DeviceManager.
|
||||
self.conf.register_opt(cfg.BoolOpt('enable_isolated_metadata',
|
||||
default=False))
|
||||
plugin = mock.Mock()
|
||||
mgr = dhcp.DeviceManager(self.conf, plugin)
|
||||
load_interface_driver.assert_called_with(self.conf)
|
||||
|
||||
# Setup with no existing DHCP port - expect a new DHCP port to
|
||||
# be created.
|
||||
network = FakeDeviceManagerNetwork()
|
||||
network.tenant_id = 'Tenant A'
|
||||
|
||||
def mock_create(dict):
|
||||
port = dhcp.DictModel(dict['port'])
|
||||
port.id = 'abcd-123456789'
|
||||
port.mac_address = '00-12-34-56-78-90'
|
||||
port.fixed_ips = [
|
||||
dhcp.DictModel({'subnet_id': ip['subnet_id'],
|
||||
'ip_address': 'unique-IP-address'})
|
||||
for ip in port.fixed_ips
|
||||
]
|
||||
return port
|
||||
|
||||
plugin.create_dhcp_port.side_effect = mock_create
|
||||
mgr.driver.get_device_name.return_value = 'ns-XXX'
|
||||
ip_lib.ensure_device_is_ready.return_value = True
|
||||
mgr.setup(network)
|
||||
plugin.create_dhcp_port.assert_called_with(mock.ANY)
|
||||
|
||||
mgr.driver.init_l3.assert_called_with('ns-XXX',
|
||||
mock.ANY,
|
||||
namespace='qdhcp-ns')
|
||||
cidrs = set(mgr.driver.init_l3.call_args[0][1])
|
||||
self.assertEqual(cidrs, set(['unique-IP-address/24',
|
||||
'unique-IP-address/64']))
|
||||
|
||||
# Now call setup again. This time we go through the existing
|
||||
# port code path, and the driver's init_l3 method is called
|
||||
# again.
|
||||
plugin.create_dhcp_port.reset_mock()
|
||||
mgr.driver.init_l3.reset_mock()
|
||||
mgr.setup(network)
|
||||
mgr.driver.init_l3.assert_called_with('ns-XXX',
|
||||
mock.ANY,
|
||||
namespace='qdhcp-ns')
|
||||
cidrs = set(mgr.driver.init_l3.call_args[0][1])
|
||||
self.assertEqual(cidrs, set(['unique-IP-address/24',
|
||||
'unique-IP-address/64']))
|
||||
self.assertFalse(plugin.create_dhcp_port.called)
|
||||
|
||||
@mock.patch('neutron.agent.linux.dhcp.ip_lib')
|
||||
@mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
|
||||
def test_setup_reserved(self, load_interface_driver, ip_lib):
|
||||
"""Test reserved port case of DeviceManager's DHCP port setup
|
||||
logic.
|
||||
"""
|
||||
|
||||
# Create DeviceManager.
|
||||
self.conf.register_opt(cfg.BoolOpt('enable_isolated_metadata',
|
||||
default=False))
|
||||
plugin = mock.Mock()
|
||||
mgr = dhcp.DeviceManager(self.conf, plugin)
|
||||
load_interface_driver.assert_called_with(self.conf)
|
||||
|
||||
# Setup with a reserved DHCP port.
|
||||
network = FakeDualNetworkReserved()
|
||||
network.tenant_id = 'Tenant A'
|
||||
reserved_port = network.ports[-1]
|
||||
|
||||
def mock_update(port_id, dict):
|
||||
port = reserved_port
|
||||
port.network_id = dict['port']['network_id']
|
||||
port.device_id = dict['port']['device_id']
|
||||
return port
|
||||
|
||||
plugin.update_dhcp_port.side_effect = mock_update
|
||||
mgr.driver.get_device_name.return_value = 'ns-XXX'
|
||||
ip_lib.ensure_device_is_ready.return_value = True
|
||||
mgr.setup(network)
|
||||
plugin.update_dhcp_port.assert_called_with(reserved_port.id, mock.ANY)
|
||||
|
||||
mgr.driver.init_l3.assert_called_with('ns-XXX',
|
||||
['192.168.0.6/24'],
|
||||
namespace='qdhcp-ns')
|
||||
|
||||
@mock.patch('neutron.agent.linux.dhcp.ip_lib')
|
||||
@mock.patch('neutron.agent.linux.dhcp.common_utils.load_interface_driver')
|
||||
def test_setup_reserved_2(self, load_interface_driver, ip_lib):
|
||||
"""Test scenario where a network has two reserved ports, and
|
||||
update_dhcp_port fails for the first of those.
|
||||
"""
|
||||
|
||||
# Create DeviceManager.
|
||||
self.conf.register_opt(cfg.BoolOpt('enable_isolated_metadata',
|
||||
default=False))
|
||||
plugin = mock.Mock()
|
||||
mgr = dhcp.DeviceManager(self.conf, plugin)
|
||||
load_interface_driver.assert_called_with(self.conf)
|
||||
|
||||
# Setup with a reserved DHCP port.
|
||||
network = FakeDualNetworkReserved2()
|
||||
network.tenant_id = 'Tenant A'
|
||||
reserved_port_1 = network.ports[-2]
|
||||
reserved_port_2 = network.ports[-1]
|
||||
|
||||
def mock_update(port_id, dict):
|
||||
if port_id == reserved_port_1.id:
|
||||
return None
|
||||
|
||||
port = reserved_port_2
|
||||
port.network_id = dict['port']['network_id']
|
||||
port.device_id = dict['port']['device_id']
|
||||
return port
|
||||
|
||||
plugin.update_dhcp_port.side_effect = mock_update
|
||||
mgr.driver.get_device_name.return_value = 'ns-XXX'
|
||||
ip_lib.ensure_device_is_ready.return_value = True
|
||||
mgr.setup(network)
|
||||
plugin.update_dhcp_port.assert_called_with(reserved_port_2.id,
|
||||
mock.ANY)
|
||||
|
||||
mgr.driver.init_l3.assert_called_with('ns-XXX',
|
||||
['192.168.0.6/24'],
|
||||
namespace='qdhcp-ns')
|
||||
|
|
|
|||
Loading…
Reference in New Issue