Support syslog rate limit configuration for containers and host (#2454)

* Support syslog rate limit configuration for containers and host

* Fix LGTM

* Fix unit test issue

* Add syslog_util as a package

* Fix review comment

* Update feature.py

config/syslog.py

@@ -6,6 +6,7 @@
 
 import utilities_common.cli as clicommon
 from sonic_py_common import logger
+from syslog_util import common as syslog_common
 
 
 SYSLOG_TABLE_CDB = "SYSLOG_SERVER"
@@ -447,3 +448,26 @@ def delete(db, server_ip_address):
     except Exception as e:
         log.log_error("Failed to remove remote syslog logging: {}".format(str(e)))
         ctx.fail(str(e))
+
+
+@syslog.command("rate-limit-host")
+@click.option("-i", "--interval", help="Configures syslog rate limit interval in seconds for host", type=click.IntRange(0, 2147483647))
+@click.option("-b", "--burst", help="Configures syslog rate limit burst in number of messages for host", type=click.IntRange(0, 2147483647))
+@clicommon.pass_db
+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.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))
+@clicommon.pass_db
+def rate_limit_container(db, service_name, interval, burst):
+    """ Configure syslog rate limit for containers """
+    syslog_common.rate_limit_validator(interval, burst)
+    feature_data = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
+    syslog_common.service_validator(feature_data, service_name)
+    syslog_common.save_rate_limit_to_db(db, service_name, interval, burst, log)

setup.py

@@ -48,6 +48,7 @@
         'pddf_psuutil',
         'pddf_thermalutil',
         'pddf_ledutil',
+        'syslog_util',
         'show',
         'show.interfaces',
         'show.plugins',

show/syslog.py

@@ -4,6 +4,7 @@
 from natsort import natsorted
 
 import utilities_common.cli as clicommon
+from syslog_util import common as syslog_common
 
 
 SYSLOG_TABLE = "SYSLOG_SERVER"
@@ -28,10 +29,14 @@ def format(header, body):
     cls=clicommon.AliasedGroup,
     invoke_without_command=True
 )
+@click.pass_context
 @clicommon.pass_db
-def syslog(db):
+def syslog(db, ctx):
     """ Show syslog server configuration """
 
+    if ctx.invoked_subcommand is not None:
+        return
+
     header = [
         "SERVER IP",
         "SOURCE IP",
@@ -51,3 +56,59 @@ def syslog(db):
         body.append(row)
 
     click.echo(format(header, body))
+
+@syslog.command(
+    name='rate-limit-host'
+)
+@clicommon.pass_db
+def rate_limit_host(db):
+    """ Show syslog rate limit configuration for host """
+
+    header = [
+        "INTERVAL",
+        "BURST",
+    ]
+    body = []
+    entry = db.cfgdb.get_entry(syslog_common.SYSLOG_CONFIG_TABLE, syslog_common.SYSLOG_CONFIG_GLOBAL_KEY)
+    if entry:
+        body.append([entry.get(syslog_common.SYSLOG_RATE_LIMIT_INTERVAL, 'N/A'),
+                    entry.get(syslog_common.SYSLOG_RATE_LIMIT_BURST, 'N/A')])
+    else:
+        body.append('N/A', 'N/A')
+
+    click.echo(format(header, body))
+
+
+@syslog.command(
+    name='rate-limit-container'
+)
+@click.argument('service_name', metavar='<service_name>', required=False)
+@clicommon.pass_db
+def rate_limit_container(db, service_name):
+    """ Show syslog rate limit configuration for containers """
+
+    header = [
+        "SERVICE",
+        "INTERVAL",
+        "BURST",
+    ]
+    body = []
+    features = db.cfgdb.get_table(syslog_common.FEATURE_TABLE)
+
+    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)
+    for service in natsorted(service_list):
+        if service in syslog_configs:
+            entry = syslog_configs[service]
+            body.append([service,
+                        entry.get(syslog_common.SYSLOG_RATE_LIMIT_INTERVAL, 'N/A'),
+                        entry.get(syslog_common.SYSLOG_RATE_LIMIT_BURST, 'N/A')])
+        else:
+            body.append([service, 'N/A', 'N/A'])
+
+    click.echo(format(header, body))

