Merge "Fix missing GARP with UDP listeners on SINGLE LB"

octavia/amphorae/backends/agent/api_server/keepalivedlvs.py

@@ -201,6 +201,15 @@ class KeepalivedLvs(lvs_listener_base.LvsListenerApiServerBase):
                             f"{listener_id}"),
                 'details': e.output}, status=500)
 
+        is_vrrp = (CONF.controller_worker.loadbalancer_topology ==
+                   consts.TOPOLOGY_ACTIVE_STANDBY)
+        # TODO(gthiemonge) remove RESTART from the list (same as previous todo
+        # in this function)
+        if not is_vrrp and action in [consts.AMP_ACTION_START,
+                                      consts.AMP_ACTION_RESTART,
+                                      consts.AMP_ACTION_RELOAD]:
+            util.send_vip_advertisements(listener_id=listener_id)
+
         return webob.Response(
             json={'message': 'OK',
                   'details': (f'keepalivedlvs listener {listener_id} '

octavia/amphorae/backends/agent/api_server/util.py

@@ -347,7 +347,37 @@ def get_haproxy_vip_addresses(lb_id):
     return vips
 
 
-def send_vip_advertisements(lb_id):
+def get_lvs_vip_addresses(listener_id: str) -> list[str]:
+    """Get the VIP addresses for a LVS load balancer.
+
+    :param listener_id: The listener ID to get VIP addresses from.
+    :returns: List of VIP addresses (IPv4 and IPv6)
+    """
+    vips = []
+    # Extract the VIP addresses from keepalived configuration
+    # Format is
+    # virtual_server_group ipv<n>-group {
+    #     vip_address1 port1
+    #     vip_address2 port2
+    # }
+    # it can be repeated in case of dual-stack LBs
+    with open(keepalived_lvs_cfg_path(listener_id), encoding='utf-8') as file:
+        vsg_section = False
+        for line in file:
+            current_line = line.strip()
+            if vsg_section:
+                if current_line.startswith('}'):
+                    vsg_section = False
+                else:
+                    vip_address = current_line.split(' ')[0]
+                    vips.append(vip_address)
+            elif line.startswith('virtual_server_group '):
+                vsg_section = True
+    return vips
+
+
+def send_vip_advertisements(lb_id: tp.Optional[str] = None,
+                            listener_id: tp.Optional[str] = None):
     """Sends address advertisements for each load balancer VIP.
 
     This method will send either GARP (IPv4) or neighbor advertisements (IPv6)
@@ -357,7 +387,10 @@ def send_vip_advertisements(lb_id):
     :returns: None
     """
     try:
-        vips = get_haproxy_vip_addresses(lb_id)
+        if lb_id:
+            vips = get_haproxy_vip_addresses(lb_id)
+        else:
+            vips = get_lvs_vip_addresses(listener_id)
 
         for vip in vips:
             interface = network_utils.get_interface_name(

octavia/tests/unit/amphorae/backends/agent/api_server/test_util.py

@@ -356,13 +356,57 @@ class TestUtil(base.TestCase):
         self.assertEqual([], util.get_haproxy_vip_addresses(LB_ID1))
         mock_cfg_path.assert_called_once_with(LB_ID1)
 
+    @mock.patch('octavia.amphorae.backends.agent.api_server.util.'
+                'keepalived_lvs_cfg_path')
+    def test_get_lvs_vip_addresses(self, mock_cfg_path):
+        FAKE_PATH = 'fake_path'
+        mock_cfg_path.return_value = FAKE_PATH
+        self.useFixture(
+            test_utils.OpenFixture(FAKE_PATH, 'no match')).mock_open()
+
+        # Test with no matching lines in the config file
+        self.assertEqual([], util.get_lvs_vip_addresses(LB_ID1))
+        mock_cfg_path.assert_called_once_with(LB_ID1)
+
+        # Test with 2 matching lines
+        mock_cfg_path.reset_mock()
+        test_data = ('virtual_server_group ipv4-group {\n'
+                     '    203.0.113.43 1\n'
+                     '    203.0.113.44 1\n'
+                     '}\n')
+        self.useFixture(
+            test_utils.OpenFixture(FAKE_PATH, test_data)).mock_open()
+        expected_result = ['203.0.113.43', '203.0.113.44']
+        self.assertEqual(expected_result,
+                         util.get_lvs_vip_addresses(LB_ID1))
+        mock_cfg_path.assert_called_once_with(LB_ID1)
+
+        # Test with 2 groups
+        mock_cfg_path.reset_mock()
+        test_data = ('virtual_server_group ipv4-group {\n'
+                     '    203.0.113.43 1\n'
+                     '}\n'
+                     'virtual_server_group ipv6-group {\n'
+                     '    2d01:27::1 2\n'
+                     '    2d01:27::2 2\n'
+                     '}\n')
+        self.useFixture(
+            test_utils.OpenFixture(FAKE_PATH, test_data)).mock_open()
+        expected_result = ['203.0.113.43', '2d01:27::1', '2d01:27::2']
+        self.assertEqual(expected_result,
+                         util.get_lvs_vip_addresses(LB_ID1))
+        mock_cfg_path.assert_called_once_with(LB_ID1)
+
     @mock.patch('octavia.amphorae.backends.utils.ip_advertisement.'
                 'send_ip_advertisement')
     @mock.patch('octavia.amphorae.backends.utils.network_utils.'
                 'get_interface_name')
     @mock.patch('octavia.amphorae.backends.agent.api_server.util.'
                 'get_haproxy_vip_addresses')
-    def test_send_vip_advertisements(self, mock_get_vip_addrs,
+    @mock.patch('octavia.amphorae.backends.agent.api_server.util.'
+                'get_lvs_vip_addresses')
+    def test_send_vip_advertisements(self, mock_get_lvs_vip_addrs,
+                                     mock_get_vip_addrs,
                                      mock_get_int_name, mock_send_advert):
         mock_get_vip_addrs.side_effect = [[], ['203.0.113.46'],
                                           Exception('boom')]
@@ -371,6 +415,7 @@ class TestUtil(base.TestCase):
         # Test no VIPs
         util.send_vip_advertisements(LB_ID1)
         mock_get_vip_addrs.assert_called_once_with(LB_ID1)
+        mock_get_lvs_vip_addrs.assert_not_called()
         mock_get_int_name.assert_not_called()
         mock_send_advert.assert_not_called()
 
@@ -380,6 +425,7 @@ class TestUtil(base.TestCase):
         mock_send_advert.reset_mock()
         util.send_vip_advertisements(LB_ID1)
         mock_get_vip_addrs.assert_called_once_with(LB_ID1)
+        mock_get_lvs_vip_addrs.assert_not_called()
         mock_get_int_name.assert_called_once_with(
             '203.0.113.46', net_ns=consts.AMPHORA_NAMESPACE)
         mock_send_advert.assert_called_once_with(
@@ -393,6 +439,48 @@ class TestUtil(base.TestCase):
         mock_get_int_name.assert_not_called()
         mock_send_advert.assert_not_called()
 
+    @mock.patch('octavia.amphorae.backends.utils.ip_advertisement.'
+                'send_ip_advertisement')
+    @mock.patch('octavia.amphorae.backends.utils.network_utils.'
+                'get_interface_name')
+    @mock.patch('octavia.amphorae.backends.agent.api_server.util.'
+                'get_haproxy_vip_addresses')
+    @mock.patch('octavia.amphorae.backends.agent.api_server.util.'
+                'get_lvs_vip_addresses')
+    def test_send_vip_advertisements_udp(self, mock_get_lvs_vip_addrs,
+                                         mock_get_vip_addrs,
+                                         mock_get_int_name, mock_send_advert):
+        mock_get_lvs_vip_addrs.side_effect = [[], ['203.0.113.46'],
+                                              Exception('boom')]
+        mock_get_int_name.return_value = 'fake0'
+
+        # Test no VIPs
+        util.send_vip_advertisements(listener_id=LISTENER_ID1)
+        mock_get_lvs_vip_addrs.assert_called_once_with(LISTENER_ID1)
+        mock_get_vip_addrs.assert_not_called()
+        mock_get_int_name.assert_not_called()
+        mock_send_advert.assert_not_called()
+
+        # Test with a VIP
+        mock_get_lvs_vip_addrs.reset_mock()
+        mock_get_int_name.reset_mock()
+        mock_send_advert.reset_mock()
+        util.send_vip_advertisements(listener_id=LISTENER_ID1)
+        mock_get_lvs_vip_addrs.assert_called_once_with(LISTENER_ID1)
+        mock_get_vip_addrs.assert_not_called()
+        mock_get_int_name.assert_called_once_with(
+            '203.0.113.46', net_ns=consts.AMPHORA_NAMESPACE)
+        mock_send_advert.assert_called_once_with(
+            'fake0', '203.0.113.46', net_ns=consts.AMPHORA_NAMESPACE)
+
+        # Test with an exception (should not raise)
+        mock_get_lvs_vip_addrs.reset_mock()
+        mock_get_int_name.reset_mock()
+        mock_send_advert.reset_mock()
+        util.send_vip_advertisements(listener_id=LISTENER_ID1)
+        mock_get_int_name.assert_not_called()
+        mock_send_advert.assert_not_called()
+
     @mock.patch('octavia.amphorae.backends.utils.ip_advertisement.'
                 'send_ip_advertisement')
     @mock.patch('octavia.amphorae.backends.utils.network_utils.'

releasenotes/notes/fix-garp-for-udp-listeners-6bf2ec8d491d1e1b.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed an issue with SINGLE topology load balancer with UDP listeners, the
+    Amphora now sends a Gratuitous ARP packet when a UDP pool is added, it
+    makes the VIP address more quickly reachable after a failover or when
+    reusing a previously allocated IP address.