From eb23c84880a51e301530e43af07e86a6a9043841 Mon Sep 17 00:00:00 2001 From: Yizhen Zhang <98562104+yizhenzha@users.noreply.github.com> Date: Fri, 31 Jul 2026 15:53:05 -0700 Subject: [PATCH] Abort config reload if warm-boot or fast-reboot is still in progress (#4721) config reload can inherit a stale WARM_RESTART_ENABLE_TABLE|system=true flag left by an unfinished warm-boot or fast-reboot finalization (e.g. BGP reconcile still pending). syncd.sh::check_warm_boot() reads this flag and issues `syncd_request_shutdown --warm` without a preceding `--pre` (pre-shutdown). - Added a boot-in-progress guard in config/main.py (reload()) that uses swsscommon.RestartWaiter to detect if a warm-boot or fast-reboot is still finalizing; if so, config reload aborts with a clear message before syncd is stopped. - isFastBootInProgress is checked first: fast-reboot sets both FAST_RESTART_ENABLE_TABLE and WARM_RESTART_ENABLE_TABLE, so checking fast first ensures the correct label. - Guard is skipped when --no-service-restart (-n) is passed since no syncd teardown occurs in that path. - Added four inline tests in tests/config_test.py. (cherry picked from commit cd4953c3bcb92b56a2d0c288e0cd4cbb79b98f9d) Signed-off-by: Yizhen Zhang --- config/main.py | 18 ++++++++++ tests/config_test.py | 84 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/config/main.py b/config/main.py index 45dce1b13..ed76e947c 100644 --- a/config/main.py +++ b/config/main.py @@ -2195,6 +2195,24 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form config_file_yang_validation(filename) #Stop services before config push + if not no_service_restart: + try: + _db = swsscommon.DBConnector('STATE_DB', 0) + boot_type = None + # fast-reboot sets both FAST_RESTART_ENABLE_TABLE and WARM_RESTART_ENABLE_TABLE; + # check fast first so the label is correct when both flags are set. + if swsscommon.RestartWaiter.isFastBootInProgress(_db): + boot_type = 'fast-reboot' + elif swsscommon.RestartWaiter.isWarmBootInProgress(_db): + boot_type = 'warm-boot' + if boot_type: + click.echo(f"A {boot_type} is still in progress. Please wait for finalization to complete.") + sys.exit(CONFIG_RELOAD_NOT_READY) + except SystemExit: + raise + except Exception: + pass + if not no_service_restart: log.log_notice("'reload' stopping services...") _stop_services() diff --git a/tests/config_test.py b/tests/config_test.py index c0e29c70a..19be461e6 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -754,6 +754,90 @@ def test_config_reload(self, get_cmd_module, setup_single_broadcom_asic): assert "\n".join([line.rstrip() for line in result.output.split('\n')][:2]) == \ reload_config_with_sys_info_command_output.format(config.SYSTEM_RELOAD_LOCK) + def test_config_reload_blocked_during_warm_boot(self, get_cmd_module, setup_single_broadcom_asic): + """config reload must abort when a warm-boot is in progress (Redmine #5154490).""" + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db.json") + + mock_swsscommon = mock.MagicMock() + mock_swsscommon.RestartWaiter.isWarmBootInProgress.return_value = True + mock_swsscommon.RestartWaiter.isFastBootInProgress.return_value = False + + with mock.patch.object(config, "DEFAULT_CONFIG_DB_FILE", jsonfile_config), \ + mock.patch('config.main.swsscommon', mock_swsscommon), \ + mock.patch('config.main._is_system_starting', return_value=False), \ + mock.patch('config.main._swss_ready', return_value=True), \ + mock.patch.object(config, 'config_file_yang_validation'): + result = CliRunner().invoke(config.config.commands["reload"], ["-y"]) + assert result.exit_code != 0 + assert "warm-boot" in result.output + assert "still in progress" in result.output + + def test_config_reload_blocked_during_fast_boot(self, get_cmd_module, setup_single_broadcom_asic): + """config reload must abort when a fast-reboot is in progress. + fast-reboot sets both FAST_RESTART_ENABLE_TABLE and WARM_RESTART_ENABLE_TABLE; + isFastBootInProgress is checked first so the label is correct.""" + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db.json") + + mock_swsscommon = mock.MagicMock() + mock_swsscommon.RestartWaiter.isWarmBootInProgress.return_value = True + mock_swsscommon.RestartWaiter.isFastBootInProgress.return_value = True + + with mock.patch.object(config, "DEFAULT_CONFIG_DB_FILE", jsonfile_config), \ + mock.patch('config.main.swsscommon', mock_swsscommon), \ + mock.patch('config.main._is_system_starting', return_value=False), \ + mock.patch('config.main._swss_ready', return_value=True), \ + mock.patch.object(config, 'config_file_yang_validation'): + result = CliRunner().invoke(config.config.commands["reload"], ["-y"]) + assert result.exit_code != 0 + assert "fast-reboot" in result.output + assert "still in progress" in result.output + + def test_config_reload_not_blocked_when_no_boot_in_progress(self, get_cmd_module, setup_single_broadcom_asic): + """Guard runs but does not block when no boot is in progress; both checks are invoked.""" + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db.json") + + mock_swsscommon = mock.MagicMock() + mock_swsscommon.RestartWaiter.isWarmBootInProgress.return_value = False + mock_swsscommon.RestartWaiter.isFastBootInProgress.return_value = False + + with mock.patch.object(config, "DEFAULT_CONFIG_DB_FILE", jsonfile_config), \ + mock.patch('config.main.swsscommon', mock_swsscommon), \ + mock.patch.object(config, 'config_file_yang_validation'), \ + mock.patch('config.main._is_system_starting', return_value=False), \ + mock.patch('config.main._swss_ready', return_value=True), \ + mock.patch('config.main._stop_services'), \ + mock.patch('config.main._restart_services'), \ + mock.patch('config.main._reset_failed_services'), \ + mock.patch("utilities_common.cli.run_command", + mock.MagicMock(side_effect=mock_run_command_side_effect)): + result = CliRunner().invoke(config.config.commands["reload"], ["-y"]) + assert result.exit_code == 0 + assert "still in progress" not in result.output + mock_swsscommon.RestartWaiter.isFastBootInProgress.assert_called_once() + mock_swsscommon.RestartWaiter.isWarmBootInProgress.assert_called_once() + + def test_config_reload_no_service_restart_skips_guard(self, get_cmd_module, setup_single_broadcom_asic): + """With -n (no_service_restart), guard is skipped — no syncd teardown, no risk.""" + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db.json") + + mock_swsscommon = mock.MagicMock() + mock_swsscommon.RestartWaiter.isWarmBootInProgress.return_value = True + + with mock.patch.object(config, "DEFAULT_CONFIG_DB_FILE", jsonfile_config), \ + mock.patch('config.main.swsscommon', mock_swsscommon), \ + mock.patch.object(config, 'config_file_yang_validation'), \ + mock.patch("utilities_common.cli.run_command", + mock.MagicMock(side_effect=mock_run_command_side_effect)): + result = CliRunner().invoke(config.config.commands["reload"], ["-y", "-n"]) + assert result.exit_code == 0 + assert "still in progress" not in result.output + mock_swsscommon.RestartWaiter.isFastBootInProgress.assert_not_called() + mock_swsscommon.RestartWaiter.isWarmBootInProgress.assert_not_called() + def test_config_reload_stdin(self, get_cmd_module, setup_single_broadcom_asic): def mock_json_load(f): device_metadata = {