diff --git a/sonic-bmcctld/scripts/bmcctld b/sonic-bmcctld/scripts/bmcctld index 35acfde..61d4b37 100644 --- a/sonic-bmcctld/scripts/bmcctld +++ b/sonic-bmcctld/scripts/bmcctld @@ -226,7 +226,9 @@ DEFAULT_RACK_MGR_CRITICAL_ALERT_ACTION = ACTION_SYSLOG_ONLY DEFAULT_RACK_MGR_MINOR_ALERT_ACTION = ACTION_SYSLOG_ONLY # Default timing values (seconds) -DEFAULT_POWER_ON_DELAY_SECS = 0 # 0 seconds - power on Switch-Host immediately by default +DEFAULT_POWER_ON_DELAY_SECS = 300 # liquid-cooled: allow Rack Manager time to verify liquid flow + # before bmcctld auto-powers Switch-Host. Unused on air-cooled + # (startup path skips _initial_power_on_sequence). DEFAULT_SHUTDOWN_DELAY_SECS = 0 # default 0 until real GNOI graceful shutdown is supported; # operator may override via CHASSIS_MODULE|SWITCH-HOST/graceful_shutdown_timeout @@ -315,8 +317,10 @@ class SwitchHostController(EventLogger): self.chassis = chassis self._switch_host_module = None self.state_db = daemon_base.db_connect("STATE_DB") + self.config_db = daemon_base.db_connect("CONFIG_DB") self.host_state_table = swsscommon.Table(self.state_db, HOST_STATE_TABLE) self.chassis_module_info_table = swsscommon.Table(self.state_db, CHASSIS_MODULE_INFO_TABLE) + self.chassis_module_config_table = swsscommon.Table(self.config_db, CHASSIS_MODULE_TABLE) def _get_switch_host_module(self): """Return the Switch-Host Module object from the chassis module list.""" @@ -483,13 +487,29 @@ class SwitchHostController(EventLogger): power_state = POWER_STATE_ON if str(oper_status).upper() == SWITCH_HOST_ONLINE else POWER_STATE_OFF self._update_host_state(power_state, oper_status) - def initialize_chassis_module_info(self, admin_status): - """Populate CHASSIS_MODULE_TABLE|SWITCH-HOST in STATE_DB at daemon startup. + def initialize_chassis_module(self, admin_status): + """Populate CHASSIS_MODULE|SWITCH-HOST in both CONFIG_DB and STATE_DB at daemon startup. - Reads module identity fields from the platform and writes them alongside - the current admin_status and oper_status. Called once from - BmcctldDaemon.__init__ after policy_reader is available. + CONFIG_DB|CHASSIS_MODULE|SWITCH-HOST is seeded with the supplied admin_status + and default timings (power_on_delay=0, graceful_shutdown_timeout=0) only when + no entry exists, so any operator override persisted from a prior boot is preserved. + STATE_DB|CHASSIS_MODULE_TABLE|SWITCH-HOST is always (re)populated with module + identity, current oper_status, and the supplied admin_status. """ + # 1. Seed CONFIG_DB defaults if absent (don't clobber operator config). + existing = self.chassis_module_config_table.get(SWITCH_HOST_MODULE_KEY) + if not (existing and existing[0]): + self.chassis_module_config_table.set(SWITCH_HOST_MODULE_KEY, swsscommon.FieldValuePairs([ + (CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD, admin_status), + (FIELD_POWER_ON_DELAY, str(DEFAULT_POWER_ON_DELAY_SECS)), + (FIELD_GRACEFUL_SHUTDOWN_TIMEOUT, str(DEFAULT_SHUTDOWN_DELAY_SECS)), + ])) + self.log_info( + "CONFIG_DB CHASSIS_MODULE|SWITCH-HOST seeded: " + "admin_status={} power_on_delay={} graceful_shutdown_timeout={}".format( + admin_status, DEFAULT_POWER_ON_DELAY_SECS, DEFAULT_SHUTDOWN_DELAY_SECS)) + + # 2. Populate STATE_DB CHASSIS_MODULE_TABLE with module identity + status. module = self._get_switch_host_module() if module is None: self.log_error("Cannot initialize CHASSIS_MODULE_INFO: Switch-Host module not found") @@ -545,9 +565,14 @@ class PolicyReader(EventLogger): return fvs_to_dict(result[1]) return {} - def get_switch_host_admin_status(self): - """Return admin_status for SWITCH-HOST from CHASSIS_MODULE|SWITCH-HOST (default: down).""" - return self._get_chassis_module_entry().get(FIELD_ADMIN_STATUS, ADMIN_DOWN) + def get_switch_host_admin_status(self, default=ADMIN_DOWN): + """Return admin_status for SWITCH-HOST from CHASSIS_MODULE|SWITCH-HOST. + + When no entry exists, the supplied default is returned. Callers pass + ADMIN_UP for air-cooled systems (no liquid-flow safety gate) and + ADMIN_DOWN for liquid-cooled systems (must wait for explicit admin-up). + """ + return self._get_chassis_module_entry().get(FIELD_ADMIN_STATUS, default) def get_power_on_delay(self): """Return power_on_delay in seconds from CHASSIS_MODULE|SWITCH-HOST (default: 0).""" @@ -1064,9 +1089,13 @@ class BmcctldDaemon(daemon_base.DaemonBase): self.controller, ) - # Populate CHASSIS_MODULE_INFO_TABLE at startup with module identity and status - admin_status = self.policy_reader.get_switch_host_admin_status() - self.controller.initialize_chassis_module_info(admin_status) + # Populate CHASSIS_MODULE_INFO_TABLE at startup with module identity and status. + # Air-cooled boxes default to admin_status=up (no liquid-flow gate); liquid-cooled + # boxes default to admin_status=down and require explicit admin-up to power on. + default_admin_status = ADMIN_DOWN if self.chassis.is_liquid_cooled() else ADMIN_UP + + admin_status = self.policy_reader.get_switch_host_admin_status(default=default_admin_status) + self.controller.initialize_chassis_module(admin_status) for signum in self.FATAL_SIGNALS + self.NONFATAL_SIGNALS: try: @@ -1143,8 +1172,24 @@ class BmcctldDaemon(daemon_base.DaemonBase): if reboot_cause == ChassisBase.REBOOT_CAUSE_POWER_LOSS: boot_delay = self.policy_reader.get_power_on_delay() - self.log_notice("Waiting {}s before initial Switch-Host power-on check " - "(SWITCH_HOST_POWER_ON_DELAY, reboot_cause={})".format(boot_delay, reboot_cause)) + # Use system uptime so that a bmcctld restart mid-boot (or any later restart) + # doesn't re-arm the full SWITCH_HOST_POWER_ON_DELAY. We only need to wait + # for any time remaining relative to system uptime, not start a fresh timer. + # CLOCK_BOOTTIME is monotonic and includes any suspend time. + try: + system_uptime = time.clock_gettime(time.CLOCK_BOOTTIME) + except Exception as e: + self.log_warning("STARTUP: failed to read CLOCK_BOOTTIME ({}); " + "assuming uptime=0".format(repr(e))) + system_uptime = 0 + boot_delay = max(0, int(boot_delay - system_uptime)) + if boot_delay > 0: + self.log_notice("Waiting {}s before initial Switch-Host power-on check " + "(SWITCH_HOST_POWER_ON_DELAY, uptime={:.1f}s, reboot_cause={})".format( + boot_delay, system_uptime, reboot_cause)) + else: + self.log_notice("Skipping SWITCH_HOST_POWER_ON_DELAY: " + "system uptime ({:.1f}s) already exceeds power_on_delay".format(system_uptime)) else: self.log_notice("Skipping SWITCH_HOST_POWER_ON_DELAY (reboot_cause={}, not a full power loss)".format(reboot_cause)) diff --git a/sonic-bmcctld/tests/test_bmcctld.py b/sonic-bmcctld/tests/test_bmcctld.py index 5ebc20c..bd4faa3 100644 --- a/sonic-bmcctld/tests/test_bmcctld.py +++ b/sonic-bmcctld/tests/test_bmcctld.py @@ -7,6 +7,7 @@ import os import sys import threading +import time import importlib.util import importlib.machinery @@ -1110,13 +1111,30 @@ def test_refreshes_state_when_already_online(self, chassis): daemon.controller.power_on.assert_not_called() daemon.controller.init_host_state.assert_called_once() + def test_boot_delay_skipped_when_system_uptime_exceeds_delay(self, chassis): + """If system uptime already exceeds power_on_delay, boot delay is skipped (no fresh timer).""" + chassis.switch_host.set_oper_status(MockModule.MODULE_STATUS_OFFLINE) + daemon = self._make_daemon(chassis) + daemon.policy_reader.get_power_on_delay = MagicMock(return_value=60) + daemon.critical_event_checker.has_any_critical_event = MagicMock(return_value=False) + daemon.controller.power_on = MagicMock(return_value=True) + # Pretend the system has been up for 10 minutes — bmcctld restart mid-life + # must not re-arm the full 60s delay. + with patch('time.clock_gettime', return_value=600): + t0 = time.monotonic() + daemon._initial_power_on_sequence() + elapsed = time.monotonic() - t0 + assert elapsed < 1.0, "boot delay should have been skipped (elapsed={:.2f}s)".format(elapsed) + daemon.controller.power_on.assert_called_once() + def test_stop_event_during_boot_delay_skips_sequence(self, chassis): daemon = self._make_daemon(chassis) daemon.policy_reader.get_power_on_delay = MagicMock(return_value=60) daemon.critical_event_checker.has_any_critical_event = MagicMock(return_value=False) daemon.controller.power_on = MagicMock() daemon.stop_event.set() # Signal stop before delay expires - daemon._initial_power_on_sequence() + with patch('time.clock_gettime', return_value=0): + daemon._initial_power_on_sequence() daemon.controller.power_on.assert_not_called() def test_boot_delay_processes_queued_actions(self, chassis): @@ -1130,7 +1148,9 @@ def test_boot_delay_processes_queued_actions(self, chassis): # Simulate a POWER_OFF arriving from Rack Manager during boot delay chassis.switch_host.set_oper_status(MockModule.MODULE_STATUS_ONLINE) daemon.action_queue.put(bmcctld.ActionItem(bmcctld.ACTION_POWER_OFF, "RACK_MGR_BOOT_DELAY")) - daemon._initial_power_on_sequence() + # Force system uptime to 0 so the full configured delay applies. + with patch('time.clock_gettime', return_value=0): + daemon._initial_power_on_sequence() # The POWER_OFF must have been consumed from the queue during the delay assert daemon.action_queue.empty() daemon.controller.power_off.assert_called_once() @@ -1329,10 +1349,10 @@ def test_run_liquid_cooled_runs_full_sequence(self, chassis): class TestChassisModuleInfo: - def test_initialize_chassis_module_info_all_fields(self, chassis, controller): - """initialize_chassis_module_info writes all expected fields to CHASSIS_MODULE_TABLE.""" + def test_initialize_chassis_module_all_fields(self, chassis, controller): + """initialize_chassis_module writes all expected fields to CHASSIS_MODULE_TABLE.""" chassis.switch_host.set_oper_status(MockModule.MODULE_STATUS_OFFLINE) - controller.initialize_chassis_module_info(bmcctld.ADMIN_UP) + controller.initialize_chassis_module(bmcctld.ADMIN_UP) result = controller.chassis_module_info_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) assert result[0] is True info = dict(result[1]) @@ -1342,25 +1362,25 @@ def test_initialize_chassis_module_info_all_fields(self, chassis, controller): assert info[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_UP assert info[bmcctld.CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] == bmcctld.SWITCH_HOST_OFFLINE - def test_initialize_chassis_module_info_oper_status_online(self, chassis, controller): + def test_initialize_chassis_module_oper_status_online(self, chassis, controller): """oper_status reflects live module state at initialization time.""" chassis.switch_host.set_oper_status(MockModule.MODULE_STATUS_ONLINE) - controller.initialize_chassis_module_info(bmcctld.ADMIN_UP) + controller.initialize_chassis_module(bmcctld.ADMIN_UP) result = controller.chassis_module_info_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) info = dict(result[1]) assert info[bmcctld.CHASSIS_MODULE_INFO_OPERSTATUS_FIELD] == bmcctld.SWITCH_HOST_ONLINE - def test_initialize_chassis_module_info_admin_down(self, chassis, controller): + def test_initialize_chassis_module_admin_down(self, chassis, controller): """admin_status=down is stored when module is initially down.""" - controller.initialize_chassis_module_info(bmcctld.ADMIN_DOWN) + controller.initialize_chassis_module(bmcctld.ADMIN_DOWN) result = controller.chassis_module_info_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) info = dict(result[1]) assert info[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_DOWN - def test_initialize_chassis_module_info_no_module(self, controller): + def test_initialize_chassis_module_no_module(self, controller): """When module not found, initialize logs an error and does not write the table.""" controller._get_switch_host_module = MagicMock(return_value=None) - controller.initialize_chassis_module_info(bmcctld.ADMIN_UP) + controller.initialize_chassis_module(bmcctld.ADMIN_UP) result = controller.chassis_module_info_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) assert result[0] is False @@ -1393,7 +1413,7 @@ def test_init_host_state_mirrors_oper_status(self, chassis, controller): def test_initialize_then_power_off_preserves_static_fields(self, chassis, controller): """oper_status update via power_off merges into entry; static fields are preserved.""" chassis.switch_host.set_oper_status(MockModule.MODULE_STATUS_ONLINE) - controller.initialize_chassis_module_info(bmcctld.ADMIN_UP) + controller.initialize_chassis_module(bmcctld.ADMIN_UP) # Now power off — oper_status should update but name/serial/etc. must survive controller.power_off() result = controller.chassis_module_info_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) @@ -1406,7 +1426,7 @@ def test_initialize_then_power_off_preserves_static_fields(self, chassis, contro def test_admin_status_updated_on_chassis_module_event(self, event_handler, controller): """CHASSIS_MODULE admin_status event mirrors admin_status to CHASSIS_MODULE_TABLE.""" # Initialize the table first so merging has something to merge into - controller.initialize_chassis_module_info(bmcctld.ADMIN_DOWN) + controller.initialize_chassis_module(bmcctld.ADMIN_DOWN) # Simulate an admin_up event while host is OFFLINE and a critical leak blocks power-on _set_table_entry(controller.host_state_table, bmcctld.HOST_STATE_KEY, {bmcctld.FIELD_DEVICE_STATUS: bmcctld.SWITCH_HOST_OFFLINE}) @@ -1422,7 +1442,7 @@ def test_admin_status_updated_on_chassis_module_event(self, event_handler, contr def test_admin_down_event_mirrors_admin_status(self, event_handler, controller): """CHASSIS_MODULE admin_down event mirrors admin_status=down to CHASSIS_MODULE_TABLE.""" - controller.initialize_chassis_module_info(bmcctld.ADMIN_UP) + controller.initialize_chassis_module(bmcctld.ADMIN_UP) _set_table_entry(controller.host_state_table, bmcctld.HOST_STATE_KEY, {bmcctld.FIELD_DEVICE_STATUS: bmcctld.SWITCH_HOST_ONLINE}) event_handler._handle_chassis_module( @@ -1433,7 +1453,7 @@ def test_admin_down_event_mirrors_admin_status(self, event_handler, controller): info = dict(result[1]) assert info[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_DOWN - def test_daemon_init_calls_initialize_chassis_module_info(self, chassis): + def test_daemon_init_calls_initialize_chassis_module(self, chassis): """BmcctldDaemon.__init__ populates CHASSIS_MODULE_TABLE at startup.""" with patch('sonic_platform.platform.Platform') as MockPlatform: MockPlatform.return_value.get_chassis.return_value = chassis @@ -1445,3 +1465,53 @@ def test_daemon_init_calls_initialize_chassis_module_info(self, chassis): assert bmcctld.CHASSIS_MODULE_INFO_OPERSTATUS_FIELD in info assert bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD in info + def test_initialize_chassis_module_seeds_config_db_defaults(self, chassis, controller): + """When CONFIG_DB CHASSIS_MODULE|SWITCH-HOST is absent, seed defaults using passed admin_status.""" + controller.chassis_module_config_table._del(bmcctld.SWITCH_HOST_MODULE_KEY) + controller.initialize_chassis_module(bmcctld.ADMIN_DOWN) + result = controller.chassis_module_config_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) + assert result[0] is True + cfg = dict(result[1]) + assert cfg[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_DOWN + assert cfg[bmcctld.FIELD_POWER_ON_DELAY] == str(bmcctld.DEFAULT_POWER_ON_DELAY_SECS) + assert cfg[bmcctld.FIELD_GRACEFUL_SHUTDOWN_TIMEOUT] == str(bmcctld.DEFAULT_SHUTDOWN_DELAY_SECS) + + def test_initialize_chassis_module_preserves_operator_config(self, chassis, controller): + """Existing operator CONFIG_DB entry must not be clobbered by daemon startup.""" + controller.chassis_module_config_table.set( + bmcctld.SWITCH_HOST_MODULE_KEY, + FieldValuePairs([ + (bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD, bmcctld.ADMIN_UP), + (bmcctld.FIELD_POWER_ON_DELAY, "45"), + (bmcctld.FIELD_GRACEFUL_SHUTDOWN_TIMEOUT, "90"), + ]), + ) + controller.initialize_chassis_module(bmcctld.ADMIN_DOWN) + result = controller.chassis_module_config_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) + cfg = dict(result[1]) + assert cfg[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_UP + assert cfg[bmcctld.FIELD_POWER_ON_DELAY] == "45" + assert cfg[bmcctld.FIELD_GRACEFUL_SHUTDOWN_TIMEOUT] == "90" + + def test_daemon_init_air_cooled_defaults_admin_up(self, chassis): + """Air-cooled boxes default CONFIG_DB admin_status=up when no operator entry exists.""" + chassis.set_liquid_cooled(False) + with patch('sonic_platform.platform.Platform') as MockPlatform: + MockPlatform.return_value.get_chassis.return_value = chassis + daemon = bmcctld.BmcctldDaemon(bmcctld.SYSLOG_IDENTIFIER) + result = daemon.controller.chassis_module_config_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) + assert result[0] is True + cfg = dict(result[1]) + assert cfg[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_UP + + def test_daemon_init_liquid_cooled_defaults_admin_down(self, chassis): + """Liquid-cooled boxes default CONFIG_DB admin_status=down when no operator entry exists.""" + chassis.set_liquid_cooled(True) + with patch('sonic_platform.platform.Platform') as MockPlatform: + MockPlatform.return_value.get_chassis.return_value = chassis + daemon = bmcctld.BmcctldDaemon(bmcctld.SYSLOG_IDENTIFIER) + result = daemon.controller.chassis_module_config_table.get(bmcctld.SWITCH_HOST_MODULE_KEY) + assert result[0] is True + cfg = dict(result[1]) + assert cfg[bmcctld.CHASSIS_MODULE_INFO_ADMIN_STATUS_FIELD] == bmcctld.ADMIN_DOWN +