Add a command to update log level and refresh configuration (#3428)

* Add a command to update log level and refresh configuration

* Multi ASIC support

* Fix pre-commit issue

* Fix comment

Fix review comment

Author: Junchao-Mellanox

config/syslog.py

@@ -642,3 +642,57 @@ def disable_rate_limit_feature(db, service_name, namespace):
 
         if not failed:
             click.echo(f'Disabled syslog rate limit feature for {feature_name}')
+
+
+@syslog.command('level')
+@click.option("-i", "--identifier",
+              required=True,
+              help="Log identifier in DB for which loglevel is applied (provided with -l)")
+@click.option("-l", "--level",
+              required=True,
+              help="Loglevel value",
+              type=click.Choice(['DEBUG', 'INFO', 'NOTICE', 'WARN', 'ERROR']))
+@click.option("--container",
+              help="Container name to which the SIGHUP is sent (provided with --pid or --program)")
+@click.option("--program",
+              help="Program name to which the SIGHUP is sent (provided with --container)")
+@click.option("--pid",
+              help="Process ID to which the SIGHUP is sent (provided with --container if PID is from container)")
+@click.option('--namespace', '-n', 'namespace', default=None,
+              type=click.Choice(multi_asic_util.multi_asic_ns_choices()),
+              show_default=True, help='Namespace name')
+@clicommon.pass_db
+def level(db, identifier, level, container, program, pid, namespace):
+    """ Configure log level """
+    if program and not container:
+        raise click.UsageError('--program must be specified with --container')
+
+    if container and not program and not pid:
+        raise click.UsageError('--container must be specified with --pid or --program')
+
+    if not namespace:
+        cfg_db = db.cfgdb
+    else:
+        asic_id = multi_asic.get_asic_id_from_name(namespace)
+        container = f'{container}{asic_id}'
+        cfg_db = db.cfgdb_clients[namespace]
+
+    cfg_db.mod_entry('LOGGER', identifier, {'LOGLEVEL': level})
+    if not container and not program and not pid:
+        return
+
+    log_config = cfg_db.get_entry('LOGGER', identifier)
+    require_manual_refresh = log_config.get('require_manual_refresh')
+    if not require_manual_refresh:
+        return
+
+    if container:
+        if program:
+            command = ['docker', 'exec', '-i', container, 'supervisorctl', 'signal', 'HUP', program]
+        else:
+            command = ['docker', 'exec', '-i', container, 'kill', '-s', 'SIGHUP', pid]
+    else:
+        command = ['kill', '-s', 'SIGHUP', pid]
+    output, ret = clicommon.run_command(command, return_cmd=True)
+    if ret != 0:
+        raise click.ClickException(f'Failed: {output}')

doc/Command-Reference.md

@@ -10858,6 +10858,35 @@ This command is used to disable syslog rate limit feature.
   config syslog rate-limit-feature disable database -n asci0
   ```
 
+**config syslog level**
+
+This command is used to configure log level for a given log identifier.
+
+- Usage:
+  ```
+  config syslog level -i <log_identifier> -l <log_level> --container [<container_name>] --program [<program_name>]
+
+  config syslog level -i <log_identifier> -l <log_level> --container [<container_name>] --pid [<process_id>]
+
+  config syslog level -i <log_identifier> -l <log_level> ---pid [<process_id>]
+  ```
+
+- Example:
+
+  ```
+  # Update the log level without refresh the configuration
+  config syslog level -i xcvrd -l DEBUG
+
+  # Update the log level and send SIGHUP to xcvrd running in PMON
+  config syslog level -i xcvrd -l DEBUG --container pmon --program xcvrd
+
+  # Update the log level and send SIGHUP to PID 20 running in PMON
+  config syslog level -i xcvrd -l DEBUG --container pmon --pid 20
+
+  # Update the log level and send SIGHUP to PID 20 running in host
+  config syslog level -i xcvrd -l DEBUG --pid 20
+  ```
+
 Go Back To [Beginning of the document](#) or [Beginning of this section](#syslog)
 
 ## System State

tests/syslog_multi_asic_test.py

@@ -279,3 +279,19 @@ def test_disable_syslog_rate_limit_feature(self, setup_cmd_module):
             ['database', '-n', 'asic0']
         )
         assert result.exit_code == 0
+
+    @mock.patch('config.syslog.clicommon.run_command')
+    def test_config_log_level(self, mock_run, setup_cmd_module):
+        _, config = setup_cmd_module
+        db = Db()
+        runner = CliRunner()
+
+        mock_run.return_value = ('something', 0)
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'component', '-l', 'DEBUG', '-n', 'asic0'], obj=db
+        )
+        assert result.exit_code == 0
+        cfg_db = db.cfgdb_clients['asic0']
+        data = cfg_db.get_entry('LOGGER', 'component')
+        assert data.get('LOGLEVEL') == 'DEBUG'

tests/syslog_test.py

@@ -484,3 +484,73 @@ def side_effect(*args, **kwargs):
             config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db
         )
         assert result.exit_code == SUCCESS
+
+    @mock.patch('config.syslog.clicommon.run_command')
+    def test_config_log_level(self, mock_run):
+        db = Db()
+        db.cfgdb.set_entry('LOGGER', 'log1', {'require_manual_refresh': 'true'})
+
+        runner = CliRunner()
+
+        mock_run.return_value = ('something', 0)
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'component', '-l', 'DEBUG'], obj=db
+        )
+        assert result.exit_code == SUCCESS
+        data = db.cfgdb.get_entry('LOGGER', 'component')
+        assert data.get('LOGLEVEL') == 'DEBUG'
+
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'component', '-l', 'DEBUG', '--pid', '123'], obj=db
+        )
+        assert result.exit_code == SUCCESS
+
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'component', '-l', 'DEBUG', '--container', 'pmon', '--pid', '123'], obj=db
+        )
+        assert result.exit_code == SUCCESS
+
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'component', '-l', 'DEBUG', '--container', 'pmon', '--program', 'xcvrd'], obj=db
+        )
+        assert result.exit_code == SUCCESS
+
+    @mock.patch('config.syslog.clicommon.run_command')
+    def test_config_log_level_negative(self, mock_run):
+        db = Db()
+
+        runner = CliRunner()
+
+        mock_run.return_value = ('something', 0)
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'log1', '-l', 'DEBUG', '--container', 'pmon'], obj=db
+        )
+        assert result.exit_code != SUCCESS
+
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'log1', '-l', 'DEBUG', '--program', 'xcvrd'], obj=db
+        )
+        assert result.exit_code != SUCCESS
+
+        mock_run.reset_mock()
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'log1', '-l', 'DEBUG', '--container', 'swss', '--program', 'orchagent'], obj=db
+        )
+        assert result.exit_code == SUCCESS
+        # Verify it does not send signal to orchagent if require_manual_refresh is not true
+        assert mock_run.call_count == 0
+
+        mock_run.return_value = ('something', -1)
+        db.cfgdb.set_entry('LOGGER', 'log1', {'require_manual_refresh': 'true'})
+        result = runner.invoke(
+            config.config.commands["syslog"].commands["level"],
+            ['-i', 'log1', '-l', 'DEBUG', '--container', 'pmon', '--program', 'xcvrd'], obj=db
+        )
+        assert result.exit_code != SUCCESS