Merge pull request #189 from stackhpc/upstream/2023.1-2024-11-18

Synchronise 2023.1 with upstream

Author: priteau

neutron/db/ovn_revision_numbers_db.py

@@ -88,6 +88,12 @@ class UnknownResourceType(n_exc.NeutronException):
     message = 'Uknown resource type: %(resource_type)s'
 
 
+# NOTE(ralonsoh): to be moved to neutron-lib
+class RevisionNumberNotDefined(n_exc.NeutronException):
+    message = ('Unique revision number not found for %(resource_uuid)s, '
+               'the resource type is required in query')
+
+
 def _get_standard_attr_id(context, resource_uuid, resource_type):
     try:
         row = context.session.query(STD_ATTR_MAP[resource_type]).filter_by(
@@ -151,14 +157,26 @@ def _ensure_revision_row_exist(context, resource, resource_type, std_attr_id):
 
 
 @db_api.retry_if_session_inactive()
-def get_revision_row(context, resource_uuid):
+@db_api.CONTEXT_READER
+def get_revision_row(context, resource_uuid, resource_type=None):
+    """Retrieve the resource revision number
+
+    Only the Neutron ports can have two revision number registers, one for the
+    Logical_Switch_Port and another for the Logical_Router_Port, if this port
+    is a router interface. It is not strictly needed to filter by resource_type
+    if the resource is not a port.
+    """
     try:
-        with db_api.CONTEXT_READER.using(context):
-            return context.session.query(
-                ovn_models.OVNRevisionNumbers).filter_by(
-                    resource_uuid=resource_uuid).one()
+        filters = {'resource_uuid': resource_uuid}
+        if resource_type:
+            filters['resource_type'] = resource_type
+        return context.session.query(
+            ovn_models.OVNRevisionNumbers).filter_by(
+                **filters).one()
     except exc.NoResultFound:
         pass
+    except exc.MultipleResultsFound:
+        raise RevisionNumberNotDefined(resource_uuid=resource_uuid)
 
 
 @db_api.retry_if_session_inactive()

neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py

@@ -403,6 +403,11 @@ def _create_security_group_precommit(self, resource, event, trigger,
             context, security_group['id'],
             ovn_const.TYPE_SECURITY_GROUPS,
             std_attr_id=security_group['standard_attr_id'])
+        for sg_rule in security_group['security_group_rules']:
+            ovn_revision_numbers_db.create_initial_revision(
+                context, sg_rule['id'],
+                ovn_const.TYPE_SECURITY_GROUP_RULES,
+                std_attr_id=sg_rule['standard_attr_id'])
 
     def _create_security_group(self, resource, event, trigger, payload):
         context = payload.context

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -32,6 +32,7 @@
 from neutron_lib.plugins import utils as p_utils
 from neutron_lib.services.logapi import constants as log_const
 from neutron_lib.services.qos import constants as qos_consts
+from neutron_lib.services.trunk import constants as trunk_const
 from neutron_lib.utils import helpers
 from neutron_lib.utils import net as n_net
 from oslo_config import cfg
@@ -289,7 +290,18 @@ def update_lsp_host_info(self, context, db_port, up=True):
                    Defaults to True.
         """
         cmd = []
+        if db_port.device_owner == trunk_const.TRUNK_SUBPORT_OWNER:
+            # NOTE(ralonsoh): OVN subports don't have host ID information.
+            return
+
+        port_up = self._nb_idl.lsp_get_up(db_port.id).execute(
+            check_error=True)
         if up:
+            if not port_up:
+                LOG.warning('Logical_Switch_Port %s host information not '
+                            'updated, the port state is down')
+                return
+
             if not db_port.port_bindings:
                 return
 
@@ -311,6 +323,11 @@ def update_lsp_host_info(self, context, db_port, up=True):
                 self._nb_idl.db_set(
                     'Logical_Switch_Port', db_port.id, ext_ids))
         else:
+            if port_up:
+                LOG.warning('Logical_Switch_Port %s host information not '
+                            'removed, the port state is up')
+                return
+
             cmd.append(
                 self._nb_idl.db_remove(
                     'Logical_Switch_Port', db_port.id, 'external_ids',
@@ -818,7 +835,8 @@ def delete_port(self, context, port_id, port_object=None):
             # to allow at least one maintenance cycle  before we delete the
             # revision number so that the port doesn't stale and eventually
             # gets deleted by the maintenance task.
-            rev_row = db_rev.get_revision_row(context, port_id)
+            rev_row = db_rev.get_revision_row(
+                context, port_id, resource_type=ovn_const.TYPE_PORTS)
             time_ = (timeutils.utcnow() - datetime.timedelta(
                 seconds=ovn_const.DB_CONSISTENCY_CHECK_INTERVAL + 30))
             if rev_row and rev_row.created_at >= time_:
@@ -2425,6 +2443,9 @@ def create_security_group(self, context, security_group):
                 self.is_allow_stateless_supported())
         db_rev.bump_revision(
             context, security_group, ovn_const.TYPE_SECURITY_GROUPS)
+        for sg_rule in security_group['security_group_rules']:
+            db_rev.bump_revision(
+                context, sg_rule, ovn_const.TYPE_SECURITY_GROUP_RULES)
 
     def _add_port_to_drop_port_group(self, port, txn):
         txn.add(self._nb_idl.pg_add_ports(ovn_const.OVN_DROP_PORT_GROUP_NAME,

neutron/services/ovn_l3/plugin.py

@@ -366,7 +366,18 @@ def update_router_gateway_port_bindings(self, router, host):
                 port = self._plugin.update_port(
                     context, port['id'],
                     {'port': {portbindings.HOST_ID: host}})
-
+                # Updates OVN NB database with hostname for lsp router
+                # gateway port
+                with self._nb_ovn.transaction(check_error=True) as txn:
+                    ext_ids = (
+                        "external_ids",
+                        {ovn_const.OVN_HOST_ID_EXT_ID_KEY: host},
+                    )
+                    txn.add(
+                        self._nb_ovn.db_set(
+                            "Logical_Switch_Port", port["id"], ext_ids
+                        )
+                    )
             if port['status'] != status:
                 self._plugin.update_port_status(context, port['id'], status)
 

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py

@@ -371,7 +371,8 @@ def test_port(self):
         # Assert the revision number no longer exists
         self.assertIsNone(db_rev.get_revision_row(
             self.context,
-            neutron_obj['id']))
+            neutron_obj['id'],
+            resource_type=ovn_const.TYPE_PORTS))
 
     def test_subnet_global_dhcp4_opts(self):
         obj_name = 'globaltestsubnet'

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_resources.py

@@ -26,6 +26,7 @@
 from neutron.common.ovn import utils
 from neutron.common import utils as n_utils
 from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as ovn_config
+from neutron.db import ovn_revision_numbers_db as rev_db
 from neutron.tests.functional import base
 
 
@@ -922,6 +923,25 @@ def test_port_security_port_group(self):
         self._verify_port_acls(port_id, expected_acls_with_sg_ps_enabled)
 
 
+class TestSecurityGroups(base.TestOVNFunctionalBase):
+
+    def test_security_group_creation_and_deletion(self):
+        sg = self._make_security_group(self.fmt)['security_group']
+        rev_num = rev_db.get_revision_row(self.context, sg['id'])
+        self.assertEqual(1, rev_num.revision_number)
+        for sg_rule in sg['security_group_rules']:
+            rev_num = rev_db.get_revision_row(self.context, sg_rule['id'])
+            self.assertEqual(0, rev_num.revision_number)
+
+        self._delete('security-groups', sg['id'])
+        self.assertIsNone(rev_db.get_revision_row(self.context, sg['id']))
+        # NOTE(ralonsoh): the deletion of the revision numbers of the SG rules
+        # will be fixed in a follow-up patch.
+        # for sg_rule in sg['security_group_rules']:
+        #     self.assertIsNone(rev_db.get_revision_row(self.context,
+        #                                               sg_rule['id']))
+
+
 class TestDNSRecords(base.TestOVNFunctionalBase):
     _extension_drivers = ['port_security', 'dns']
 

neutron/tests/functional/services/ovn_l3/test_plugin.py

@@ -443,6 +443,12 @@ def fake_select(*args, **kwargs):
             self.assertGreaterEqual(client_select.call_count, 3)
             self.assertGreaterEqual(plugin_select.call_count, 3)
 
+    def _find_port_binding(self, port_id):
+        cmd = self.sb_api.db_find_rows('Port_Binding',
+                                       ('logical_port', '=', port_id))
+        rows = cmd.execute(check_error=True)
+        return rows[0] if rows else None
+
     def test_router_gateway_port_binding_host_id(self):
         # Test setting chassis on chassisredirect port in Port_Binding table,
         # will update host_id of corresponding router gateway port
@@ -465,14 +471,30 @@ def test_router_gateway_port_binding_host_id(self):
                              may_exist=True).execute(check_error=True)
 
         def check_port_binding_host_id(port_id):
+            # Get port from Neutron DB
             port = core_plugin.get_ports(
                 self.context, filters={'id': [port_id]})[0]
-            return port[portbindings.HOST_ID] == host_id
+            # Get port from OVN DB
+            bp = self._find_port_binding(port_id)
+            ovn_host_id = bp.external_ids.get(ovn_const.OVN_HOST_ID_EXT_ID_KEY)
+            return port[portbindings.HOST_ID] == host_id == ovn_host_id
 
         # Test if router gateway port updated with this chassis
         n_utils.wait_until_true(lambda: check_port_binding_host_id(
             gw_port_id))
 
+        # Simulate failover to another chassis and check host_id in Neutron DB
+        # and external_ids:neutron:host_id in OVN DB are updated
+        chassis = idlutils.row_by_value(
+            self.sb_api.idl, "Chassis", "name", self.chassis2
+        )
+        host_id = chassis.hostname
+        self.sb_api.lsp_unbind(logical_port).execute(check_error=True)
+        self.sb_api.lsp_bind(logical_port, self.chassis2).execute(
+            check_error=True
+        )
+        n_utils.wait_until_true(lambda: check_port_binding_host_id(gw_port_id))
+
     def _validate_router_ipv6_ra_configs(self, lrp_name, expected_ra_confs):
         lrp = idlutils.row_by_value(self.nb_api.idl,
                                     'Logical_Router_Port', 'name', lrp_name)

neutron/tests/unit/db/test_db_base_plugin_v2.py

@@ -246,6 +246,14 @@ def _req(self, method, resource, data=None, fmt=None, id=None, params=None,
                                           query_string=params, context=context,
                                           headers=headers)
 
+    def _check_http_response(self, res):
+        # Things can go wrong - raise HTTP exc with res code only
+        # so it can be caught by unit tests
+        if res.status_int >= webob.exc.HTTPClientError.code:
+            res.charset = 'utf8'
+            raise webob.exc.HTTPClientError(explanation=str(res),
+                                            code=res.status_int)
+
     def new_create_request(self, resource, data, fmt=None, id=None,
                            subresource=None, context=None):
         return self._req('POST', resource, data, fmt, id=id,
@@ -523,10 +531,7 @@ def _make_network(self, fmt, name, admin_state_up, **kwargs):
         res = self._create_network(fmt, name, admin_state_up, **kwargs)
         # TODO(salvatore-orlando): do exception handling in this test module
         # in a uniform way (we do it differently for ports, subnets, and nets
-        # Things can go wrong - raise HTTP exc with res code only
-        # so it can be caught by unit tests
-        if res.status_int >= webob.exc.HTTPClientError.code:
-            raise webob.exc.HTTPClientError(code=res.status_int)
+        self._check_http_response(res)
         return self.deserialize(fmt, res)
 
     def _make_subnet(self, fmt, network, gateway, cidr, subnetpool_id=None,
@@ -551,10 +556,7 @@ def _make_subnet(self, fmt, network, gateway, cidr, subnetpool_id=None,
                                   ipv6_ra_mode=ipv6_ra_mode,
                                   ipv6_address_mode=ipv6_address_mode,
                                   set_context=set_context)
-        # Things can go wrong - raise HTTP exc with res code only
-        # so it can be caught by unit tests
-        if res.status_int >= webob.exc.HTTPClientError.code:
-            raise webob.exc.HTTPClientError(code=res.status_int)
+        self._check_http_response(res)
         return self.deserialize(fmt, res)
 
     def _make_v6_subnet(self, network, ra_addr_mode, ipv6_pd=False):
@@ -580,18 +582,30 @@ def _make_subnetpool(self, fmt, prefixes, admin=False, **kwargs):
                                       **kwargs)
         # Things can go wrong - raise HTTP exc with res code only
         # so it can be caught by unit tests
-        if res.status_int >= webob.exc.HTTPClientError.code:
-            raise webob.exc.HTTPClientError(code=res.status_int)
+        self._check_http_response(res)
         return self.deserialize(fmt, res)
 
     def _make_port(self, fmt, net_id, expected_res_status=None, **kwargs):
         res = self._create_port(fmt, net_id, expected_res_status, **kwargs)
         # Things can go wrong - raise HTTP exc with res code only
         # so it can be caught by unit tests
-        if res.status_int >= webob.exc.HTTPClientError.code:
-            raise webob.exc.HTTPClientError(code=res.status_int)
+        self._check_http_response(res)
         return self.deserialize(fmt, res)
 
+    def _make_security_group(self, fmt, name=None, expected_res_status=None,
+                             project_id=None, is_admin=False):
+        name = name or 'sg-{}'.format(uuidutils.generate_uuid())
+        project_id = project_id or self._tenant_id
+        data = {'security_group': {'name': name,
+                                   'description': name,
+                                   'project_id': project_id}}
+        sg_req = self.new_create_request('security-groups', data, fmt)
+        sg_res = sg_req.get_response(self.api)
+        if expected_res_status:
+            self.assertEqual(expected_res_status, sg_res.status_int)
+        self._check_http_response(sg_res)
+        return self.deserialize(fmt, sg_res)
+
     def _create_qos_rule(self, fmt, qos_policy_id, rule_type, max_kbps=None,
                          max_burst_kbps=None, dscp_mark=None, min_kbps=None,
                          direction=constants.EGRESS_DIRECTION,

neutron/tests/unit/db/test_l3_db.py

@@ -32,7 +32,6 @@
 from neutron_lib.plugins import utils as plugin_utils
 from oslo_utils import uuidutils
 import testtools
-import webob.exc
 
 from neutron.db import extraroute_db
 from neutron.db import l3_db
@@ -951,8 +950,7 @@ def _create_external_network(self, name=None, **kwargs):
             res = self._create_network(
                 self.fmt, name, True,
                 arg_list=(extnet_apidef.EXTERNAL,), **kwargs)
-            if res.status_int >= webob.exc.HTTPClientError.code:
-                raise webob.exc.HTTPClientError(code=res.status_int)
+            self._check_http_response(res)
             return self.deserialize(self.fmt, res)
 
     def test_update_router_gw_notify(self):

neutron/tests/unit/db/test_ovn_revision_numbers_db.py

@@ -24,6 +24,7 @@
 
 from neutron.api import extensions
 from neutron.common import config
+from neutron.common.ovn import constants as ovn_const
 from neutron.db.models import ovn as ovn_models
 from neutron.db import ovn_revision_numbers_db as ovn_rn_db
 import neutron.extensions
@@ -32,7 +33,6 @@
 from neutron.tests.unit.extensions import test_l3
 from neutron.tests.unit.extensions import test_securitygroup
 
-
 EXTENSIONS_PATH = ':'.join(neutron.extensions.__path__)
 PLUGIN_CLASS = (
     'neutron.tests.unit.db.test_ovn_revision_numbers_db.TestMaintenancePlugin')
@@ -123,6 +123,24 @@ def test_create_initial_revision_may_exist_duplicated_entry(self):
                 self.fail("create_initial_revision shouldn't raise "
                           "DBDuplicateEntry when may_exist is True")
 
+    def test_get_revision_row_ports(self):
+        res = self._create_port(self.fmt, self.net['id'])
+        port = self.deserialize(self.fmt, res)['port']
+        with db_api.CONTEXT_WRITER.using(self.ctx):
+            for resource_type in (ovn_const.TYPE_PORTS,
+                                  ovn_const.TYPE_ROUTER_PORTS):
+                self._create_initial_revision(port['id'], resource_type)
+
+        for resource_type in (ovn_const.TYPE_PORTS,
+                              ovn_const.TYPE_ROUTER_PORTS):
+            row = ovn_rn_db.get_revision_row(
+                self.ctx, port['id'], resource_type=resource_type)
+            self.assertEqual(resource_type, row.resource_type)
+            self.assertEqual(port['id'], row.resource_uuid)
+
+        self.assertRaises(ovn_rn_db.RevisionNumberNotDefined,
+                          ovn_rn_db.get_revision_row, self.ctx, port['id'])
+
 
 class TestMaintenancePlugin(test_securitygroup.SecurityGroupTestPlugin,
                             test_l3.TestL3NatBasePlugin):