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

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index 3fa9504f3e74d164234a63879458caeaa8a38767..db5c102a85819e06c22d662c216698fbdb54f204 100644 (file)
--- 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 1c4aa49b54346a99f2e75e366d65a02354ae6854..ba32922127164e00d069827e7fa9fda1036c3d90 100644 (file)
--- 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 52a9377fc81b5929d1412255a908289e28bb5f3c..221a26bae18924e9ae012982e215e4fc19fa7610 100644 (file)
--- 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 b0147bd9b9f1f316988a428e60fab96f61d30eb5..6db2bea96d7ac7c0db54812264f95e88e425a606 100755 (executable)
--- 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 486c63dbf9beee941cbfabf8435608c8868b3484..e91e1468b36f61cd8688579db567cfce5f6525c6 100644 (file)
--- 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 a34ee482a8501135c9545bd3b0f19bf3e26804e9..1111e3fd1869ac5cec35d5ebd1c12be3490be132 100644 (file)
--- 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')