Revert "Track all interfaces in Keepalived"

This reverts commit bee07de.

Reason for revert:
The only interfaces tracked by keepalived are the HA interfaces. The
fixed IP / floating IPs / routes are linked to the internal router interfaces or the gateway interface, that are interfaces not tracked
by keepalived. These IP addresses / routes should have the suffix
"no_track" in the configuration entry.

This is commented in [1], when the keepalived VIP HA configuration
was fixed, excluding any IP address other than the VIP of the HA interface, that are placed in the "virtual_ipaddress_excluded" and
belong to no tracked interfaces.
    
[1]https://github.com/openstack/neutron/blob/ad628353da68dcbf9e6ff25c47fd6753eee22683/neutron/agent/linux/keepalived.py#L271-L281

Change-Id: I4dfd89606042ba545559eb03d47fceee3b0895fc
Closes-Bug: #2097770
(cherry picked from commit 438808f)

Author: ralonsoh

neutron/agent/linux/keepalived.py

@@ -127,12 +127,11 @@ class KeepalivedVirtualRoute(object):
     """A virtual route entry of a keepalived configuration."""
 
     def __init__(self, destination, nexthop, interface_name=None,
-                 scope=None, track=True):
+                 scope=None):
         self.destination = destination
         self.nexthop = nexthop
         self.interface_name = interface_name
         self.scope = scope
-        self.track = track
 
     def build_config(self):
         output = self.destination
@@ -142,7 +141,7 @@ def build_config(self):
             output += ' dev %s' % self.interface_name
         if self.scope:
             output += ' scope %s' % self.scope
-        if not self.track and _is_keepalived_use_no_track_supported():
+        if _is_keepalived_use_no_track_supported():
             output += ' no_track'
         # NOTE(mstinsky): neutron and keepalived are adding the same routes on
         # primary routers. With this we ensure that both are adding the routes
@@ -225,7 +224,7 @@ def set_authentication(self, auth_type, password):
         self.authentication = (auth_type, password)
 
     def add_vip(self, ip_cidr, interface_name, scope):
-        track = interface_name not in self.track_interfaces
+        track = interface_name in self.track_interfaces
         vip = KeepalivedVipAddress(ip_cidr, interface_name, scope, track=track)
         if vip not in self.vips:
             self.vips.append(vip)

neutron/tests/functional/agent/l3/framework.py

