Merge "Consider logging options when using OVNdbsync" into stable/2025.1

Author: Zuul

neutron/common/ovn/acl.py

@@ -148,7 +148,8 @@ def add_acls_for_drop_port_group(pg_name):
                "name": [],
                "severity": [],
                "direction": direction,
-               "match": '{} == @{} && ip'.format(p, pg_name)}
+               "match": '{} == @{} && ip'.format(p, pg_name),
+               "meter": []}
         acl_list.append(acl)
     return acl_list
 
@@ -167,6 +168,7 @@ def drop_all_ip_traffic_for_port(port):
                "severity": [],
                "direction": direction,
                "match": '{} == "{}" && ip'.format(p, port['id']),
+               "meter": [],
                "external_ids": {'neutron:lport': port['id']}}
         acl_list.append(acl)
     return acl_list
@@ -187,6 +189,7 @@ def add_sg_rule_acl_for_port_group(port_group, r, stateful, match):
            "severity": [],
            "direction": dir_map[r['direction']],
            "match": match,
+           "meter": [],
            ovn_const.OVN_SG_RULE_EXT_ID_KEY: r['id']}
     return acl
 

neutron/common/ovn/constants.py

@@ -259,7 +259,7 @@
 
 ACL_EXPECTED_COLUMNS_NBDB = (
     'external_ids', 'direction', 'log', 'priority',
-    'name', 'action', 'severity', 'match')
+    'name', 'action', 'severity', 'match', 'meter')
 
 # Resource types
 TYPE_NETWORKS = 'networks'

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

@@ -35,6 +35,7 @@
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions import qos \
     as ovn_qos
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
+from neutron.services.logapi.drivers.ovn import driver as log_driver
 from neutron.services.segments import db as segments_db
 
 
@@ -82,6 +83,18 @@ def __init__(self, core_plugin, ovn_api, sb_ovn, mode, ovn_driver):
             self.segments_plugin = (
                 manager.NeutronManager.load_class_for_provider(
                     'neutron.service_plugins', 'segments')())
+        self.log_plugin = directory.get_plugin(plugin_constants.LOG_API)
+        if not self.log_plugin:
+            self.log_plugin = (
+                manager.NeutronManager.load_class_for_provider(
+                    'neutron.service_plugins', 'log')())
+            directory.add_plugin(plugin_constants.LOG_API, self.log_plugin)
+        for driver in self.log_plugin.driver_manager.drivers:
+            if driver.name == "ovn":
+                self.ovn_log_driver = driver
+        if not hasattr(self, 'ovn_log_driver'):
+            self.ovn_log_driver = log_driver.OVNDriver()
+            self.log_plugin.driver_manager.register_driver(self.ovn_log_driver)
 
     def stop(self):
         if utils.is_ovn_l3(self.l3_plugin):
@@ -233,6 +246,9 @@ def sync_port_groups(self, ctx):
 
     def _get_acls_from_port_groups(self):
         ovn_acls = []
+        # Options and label columns are only present for OVN >= 22.03.
+        # Furthermore label is a randint so it cannot be compared with any
+        # expected neutron value. They are added later on the ACL addition.
         acl_columns = (self.ovn_api._tables['ACL'].columns.keys() &
                        set(ovn_const.ACL_EXPECTED_COLUMNS_NBDB))
         acl_columns.discard('external_ids')
@@ -244,7 +260,16 @@ def _get_acls_from_port_groups(self):
                 acl_string['port_group'] = pg.name
                 if id_key in acl.external_ids:
                     acl_string[id_key] = acl.external_ids[id_key]
+                # This properties are present as lists of one item,
+                # converting them to string.
+                if acl_string['name']:
+                    acl_string['name'] = acl_string['name'][0]
+                if acl_string['meter']:
+                    acl_string['meter'] = acl_string['meter'][0]
+                if acl_string['severity']:
+                    acl_string['severity'] = acl_string['severity'][0]
                 ovn_acls.append(acl_string)
+
         return ovn_acls
 
     def sync_acls(self, ctx):
@@ -274,6 +299,10 @@ def sync_acls(self, ctx):
         neutron_default_acls = acl_utils.add_acls_for_drop_port_group(
             ovn_const.OVN_DROP_PORT_GROUP_NAME)
 
+        # Add logging options
+        self.ovn_log_driver.add_logging_options_to_acls(neutron_acls, ctx)
+        self.ovn_log_driver.add_logging_options_to_acls(neutron_default_acls,
+                                                        ctx)
         ovn_acls = self._get_acls_from_port_groups()
         # Sort the acls in the ovn database according to the security
         # group rule id for easy comparison in the future.
