Add multi ASIC support for syslog rate limit feature

config/syslog.py

@@ -5,7 +5,9 @@
 import subprocess
 
 import utilities_common.cli as clicommon
+import utilities_common.multi_asic as multi_asic_util
 from sonic_py_common import logger
+from sonic_py_common import multi_asic
 from syslog_util import common as syslog_common
 
 
@@ -457,20 +459,46 @@ def delete(db, server_ip_address):
 def rate_limit_host(db, interval, burst):
     """ Configure syslog rate limit for host """
     syslog_common.rate_limit_validator(interval, burst)
-    syslog_common.save_rate_limit_to_db(db, None, interval, burst, log)
+    syslog_common.save_rate_limit_to_db(db.cfgdb, None, interval, burst, log)
 
 
 @syslog.command("rate-limit-container")
 @click.argument("service_name", required=True)
 @click.option("-i", "--interval", help="Configures syslog rate limit interval in seconds for specified containers", type=click.IntRange(0, 2147483647))
 @click.option("-b", "--burst", help="Configures syslog rate limit burst in number of messages for specified containers", type=click.IntRange(0, 2147483647))
+@click.option('--namespace', '-n', 'namespace', default=None, 
+              type=click.Choice(multi_asic_util.multi_asic_ns_choices() + ['default']), 
+              show_default=True, help='Namespace name or all')
 @clicommon.pass_db
-def rate_limit_container(db, service_name, interval, burst):
+def rate_limit_container(db, service_name, interval, burst, namespace):
     """ Configure syslog rate limit for containers """
     syslog_common.rate_limit_validator(interval, burst)
-    feature_data = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
+    features = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
+    syslog_common.service_validator(features, service_name)
+
+    global_feature_data, per_ns_feature_data = syslog_common.extract_feature_data(features)
+    if not namespace:
+        # for all namespaces
+        for namespace, cfg_db in db.cfgdb_clients.items():
+            if namespace == multi_asic.DEFAULT_NAMESPACE:
+                feature_data = global_feature_data
+            else:
+                feature_data = per_ns_feature_data
+            if service_name and service_name not in feature_data:
+                continue
+            syslog_common.service_validator(feature_data, service_name)
+            syslog_common.save_rate_limit_to_db(cfg_db, service_name, interval, burst, log)
+        return
+    elif namespace == 'default':
+        # for default/global namespace only
+        namespace = multi_asic.DEFAULT_NAMESPACE
+        feature_data = global_feature_data
+    else:
+        # for a specific namespace
+        feature_data = per_ns_feature_data
+
     syslog_common.service_validator(feature_data, service_name)
