From 318fcc8b0ac3bd0ea8154b2ebd61f98a1f8b64a2 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Mon, 5 Sep 2022 03:10:12 +0000 Subject: [PATCH 1/4] [minigraph] add option to specify golden path in load_minigraph --- config/main.py | 14 +++++++++++--- tests/config_test.py | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/config/main.py b/config/main.py index 53180cf519..5e9f30e04f 100644 --- a/config/main.py +++ b/config/main.py @@ -1718,8 +1718,9 @@ def load_mgmt_config(filename): expose_value=False, prompt='Reload config from minigraph?') @click.option('-n', '--no_service_restart', default=False, is_flag=True, help='Do not restart docker services') @click.option('-t', '--traffic_shift_away', default=False, is_flag=True, help='Keep device in maintenance with TSA') +@click.option('-p', '--golden_config_path', help='specify Golden Config path') @clicommon.pass_db -def load_minigraph(db, no_service_restart, traffic_shift_away): +def load_minigraph(db, no_service_restart, traffic_shift_away, golden_config_path): """Reconfigure based on minigraph.""" log.log_info("'load_minigraph' executing...") @@ -1797,8 +1798,15 @@ def load_minigraph(db, no_service_restart, traffic_shift_away): click.secho("[WARNING] Golden configuration may override Traffic-shift-away state. Please execute TSC to check the current System mode") # Load golden_config_db.json - if os.path.isfile(DEFAULT_GOLDEN_CONFIG_DB_FILE): - override_config_by(DEFAULT_GOLDEN_CONFIG_DB_FILE) + if golden_config_path: + if not os.path.isfile(golden_config_path): + click.secho("Cannot find '{}'!".format(golden_config_path), + fg='magenta') + raise click.Abort() + override_config_by(golden_config_path) + else: + if os.path.isfile(DEFAULT_GOLDEN_CONFIG_DB_FILE): + override_config_by(DEFAULT_GOLDEN_CONFIG_DB_FILE) # We first run "systemctl reset-failed" to remove the "failed" # status from all services before we attempt to restart them diff --git a/tests/config_test.py b/tests/config_test.py index a9f4982548..80e41eb9c7 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -430,6 +430,20 @@ def is_file_side_effect(filename): assert result.exit_code == 0 assert expected_output in result.output + def test_load_minigraph_with_golden_config_path(self, get_cmd_module): + def is_file_side_effect(filename): + return True if 'golden_config' in filename else False + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \ + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)): + (config, show) = get_cmd_module + runner = CliRunner() + result = runner.invoke(config.config.commands["load_minigraph"], ["-p", "golden_config.json", "-y"]) + print(result.exit_code) + print(result.output) + traceback.print_tb(result.exc_info[2]) + assert result.exit_code == 0 + assert "config override-config-table golden_config.json" in result.output + def test_load_minigraph_with_traffic_shift_away(self, get_cmd_module): with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command: (config, show) = get_cmd_module From 416c29a235db68310e207006d4fc5e83a0e52daf Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Mon, 19 Sep 2022 05:21:42 +0000 Subject: [PATCH 2/4] coverage for non exist golden path --- tests/config_test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/config_test.py b/tests/config_test.py index 80e41eb9c7..1374181f8b 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -430,6 +430,17 @@ def is_file_side_effect(filename): assert result.exit_code == 0 assert expected_output in result.output + def test_load_minigraph_with_non_exist_golden_config_path(self, get_cmd_module): + def is_file_side_effect(filename): + return True if 'golden_config' in filename else False + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \ + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)): + (config, show) = get_cmd_module + runner = CliRunner() + result = runner.invoke(config.config.commands["load_minigraph"], ["-p", "non_exist.json", "-y"]) + assert result.exit_code != 0 + assert "Cannot find 'non_exist.json'" in result.output + def test_load_minigraph_with_golden_config_path(self, get_cmd_module): def is_file_side_effect(filename): return True if 'golden_config' in filename else False From 7674171640064f7367fda3197b25b173535c887c Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Tue, 20 Sep 2022 05:09:52 +0000 Subject: [PATCH 3/4] tune up TSA --- config/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/main.py b/config/main.py index 5e9f30e04f..c3de2d5fef 100644 --- a/config/main.py +++ b/config/main.py @@ -1793,7 +1793,7 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, golden_config_pat # Keep device isolated with TSA if traffic_shift_away: clicommon.run_command("TSA", display_cmd=True) - if os.path.isfile(DEFAULT_GOLDEN_CONFIG_DB_FILE): + if golden_config_path or not golden_config_path and os.path.isfile(DEFAULT_GOLDEN_CONFIG_DB_FILE): log.log_warning("Golden configuration may override System Maintenance state. Please execute TSC to check the current System mode") click.secho("[WARNING] Golden configuration may override Traffic-shift-away state. Please execute TSC to check the current System mode") From 1a44732d0f6c4ee6367586e7d1bdb30706b5b7e0 Mon Sep 17 00:00:00 2001 From: Jingwen Xie Date: Tue, 20 Sep 2022 05:17:33 +0000 Subject: [PATCH 4/4] Improve help text --- config/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/main.py b/config/main.py index c3de2d5fef..c92a56a378 100644 --- a/config/main.py +++ b/config/main.py @@ -1718,7 +1718,7 @@ def load_mgmt_config(filename): expose_value=False, prompt='Reload config from minigraph?') @click.option('-n', '--no_service_restart', default=False, is_flag=True, help='Do not restart docker services') @click.option('-t', '--traffic_shift_away', default=False, is_flag=True, help='Keep device in maintenance with TSA') -@click.option('-p', '--golden_config_path', help='specify Golden Config path') +@click.option('-p', '--golden_config_path', help='The path of golden config file') @clicommon.pass_db def load_minigraph(db, no_service_restart, traffic_shift_away, golden_config_path): """Reconfigure based on minigraph."""