@@ -304,14 +333,24 @@ def sync_acls(self, ctx):
                 o_index += 1
             elif n_id == o_id:
                 if any(item not in na.items() for item in oa.items()):
+                    for item in oa.items():
+                        if item not in na.items():
+                            LOG.warning('Property %(item)s from OVN ACL not '
+                                        'found in Neutron ACL: %(n_acl)s',
+                                        {'item': item,
+                                         'n_acl': na})
                     add_acls.append(na)
                     remove_acls.append(oa)
                 n_index += 1
                 o_index += 1
             elif n_id > o_id:
+                LOG.warning('ACL should not be present in OVN, removing'
+                            '%(acl)s', {'acl': oa})
                 remove_acls.append(oa)
                 o_index += 1
             else:
+                LOG.warning('ACL should be present in OVN but is not, adding:'
+                            '%(acl)s', {'acl': na})
                 add_acls.append(na)
                 n_index += 1
 
@@ -351,15 +390,41 @@ def get_num_acls(ovn_acls):
         if (self.mode == ovn_const.OVN_DB_SYNC_MODE_REPAIR and
                 (num_acls_to_add or num_acls_to_remove)):
             one_time_pg_resync = True
+            with self.ovn_api.transaction(check_error=True) as txn:
+                for aclr in ovn_acls:
+                    LOG.warning('ACLs found in OVN NB DB but not in '
+                                'Neutron for port group %s',
+                                aclr['port_group'])
+                    txn.add(self.ovn_api.pg_acl_del(aclr['port_group'],
+                                                    aclr['direction'],
+                                                    aclr['priority'],
+                                                    aclr['match']))
+                for aclr in ovn_acls_from_ls:
+                    # Remove all the ACLs from any Logical Switch if they have
+                    # any. Elements are (lswitch_name, list_of_acls).
+                    if len(aclr[1]) > 0:
+                        LOG.warning('Removing ACLs from OVN from Logical '
+                                    'Switch %s', aclr[0])
+                        txn.add(self.ovn_api.acl_del(aclr[0]))
             while True:
                 try:
                     with self.ovn_api.transaction(check_error=True) as txn:
                         for acla in neutron_acls:
                             LOG.warning('ACL found in Neutron but not in '
                                         'OVN NB DB for port group %s',
                                         acla['port_group'])
-                            txn.add(self.ovn_api.pg_acl_add(
-                                **acla, may_exist=True))
+                            acl = txn.add(self.ovn_api.pg_acl_add(**acla,
+                                                               may_exist=True))
+                            # We need to do this now since label should be
+                            # random and not 0. We can use options as a way
+                            # to see if label is supported or not.
+                            if acla.get('log'):
+                                self.ovn_log_driver.add_label_related(acla,
+                                                                      ctx)
+                                txn.add(self.ovn_api.db_set('ACL', acl,
+                                    label=acla['label'],
+                                    options=acla['options']))
+
                 except idlutils.RowNotFound as row_err:
                     if row_err.msg.startswith("Cannot find Port_Group"):
                         if one_time_pg_resync:
@@ -377,23 +442,6 @@ def get_num_acls(ovn_acls):
                         raise
                 break
 
-            with self.ovn_api.transaction(check_error=True) as txn:
-                for aclr in ovn_acls:
-                    LOG.warning('ACLs found in OVN NB DB but not in '
-                                'Neutron for port group %s',
-                                aclr['port_group'])
-                    txn.add(self.ovn_api.pg_acl_del(aclr['port_group'],
-                                                    aclr['direction'],
-                                                    aclr['priority'],
-                                                    aclr['match']))
-                for aclr in ovn_acls_from_ls:
-                    # Remove all the ACLs from any Logical Switch if they have
-                    # any. Elements are (lswitch_name, list_of_acls).
-                    if len(aclr[1]) > 0:
-                        LOG.warning('Removing ACLs from OVN from Logical '
-                                    'Switch %s', aclr[0])
-                        txn.add(self.ovn_api.acl_del(aclr[0]))
-
         LOG.debug('OVN-NB Sync ACLs completed @ %s', str(datetime.now()))
 
     def _calculate_routes_differences(self, ovn_routes, db_routes):

neutron/services/logapi/drivers/ovn/driver.py

@@ -409,6 +409,80 @@ def resource_update(self, context, log_objs):
         with self.ovn_nb.transaction(check_error=True) as ovn_txn:
             self._update_log_objs(context, ovn_txn, log_objs)
 
