From f657d5f52579f2989dfdbf3236137c8f0eb79d79 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Fri, 23 Feb 2024 05:57:53 +0800 Subject: [PATCH 1/2] [config] Check golden config exist early if flag is set (#3169) ### What I did Fix https://github.com/sonic-net/sonic-utilities/issues/3164 Check Golden Config earlier before service is down. #### How I did it Move the check at the begining #### How to verify it Unit test --- config/main.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/config/main.py b/config/main.py index 401359b29c..a8e04dfe6e 100644 --- a/config/main.py +++ b/config/main.py @@ -1709,6 +1709,15 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, argv_str = ' '.join(['config', *sys.argv[1:]]) log.log_notice(f"'load_minigraph' executing with command: {argv_str}") + # check if golden_config exists if override flag is set + if override_config: + if golden_config_path is None: + golden_config_path = DEFAULT_GOLDEN_CONFIG_DB_FILE + if not os.path.isfile(golden_config_path): + click.secho("Cannot find '{}'!".format(golden_config_path), + fg='magenta') + raise click.Abort() + #Stop services before config push if not no_service_restart: log.log_notice("'load_minigraph' stopping services...") @@ -1780,12 +1789,6 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, # Load golden_config_db.json if override_config: - if golden_config_path is None: - golden_config_path = DEFAULT_GOLDEN_CONFIG_DB_FILE - 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) # Invoke platform script if available before starting the services From 47a27858f1bc1c050b84055531ab9f66031c6e98 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Tue, 18 Jun 2024 12:37:49 +0800 Subject: [PATCH 2/2] [config] no op if Golden Config is invalid (#3367) ADO: 27941719 What I did Improve Golden Config workflow and make sure no op if invalid config detected How I did it Add table dependency check right after Golden Config path is enabled How to verify it Unit test --- config/main.py | 8 +++++++ tests/config_test.py | 50 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/config/main.py b/config/main.py index a8e04dfe6e..4417617322 100644 --- a/config/main.py +++ b/config/main.py @@ -1718,6 +1718,14 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, fg='magenta') raise click.Abort() + # Dependency check golden config json + config_to_check = read_json_file(golden_config_path) + if multi_asic.is_multi_asic(): + host_config = config_to_check.get('localhost', {}) + else: + host_config = config_to_check + table_hard_dependency_check(host_config) + #Stop services before config push if not no_service_restart: log.log_notice("'load_minigraph' stopping services...") diff --git a/tests/config_test.py b/tests/config_test.py index cc0ac22e98..0d8fe44e35 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -519,8 +519,13 @@ def is_file_side_effect(filename): def test_load_minigraph_with_specified_golden_config_path(self, get_cmd_module): def is_file_side_effect(filename): return True if 'golden_config' in filename else False + + def read_json_file_side_effect(filename): + return {} + 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)): + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): (config, show) = get_cmd_module runner = CliRunner() result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "--golden_config_path", "golden_config.json", "-y"]) @@ -531,14 +536,48 @@ def is_file_side_effect(filename): def test_load_minigraph_with_default_golden_config_path(self, get_cmd_module): def is_file_side_effect(filename): return True if 'golden_config' in filename else False + + def read_json_file_side_effect(filename): + return {} + 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)): + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): (config, show) = get_cmd_module runner = CliRunner() result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"]) assert result.exit_code == 0 assert "config override-config-table /etc/sonic/golden_config_db.json" in result.output + @mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', + mock.MagicMock(return_value=("dummy_path", None))) + def test_load_minigraph_hard_dependency_check(self, get_cmd_module): + def is_file_side_effect(filename): + return True if 'golden_config' in filename else False + + def read_json_file_side_effect(filename): + return { + "AAA": { + "authentication": { + "login": "tacacs+" + } + }, + "TACPLUS": { + "global": { + "passkey": "" + } + } + } + + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)), \ + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): + (config, _) = get_cmd_module + runner = CliRunner() + result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"]) + assert result.exit_code != 0 + assert "Authentication with 'tacacs+' is not allowed when passkey not exits." in result.output + @mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', mock.MagicMock(return_value=("dummy_path", None))) 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: @@ -556,7 +595,12 @@ def test_load_minigraph_with_traffic_shift_away_with_golden_config(self, get_cmd with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command: def is_file_side_effect(filename): return True if 'golden_config' in filename else False - with mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)): + + def read_json_file_side_effect(filename): + return {} + + with mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): (config, show) = get_cmd_module db = Db() golden_config = {}