Fix duplicated routes exceptions

Since the train release neutron adds routes with protocol static.
Keepalived also adds the same routes with different protocols depending
on the keepalived version. This can result in duplicated routes inside
network namespaces. On l3 agent restarts those duplicate routes
then prevent the l3 agent from updating its router state
because it runs into 'RTNETLINK answers: File exists expections'
when it tries to execute 'ip route' commands.

This patch adds the same protocol static to each virtual route of
keepalived's configuration so network namespaces do not run into
duplicated routes anymore.

Closes-Bug: #1956846
Change-Id: Ic35b5d4b9110b832c10345c45ec62c0923237cfd
(cherry picked from commit c813b65)

Author: mstinsky

neutron/agent/linux/keepalived.py

@@ -152,6 +152,11 @@ def build_config(self):
                 LOG.warning("keepalived_use_no_track cfg option is True but "
                             "keepalived on host seems to not support this "
                             "option")
+        # NOTE(mstinsky): neutron and keepalived are adding the same routes on
+        # primary routers. With this we ensure that both are adding the routes
+        # with the same procotol and prevent duplicated routes which result in
+        # neutron exception for ip route commands.
+        output += ' protocol static'
         return output
 
 

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

@@ -78,11 +78,11 @@
         %(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 no_track
-        8.8.8.0/24 via 19.4.4.4 no_track
-        %(extra_subnet_cidr)s dev %(ex_device_name)s scope link no_track
+        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
 
 
 def get_ovs_bridge(br_name):

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

@@ -164,7 +164,7 @@ class KeepalivedConfTestCase(KeepalivedBaseTestCase,
                 192.168.55.0/24 dev eth10 no_track
             }
             virtual_routes {
-                0.0.0.0/0 via 192.168.1.1 dev eth1 no_track
+                0.0.0.0/0 via 192.168.1.1 dev eth1 no_track protocol static
             }
         }
         vrrp_instance VR_2 {
@@ -247,7 +247,7 @@ class KeepalivedConfWithoutNoTrackTestCase(KeepalivedConfTestCase):
                 192.168.55.0/24 dev eth10
             }
             virtual_routes {
-                0.0.0.0/0 via 192.168.1.1 dev eth1
+                0.0.0.0/0 via 192.168.1.1 dev eth1 protocol static
             }
         }
         vrrp_instance VR_2 {
@@ -323,11 +323,11 @@ def test_remove_routes_on_interface(self):
 
     def test_build_config(self):
         expected = """    virtual_routes {
-        0.0.0.0/0 via 1.0.0.254 dev eth0 no_track
-        ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 no_track
-        10.0.0.0/8 via 1.0.0.1 no_track
-        20.0.0.0/8 via 2.0.0.2 no_track
-        30.0.0.0/8 dev eth0 scope link no_track
+        0.0.0.0/0 via 1.0.0.254 dev eth0 no_track protocol static
+        ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 no_track protocol static
+        10.0.0.0/8 via 1.0.0.1 no_track protocol static
+        20.0.0.0/8 via 2.0.0.2 no_track protocol static
+        30.0.0.0/8 dev eth0 scope link no_track protocol static
     }"""
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
@@ -337,11 +337,11 @@ def test_build_config(self):
 
     def _get_no_track_less_expected_config(self):
         expected = """    virtual_routes {
-        0.0.0.0/0 via 1.0.0.254 dev eth0
-        ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1
-        10.0.0.0/8 via 1.0.0.1
-        20.0.0.0/8 via 2.0.0.2
-        30.0.0.0/8 dev eth0 scope link
+        0.0.0.0/0 via 1.0.0.254 dev eth0 protocol static
+        ::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1 protocol static
+        10.0.0.0/8 via 1.0.0.1 protocol static
+        20.0.0.0/8 via 2.0.0.2 protocol static
+        30.0.0.0/8 dev eth0 scope link protocol static
     }"""
         return expected
 
@@ -395,7 +395,7 @@ def _test_remove_addresses_by_interface(self, no_track_value):
                     192.168.1.0/24 dev eth1%(no_track)s
                 }
                 virtual_routes {
-                    0.0.0.0/0 via 192.168.1.1 dev eth1%(no_track)s
+                    0.0.0.0/0 via 192.168.1.1 dev eth1%(no_track)s protocol static
                 }
             }
             vrrp_instance VR_2 {
@@ -416,7 +416,7 @@ def _test_remove_addresses_by_interface(self, no_track_value):
                     192.168.3.0/24 dev eth6%(no_track)s
                     192.168.55.0/24 dev eth10%(no_track)s
                 }
-            }""" % {'no_track': no_track_value})
+            }""" % {'no_track': no_track_value})  # noqa: E501 # pylint: disable=line-too-long
 
         self.assertEqual(expected, config.get_config_str())
 
@@ -501,45 +501,47 @@ 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 no_track',
-                             route.build_config())
+            self.assertEqual(
+                '0.0.0.0/0 via 1.2.3.4 dev eth0 no_track protocol static',
+                route.build_config())
 
     def test_virtual_route_with_dev_no_track_not_supported(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')
-            self.assertEqual('0.0.0.0/0 via 1.2.3.4 dev 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_without_no_track(self):
         cfg.CONF.set_override('keepalived_use_no_track', False)
         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',
+        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')
-            self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track',
+            self.assertEqual('50.0.0.0/8 via 1.2.3.4 no_track protocol static',
                              route.build_config())
 
     def test_virtual_route_without_dev_no_track_not_supported(self):
         with mock.patch.object(
                 keepalived, '_is_keepalived_use_no_track_supported',
                 return_value=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',
+            self.assertEqual('50.0.0.0/8 via 1.2.3.4 protocol static',
                              route.build_config())
 
     def test_virtual_route_without_dev_without_no_track(self):
         cfg.CONF.set_override('keepalived_use_no_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', route.build_config())
+        self.assertEqual('50.0.0.0/8 via 1.2.3.4 protocol static',
+                         route.build_config())
 
 
 class KeepalivedTrackScriptTestCase(KeepalivedBaseTestCase):