@@ -71,16 +71,16 @@
         169.254.0.1/24 dev %(ha_device_name)s
     }
     virtual_ipaddress_excluded {
-        %(floating_ip_cidr)s dev %(ex_device_name)s
-        %(external_device_cidr)s dev %(ex_device_name)s
-        %(internal_device_cidr)s dev %(internal_device_name)s
-        %(ex_port_ipv6)s dev %(ex_device_name)s scope link
-        %(int_port_ipv6)s dev %(internal_device_name)s scope link
+        %(floating_ip_cidr)s dev %(ex_device_name)s no_track
+        %(external_device_cidr)s dev %(ex_device_name)s no_track
+        %(internal_device_cidr)s dev %(internal_device_name)s no_track
+        %(ex_port_ipv6)s dev %(ex_device_name)s scope link no_track
+        %(int_port_ipv6)s dev %(internal_device_name)s scope link no_track
     }
     virtual_routes {
-        0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s protocol static
-        8.8.8.0/24 via 19.4.4.4 protocol static
-        %(extra_subnet_cidr)s dev %(ex_device_name)s scope link protocol static
+        0.0.0.0/0 via %(default_gateway_ip)s dev %(ex_device_name)s no_track protocol static
+        8.8.8.0/24 via 19.4.4.4 no_track protocol static
+        %(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track protocol static
     }
 }"""  # noqa: E501 # pylint: disable=line-too-long
 

neutron/tests/unit/agent/linux/test_keepalived.py

@@ -87,7 +87,7 @@ def test_get_free_range_not_found(self):
 
 class KeepalivedConfBaseMixin(object):
 
-    def _get_config(self, track=True):
+    def _get_config(self):
         config = keepalived.KeepalivedConf()
 
         instance1 = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
@@ -97,16 +97,16 @@ def _get_config(self, track=True):
         instance1.track_interfaces.append("eth0")
 
         vip_address1 = keepalived.KeepalivedVipAddress('192.168.1.0/24',
-                                                       'eth1', track=track)
+                                                       'eth1', track=False)
 
         vip_address2 = keepalived.KeepalivedVipAddress('192.168.2.0/24',
-                                                       'eth2', track=track)
+                                                       'eth2', track=False)
 
         vip_address3 = keepalived.KeepalivedVipAddress('192.168.3.0/24',
-                                                       'eth2', track=track)
+                                                       'eth2', track=False)
 
         vip_address_ex = keepalived.KeepalivedVipAddress('192.168.55.0/24',
-                                                         'eth10', track=track)
+                                                         'eth10', track=False)
 
         instance1.vips.append(vip_address1)
         instance1.vips.append(vip_address2)
@@ -115,7 +115,7 @@ def _get_config(self, track=True):
 
         virtual_route = keepalived.KeepalivedVirtualRoute(n_consts.IPv4_ANY,
                                                           "192.168.1.1",
-                                                          "eth1", track=track)
+                                                          "eth1")
         instance1.virtual_routes.gateway_routes = [virtual_route]
 
         instance2 = keepalived.KeepalivedInstance('MASTER', 'eth4', 2,
@@ -124,7 +124,7 @@ def _get_config(self, track=True):
         instance2.track_interfaces.append("eth4")
 
         vip_address1 = keepalived.KeepalivedVipAddress('192.168.3.0/24',
-                                                       'eth6', track=track)
+                                                       'eth6', track=False)
 
         instance2.vips.append(vip_address1)
         instance2.vips.append(vip_address2)
@@ -192,8 +192,7 @@ def test_config_generation(self):
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=True):
             config = self._get_config()
-            self.assertEqual(self.expected.replace(' no_track', ''),
-                             config.get_config_str())
+            self.assertEqual(self.expected, config.get_config_str())
 
     def test_config_generation_no_track_not_supported(self):
         self._mock_no_track_supported.start().return_value = False
@@ -208,7 +207,7 @@ def test_config_with_reset(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=True):
-            config = self._get_config(track=False)
+            config = self._get_config()
             self.assertEqual(self.expected, config.get_config_str())
 
         config.reset()
@@ -239,24 +238,20 @@ def test_state_exception(self):
 
 class KeepalivedInstanceRoutesTestCase(KeepalivedBaseTestCase):
     @classmethod
-    def _get_instance_routes(cls, track=True):
+    def _get_instance_routes(cls):
         routes = keepalived.KeepalivedInstanceRoutes()
         default_gw_eth0 = keepalived.KeepalivedVirtualRoute(
-            '0.0.0.0/0', '1.0.0.254', 'eth0', track=track)
+            '0.0.0.0/0', '1.0.0.254', 'eth0')
         default_gw_eth1 = keepalived.KeepalivedVirtualRoute(
-            '::/0', 'fe80::3e97:eff:fe26:3bfa/64', 'eth1',
-            track=track)
+            '::/0', 'fe80::3e97:eff:fe26:3bfa/64', 'eth1')
         routes.gateway_routes = [default_gw_eth0, default_gw_eth1]
         extra_routes = [
-            keepalived.KeepalivedVirtualRoute(
-                '10.0.0.0/8', '1.0.0.1', track=track),
-            keepalived.KeepalivedVirtualRoute(
-                '20.0.0.0/8', '2.0.0.2', track=track)]
+            keepalived.KeepalivedVirtualRoute('10.0.0.0/8', '1.0.0.1'),
+            keepalived.KeepalivedVirtualRoute('20.0.0.0/8', '2.0.0.2')]
         routes.extra_routes = extra_routes
         extra_subnets = [
             keepalived.KeepalivedVirtualRoute(
-                '30.0.0.0/8', None, 'eth0', scope='link',
-                track=track)]
+                '30.0.0.0/8', None, 'eth0', scope='link')]
         routes.extra_subnets = extra_subnets
         return routes
 
@@ -282,7 +277,7 @@ def test_build_config(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=True):
-            routes = self._get_instance_routes(track=False)
+            routes = self._get_instance_routes()
             self.assertEqual(expected, '\n'.join(routes.build_config()))
 
     def _get_no_track_less_expected_config(self):
@@ -295,19 +290,11 @@ def _get_no_track_less_expected_config(self):
     }"""
         return expected
 
