From ac95db26f3b394064e7bb16df7aaaee6b4f63d5a Mon Sep 17 00:00:00 2001 From: Assaf Muller Date: Tue, 24 Mar 2015 20:27:29 -0400 Subject: [PATCH] Rename/move/remove HaRouter methods All of the methods that use verify_ha are now part of the HaRouter class. The check has outlived its usefulness and may die a peaceful yet horrifying death. At its point of death, verify_ha is akin to a guy walking down the street and yelling: 'Am I me? Am I me?' - Yes, you're you, shut up. An HA router is an HA router, there's no point in the class checking if it's indeed an HA router. * keepalived_manager._get_full_config_file_path was being used outside of the keepalived_manager, removed the leading underscore * _ha_get_existing_cidrs doesn't need an 'ha' prefix, it's already a part of the HA router class. This patch renames the method to a more descriptive name * HARouter._get_ipv6_lladdr had nothing to do with HA routers, nor did it use the HARouter state. Moved it to ip_lib Partially-Implements: bp/restructure-l3-agent Change-Id: I6a7df0b08ca3fc8c2c1b310aa8dd897111f52974 --- neutron/agent/l3/ha_router.py | 23 ++++--------------- neutron/agent/linux/ip_lib.py | 4 ++++ neutron/agent/linux/keepalived.py | 6 ++--- .../tests/functional/agent/test_l3_agent.py | 6 ++--- neutron/tests/unit/agent/l3/test_ha_router.py | 2 +- neutron/tests/unit/test_l3_agent.py | 2 +- 6 files changed, 16 insertions(+), 27 deletions(-) diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 3fa9504f3..db5c102a8 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -38,12 +38,6 @@ class HaRouter(router.RouterInfo): self.ha_port = None self.keepalived_manager = None - def _verify_ha(self): - # TODO(Carl) Remove when is_ha below is removed. - if not self.is_ha: - raise ValueError(_('Router %s is not a HA router') % - self.router_id) - @property def is_ha(self): # TODO(Carl) Remove when refactoring to use sub-classes is complete. @@ -51,18 +45,15 @@ class HaRouter(router.RouterInfo): @property def ha_priority(self): - self._verify_ha() return self.router.get('priority', keepalived.HA_DEFAULT_PRIORITY) @property def ha_vr_id(self): - self._verify_ha() return self.router.get('ha_vr_id') @property def ha_state(self): - self._verify_ha() - ha_state_path = self.keepalived_manager._get_full_config_file_path( + ha_state_path = self.keepalived_manager.get_full_config_file_path( 'state') try: with open(ha_state_path, 'r') as f: @@ -73,8 +64,7 @@ class HaRouter(router.RouterInfo): @ha_state.setter def ha_state(self, new_state): - self._verify_ha() - ha_state_path = self.keepalived_manager._get_full_config_file_path( + ha_state_path = self.keepalived_manager.get_full_config_file_path( 'state') try: with open(ha_state_path, 'w') as f: @@ -158,12 +148,12 @@ class HaRouter(router.RouterInfo): instance = self._get_keepalived_instance() instance.remove_vips_vroutes_by_interface(interface) - def _ha_get_existing_cidrs(self, interface_name): + def _get_cidrs_from_keepalived(self, interface_name): instance = self._get_keepalived_instance() return instance.get_existing_vip_ip_addresses(interface_name) def get_router_cidrs(self, device): - return set(self._ha_get_existing_cidrs(device.name)) + return set(self._get_cidrs_from_keepalived(device.name)) def routes_updated(self): new_routes = self.router['routes'] @@ -196,9 +186,6 @@ class HaRouter(router.RouterInfo): keepalived.KeepalivedVirtualRoute( default_gw, gw_ip, interface_name)) - def _get_ipv6_lladdr(self, mac_addr): - return '%s/64' % netaddr.EUI(mac_addr).ipv6_link_local() - def _should_delete_ipv6_lladdr(self, ipv6_lladdr): """Only the master should have any IP addresses configured. Let keepalived manage IPv6 link local addresses, the same way we let @@ -219,7 +206,7 @@ class HaRouter(router.RouterInfo): will only be present on the master. """ device = ip_lib.IPDevice(interface_name, namespace=self.ns_name) - ipv6_lladdr = self._get_ipv6_lladdr(device.link.address) + ipv6_lladdr = ip_lib.get_ipv6_lladdr(device.link.address) if self._should_delete_ipv6_lladdr(ipv6_lladdr): device.addr.flush(n_consts.IP_VERSION_6) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 1c4aa49b5..ba3292212 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -717,3 +717,7 @@ def add_namespace_to_cmd(cmd, namespace=None): def get_ip_version(ip_or_cidr): return netaddr.IPNetwork(ip_or_cidr).version + + +def get_ipv6_lladdr(mac_addr): + return '%s/64' % netaddr.EUI(mac_addr).ipv6_link_local() diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py index 52a9377fc..221a26bae 100644 --- a/neutron/agent/linux/keepalived.py +++ b/neutron/agent/linux/keepalived.py @@ -306,7 +306,7 @@ class KeepalivedManager(object): conf_dir = os.path.join(confs_dir, self.resource_id) return conf_dir - def _get_full_config_file_path(self, filename, ensure_conf_dir=True): + def get_full_config_file_path(self, filename, ensure_conf_dir=True): conf_dir = self.get_conf_dir() if ensure_conf_dir: utils.ensure_dir(conf_dir) @@ -314,13 +314,13 @@ class KeepalivedManager(object): def _output_config_file(self): config_str = self.config.get_config_str() - config_path = self._get_full_config_file_path('keepalived.conf') + config_path = self.get_full_config_file_path('keepalived.conf') utils.replace_file(config_path, config_str) return config_path def get_conf_on_disk(self): - config_path = self._get_full_config_file_path('keepalived.conf') + config_path = self.get_full_config_file_path('keepalived.conf') try: with open(config_path) as conf: return conf.read() diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py index b0147bd9b..6db2bea96 100755 --- a/neutron/tests/functional/agent/test_l3_agent.py +++ b/neutron/tests/functional/agent/test_l3_agent.py @@ -155,14 +155,12 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase): ha_device_name = router.get_ha_device_name(router.ha_port['id']) ha_device_cidr = router.ha_port['ip_cidr'] external_port = router.get_ex_gw_port() - ex_port_ipv6 = router._get_ipv6_lladdr( - external_port['mac_address']) + ex_port_ipv6 = ip_lib.get_ipv6_lladdr(external_port['mac_address']) external_device_name = router.get_external_device_name( external_port['id']) external_device_cidr = external_port['ip_cidr'] internal_port = router.router[l3_constants.INTERFACE_KEY][0] - int_port_ipv6 = router._get_ipv6_lladdr( - internal_port['mac_address']) + int_port_ipv6 = ip_lib.get_ipv6_lladdr(internal_port['mac_address']) internal_device_name = router.get_internal_device_name( internal_port['id']) internal_device_cidr = internal_port['ip_cidr'] diff --git a/neutron/tests/unit/agent/l3/test_ha_router.py b/neutron/tests/unit/agent/l3/test_ha_router.py index 486c63dbf..e91e1468b 100644 --- a/neutron/tests/unit/agent/l3/test_ha_router.py +++ b/neutron/tests/unit/agent/l3/test_ha_router.py @@ -43,5 +43,5 @@ class TestBasicRouterOperations(base.BaseTestCase): device = mock.MagicMock() device.name.return_value = 'eth2' addresses = ['15.1.2.2/24', '15.1.2.3/32'] - ri._ha_get_existing_cidrs = mock.MagicMock(return_value=addresses) + ri._get_cidrs_from_keepalived = mock.MagicMock(return_value=addresses) self.assertEqual(set(addresses), ri.get_router_cidrs(device)) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index a34ee482a..1111e3fd1 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -202,7 +202,7 @@ class BasicRouterOperationsFramework(base.BaseTestCase): '.ensure_dir').start() mock.patch('neutron.agent.linux.keepalived.KeepalivedManager' - '._get_full_config_file_path').start() + '.get_full_config_file_path').start() self.utils_exec_p = mock.patch( 'neutron.agent.linux.utils.execute') -- 2.45.2