-    syslog_common.save_rate_limit_to_db(db, service_name, interval, burst, log)
+    syslog_common.save_rate_limit_to_db(db.cfgdb_clients[namespace], service_name, interval, burst, log)
 
 
 @syslog.group(
@@ -482,14 +510,70 @@ def rate_limit_feature():
     pass
 
 
+def get_feature_names_to_proceed(db, service_name, namespace):
+    """Get feature name list to be proceed by "config syslog rate-limit-feature enable" and
+    "config syslog rate-limit-feature disable" CLIs
+
+    Args:
+        db (object): Db object
+        service_name (str): Nullable service name to be enable/disable
+        namespace (str): Namespace provided by user
+
+    Returns:
+        list: A list of feature name
+    """
+    features = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
+    if service_name:
+        syslog_common.service_validator(features, service_name)
+
+    global_feature_data, per_ns_feature_data = syslog_common.extract_feature_data(features)
+    if not namespace:
+        if not service_name:
+            feature_list = [feature_name for feature_name in global_feature_data.keys()]
+            if multi_asic.is_multi_asic():
+                asic_count = multi_asic.get_num_asics()
+                for i in range(asic_count):
+                    feature_list.extend([f'{feature_name}{i}' for feature_name in per_ns_feature_data.keys()])
+        else:
+            feature_config = features[service_name]
+            feature_list = []
+            if feature_config[syslog_common.FEATURE_HAS_GLOBAL_SCOPE].lower() == 'true':
+                feature_list.append(service_name)
+
+            if multi_asic.is_multi_asic():
+                if feature_config[syslog_common.FEATURE_HAS_PER_ASIC_SCOPE].lower() == 'true':
+                    asic_count = multi_asic.get_num_asics()
+                    for i in range(asic_count):
+                        feature_list.append(f'{service_name}{i}')
+    elif namespace == 'default':
+        if not service_name:
+            feature_list = [feature_name for feature_name in global_feature_data.keys()]
+        else:
+            syslog_common.service_validator(global_feature_data, service_name)
+            feature_list = [service_name]
+    else:
+        asic_num = multi_asic.get_asic_id_from_name(namespace)
+        if not service_name:
+            feature_list = [f'{feature_name}{asic_num}' for feature_name in per_ns_feature_data.keys()]
+        else:
+            syslog_common.service_validator(per_ns_feature_data, service_name)
+            feature_list = [f'{service_name}{asic_num}']
+    return feature_list
+
+
 @rate_limit_feature.command("enable")
+@click.argument("service_name", required=False)
+@click.option('--namespace', '-n', 'namespace', default=None, 
+              type=click.Choice(multi_asic_util.multi_asic_ns_choices() + ['default']), 
+              show_default=True, help='Namespace name or all')
 @clicommon.pass_db
-def enable_rate_limit_feature(db):
+def enable_rate_limit_feature(db, service_name, namespace):
     """ Enable syslog rate limit feature """
-    feature_data = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
-    for feature_name in feature_data.keys():
+    feature_list = get_feature_names_to_proceed(db, service_name, namespace)
+    for feature_name in feature_list:
         click.echo(f'Enabling syslog rate limit feature for {feature_name}')
-        output, _ = clicommon.run_command(['docker', 'ps', '-q', '-f', 'status=running', '-f', f'name={feature_name}'], return_cmd=True)
+        shell_cmd = f'docker ps -f status=running --format "{{{{.Names}}}}" | grep -E "^{feature_name}$"'
+        output, _ = clicommon.run_command(shell_cmd, return_cmd=True, shell=True)
         if not output:
             click.echo(f'{feature_name} is not running, ignoring...')
             continue
@@ -517,16 +601,21 @@ def enable_rate_limit_feature(db):
 
         if not failed:
             click.echo(f'Enabled syslog rate limit feature for {feature_name}')
-        
-            
+
+
 @rate_limit_feature.command("disable")
+@click.argument("service_name", required=False)
+@click.option('--namespace', '-n', 'namespace', default=None, 
+              type=click.Choice(multi_asic_util.multi_asic_ns_choices() + ['default']), 
+              show_default=True, help='Namespace name or all')
 @clicommon.pass_db
-def disable_rate_limit_feature(db):
+def disable_rate_limit_feature(db, service_name, namespace):
     """ Disable syslog rate limit feature """
-    feature_data = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
-    for feature_name in feature_data.keys():
+    feature_list = get_feature_names_to_proceed(db, service_name, namespace)
+    for feature_name in feature_list:
         click.echo(f'Disabling syslog rate limit feature for {feature_name}')
-        output, _ = clicommon.run_command(['docker', 'ps', '-q', '-f', 'status=running', '-f', f'name={feature_name}'], return_cmd=True)
+        shell_cmd = f'docker ps -f status=running --format "{{{{.Names}}}}" | grep -E "^{feature_name}$"'
+        output, _ = clicommon.run_command(shell_cmd, return_cmd=True, shell=True)
         if not output:
             click.echo(f'{feature_name} is not running, ignoring...')
             continue
@@ -553,4 +642,3 @@ def disable_rate_limit_feature(db):
 
         if not failed:
             click.echo(f'Disabled syslog rate limit feature for {feature_name}')
-

show/syslog.py

@@ -1,9 +1,12 @@
+from unicodedata import name
 import click
 
 import tabulate
 from natsort import natsorted
 
 import utilities_common.cli as clicommon
+import utilities_common.multi_asic as multi_asic_util
+from sonic_py_common import multi_asic
 from syslog_util import common as syslog_common
 
 
@@ -83,25 +86,69 @@ def rate_limit_host(db):
     name='rate-limit-container'
 )
 @click.argument('service_name', metavar='<service_name>', required=False)