+    def add_logging_options_to_acls(self, neutron_acls, context):
+        log_objs = self._get_logs(context)
+        for log_obj in log_objs:
+            pgs = self._pgs_from_log_obj(context, log_obj)
+            actions_enabled = self._acl_actions_enabled(log_obj)
+            self._set_neutron_acls_log(pgs, context, actions_enabled,
+                                       utils.ovn_name(log_obj.id),
+                                       neutron_acls)
+
+    # This function is a version of set_acls_log meant to change neutron
+    # defined acls, mostly thought for ovndbsync consistency check.
+    def _set_neutron_acls_log(self, pgs, context, actions_enabled, log_name,
+                              neutron_acls):
+        acl_changes, acl_visits = 0, 0
+        for pg in pgs:
+            meter_name = self.meter_name
+            if pg['name'] != ovn_const.OVN_DROP_PORT_GROUP_NAME:
+                sg = sg_obj.SecurityGroup.get_sg_by_id(context,
+                        pg['external_ids'][ovn_const.OVN_SG_EXT_ID_KEY])
+                if not sg:
+                    LOG.warning("Port Group %s is missing a corresponding "
+                                "security group, skipping its network log "
+                                "setting...", pg["name"])
+                    continue
+                if not sg.stateful:
+                    meter_name = meter_name + ("_stateless")
+            # We need to get the OVN ACL because UUID is not listed as a
+            # property on neutron defined ACLs (and it shouldn't), so we need
+            # to check which ACL is that UUID referring to, using match as
+            # differentiating value.
+            for acl in neutron_acls:
+                acl_visits += 1
+                # skip acls used by a different network log
+                n_acl_name = acl['name']
+                if n_acl_name and n_acl_name != log_name:
+                    continue
+                action = acl['action'] in actions_enabled
+                acl['log'] = action
+                acl['meter'] = meter_name
+                acl['name'] = log_name
+                acl['severity'] = "info"
+                if acl.get('options'):
+                    acl["options"] = {'log-related': "true"}
+                # label is not set because the actual number should not
+                # be compared or taken into account, we only need it to be
+                # different from 0.
+                acl_changes += 1
+        LOG.info("Set %d (out of %d visited) Neutron ACLs for network log %s",
+                 acl_changes, acl_visits, log_name)
+
+    def _get_all_log_pgs(self, ctx):
+        """Get all Port Group names associated to a Log Object.
+
+        :param log_plugin: Currently loaded log_plugging.
+        :param ctx: current running context information
+        """
+        log_objs = self._get_logs(ctx)
+        log_pgs = []
+        for log_obj in log_objs:
+            log_pgs.extend(self._pgs_from_log_obj(ctx, log_obj))
+        return log_pgs
+
+    def add_label_related(self, n_acl, ctx):
+        # Get acls to be able to check if label is present in OVN ACLs and
+        # also check old label value for ACL if it was already present.
+        acls = [acl for pg in self._get_all_log_pgs(ctx) for acl in pg["acls"]]
+        if not acls:
+            return
+        acl = self.ovn_nb.lookup("ACL", acls[0], default=None)
+        if not hasattr(acl, 'label'):
+            return
+        n_acl["label"] = secrets.SystemRandom().randrange(1, MAX_INT_LABEL)
+        n_acl["options"] = {'log-related': 'true'}
+
 
 def register(plugin_driver):
     """Register the driver."""

neutron/tests/functional/base.py

@@ -50,6 +50,7 @@
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import worker
 from neutron.plugins.ml2.drivers import type_geneve  # noqa
 from neutron import service  # noqa
+from neutron.services.logapi.drivers.ovn import driver as log_driver
 from neutron.tests import base
 from neutron.tests.common import base as common_base
 from neutron.tests.common import helpers
@@ -175,6 +176,7 @@ def setUp(self, maintenance_worker=False, service_plugins=None):
         self.addCleanup(exts.PluginAwareExtensionManager.clear_instance)
         self.ovsdb_server_mgr = None
         self._service_plugins = service_plugins
+        log_driver.DRIVER = None
         super().setUp()
         self.test_log_dir = os.path.join(DEFAULT_LOG_DIR, self.id())
         base.setup_test_logging(
@@ -197,6 +199,11 @@ def setUp(self, maintenance_worker=False, service_plugins=None):
                 self.mech_driver.log_driver)
         self.mech_driver.log_driver.plugin_driver = self.mech_driver
         self.mech_driver.log_driver._log_plugin_property = None
+        for driver in self.log_plugin.driver_manager.drivers:
+            if driver.name == "ovn":
+                self.ovn_log_driver = driver
+        if not hasattr(self, 'ovn_log_driver'):
+            self.ovn_log_driver = log_driver.OVNDriver()
         self.ovn_northd_mgr = None
         self.maintenance_worker = maintenance_worker
         mock.patch(