-    def test_build_config_without_no_track(self):
-        with mock.patch.object(
-                keepalived, '_is_keepalived_use_no_track_supported',
-                return_value=True):
-            routes = self._get_instance_routes()
-            self.assertEqual(self._get_no_track_less_expected_config(),
-                             '\n'.join(routes.build_config()))
-
     def test_build_config_no_track_not_supported(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=False):
-            routes = self._get_instance_routes(track=False)
+            routes = self._get_instance_routes()
             self.assertEqual(self._get_no_track_less_expected_config(),
                              '\n'.join(routes.build_config()))
 
@@ -319,14 +306,12 @@ def test_get_primary_vip(self):
                                                  ['169.254.192.0/18'])
         self.assertEqual('169.254.0.42/24', instance.get_primary_vip())
 
-    def _test_remove_addresses_by_interface(self, track=True):
-        config = self._get_config(track=track)
+    def _test_remove_addresses_by_interface(self, no_track_value):
+        config = self._get_config()
         instance = config.get_instance(1)
         instance.remove_vips_vroutes_by_interface('eth2')
         instance.remove_vips_vroutes_by_interface('eth10')
 
-        no_track_value = ' no_track' if not track else ''
-
         expected = KEEPALIVED_GLOBAL_CONFIG + textwrap.dedent("""
             vrrp_instance VR_1 {
                 state MASTER
@@ -378,19 +363,13 @@ def test_remove_addresses_by_interface(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=True):
-            self._test_remove_addresses_by_interface()
-
-    def test_remove_addresses_by_interface_with_no_track(self):
-        with mock.patch.object(
-                keepalived, '_is_keepalived_use_no_track_supported',
-                return_value=True):
-            self._test_remove_addresses_by_interface(track=False)
+            self._test_remove_addresses_by_interface(" no_track")
 
     def test_remove_address_by_interface_no_track_not_supported(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=False):
-            self._test_remove_addresses_by_interface()
+            self._test_remove_addresses_by_interface("")
 
     def test_build_config_no_vips(self):
         expected = textwrap.dedent("""\
@@ -457,16 +436,6 @@ def test_virtual_route_with_dev(self):
                 return_value=True):
             route = keepalived.KeepalivedVirtualRoute(
                 n_consts.IPv4_ANY, '1.2.3.4', 'eth0')
-            self.assertEqual(
-                '0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static',
-                route.build_config())
-
-    def test_virtual_route_with_dev_supported_no_track(self):
-        with mock.patch.object(
-                keepalived, '_is_keepalived_use_no_track_supported',
-                return_value=True):
-            route = keepalived.KeepalivedVirtualRoute(
-                n_consts.IPv4_ANY, '1.2.3.4', 'eth0', track=False)
             self.assertEqual(
                 '0.0.0.0/0 via 1.2.3.4 dev eth0 no_track protocol static',
                 route.build_config())
@@ -480,21 +449,11 @@ def test_virtual_route_with_dev_no_track_not_supported(self):
             self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static',
                              route.build_config())
 
-    def test_virtual_route_with_dev_no_track_not_supported_not_track(self):
-        with mock.patch.object(
-                keepalived, '_is_keepalived_use_no_track_supported',
-                return_value=False):
-            route = keepalived.KeepalivedVirtualRoute(
-                n_consts.IPv4_ANY, '1.2.3.4', 'eth0', track=False)
-            self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev eth0 protocol static',
-                             route.build_config())
-
     def test_virtual_route_without_dev(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=True):
-            route = keepalived.KeepalivedVirtualRoute(
-                '50.0.0.0/8', '1.2.3.4', track=False)
+            route = keepalived.KeepalivedVirtualRoute('50.0.0.0/8', '1.2.3.4')
             self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track protocol static',
                              route.build_config())