Merge pull request #44 from stackhpc/upstream/2024.1-2025-04-21

Synchronise 2024.1 with upstream

Author: priteau

octavia/amphorae/drivers/keepalived/jinja/jinja_cfg.py

@@ -123,7 +123,6 @@ def build_keepalived_config(self, loadbalancer, amphora, amp_net_config):
                 peers_ips.append(amp.vrrp_ip)
         return self.get_template(self.keepalived_template).render(
             {'vrrp_group_name': loadbalancer.vrrp_group.vrrp_group_name,
-             'amp_role': amphora.role,
              'amp_intf': amphora.vrrp_interface,
              'amp_vrrp_id': amphora.vrrp_id,
              'amp_priority': amphora.vrrp_priority,

octavia/amphorae/drivers/keepalived/jinja/templates/keepalived_base.template

@@ -21,7 +21,6 @@ vrrp_script check_script {
 }
 
 vrrp_instance {{ vrrp_group_name }} {
-    state {{ amp_role }}
     interface {{ amp_intf }}
     virtual_router_id {{ amp_vrrp_id }}
     priority {{ amp_priority }}

octavia/controller/worker/v2/tasks/network_tasks.py

@@ -984,10 +984,9 @@ def revert(self, result, vip, vip_sg_id, amphora_id, additional_vips,
             return
         try:
             port_name = constants.AMP_BASE_PORT_PREFIX + amphora_id
-            for port in result:
-                self.network_driver.delete_port(port.id)
-                LOG.info('Deleted port %s with ID %s for amphora %s due to a '
-                         'revert.', port_name, port.id, amphora_id)
+            self.network_driver.delete_port(result[constants.ID])
+            LOG.info('Deleted port %s with ID %s for amphora %s due to a '
+                     'revert.', port_name, result[constants.ID], amphora_id)
         except Exception as e:
             LOG.error('Failed to delete port %s. Resources may still be in '
                       'use for a port intended for amphora %s due to error '

octavia/db/base_models.py

@@ -12,13 +12,17 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from wsme import types as wtypes
+
 from oslo_db.sqlalchemy import models
 from oslo_utils import strutils
 from oslo_utils import uuidutils
 import sqlalchemy as sa
 from sqlalchemy.orm import collections
 from sqlalchemy.orm import declarative_base
 
+from octavia.common import constants
+
 
 class OctaviaBase(models.ModelBase):
 
@@ -112,12 +116,20 @@ def to_data_model(self, _graph_nodes=None):
 
     @staticmethod
     def apply_filter(query, model, filters):
+        # Convert boolean filters to proper type
+        for key in filters:
+            attr = getattr(model.__v2_wsme__, key, None)
+            if isinstance(attr, wtypes.wsattr) and attr.datatype == bool:
+                filters[key] = strutils.bool_from_string(filters[key])
+        # Special case for 'enabled', it's 'admin_state_up' in the WSME class
+        # definition and the attribute has already been renamed to 'enabled' by
+        # a previous pagination filter
+        if constants.ENABLED in filters:
+            filters[constants.ENABLED] = strutils.bool_from_string(
+                filters[constants.ENABLED])
+
         translated_filters = {}
         child_map = {}
-        # Convert enabled to proper type
-        if 'enabled' in filters:
-            filters['enabled'] = strutils.bool_from_string(
-                filters['enabled'])
         for attr, name_map in model.__v2_wsme__._child_map.items():
             for k, v in name_map.items():
                 if attr in filters and k in filters[attr]:

octavia/tests/unit/amphorae/drivers/keepalived/jinja/test_jinja_cfg.py

@@ -71,7 +71,6 @@ def setUp(self):
             "}\n"
             "\n"
             "vrrp_instance TESTGROUP {\n"
-            "    state MASTER\n"
             "    interface eth1\n"
             "    virtual_router_id 1\n"
             "    priority 100\n"
@@ -124,7 +123,6 @@ def setUp(self):
             "}\n"
             "\n"
             "vrrp_instance TESTGROUP {\n"
-            "    state MASTER\n"
             "    interface eth1\n"
             "    virtual_router_id 1\n"
             "    priority 100\n"
@@ -170,7 +168,6 @@ def setUp(self):
             "}\n"
             "\n"
             "vrrp_instance TESTGROUP {\n"
-            "    state MASTER\n"
             "    interface eth1\n"
             "    virtual_router_id 1\n"
             "    priority 100\n"
@@ -220,7 +217,6 @@ def setUp(self):
             "}\n"
             "\n"
             "vrrp_instance TESTGROUP {\n"
-            "    state MASTER\n"
             "    interface eth1\n"
             "    virtual_router_id 1\n"
             "    priority 100\n"

octavia/tests/unit/api/common/test_pagination.py

@@ -103,6 +103,16 @@ def test_filter_correct_params(self, request_mock):
         helper.apply(query_mock, models.LoadBalancer)
         self.assertEqual(params, helper.filters)
 
+    @mock.patch('octavia.api.common.pagination.request')
+    def test_filter_with_booleans(self, request_mock):
+        params = {'backup': 'True', 'admin_state_up': 'false'}
+        expected_params = {'backup': True, 'enabled': False}
+        helper = pagination.PaginationHelper(params)
+        query_mock = mock.MagicMock()
+
+        helper.apply(query_mock, models.Member)
+        self.assertEqual(expected_params, helper.filters)
+
     @mock.patch('octavia.api.common.pagination.request')
     def test_filter_mismatched_params(self, request_mock):
         params = {

octavia/tests/unit/cmd/test_prometheus_proxy.py

@@ -147,6 +147,7 @@ def test_shutdown_thread(self, mock_signal, mock_exit_event):
         mock_http.shutdown.assert_called_once()
 
     @mock.patch('threading.Thread')
+    @mock.patch('http.server.ThreadingHTTPServer.__init__')
     @mock.patch('http.server.ThreadingHTTPServer.serve_forever')
     @mock.patch('octavia.amphorae.backends.utils.network_namespace.'
                 'NetworkNamespace.__exit__')
@@ -155,12 +156,18 @@ def test_shutdown_thread(self, mock_signal, mock_exit_event):
     @mock.patch('octavia.cmd.prometheus_proxy.EXIT_EVENT')
     @mock.patch('octavia.cmd.prometheus_proxy.SignalHandler')
     def test_main(self, mock_signal_handler, mock_exit_event, mock_netns_enter,
-                  mock_netns_exit, mock_serve_forever, mock_thread):
+                  mock_netns_exit, mock_serve_forever, mock_server_init,
+                  mock_thread):
 
         mock_exit_event.is_set.side_effect = [False, False, True]
         mock_netns_enter.side_effect = [Exception('boom'), True]
 
+        mock_server_init.return_value = None
+
         prometheus_proxy.main()
 
         mock_signal_handler.assert_called_once()
+        mock_server_init.assert_called_once_with(
+            ('127.0.0.1', 9102),
+            prometheus_proxy.PrometheusProxy)
         mock_serve_forever.assert_called_once()

octavia/tests/unit/controller/worker/v2/tasks/test_network_tasks.py

@@ -1721,15 +1721,19 @@ def test_create_vip_base_port(self, mock_get_net_driver):
         # Test revert
         mock_driver.reset_mock()
 
-        net_task.revert([port_mock], vip_dict, VIP_SG_ID, AMP_ID,
+        # The execute path generates a port dict, so this will be the result
+        # passed into the revert method by Taskflow
+        port_dict = {constants.ID: PORT_ID}
+
+        net_task.revert(port_dict, vip_dict, VIP_SG_ID, AMP_ID,
                         additional_vips)
 
         mock_driver.delete_port.assert_called_once_with(PORT_ID)
 
         # Test revert exception
         mock_driver.reset_mock()
 
-        net_task.revert([port_mock], vip_dict, VIP_SG_ID, AMP_ID,
+        net_task.revert(port_dict, vip_dict, VIP_SG_ID, AMP_ID,
                         additional_vips)
 
         mock_driver.delete_port.assert_called_once_with(PORT_ID)

releasenotes/notes/Fix-failover-revert-port-leak-d9879523506c6ff3.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixed an issue when a failover reverts, a neutron port may get abandoned.
+    The issue was logged with "Failed to delete port",
+    "Resources may still be in use for a port intended for amphora", and
+    "Search for a port named octavia-lb-vrrp-<uuid>".

releasenotes/notes/fix-filtering-with-boolean-attributes-15df51820753a900.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixed an issue when filtering resources with a boolean attribute in the GET
+    calls in the Octavia API.