From 127de06c7e09e1468f2855a3033fb6193a6b9365 Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Wed, 6 May 2015 22:40:39 +0200 Subject: [PATCH] Clean only floating-ip related connection states Currently init_l3 deletes connection states related to ALL ips deleted in init_l3 but it's required only when floating-ips are deleted[1]. This change deletes only connection states related to floating-ips deleted in init_l3 ... it avoids to delete connection states in dhcp agents and on router internal ports! [1] look at change Ia9bd7ae243a0859dcb97e2fa939f7d16f9c2456c Closes-Bug: #1452434 Related-Bug: #1334926 Change-Id: Icfcfc585df6fd41de1e1345fd731e4631a6950ce --- neutron/agent/l3/dvr_fip_ns.py | 3 +- neutron/agent/l3/router_info.py | 3 +- neutron/agent/linux/interface.py | 9 ++++-- neutron/tests/unit/agent/l3/test_agent.py | 9 ++++-- .../tests/unit/agent/linux/test_interface.py | 32 ++++++++++++++++--- 5 files changed, 44 insertions(+), 12 deletions(-) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index e2e63eb2700..9b7eee99a88 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -103,7 +103,8 @@ class FipNamespace(namespaces.Namespace): prefix=FIP_EXT_DEV_PREFIX) ip_cidrs = common_utils.fixed_ip_cidrs(ex_gw_port['fixed_ips']) - self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name) + self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name, + clean_connections=True) for fixed_ip in ex_gw_port['fixed_ips']: ip_lib.send_gratuitous_arp(ns_name, diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index 6e213d2f24f..0dfbc13ef58 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -462,7 +462,8 @@ class RouterInfo(object): gateway_ips=gateway_ips, extra_subnets=ex_gw_port.get('extra_subnets', []), preserve_ips=preserve_ips, - enable_ra_on_gw=enable_ra_on_gw) + enable_ra_on_gw=enable_ra_on_gw, + clean_connections=True) for fixed_ip in ex_gw_port['fixed_ips']: ip_lib.send_gratuitous_arp(ns_name, interface_name, diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index ed1e91e98f7..470e8f34f25 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -79,13 +79,15 @@ class LinuxInterfaceDriver(object): def init_l3(self, device_name, ip_cidrs, namespace=None, preserve_ips=[], gateway_ips=None, extra_subnets=[], - enable_ra_on_gw=False): + enable_ra_on_gw=False, clean_connections=False): """Set the L3 settings for the interface using data from the port. ip_cidrs: list of 'X.X.X.X/YY' strings preserve_ips: list of ip cidrs that should not be removed from device gateway_ips: For gateway ports, list of external gateway ip addresses enable_ra_on_gw: Boolean to indicate configuring acceptance of IPv6 RA + clean_connections: Boolean to indicate if we should cleanup connections + associated to removed ips """ device = ip_lib.IPDevice(device_name, namespace=namespace) @@ -113,7 +115,10 @@ class LinuxInterfaceDriver(object): # clean up any old addresses for ip_cidr in previous: if ip_cidr not in preserve_ips: - device.delete_addr_and_conntrack_state(ip_cidr) + if clean_connections: + device.delete_addr_and_conntrack_state(ip_cidr) + else: + device.addr.delete(ip_cidr) for gateway_ip in gateway_ips or []: device.route.add_gateway(gateway_ip) diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 52dbb073fb6..5e67dda6ae0 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -611,7 +611,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ips': gateway_ips, 'namespace': 'qrouter-' + router['id'], 'extra_subnets': [], - 'enable_ra_on_gw': enable_ra_on_gw} + 'enable_ra_on_gw': enable_ra_on_gw, + 'clean_connections': True} else: exp_arp_calls = [mock.call(ri.ns_name, interface_name, '20.0.0.30', mock.ANY)] @@ -632,7 +633,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ips': gateway_ips, 'namespace': 'qrouter-' + router['id'], 'extra_subnets': [{'cidr': '172.16.0.0/24'}], - 'enable_ra_on_gw': enable_ra_on_gw} + 'enable_ra_on_gw': enable_ra_on_gw, + 'clean_connections': True} self.mock_driver.init_l3.assert_called_with(interface_name, ip_cidrs, **kwargs) @@ -799,7 +801,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): 'gateway_ips': gateway_ips, 'namespace': 'qrouter-' + router['id'], 'extra_subnets': [{'cidr': '172.16.0.0/24'}], - 'enable_ra_on_gw': False} + 'enable_ra_on_gw': False, + 'clean_connections': True} self.mock_driver.init_l3.assert_called_with(interface_name, ip_cidrs, **kwargs) diff --git a/neutron/tests/unit/agent/linux/test_interface.py b/neutron/tests/unit/agent/linux/test_interface.py index 8bbc210dd9b..1834524e899 100644 --- a/neutron/tests/unit/agent/linux/test_interface.py +++ b/neutron/tests/unit/agent/linux/test_interface.py @@ -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().delete_addr_and_conntrack_state('172.16.77.240/24'), + mock.call().addr.delete('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')]) @@ -129,6 +129,29 @@ class TestABCDriver(TestBase): self.assertFalse(self.ip_dev().addr.delete.called) self.assertFalse(self.ip_dev().delete_addr_and_conntrack_state.called) + def _test_l3_init_clean_connections(self, clean_connections): + addresses = [ + dict(scope='global', dynamic=False, cidr='10.0.0.1/24'), + dict(scope='global', dynamic=False, cidr='10.0.0.3/32')] + self.ip_dev().addr.list = mock.Mock(return_value=addresses) + + bc = BaseChild(self.conf) + ns = '12345678-1234-5678-90ab-ba0987654321' + bc.init_l3('tap0', ['10.0.0.1/24'], namespace=ns, + clean_connections=clean_connections) + + delete = self.ip_dev().delete_addr_and_conntrack_state + if clean_connections: + delete.assert_called_once_with('10.0.0.3/32') + else: + self.assertFalse(delete.called) + + def test_l3_init_with_clean_connections(self): + self._test_l3_init_clean_connections(True) + + def test_l3_init_without_clean_connections(self): + self._test_l3_init_clean_connections(False) + def _test_l3_init_with_ipv6(self, include_gw_ip): addresses = [dict(scope='global', dynamic=False, @@ -148,8 +171,7 @@ 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().delete_addr_and_conntrack_state( - '2001:db8:a::123/64')]) + mock.call().addr.delete('2001:db8:a::123/64')]) if include_gw_ip: expected_calls += ( [mock.call().route.add_gateway('2001:db8:a::1')]) @@ -182,8 +204,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().delete_addr_and_conntrack_state('172.16.77.240/24'), - mock.call().delete_addr_and_conntrack_state('2001:db8:a::123/64'), + mock.call().addr.delete('172.16.77.240/24'), + mock.call().addr.delete('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')],