+@click.option('--namespace', '-n', 'namespace', default=None, 
+              type=click.Choice(multi_asic_util.multi_asic_ns_choices() + ['default']), 
+              show_default=True, help='Namespace name or all')
 @clicommon.pass_db
-def rate_limit_container(db, service_name):
+def rate_limit_container(db, service_name, namespace):
     """ Show syslog rate limit configuration for containers """
 
     header = [
         "SERVICE",
         "INTERVAL",
         "BURST",
     ]
-    body = []
+
+    # Feature configuration in global DB
     features = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
-
+    if service_name:
+        syslog_common.service_validator(features, service_name)
+
+    global_feature_data, per_ns_feature_data = syslog_common.extract_feature_data(features)
+    if not namespace:
+        # for all namespaces
+        is_first = True
+        for namespace, cfg_db in natsorted(db.cfgdb_clients.items()):
+            if is_first:
+                is_first = False
+            else:
+                # add a new blank line between each namespace
+                click.echo('\n')
+
+            if namespace == multi_asic.DEFAULT_NAMESPACE:
+                if service_name and service_name not in global_feature_data:
+                    continue
+                echo_rate_limit_config(header, cfg_db, service_name, global_feature_data)
+            else:
+                if service_name and service_name not in per_ns_feature_data:
+                    continue
+                echo_rate_limit_config(header, cfg_db, service_name, per_ns_feature_data, namespace)
+    elif namespace == 'default':
+        # for default/global namespace only
+        echo_rate_limit_config(header, db.cfgdb, service_name, global_feature_data)
+    else:
+        # for a specific namespace
+        echo_rate_limit_config(header, db.cfgdb_clients[namespace], service_name, per_ns_feature_data, namespace)
+
+
+def echo_rate_limit_config(header, db, service_name, features, namespace=None):
+    """Echo rate limit configuration
+
+    Args:
+        header (list): CLI headers
+        db (object): Db object
+        service_name (str): Nullable service name to be printed.
+        features (dict): Feature data got from CONFIG DB
+        namespace (str, optional): Namespace provided by user. Defaults to None.
+    """
+    body = []
     if service_name:
         syslog_common.service_validator(features, service_name)
         service_list = [service_name]
     else:
-        service_list = [name for name, service_config in features.items() if service_config.get(syslog_common.SUPPORT_RATE_LIMIT, '').lower() == 'true']
-
-    syslog_configs = db.cfgdb.get_table(syslog_common.SYSLOG_CONFIG_FEATURE_TABLE)
+        service_list = features.keys()
+    
+    syslog_configs = db.get_table(syslog_common.SYSLOG_CONFIG_FEATURE_TABLE)
     for service in natsorted(service_list):
         if service in syslog_configs:
             entry = syslog_configs[service]
@@ -110,5 +157,11 @@ def rate_limit_container(db, service_name):
                         entry.get(syslog_common.SYSLOG_RATE_LIMIT_BURST, 'N/A')])
         else:
             body.append([service, 'N/A', 'N/A'])
-
-    click.echo(format(header, body))
+
+    if namespace:
+        click.echo(f'Namespace {namespace}:')
+
+    if body:
+        click.echo(format(header, body))
+    else:
+        click.echo('N/A')

syslog_util/common.py

@@ -1,4 +1,5 @@
 import click
+from sonic_py_common import multi_asic
 
 
 FEATURE_TABLE = "FEATURE"
@@ -9,6 +10,8 @@
 SYSLOG_RATE_LIMIT_INTERVAL = 'rate_limit_interval'
 SYSLOG_RATE_LIMIT_BURST = 'rate_limit_burst'
 SUPPORT_RATE_LIMIT = 'support_syslog_rate_limit'
+FEATURE_HAS_GLOBAL_SCOPE = 'has_global_scope'
+FEATURE_HAS_PER_ASIC_SCOPE = 'has_per_asic_scope'
 
 
 def rate_limit_validator(interval, burst):
@@ -70,7 +73,33 @@ def save_rate_limit_to_db(db, service_name, interval, burst, log):
         data[SYSLOG_RATE_LIMIT_INTERVAL] = interval
     if burst is not None:
         data[SYSLOG_RATE_LIMIT_BURST] = burst
