From 99795c64d48e8efd5ab0eb6792a496fe7b477841 Mon Sep 17 00:00:00 2001 From: shi-su Date: Thu, 3 Sep 2020 23:49:58 +0000 Subject: [PATCH 1/4] Add CLI for configuring synchronous mode --- scripts/sonic-sync-mode | 33 +++++++++++++++++++++++++++++++++ setup.py | 1 + 2 files changed, 34 insertions(+) create mode 100755 scripts/sonic-sync-mode diff --git a/scripts/sonic-sync-mode b/scripts/sonic-sync-mode new file mode 100755 index 0000000000..4aa22192c9 --- /dev/null +++ b/scripts/sonic-sync-mode @@ -0,0 +1,33 @@ +#!/usr/bin/python +import argparse +from swsscommon import swsscommon + +def main(): + parser=argparse.ArgumentParser(description= + 'Configure enable or disable synchronous mode between orchagent and syncd', + formatter_class=argparse.RawTextHelpFormatter, + epilog= +''' +sudo needed for configuring synchronous mode +swss restart required to apply the configuration +Options to restart swss and apply the configuration: + 1. config save + config reload -y + 2. systemctl restart swss +''') + parser.add_argument('mode', metavar='[enable|disable]', type=str, + help='Configure enable or disable synchronous mode') + args = parser.parse_args() + + if args.mode == 'enable' or args.mode == 'disable': + config_db = swsscommon.DBConnector('CONFIG_DB', 0, False) + table = swsscommon.Table(config_db, 'DEVICE_METADATA') + fvs = swsscommon.FieldValuePairs([('synchronous_mode', args.mode)]) + table.set('localhost', fvs) + print('Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration'%(args.mode)) + else: + print('Invalid argument: expect either enable or disable') + parser.print_help() + +if __name__ == '__main__': + main() diff --git a/setup.py b/setup.py index e672ef7e0d..fc7453d2b8 100644 --- a/setup.py +++ b/setup.py @@ -110,6 +110,7 @@ 'scripts/route_check.py', 'scripts/route_check_test.sh', 'scripts/sfpshow', + 'scripts/sonic-sync-mode', 'scripts/syseeprom-to-json', 'scripts/tempershow', 'scripts/update_json.py', From c648458a790e906d05892fc349f2a944dfe2db28 Mon Sep 17 00:00:00 2001 From: shi-su Date: Tue, 8 Sep 2020 16:20:48 +0000 Subject: [PATCH 2/4] Integrate switch for sync mode as a config CLI --- config/main.py | 25 +++++++++++++++++++++++++ scripts/sonic-sync-mode | 33 --------------------------------- setup.py | 1 - 3 files changed, 25 insertions(+), 34 deletions(-) delete mode 100755 scripts/sonic-sync-mode diff --git a/config/main.py b/config/main.py index a1b6c99af9..c98647c8da 100755 --- a/config/main.py +++ b/config/main.py @@ -1289,6 +1289,31 @@ def hostname(new_hostname): raise click.echo("Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.") +# +# 'synchronous_mode' command ('config synchronous_mode ...') +# +@config.command('synchronous_mode') +@click.argument('sync_mode', metavar='', required=True) +def synchronous_mode(sync_mode): + """ Enable or disable synchronous mode between orchagent and syncd \n + swss restart required to apply the configuration \n + Options to restart swss and apply the configuration: \n + 1. Keep and apply configuration after config reload \n + config save -y \n + config reload -y \n + 2. Discard configuration after config reload \n + systemctl restart swss + """ + + if sync_mode == 'enable' or sync_mode == 'disable': + config_db = ConfigDBConnector() + config_db.connect() + config_db.mod_entry('DEVICE_METADATA' , 'localhost', {"synchronous_mode" : sync_mode}) + click.echo("Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration" % sync_mode) + else: + click.echo("Error: Invalid argument %s, expect either enable or disable" % sync_mode) + sys.exit(1) + # # 'portchannel' group ('config portchannel ...') # diff --git a/scripts/sonic-sync-mode b/scripts/sonic-sync-mode deleted file mode 100755 index 4aa22192c9..0000000000 --- a/scripts/sonic-sync-mode +++ /dev/null @@ -1,33 +0,0 @@ -#!/usr/bin/python -import argparse -from swsscommon import swsscommon - -def main(): - parser=argparse.ArgumentParser(description= - 'Configure enable or disable synchronous mode between orchagent and syncd', - formatter_class=argparse.RawTextHelpFormatter, - epilog= -''' -sudo needed for configuring synchronous mode -swss restart required to apply the configuration -Options to restart swss and apply the configuration: - 1. config save - config reload -y - 2. systemctl restart swss -''') - parser.add_argument('mode', metavar='[enable|disable]', type=str, - help='Configure enable or disable synchronous mode') - args = parser.parse_args() - - if args.mode == 'enable' or args.mode == 'disable': - config_db = swsscommon.DBConnector('CONFIG_DB', 0, False) - table = swsscommon.Table(config_db, 'DEVICE_METADATA') - fvs = swsscommon.FieldValuePairs([('synchronous_mode', args.mode)]) - table.set('localhost', fvs) - print('Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration'%(args.mode)) - else: - print('Invalid argument: expect either enable or disable') - parser.print_help() - -if __name__ == '__main__': - main() diff --git a/setup.py b/setup.py index fc7453d2b8..e672ef7e0d 100644 --- a/setup.py +++ b/setup.py @@ -110,7 +110,6 @@ 'scripts/route_check.py', 'scripts/route_check_test.sh', 'scripts/sfpshow', - 'scripts/sonic-sync-mode', 'scripts/syseeprom-to-json', 'scripts/tempershow', 'scripts/update_json.py', From 9f9b284ef1c26e316c3e5467311818b202529a4b Mon Sep 17 00:00:00 2001 From: shi-su Date: Wed, 9 Sep 2020 02:45:47 +0000 Subject: [PATCH 3/4] Address review comments --- config/main.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/config/main.py b/config/main.py index c98647c8da..64b421939e 100755 --- a/config/main.py +++ b/config/main.py @@ -1298,11 +1298,9 @@ def synchronous_mode(sync_mode): """ Enable or disable synchronous mode between orchagent and syncd \n swss restart required to apply the configuration \n Options to restart swss and apply the configuration: \n - 1. Keep and apply configuration after config reload \n - config save -y \n - config reload -y \n - 2. Discard configuration after config reload \n - systemctl restart swss + 1. config save -y \n + config reload -y \n + 2. systemctl restart swss """ if sync_mode == 'enable' or sync_mode == 'disable': From 7ec2adf637bb8579dbe2c299f3f4f9b358c3a48d Mon Sep 17 00:00:00 2001 From: shi-su Date: Fri, 11 Sep 2020 22:46:46 +0000 Subject: [PATCH 4/4] Add unit test and address review comments --- config/main.py | 8 +++++--- tests/synchronous_mode_test.py | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 tests/synchronous_mode_test.py diff --git a/config/main.py b/config/main.py index 64b421939e..a3b1212e29 100755 --- a/config/main.py +++ b/config/main.py @@ -1307,10 +1307,12 @@ def synchronous_mode(sync_mode): config_db = ConfigDBConnector() config_db.connect() config_db.mod_entry('DEVICE_METADATA' , 'localhost', {"synchronous_mode" : sync_mode}) - click.echo("Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration" % sync_mode) + click.echo("""Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration: \n + Option 1. config save -y \n + config reload -y \n + Option 2. systemctl restart swss""" % sync_mode) else: - click.echo("Error: Invalid argument %s, expect either enable or disable" % sync_mode) - sys.exit(1) + raise click.BadParameter("Error: Invalid argument %s, expect either enable or disable" % sync_mode) # # 'portchannel' group ('config portchannel ...') diff --git a/tests/synchronous_mode_test.py b/tests/synchronous_mode_test.py new file mode 100644 index 0000000000..05f341710c --- /dev/null +++ b/tests/synchronous_mode_test.py @@ -0,0 +1,37 @@ +from click.testing import CliRunner +import config.main as config + +class TestSynchronousMode(object): + @classmethod + def setup_class(cls): + print("SETUP") + + def __check_result(self, result_msg, mode): + if mode == "enable" or mode == "disable": + expected_msg = """Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration: \n + Option 1. config save -y \n + config reload -y \n + Option 2. systemctl restart swss""" % mode + else: + expected_msg = "Error: Invalid argument %s, expect either enable or disable" % mode + + return expected_msg in result_msg + + + def test_synchronous_mode(self): + runner = CliRunner() + + result = runner.invoke(config.config.commands["synchronous_mode"], ["enable"]) + print(result.output) + assert result.exit_code == 0 + assert self.__check_result(result.output, "enable") + + result = runner.invoke(config.config.commands["synchronous_mode"], ["disable"]) + print(result.output) + assert result.exit_code == 0 + assert self.__check_result(result.output, "disable") + + result = runner.invoke(config.config.commands["synchronous_mode"], ["invalid-input"]) + print(result.output) + assert result.exit_code != 0 + assert self.__check_result(result.output, "invalid-input")