From a5294f58fcc5dc6ecd926ea00a725e7b1864d6c2 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Tue, 5 Dec 2023 10:08:00 +0200 Subject: [PATCH 1/6] Support disable/enable syslog rate limit feature --- config/syslog.py | 83 ++++++++++++++++++++++++++++++++++++++++++++ tests/syslog_test.py | 28 +++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/config/syslog.py b/config/syslog.py index 90fc52ec9d..dde69eb4e4 100644 --- a/config/syslog.py +++ b/config/syslog.py @@ -471,3 +471,86 @@ def rate_limit_container(db, service_name, 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) + + +@syslog.group( + name="rate-limit-feature", + cls=clicommon.AliasedGroup +) +def rate_limit_feature(): + """ Configure syslog rate limit feature """ + pass + + +@rate_limit_feature.command("enable") +@clicommon.pass_db +def enable_rate_limit_feature(db): + """ Enable syslog rate limit feature """ + feature_data = db.cfgdb.get_table(syslog_common.FEATURE_TABLE) + for feature_name in feature_data.keys(): + 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) + if not output: + click.echo(f'{feature_name} is not running, ignoring...') + continue + + output, _ = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'status', 'containercfgd'], + ignore_error=True, return_cmd=True) + if 'no such process' not in output: + click.echo(f'Syslog rate limit feature is already enabled in {feature_name}, ignoring...') + continue + + output, ret = clicommon.run_command(['docker', 'cp', '/usr/share/sonic/templates/containercfgd.conf', f'{feature_name}:/etc/supervisor/conf.d/'], return_cmd=True) + if ret != 0: + click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'reread'], return_cmd=True) + if ret != 0: + click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'update'], return_cmd=True) + if ret != 0: + click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'start', 'containercfgd'], return_cmd=True) + if ret != 0: + click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') + continue + click.echo(f'Enabled syslog rate limit feature for {feature_name}') + + +@rate_limit_feature.command("disable") +@clicommon.pass_db +def disable_rate_limit_feature(db): + """ Disable syslog rate limit feature """ + feature_data = db.cfgdb.get_table(syslog_common.FEATURE_TABLE) + for feature_name in feature_data.keys(): + 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) + if not output: + click.echo(f'{feature_name} is not running, ignoring...') + continue + + output, _ = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'status', 'containercfgd'], + ignore_error=True, return_cmd=True) + if 'no such process' in output: + click.echo(f'Syslog rate limit feature is already disabled in {feature_name}, ignoring...') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'stop', 'containercfgd'], return_cmd=True) + if ret != 0: + click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'rm', '-f', '/etc/supervisor/conf.d/containercfgd.conf'], return_cmd=True) + if ret != 0: + click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'reread'], return_cmd=True) + if ret != 0: + click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') + continue + output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'update'], return_cmd=True) + if ret != 0: + click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') + continue + click.echo(f'Disabled syslog rate limit feature for {feature_name}') + diff --git a/tests/syslog_test.py b/tests/syslog_test.py index 354100a508..f8b2379585 100644 --- a/tests/syslog_test.py +++ b/tests/syslog_test.py @@ -399,3 +399,31 @@ def test_show_syslog_rate_limit_container_negative(self, subcommand): logger.debug("\n" + result.output) logger.debug(result.exit_code) assert result.exit_code != SUCCESS + + @mock.patch('config.syslog.clicommon.run_command') + def test_enable_syslog_rate_limit_feature(self, mock_run): + db = Db() + db.cfgdb.set_entry(FEATURE_TABLE, 'bgp', {SUPPORT_RATE_LIMIT: 'true', + 'state': 'enabled'}) + + runner = CliRunner() + + mock_run.return_value = ('', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"] + ) + assert result.exit_code == SUCCESS + + @mock.patch('config.syslog.clicommon.run_command') + def test_disable_syslog_rate_limit_feature(self, mock_run): + db = Db() + db.cfgdb.set_entry(FEATURE_TABLE, 'bgp', {SUPPORT_RATE_LIMIT: 'true', + 'state': 'enabled'}) + + runner = CliRunner() + + mock_run.return_value = ('', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"] + ) + assert result.exit_code == SUCCESS From e334ee7e538d770e5d26aa27c6e55127c2fcd83e Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Thu, 7 Dec 2023 08:03:55 +0200 Subject: [PATCH 2/6] Update command ref --- doc/Command-Reference.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index d693c49d80..22739976be 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -10225,6 +10225,33 @@ This command is used to configure syslog rate limit for containers. admin@sonic:~$ sudo config syslog rate-limit-container bgp --interval 300 --burst 20000 ``` +**config syslog rate-limit-feature enable** + +This command is used to enable syslog rate limit feature. + +- Usage: + ``` + config syslog rate-limit-feature enable + ``` + +- Example: + ``` + admin@sonic:~$ sudo config syslog rate-limit-feature enable + ``` + +**config syslog rate-limit-feature disable** + +This command is used to disable syslog rate limit feature. + +- Usage: + ``` + config syslog rate-limit-feature disable + ``` + +- Example: + ``` + admin@sonic:~$ sudo config syslog rate-limit-feature disable + ``` Go Back To [Beginning of the document](#) or [Beginning of this section](#syslog) From d2df20f343c68a4232d4642c0cc41cb4d2db7106 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Fri, 8 Dec 2023 07:35:15 +0200 Subject: [PATCH 3/6] Increase coverage --- tests/syslog_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/syslog_test.py b/tests/syslog_test.py index f8b2379585..6cacdafeb7 100644 --- a/tests/syslog_test.py +++ b/tests/syslog_test.py @@ -408,7 +408,7 @@ def test_enable_syslog_rate_limit_feature(self, mock_run): runner = CliRunner() - mock_run.return_value = ('', 0) + mock_run.return_value = ('something', 0) result = runner.invoke( config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"] ) @@ -422,7 +422,7 @@ def test_disable_syslog_rate_limit_feature(self, mock_run): runner = CliRunner() - mock_run.return_value = ('', 0) + mock_run.return_value = ('something', 0) result = runner.invoke( config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"] ) From 84a8b37374250a0650df01fc40e3b0bddecf124d Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Fri, 8 Dec 2023 09:16:27 +0200 Subject: [PATCH 4/6] Fix ut issue --- tests/syslog_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/syslog_test.py b/tests/syslog_test.py index 6cacdafeb7..c164695739 100644 --- a/tests/syslog_test.py +++ b/tests/syslog_test.py @@ -410,7 +410,7 @@ def test_enable_syslog_rate_limit_feature(self, mock_run): mock_run.return_value = ('something', 0) result = runner.invoke( - config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"] + config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"], obj=db ) assert result.exit_code == SUCCESS @@ -424,6 +424,6 @@ def test_disable_syslog_rate_limit_feature(self, mock_run): mock_run.return_value = ('something', 0) result = runner.invoke( - config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"] + config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db ) assert result.exit_code == SUCCESS From 1e609ed01737d9927327ae41b0b9e5ae5d0609dd Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Sat, 9 Dec 2023 14:44:41 +0200 Subject: [PATCH 5/6] Increase UT coverage --- tests/syslog_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/syslog_test.py b/tests/syslog_test.py index c164695739..486b876c69 100644 --- a/tests/syslog_test.py +++ b/tests/syslog_test.py @@ -408,7 +408,7 @@ def test_enable_syslog_rate_limit_feature(self, mock_run): runner = CliRunner() - mock_run.return_value = ('something', 0) + mock_run.return_value = ('no such process', 0) result = runner.invoke( config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"], obj=db ) From d9ec4e908222455c6537b69dfdf7f22f1b16ae16 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox Date: Sat, 9 Dec 2023 16:37:04 +0200 Subject: [PATCH 6/6] Fix UT --- config/syslog.py | 68 ++++++++++++++++++++++---------------------- tests/syslog_test.py | 58 +++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 34 deletions(-) diff --git a/config/syslog.py b/config/syslog.py index dde69eb4e4..7533a7f71f 100644 --- a/config/syslog.py +++ b/config/syslog.py @@ -500,23 +500,23 @@ def enable_rate_limit_feature(db): click.echo(f'Syslog rate limit feature is already enabled in {feature_name}, ignoring...') continue - output, ret = clicommon.run_command(['docker', 'cp', '/usr/share/sonic/templates/containercfgd.conf', f'{feature_name}:/etc/supervisor/conf.d/'], return_cmd=True) - if ret != 0: - click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') - continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'reread'], return_cmd=True) - if ret != 0: - click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') - continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'update'], return_cmd=True) - if ret != 0: - click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') - continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'start', 'containercfgd'], return_cmd=True) - if ret != 0: - click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') - continue - click.echo(f'Enabled syslog rate limit feature for {feature_name}') + commands = [ + ['docker', 'cp', '/usr/share/sonic/templates/containercfgd.conf', f'{feature_name}:/etc/supervisor/conf.d/'], + ['docker', 'exec', '-i', feature_name, 'supervisorctl', 'reread'], + ['docker', 'exec', '-i', feature_name, 'supervisorctl', 'update'], + ['docker', 'exec', '-i', feature_name, 'supervisorctl', 'start', 'containercfgd'] + ] + + failed = False + for command in commands: + output, ret = clicommon.run_command(command, return_cmd=True) + if ret != 0: + failed = True + click.echo(f'Enable syslog rate limit feature for {feature_name} failed - {output}') + break + + if not failed: + click.echo(f'Enabled syslog rate limit feature for {feature_name}') @rate_limit_feature.command("disable") @@ -536,21 +536,21 @@ def disable_rate_limit_feature(db): if 'no such process' in output: click.echo(f'Syslog rate limit feature is already disabled in {feature_name}, ignoring...') continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'stop', 'containercfgd'], return_cmd=True) - if ret != 0: - click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') - continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'rm', '-f', '/etc/supervisor/conf.d/containercfgd.conf'], return_cmd=True) - if ret != 0: - click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') - continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'reread'], return_cmd=True) - if ret != 0: - click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') - continue - output, ret = clicommon.run_command(['docker', 'exec', '-i', feature_name, 'supervisorctl', 'update'], return_cmd=True) - if ret != 0: - click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') - continue - click.echo(f'Disabled syslog rate limit feature for {feature_name}') + + commands = [ + ['docker', 'exec', '-i', feature_name, 'supervisorctl', 'stop', 'containercfgd'], + ['docker', 'exec', '-i', feature_name, 'rm', '-f', '/etc/supervisor/conf.d/containercfgd.conf'], + ['docker', 'exec', '-i', feature_name, 'supervisorctl', 'reread'], + ['docker', 'exec', '-i', feature_name, 'supervisorctl', 'update'] + ] + failed = False + for command in commands: + output, ret = clicommon.run_command(command, return_cmd=True) + if ret != 0: + failed = True + click.echo(f'Disable syslog rate limit feature for {feature_name} failed - {output}') + break + + if not failed: + click.echo(f'Disabled syslog rate limit feature for {feature_name}') diff --git a/tests/syslog_test.py b/tests/syslog_test.py index 486b876c69..44915b6d36 100644 --- a/tests/syslog_test.py +++ b/tests/syslog_test.py @@ -414,6 +414,35 @@ def test_enable_syslog_rate_limit_feature(self, mock_run): ) assert result.exit_code == SUCCESS + # container not run + mock_run.return_value = ('', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"], obj=db + ) + assert result.exit_code == SUCCESS + + # process already running + mock_run.return_value = ('something', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"], obj=db + ) + assert result.exit_code == SUCCESS + + # one command fail + def side_effect(*args, **kwargs): + side_effect.call_count += 1 + if side_effect.call_count <= 2: + return 'no such process', 0 + else: + return '', -1 + side_effect.call_count = 0 + mock_run.side_effect = side_effect + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["enable"], obj=db + ) + assert result.exit_code == SUCCESS + + @mock.patch('config.syslog.clicommon.run_command') def test_disable_syslog_rate_limit_feature(self, mock_run): db = Db() @@ -427,3 +456,32 @@ def test_disable_syslog_rate_limit_feature(self, mock_run): config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db ) assert result.exit_code == SUCCESS + + # container not run + mock_run.return_value = ('', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db + ) + assert result.exit_code == SUCCESS + + # process already stopped + mock_run.return_value = ('no such process', 0) + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db + ) + assert result.exit_code == SUCCESS + + # one command fail + def side_effect(*args, **kwargs): + side_effect.call_count += 1 + if side_effect.call_count <= 2: + return 'something', 0 + else: + return '', -1 + side_effect.call_count = 0 + mock_run.side_effect = side_effect + result = runner.invoke( + config.config.commands["syslog"].commands["rate-limit-feature"].commands["disable"], obj=db + ) + assert result.exit_code == SUCCESS +