sonic_package_manager/manifest.py

@@ -186,6 +186,9 @@ def unmarshal(self, value):
                 ManifestArray('after', DefaultMarshaller(str)),
                 ManifestArray('before', DefaultMarshaller(str)),
             ]),
+            ManifestRoot('syslog', [
+                ManifestField('support-rate-limit', DefaultMarshaller(bool), False),
+            ]),
         ]),
         ManifestRoot('container', [
             ManifestField('privileged', DefaultMarshaller(bool), False),

sonic_package_manager/service_creator/feature.py

@@ -25,6 +25,12 @@
     'available_mem_threshold': '10.0'
 }
 
+SYSLOG_CONFIG = 'SYSLOG_CONFIG_FEATURE'
+DEFAULT_SYSLOG_FEATURE_CONFIG = {
+    'rate_limit_interval': '300',
+    'rate_limit_burst': '20000'
+}
+
 def is_enabled(cfg):
     return cfg.get('state', 'disabled').lower() == 'enabled'
 
@@ -36,7 +42,7 @@ def is_multi_instance(cfg):
 class FeatureRegistry:
     """ 1) FeatureRegistry class provides an interface to
     register/de-register new feature tables persistently.
-        2) Writes persistent configuration to FEATURE & 
+        2) Writes persistent configuration to FEATURE &
     AUTO_TECHSUPPORT_FEATURE tables
     """
 
@@ -72,10 +78,14 @@ def register(self,
             new_cfg = {**new_cfg, **non_cfg_entries}
 
             conn.set_entry(FEATURE, name, new_cfg)
-        
+
         if self.register_auto_ts(name):
             log.info(f'{name} entry is added to {AUTO_TS_FEATURE} table')
 
+        if 'syslog' in manifest['service'] and 'support-rate-limit' in manifest['service']['syslog'] and manifest['service']['syslog']['support-rate-limit']:
+            self.register_syslog_config(name)
+            log.info(f'{name} entry is added to {SYSLOG_CONFIG} table')
+
     def deregister(self, name: str):
         """ Deregister feature by name.
 
@@ -89,6 +99,7 @@ def deregister(self, name: str):
         for conn in db_connetors:
             conn.set_entry(FEATURE, name, None)
             conn.set_entry(AUTO_TS_FEATURE, name, None)
+            conn.set_entry(SYSLOG_CONFIG, name, None)
 
     def update(self,
                old_manifest: Manifest,
@@ -119,10 +130,14 @@ def update(self,
             new_cfg = {**new_cfg, **non_cfg_entries}
 
             conn.set_entry(FEATURE, new_name, new_cfg)
-        
+
         if self.register_auto_ts(new_name, old_name):
             log.info(f'{new_name} entry is added to {AUTO_TS_FEATURE} table')
 
+        if 'syslog' in new_manifest['service'] and 'support-rate-limit' in new_manifest['service']['syslog'] and new_manifest['service']['syslog']['support-rate-limit']:
+            self.register_syslog_config(new_name, old_name)
+            log.info(f'{new_name} entry is added to {SYSLOG_CONFIG} table')
+
     def is_feature_enabled(self, name: str) -> bool:
         """ Returns whether the feature is current enabled
         or not. Accesses running CONFIG DB. If no running CONFIG_DB
@@ -178,10 +193,26 @@ def register_auto_ts(self, new_name, old_name=None):
                 current_cfg = conn.get_entry(AUTO_TS_FEATURE, old_name)
                 conn.set_entry(AUTO_TS_FEATURE, old_name, None)
                 new_cfg.update(current_cfg)
-            
+
             conn.set_entry(AUTO_TS_FEATURE, new_name, new_cfg)
         return True
 
+    def register_syslog_config(self, new_name, old_name=None):
+        """ Registers syslog configuration
+
+        Args:
+            new_name (str): new table name
+            old_name (str, optional): old table name. Defaults to None.
+        """
+        for conn in self._sonic_db.get_connectors():
+            new_cfg = copy.deepcopy(DEFAULT_SYSLOG_FEATURE_CONFIG)
+            if old_name:
+                current_cfg = conn.get_entry(SYSLOG_CONFIG, old_name)
+                conn.set_entry(SYSLOG_CONFIG, old_name, None)
+                new_cfg.update(current_cfg)
+
+            conn.set_entry(SYSLOG_CONFIG, new_name, new_cfg)
+
     @staticmethod
     def get_default_feature_entries(state=None, owner=None) -> Dict[str, str]:
         """ Get configurable feature table entries:
@@ -203,4 +234,5 @@ def get_non_configurable_feature_entries(manifest) -> Dict[str, str]:
             'has_global_scope': str(manifest['service']['host-service']),
             'has_timer': str(manifest['service']['delayed']),
             'check_up_status': str(manifest['service']['check_up_status']),
+            'support_syslog_rate_limit': str(manifest['service']['syslog']['support-rate-limit']),
         }

syslog_util/common.py

@@ -0,0 +1,76 @@
+import click
+
+
+FEATURE_TABLE = "FEATURE"
+SYSLOG_CONFIG_TABLE = 'SYSLOG_CONFIG'
+SYSLOG_CONFIG_GLOBAL_KEY = 'GLOBAL'
+SYSLOG_CONFIG_FEATURE_TABLE = 'SYSLOG_CONFIG_FEATURE'
+
+SYSLOG_RATE_LIMIT_INTERVAL = 'rate_limit_interval'
+SYSLOG_RATE_LIMIT_BURST = 'rate_limit_burst'
+SUPPORT_RATE_LIMIT = 'support_syslog_rate_limit'
+
+
+def rate_limit_validator(interval, burst):
+    """Validate input interval/burst
+
+    Args:
+        interval (int): Rate limit interval
+        burst (int): Rate limit burst
+    """
+    if interval is None and burst is None:
+        raise click.UsageError('Either interval or burst must be configured')
+
+
+def service_validator(feature_data, service_name):
+    """Validate input service name
+
+    Args:
+        feature_data (dict): feature entries of FEATURE table
+        service_name (str): service name
+    """
+    if service_name not in feature_data:
+        valid_service_names = ','.join(feature_data.keys())
+        raise click.ClickException(f'Invalid service name {service_name}, please choose from: {valid_service_names}')
+
+    service_data = feature_data[service_name]
+
+    support_rate_limit = service_data.get(SUPPORT_RATE_LIMIT, '').lower() == 'true'
+    if not support_rate_limit:
+        raise click.ClickException(f'Service {service_name} does not support syslog rate limit configuration')
+
+
+def save_rate_limit_to_db(db, service_name, interval, burst, log):
+    """Save rate limit configuration to DB
+
+    Args:
+        db (object): db object
+        service_name (str): service name. None means config for host.
+        interval (int): rate limit interval
+        burst (int): rate limit burst
+        log (obj): log object
+    """
+    if service_name is None:
+        service_name = 'host'
+        table = SYSLOG_CONFIG_TABLE
+        key = SYSLOG_CONFIG_GLOBAL_KEY
+    else:
+        table = SYSLOG_CONFIG_FEATURE_TABLE
+        key = service_name
+
+    if interval == 0 or burst == 0:
+        msg = f'Disable syslog rate limit for {service_name}'
+        click.echo(msg)
+        log.log_notice(msg)
+        interval = 0
+        burst = 0
+
+    data = {}
+    if interval is not None:
+        data[SYSLOG_RATE_LIMIT_INTERVAL] = interval
+    if burst is not None:
+        data[SYSLOG_RATE_LIMIT_BURST] = burst
+    db.cfgdb.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')}")
+