From ccd5fe83884cf72c7e69091f318628884665f971 Mon Sep 17 00:00:00 2001 From: Sachi King Date: Mon, 8 Dec 2014 17:42:48 +1100 Subject: [PATCH] If router is HA, get current_cidrs from keepalived object When using L3 HA and keepalived neutron is no longer directly managing the floating IP addresses itself. Neutron should not check against which addresses are currently configured on the system, but the addresses the keepalived object has configured. Co-Authored-By: Benoit Page-Guitard Change-Id: I56045ede3a3dc1a7044a22913ee38ed382a81052 Closes-Bug: #1400217 --- neutron/agent/l3/agent.py | 8 ++++++- neutron/agent/l3/ha.py | 4 ++++ neutron/agent/linux/keepalived.py | 4 ++++ .../tests/unit/agent/linux/test_keepalived.py | 6 ++++++ neutron/tests/unit/test_l3_agent.py | 21 +++++++++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index ee2dc56da..8f3604340 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -668,6 +668,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, if ri.router['distributed']: self.floating_ip_removed_dist(ri, ip_cidr) + def _get_router_cidrs(self, ri, device): + if ri.is_ha: + return set(self._ha_get_existing_cidrs(ri, device.name)) + else: + return set([addr['cidr'] for addr in device.addr.list()]) + def process_router_floating_ip_addresses(self, ri, ex_gw_port): """Configure IP addresses on router's external gateway interface. @@ -684,7 +690,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, device = ip_lib.IPDevice(interface_name, self.root_helper, namespace=ri.ns_name) - existing_cidrs = set([addr['cidr'] for addr in device.addr.list()]) + existing_cidrs = self._get_router_cidrs(ri, device) new_cidrs = set() # Loop once to ensure that floating ips are configured. diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py index bcc941495..33cb37239 100644 --- a/neutron/agent/l3/ha.py +++ b/neutron/agent/l3/ha.py @@ -181,6 +181,10 @@ class AgentMixin(object): instance = ri.keepalived_manager.config.get_instance(ri.ha_vr_id) instance.remove_vips_vroutes_by_interface(interface) + def _ha_get_existing_cidrs(self, ri, interface_name): + instance = ri.keepalived_manager.config.get_instance(ri.ha_vr_id) + return instance.get_existing_vip_ip_addresses(interface_name) + def _add_keepalived_notifiers(self, ri): callback = ( metadata_driver.MetadataDriver._get_metadata_proxy_callback( diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index 596ee6e02..346729331 100644 --- a/neutron/agent/linux/keepalived.py +++ b/neutron/agent/linux/keepalived.py @@ -179,6 +179,10 @@ class KeepalivedInstance(object): self.vips = [vip for vip in self.vips if vip.ip_address != ip_address] + def get_existing_vip_ip_addresses(self, interface_name): + return [vip.ip_address for vip in self.vips + if vip.interface_name == interface_name] + def _build_track_interface_config(self): return itertools.chain( [' track_interface {'], diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py index 5dfe0dc0f..c29ee2dd6 100644 --- a/neutron/tests/unit/agent/linux/test_keepalived.py +++ b/neutron/tests/unit/agent/linux/test_keepalived.py @@ -187,6 +187,12 @@ vrrp_instance VR_2 { config.reset() self.assertEqual('', config.get_config_str()) + def test_get_existing_vip_ip_addresses_returns_list(self): + config = self._get_config() + instance = config.get_instance(1) + current_vips = sorted(instance.get_existing_vip_ip_addresses('eth2')) + self.assertEqual(['192.168.2.0/24', '192.168.3.0/24'], current_vips) + class KeepalivedStateExceptionTestCase(base.BaseTestCase): def test_state_exception(self): diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 8a93601b8..b5fac4207 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -981,6 +981,27 @@ class TestBasicRouterOperations(base.BaseTestCase): ) self._test_process_router_floating_ip_addresses_add(ri, agent) + def test_get_router_cidrs_returns_cidrs(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + ri = mock.MagicMock() + ri.is_ha = False + addresses = ['15.1.2.2/24', '15.1.2.3/32'] + device = mock.MagicMock() + device.addr.list.return_value = [{'cidr': addresses[0]}, + {'cidr': addresses[1]}] + self.assertEqual(set(addresses), agent._get_router_cidrs(ri, device)) + + def test_get_router_cidrs_returns_ha_cidrs(self): + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + ri = mock.MagicMock() + ri.is_ha = True + device = mock.MagicMock() + device.name.return_value = 'eth2' + addresses = ['15.1.2.2/24', '15.1.2.3/32'] + agent._ha_get_existing_cidrs = mock.MagicMock() + agent._ha_get_existing_cidrs.return_value = addresses + self.assertEqual(set(addresses), agent._get_router_cidrs(ri, device)) + # TODO(mrsmith): refactor for DVR cases @mock.patch('neutron.agent.linux.ip_lib.IPDevice') def test_process_router_floating_ip_addresses_remove(self, IPDevice): -- 2.45.2