-    db.cfgdb.mod_entry(table, key, data)
+    db.mod_entry(table, key, data)
     log.log_notice(f"Configured syslog {service_name} rate-limits: interval={data.get(SYSLOG_RATE_LIMIT_INTERVAL, 'N/A')},\
         burst={data.get(SYSLOG_RATE_LIMIT_BURST, 'N/A')}")
 
+
+def extract_feature_data(features):
+    """Extract feature data in global scope and feature data in per ASIC namespace scope
+
+    Args:
+        features (dict): Feature data got from CONFIG DB
+
+    Returns:
+        tuple: <global feature data, per namespace feature data>
+    """
+    global_feature_data = {}
+    per_ns_feature_data = {}
+    is_multi_asic = multi_asic.is_multi_asic()
+    for feature_name, feature_config in features.items():
+        if not feature_config.get(SUPPORT_RATE_LIMIT, '').lower() == 'true':
+            continue
+
+        if is_multi_asic:
+            if feature_config.get(FEATURE_HAS_GLOBAL_SCOPE, '').lower() == 'true':
+                global_feature_data[feature_name] = feature_config
+
+            if feature_config.get(FEATURE_HAS_PER_ASIC_SCOPE, '').lower() == 'true':
+                per_ns_feature_data[feature_name] = feature_config
+        else:
+            global_feature_data[feature_name] = feature_config
+    return global_feature_data, per_ns_feature_data

tests/mock_tables/asic0/config_db.json

@@ -288,5 +288,13 @@
         "ports@": "Ethernet124",
         "type": "L3",
         "stage": "ingress"
+    },
+    "SYSLOG_CONFIG_FEATURE|bgp": {
+        "rate_limit_interval": "111",
+        "rate_limit_burst": "33333"
+    },
+    "SYSLOG_CONFIG_FEATURE|database": {
+        "rate_limit_interval": "222",
+        "rate_limit_burst": "22222"
     }
 }

tests/mock_tables/asic1/config_db.json

@@ -227,5 +227,13 @@
         "holdtime": "10",
         "asn": "65200",
         "keepalive": "3"
+    },
+    "SYSLOG_CONFIG_FEATURE|bgp": {
+        "rate_limit_interval": "444",
+        "rate_limit_burst": "44444"
+    },
+    "SYSLOG_CONFIG_FEATURE|database": {
+        "rate_limit_interval": "555",
+        "rate_limit_burst": "55555"
     }
 }

tests/mock_tables/config_db.json

@@ -826,13 +826,19 @@
         "state": "enabled",
         "auto_restart": "enabled",
         "high_mem_alert": "disabled",
-        "set_owner": "local"
+        "set_owner": "local",
+        "support_syslog_rate_limit": "true",
+        "has_global_scope": "false",
+        "has_per_asic_scope": "true"
     },
     "FEATURE|database": {
         "state": "always_enabled",
         "auto_restart": "always_enabled",
         "high_mem_alert": "disabled",
-        "set_owner": "local"
+        "set_owner": "local",
+        "support_syslog_rate_limit": "true",
+        "has_global_scope": "true",
+        "has_per_asic_scope": "true"
     },
     "FEATURE|dhcp_relay": {
         "state": "enabled",
@@ -856,7 +862,10 @@
         "state": "enabled",
         "auto_restart": "enabled",
         "high_mem_alert": "disabled",
-        "set_owner": "kube"
+        "set_owner": "kube",
+        "support_syslog_rate_limit": "true",
+        "has_global_scope": "true",
+        "has_per_asic_scope": "false"
     },
     "FEATURE|radv": {
         "state": "enabled",
@@ -906,6 +915,14 @@
         "high_mem_alert": "disabled",
         "set_owner": "kube"
     },
+    "SYSLOG_CONFIG_FEATURE|database": {
+        "rate_limit_interval": "200",
+        "rate_limit_burst": "20000"
+    },
+    "SYSLOG_CONFIG_FEATURE|pmon": {
+        "rate_limit_interval": "100",
+        "rate_limit_burst": "10000"
+    },
     "DEVICE_METADATA|localhost": {
         "default_bgp_status": "down",
         "default_pfcwd_status": "enable",