Fixes for missing IPs on subnets in DHCP agent
There is a race condition server-side where a port request containing a subnet_id is processed at the same time the subnet is being deleted, the port operation may be successful without having a fixed IP on the requested subnet. This patch makes the DHCP agent resillient to this bug by checking the port response and raising a SubnetMismatchForPort to trigger a resync if it doesn't have all of the requested subnet IDs. Additionally, it avoids skipping assignment of IPv6 addresses to the interface if they are stateless. The original logic to skip assignment was only meant to be for SLAAC addresses. Both of these issues were resulting in the KeyError observed in the bug report. Related-Bug: #1627480 Closes-Bug: #1624079 Change-Id: I85ef1f4d60efd0309d6a0706e29fdbcc16f0b59d
This commit is contained in:
parent
8de9bb4b3b
commit
d1fb423830
|
@ -120,6 +120,10 @@ class DhcpAgent(manager.Manager):
|
|||
'is a conflict with its current state; please '
|
||||
'check that the network and/or its subnet(s) '
|
||||
'still exist.', {'net_id': network.id, 'action': action})
|
||||
except exceptions.SubnetMismatchForPort as e:
|
||||
# FIXME(kevinbenton): get rid of this once bug/1627480 is fixed
|
||||
LOG.debug("Error configuring DHCP port, scheduling resync: %s", e)
|
||||
self.schedule_resync(e, network.id)
|
||||
except Exception as e:
|
||||
if getattr(e, 'exc_type', '') != 'IpAddressGenerationFailure':
|
||||
# Don't resync if port could not be created because of an IP
|
||||
|
|
|
@ -1280,6 +1280,19 @@ class DeviceManager(object):
|
|||
else:
|
||||
raise exceptions.Conflict()
|
||||
|
||||
# FIXME(kevinbenton): ensure we have the IPs we actually need.
|
||||
# can be removed once bug/1627480 is fixed
|
||||
if not self.driver.use_gateway_ips:
|
||||
expected = set(dhcp_subnets)
|
||||
actual = {fip.subnet_id for fip in dhcp_port.fixed_ips}
|
||||
missing = expected - actual
|
||||
if missing:
|
||||
LOG.debug("Requested DHCP port with IPs on subnets "
|
||||
"%(expected)s but only got IPs on subnets "
|
||||
"%(actual)s.", {'expected': expected,
|
||||
'actual': actual})
|
||||
raise exceptions.SubnetMismatchForPort(
|
||||
port_id=dhcp_port.id, subnet_id=list(missing)[0])
|
||||
# Convert subnet_id to subnet dict
|
||||
fixed_ips = [dict(subnet_id=fixed_ip.subnet_id,
|
||||
ip_address=fixed_ip.ip_address,
|
||||
|
@ -1359,7 +1372,7 @@ class DeviceManager(object):
|
|||
ip_cidrs = []
|
||||
for fixed_ip in port.fixed_ips:
|
||||
subnet = fixed_ip.subnet
|
||||
if not ipv6_utils.is_auto_address_subnet(subnet):
|
||||
if not ipv6_utils.is_slaac_subnet(subnet):
|
||||
net = netaddr.IPNetwork(subnet.cidr)
|
||||
ip_cidr = '%s/%s' % (fixed_ip.ip_address, net.prefixlen)
|
||||
ip_cidrs.append(ip_cidr)
|
||||
|
|
|
@ -76,6 +76,12 @@ def is_auto_address_subnet(subnet):
|
|||
or subnet['ipv6_ra_mode'] in modes)
|
||||
|
||||
|
||||
def is_slaac_subnet(subnet):
|
||||
"""Check if subnet is a slaac subnet."""
|
||||
return (subnet['ipv6_address_mode'] == const.IPV6_SLAAC
|
||||
or subnet['ipv6_ra_mode'] == const.IPV6_SLAAC)
|
||||
|
||||
|
||||
def is_eui64_address(ip_address):
|
||||
"""Check if ip address is EUI64."""
|
||||
ip = netaddr.IPAddress(ip_address)
|
||||
|
|
|
@ -85,6 +85,8 @@ fake_meta_subnet = dhcp.DictModel(dict(id='bbbbbbbb-1111-2222-bbbbbbbbbbbb',
|
|||
|
||||
fake_fixed_ip1 = dhcp.DictModel(dict(id='', subnet_id=fake_subnet1.id,
|
||||
ip_address='172.9.9.9'))
|
||||
fake_fixed_ip_subnet2 = dhcp.DictModel(dict(id='', subnet_id=fake_subnet2.id,
|
||||
ip_address='172.9.8.9'))
|
||||
fake_fixed_ip2 = dhcp.DictModel(dict(id='', subnet_id=fake_subnet1.id,
|
||||
ip_address='172.9.9.10'))
|
||||
fake_fixed_ipv6 = dhcp.DictModel(dict(id='', subnet_id=fake_ipv6_subnet.id,
|
||||
|
@ -1531,13 +1533,26 @@ class TestDeviceManager(base.BaseTestCase):
|
|||
[{'subnet_id': fake_fixed_ip1.subnet_id}],
|
||||
'device_id': mock.ANY}})])
|
||||
|
||||
def test_create_dhcp_port_update_add_subnet(self):
|
||||
def test_create_dhcp_port_update_add_subnet_bug_1627480(self):
|
||||
# this can go away once bug/1627480 is fixed
|
||||
plugin = mock.Mock()
|
||||
dh = dhcp.DeviceManager(cfg.CONF, plugin)
|
||||
fake_network_copy = copy.deepcopy(fake_network)
|
||||
fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network)
|
||||
fake_network_copy.subnets[1].enable_dhcp = True
|
||||
plugin.update_dhcp_port.return_value = fake_network.ports[0]
|
||||
with testtools.ExpectedException(exceptions.SubnetMismatchForPort):
|
||||
dh.setup_dhcp_port(fake_network_copy)
|
||||
|
||||
def test_create_dhcp_port_update_add_subnet(self):
|
||||
plugin = mock.Mock()
|
||||
dh = dhcp.DeviceManager(cfg.CONF, plugin)
|
||||
fake_network_copy = copy.deepcopy(fake_network)
|
||||
fake_network_copy.ports[0].device_id = dh.get_device_id(fake_network)
|
||||
fake_network_copy.subnets[1].enable_dhcp = True
|
||||
updated_port = copy.deepcopy(fake_network_copy.ports[0])
|
||||
updated_port.fixed_ips.append(fake_fixed_ip_subnet2)
|
||||
plugin.update_dhcp_port.return_value = updated_port
|
||||
dh.setup_dhcp_port(fake_network_copy)
|
||||
port_body = {'port': {
|
||||
'network_id': fake_network.id,
|
||||
|
|
|
@ -86,7 +86,9 @@ class FakeReservedPort(object):
|
|||
self.device_owner = constants.DEVICE_OWNER_DHCP
|
||||
self.fixed_ips = [
|
||||
FakeIPAllocation('192.168.0.6',
|
||||
'dddddddd-dddd-dddd-dddd-dddddddddddd')]
|
||||
'dddddddd-dddd-dddd-dddd-dddddddddddd'),
|
||||
FakeIPAllocation('fdca:3ba5:a17a:4ba3::2',
|
||||
'ffffffff-ffff-ffff-ffff-ffffffffffff')]
|
||||
self.mac_address = '00:00:80:aa:bb:ee'
|
||||
self.device_id = n_const.DEVICE_ID_RESERVED_DHCP_PORT
|
||||
self.extra_dhcp_opts = []
|
||||
|
@ -2259,11 +2261,11 @@ class TestDeviceManager(TestConfBase):
|
|||
plugin.update_dhcp_port.assert_called_with(reserved_port.id,
|
||||
mock.ANY)
|
||||
|
||||
except_ips = ['192.168.0.6/24']
|
||||
expect_ips = ['192.168.0.6/24', 'fdca:3ba5:a17a:4ba3::2/64']
|
||||
if enable_isolated_metadata or force_metadata:
|
||||
except_ips.append(dhcp.METADATA_DEFAULT_CIDR)
|
||||
expect_ips.append(dhcp.METADATA_DEFAULT_CIDR)
|
||||
mgr.driver.init_l3.assert_called_with('ns-XXX',
|
||||
except_ips,
|
||||
expect_ips,
|
||||
namespace='qdhcp-ns')
|
||||
|
||||
def test_setup_reserved_and_disable_metadata(self):
|
||||
|
@ -2331,9 +2333,9 @@ class TestDeviceManager(TestConfBase):
|
|||
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')
|
||||
mgr.driver.init_l3.assert_called_with(
|
||||
'ns-XXX', ['192.168.0.6/24', 'fdca:3ba5:a17a:4ba3::2/64'],
|
||||
namespace='qdhcp-ns')
|
||||
|
||||
|
||||
class TestDictModel(base.BaseTestCase):
|
||||
|
|
Loading…
Reference in New Issue