openstack
/
neutron
Code Issues Proposed changes

Use a single method to remove an address with its conntrack state 

I just noticed a pattern and I thought I'd throw this up for
discussion.  It has occurred to me that this addition sort of breaks
the ip_lib paradigm of wrapping ip commands without any additional
useful abstraction.  Any better ideas?

Change-Id: Ibd34bf4a721c153aca916e294e58adb4a28379e4
This commit is contained in:
Carl Baldwin 2015-05-27 20:12:27 +00:00
parent 1fc3dd34af
commit 92b946e90b
5 changed files with 44 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
neutron/agent/l3/router_info.py
View File

@ -210,8 +210,7 @@ class RouterInfo(object):
raise NotImplementedError()
def remove_floating_ip(self, device, ip_cidr):
device.addr.delete(ip_cidr)
self.driver.delete_conntrack_state(namespace=self.ns_name, ip=ip_cidr)
device.delete_addr_and_conntrack_state(ip_cidr)
def get_router_cidrs(self, device):
return set([addr['cidr'] for addr in device.addr.list()])

39
neutron/agent/linux/interface.py
View File

@ -113,8 +113,7 @@ class LinuxInterfaceDriver(object):
# clean up any old addresses
for ip_cidr in previous:
if ip_cidr not in preserve_ips:
device.addr.delete(ip_cidr)
self.delete_conntrack_state(namespace=namespace, ip=ip_cidr)
device.delete_addr_and_conntrack_state(ip_cidr)
for gateway_ip in gateway_ips or []:
device.route.add_gateway(gateway_ip)
@ -131,42 +130,6 @@ class LinuxInterfaceDriver(object):
for route in existing_onlink_routes - new_onlink_routes:
device.route.delete_onlink_route(route)
def delete_conntrack_state(self, namespace, ip):
"""Delete conntrack state associated with an IP address.
This terminates any active connections through an IP. Call this soon
after removing the IP address from an interface so that new connections
cannot be created before the IP address is gone.
namespace: the name of the namespace where the IP has been configured
ip: the IP address for which state should be removed. This can be
passed as a string with or without /NN. A netaddr.IPAddress or
netaddr.Network representing the IP address can also be passed.
"""
ip_str = str(netaddr.IPNetwork(ip).ip)
ip_wrapper = ip_lib.IPWrapper(namespace=namespace)
# Delete conntrack state for ingress traffic
# If 0 flow entries have been deleted
# conntrack -D will return 1
try:
ip_wrapper.netns.execute(["conntrack", "-D", "-d", ip_str],
check_exit_code=True,
extra_ok_codes=[1])
except RuntimeError:
LOG.exception(_LE("Failed deleting ingress connection state of"
" floatingip %s"), ip_str)
# Delete conntrack state for egress traffic
try:
ip_wrapper.netns.execute(["conntrack", "-D", "-q", ip_str],
check_exit_code=True,
extra_ok_codes=[1])
except RuntimeError:
LOG.exception(_LE("Failed deleting egress connection state of"
" floatingip %s"), ip_str)
def check_bridge_exists(self, bridge):
if not ip_lib.device_exists(bridge):
raise exceptions.BridgeDoesNotExist(bridge=bridge)

35
neutron/agent/linux/ip_lib.py
View File

@ -211,6 +211,41 @@ class IPDevice(SubProcessBase):
def __str__(self):
return self.name
def delete_addr_and_conntrack_state(self, cidr):
"""Delete an address along with its conntrack state
This terminates any active connections through an IP.
cidr: the IP address for which state should be removed. This can be
passed as a string with or without /NN. A netaddr.IPAddress or
netaddr.Network representing the IP address can also be passed.
"""
self.addr.delete(cidr)
ip_str = str(netaddr.IPNetwork(cidr).ip)
ip_wrapper = IPWrapper(namespace=self.namespace)
# Delete conntrack state for ingress traffic
# If 0 flow entries have been deleted
# conntrack -D will return 1
try:
ip_wrapper.netns.execute(["conntrack", "-D", "-d", ip_str],
check_exit_code=True,
extra_ok_codes=[1])
except RuntimeError:
LOG.exception(_LE("Failed deleting ingress connection state of"
" floatingip %s"), ip_str)
# Delete conntrack state for egress traffic
try:
ip_wrapper.netns.execute(["conntrack", "-D", "-q", ip_str],
check_exit_code=True,
extra_ok_codes=[1])
except RuntimeError:
LOG.exception(_LE("Failed deleting egress connection state of"
" floatingip %s"), ip_str)
class IpCommandBase(object):
COMMAND = ''

5
neutron/tests/unit/agent/l3/test_legacy_router.py
View File

@ -46,10 +46,7 @@ class TestBasicRouterOperations(BasicRouterTestCaseFramework):
ri.remove_floating_ip(device, cidr)
device.addr.delete.assert_called_once_with(cidr)
self.driver.delete_conntrack_state.assert_called_once_with(
ip=cidr,
namespace=ri.ns_name)
device.delete_addr_and_conntrack_state.assert_called_once_with(cidr)
@mock.patch.object(ip_lib, 'send_gratuitous_arp')

10
neutron/tests/unit/agent/linux/test_interface.py
View File

@ -94,7 +94,7 @@ class TestABCDriver(TestBase):
[mock.call('tap0', namespace=ns),
mock.call().addr.list(filters=['permanent']),
mock.call().addr.add('192.168.1.2/24'),
mock.call().addr.delete('172.16.77.240/24'),
mock.call().delete_addr_and_conntrack_state('172.16.77.240/24'),
mock.call().route.list_onlink_routes(constants.IP_VERSION_4),
mock.call().route.list_onlink_routes(constants.IP_VERSION_6),
mock.call().route.add_onlink_route('172.20.0.0/24')])
@ -127,6 +127,7 @@ class TestABCDriver(TestBase):
mock.call().addr.list(filters=['permanent']),
mock.call().addr.add('192.168.1.2/24')])
self.assertFalse(self.ip_dev().addr.delete.called)
self.assertFalse(self.ip_dev().delete_addr_and_conntrack_state.called)
def _test_l3_init_with_ipv6(self, include_gw_ip):
addresses = [dict(scope='global',
@ -147,7 +148,8 @@ class TestABCDriver(TestBase):
[mock.call('tap0', namespace=ns),
mock.call().addr.list(filters=['permanent']),
mock.call().addr.add('2001:db8:a::124/64'),
mock.call().addr.delete('2001:db8:a::123/64')])
mock.call().delete_addr_and_conntrack_state(
'2001:db8:a::123/64')])
if include_gw_ip:
expected_calls += (
[mock.call().route.add_gateway('2001:db8:a::1')])
@ -180,8 +182,8 @@ class TestABCDriver(TestBase):
mock.call().addr.list(filters=['permanent']),
mock.call().addr.add('192.168.1.2/24'),
mock.call().addr.add('2001:db8:a::124/64'),
mock.call().addr.delete('172.16.77.240/24'),
mock.call().addr.delete('2001:db8:a::123/64'),
mock.call().delete_addr_and_conntrack_state('172.16.77.240/24'),
mock.call().delete_addr_and_conntrack_state('2001:db8:a::123/64'),
mock.call().route.list_onlink_routes(constants.IP_VERSION_4),
mock.call().route.list_onlink_routes(constants.IP_VERSION_6),
mock.call().route.add_onlink_route('172.20.0.0/24')],