diff --git a/sonic-chassisd/scripts/chassisd b/sonic-chassisd/scripts/chassisd index a7a363109..deb409c38 100755 --- a/sonic-chassisd/scripts/chassisd +++ b/sonic-chassisd/scripts/chassisd @@ -18,6 +18,7 @@ try: import json import glob from datetime import datetime, timezone + from typing import NamedTuple from sonic_py_common import daemon_base, logger, device_info from sonic_py_common.task_base import ProcessTaskBase @@ -105,13 +106,16 @@ MODULE_ADMIN_UP = 1 MODULE_REBOOT_CAUSE_DIR = "/host/reboot-cause/module/" MAX_HISTORY_FILES = 10 +# DPU_STATE table in CHASSIS_STATE_DB +DPU_STATE_TABLE = 'DPU_STATE' + DP_STATE = 'dpu_data_plane_state' DP_UPDATE_TIME = 'dpu_data_plane_time' CP_STATE = 'dpu_control_plane_state' CP_UPDATE_TIME = 'dpu_control_plane_time' MP_STATE = 'dpu_midplane_link_state' -# New DPU recovery DB fields (CHASSIS_STATE_DB DPU_STATE table) +# DPU recovery DB fields (CHASSIS_STATE_DB DPU_STATE table) READY_STATUS = 'ready_status' RECOVERY_STATUS = 'recovery_status' RESET_COUNT = 'reset_count' @@ -122,11 +126,16 @@ LAST_READY_TIME = 'last_ready_time' RECOVERY_RECOVERABLE = 'recoverable' RECOVERY_UNRECOVERABLE = 'unrecoverable' -# DPU auto-recovery feature name in CONFIG_DB FEATURE table -DPU_AUTO_RECOVERY_FEATURE = 'dpu-auto-recovery' -DPU_AUTO_RECOVERY_STATE_ENABLED = 'enabled' -DPU_AUTO_RECOVERY_STATE_DISABLED = 'disabled' -DPU_AUTO_RECOVERY_STATE_ALWAYS_DISABLED = 'always_disabled' +# DPU auto-recovery field in CONFIG_DB DEVICE_METADATA|localhost +DPU_AUTO_RECOVERY_FIELD = 'dpu_auto_recovery' +DPU_AUTO_RECOVERY_ENABLED = 'enable' +DPU_AUTO_RECOVERY_DISABLED = 'disable' + +# Platform.json field names for recovery thresholds +PLATFORM_CFG_DPU_REBOOT_TIMEOUT = 'dpu_reboot_timeout' +PLATFORM_CFG_DPU_RESET_LIMIT = 'dpu_reset_limit' +PLATFORM_CFG_DPU_BOOT_TIMEOUT = 'dpu_boot_timeout' +PLATFORM_CFG_DPU_SELF_RECOVERY_TIMEOUT = 'dpu_self_recovery_timeout' # Default thresholds (configurable via platform.json) DEFAULT_DPU_BOOT_TIMEOUT = 600 @@ -137,10 +146,12 @@ MINIMUM_SELF_RECOVERY_POLL_COUNT = 3 # Reboot cause file used to detect NPU kernel crash on chassisd startup REBOOT_CAUSE_FILE = "/host/reboot-cause/reboot-cause.txt" -NPU_CRASH_REBOOT_CAUSE_KEYWORDS = ("kernel panic", "unknown") -# Planned operation tracking (STATE_DB CHASSIS_MODULE_TABLE) -STATE_TRANSITION_IN_PROGRESS = 'state_transition_in_progress' +# Internal recovery dict key for tracking prior Unrecoverable state +WAS_UNRECOVERABLE_KEY = 'was_unrecoverable' + +# Transition type value used by chassisd when acquiring the state-transition lock +TRANSITION_TYPE_RECOVERY = 'recovery' # DPU recovery states DPU_STATE_BOOTING = 'Booting' @@ -148,10 +159,17 @@ DPU_STATE_READY = 'Ready' DPU_STATE_WAIT_FOR_SELF_RECOVERY = 'WaitForSelfRecovery' DPU_STATE_POWER_CYCLE = 'PowerCycle' DPU_STATE_MANUAL_INTERVENTION = 'ManualIntervention' -DPU_STATE_OFFLINE = 'Offline' +DPU_STATE_ADMIN_DOWN = 'AdminDown' DPU_STATE_UNRECOVERABLE = 'Unrecoverable' +class DpuStates(NamedTuple): + """Container for DPU plane states to avoid positional confusion.""" + mp_state: str + cp_state: str + dp_state: str + + # This daemon should return non-zero exit code so that supervisord will # restart it automatically. exit_code = 0 @@ -752,10 +770,14 @@ class SmartSwitchModuleUpdater(ModuleUpdater): self.hostname_table = swsscommon.Table(self.chassis_state_db, CHASSIS_MODULE_HOSTNAME_TABLE) self.module_reboot_table = swsscommon.Table(self.chassis_state_db, CHASSIS_MODULE_REBOOT_INFO_TABLE) - self.dpu_state_table = swsscommon.Table(self.chassis_state_db, 'DPU_STATE') + self.dpu_state_table = swsscommon.Table(self.chassis_state_db, DPU_STATE_TABLE) self.down_modules = {} self.chassis_app_db_clean_sha = None + # Cache CONFIG_DB DEVICE_METADATA for auto-recovery checks + self.config_db = daemon_base.db_connect("CONFIG_DB") + self.device_metadata_table = swsscommon.Table(self.config_db, 'DEVICE_METADATA') + self.midplane_initialized = try_get(chassis.init_midplane_switch, default=False) if not self.midplane_initialized: self.log_error("Chassisd midplane intialization failed") @@ -771,10 +793,10 @@ class SmartSwitchModuleUpdater(ModuleUpdater): try: with open(PLATFORM_JSON_FILE, 'r') as f: platform_cfg = json.load(f) - self.dpu_reboot_timeout = int(platform_cfg.get("dpu_reboot_timeout", DEFAULT_DPU_REBOOT_TIMEOUT)) - self.reset_limit = int(platform_cfg.get("dpu_reset_limit", DEFAULT_DPU_RESET_LIMIT)) - self.dpu_boot_timeout = int(platform_cfg.get("dpu_boot_timeout", DEFAULT_DPU_BOOT_TIMEOUT)) - self.dpu_self_recovery_timeout = int(platform_cfg.get("dpu_self_recovery_timeout", DEFAULT_DPU_SELF_RECOVERY_TIMEOUT)) + self.dpu_reboot_timeout = int(platform_cfg.get(PLATFORM_CFG_DPU_REBOOT_TIMEOUT, DEFAULT_DPU_REBOOT_TIMEOUT)) + self.reset_limit = int(platform_cfg.get(PLATFORM_CFG_DPU_RESET_LIMIT, DEFAULT_DPU_RESET_LIMIT)) + self.dpu_boot_timeout = int(platform_cfg.get(PLATFORM_CFG_DPU_BOOT_TIMEOUT, DEFAULT_DPU_BOOT_TIMEOUT)) + self.dpu_self_recovery_timeout = int(platform_cfg.get(PLATFORM_CFG_DPU_SELF_RECOVERY_TIMEOUT, DEFAULT_DPU_SELF_RECOVERY_TIMEOUT)) except (json.JSONDecodeError, ValueError) as e: self.log_error("Error parsing {}: {}".format(PLATFORM_JSON_FILE, e)) except Exception as e: @@ -1164,7 +1186,7 @@ class SmartSwitchModuleUpdater(ModuleUpdater): self.log_notice("Module {} midplane connectivity is up".format(module_key)) # Update midplane state in the chassisStateDB DPU_STATE table - key = "DPU_STATE|" + module_key + key = DPU_STATE_TABLE + "|" + module_key dpu_mp_state = self.get_dpu_midplane_state(key) if midplane_access and dpu_mp_state != 'up': self.update_dpu_state(key, 'up') @@ -1188,37 +1210,34 @@ class SmartSwitchModuleUpdater(ModuleUpdater): keys = self.chassis_state_db.keys(pattern) if keys: for key in keys: - if not "DPU_STATE" in key and not "REBOOT_CAUSE" in key: + if DPU_STATE_TABLE not in key and "REBOOT_CAUSE" not in key: self.chassis_state_db.delete(key) return def _is_auto_recovery_enabled(self): - """Check if dpu-auto-recovery feature is enabled in CONFIG_DB FEATURE table. + """Check if dpu_auto_recovery is enabled in CONFIG_DB DEVICE_METADATA|localhost. - Returns True only when ``state == 'enabled'``. Both ``'disabled'`` and - ``'always_disabled'`` suppress automatic power-cycle recovery; chassisd - still updates DPU state fields in CHASSIS_STATE_DB but will not - initiate a power-cycle. + Returns True when the field is 'enable'. + Returns False otherwise. """ try: - config_db = daemon_base.db_connect("CONFIG_DB") - feature_table = swsscommon.Table(config_db, 'FEATURE') - fvs = feature_table.get(DPU_AUTO_RECOVERY_FEATURE) + fvs = self.device_metadata_table.get('localhost') if isinstance(fvs, list) and fvs[0] is True: fvs = dict(fvs[-1]) - state = fvs.get('state', DPU_AUTO_RECOVERY_STATE_ENABLED) - return state == DPU_AUTO_RECOVERY_STATE_ENABLED + state = fvs.get(DPU_AUTO_RECOVERY_FIELD, DPU_AUTO_RECOVERY_DISABLED) + return state == DPU_AUTO_RECOVERY_ENABLED except Exception as e: - self.log_error("Failed to read dpu-auto-recovery feature state: {}".format(e)) + self.log_error("Failed to read {} from DEVICE_METADATA: {}".format( + DPU_AUTO_RECOVERY_FIELD, e)) return False # Default to disabled def _get_dpu_states(self, module_name): """Get current DPU states from CHASSIS_STATE_DB.""" - dpu_key = "DPU_STATE|" + module_name + dpu_key = DPU_STATE_TABLE + "|" + module_name mp_state = self.get_dpu_midplane_state(dpu_key) _, cp_state = self.dpu_state_table.hget(module_name, CP_STATE) _, dp_state = self.dpu_state_table.hget(module_name, DP_STATE) - return mp_state, cp_state, dp_state + return DpuStates(mp_state=mp_state, cp_state=cp_state, dp_state=dp_state) def _set_ready_status(self, module_name, status): """Set ready_status for a DPU in CHASSIS_STATE_DB.""" @@ -1240,45 +1259,120 @@ class SmartSwitchModuleUpdater(ModuleUpdater): """Set last_ready_time for a DPU in CHASSIS_STATE_DB.""" self.dpu_state_table.hset(module_name, LAST_READY_TIME, get_formatted_time()) - def _is_planned_transition_in_progress(self, module_name): + def _is_planned_transition_in_progress(self, module_name, module_index): """Check if a planned operation (shutdown/reboot) is in progress for this DPU. - Reads ``state_transition_in_progress`` from STATE_DB CHASSIS_MODULE_TABLE. - Returns True when a planned transition is active, suppressing auto-recovery. + Uses the platform API get_module_state_transition() which handles + timeout-based auto-clearing. Returns True when a non-recovery + transition is active, suppressing auto-recovery. + Ignores transition_type == TRANSITION_TYPE_RECOVERY so chassisd does not suppress itself. """ + module = self.chassis.get_module(module_index) + in_progress = try_get(module.get_module_state_transition, module_name, default=False) + if not in_progress: + return False + # Check transition_type — only suppress for non-recovery transitions try: fvs = self.module_table.get(module_name) if isinstance(fvs, list) and fvs[0] is True: fvs = dict(fvs[-1]) - return fvs.get(STATE_TRANSITION_IN_PROGRESS, 'False') == 'True' - except Exception as e: - self.log_warning("{}: Failed to read state_transition_in_progress: {}".format( - module_name, e)) - return False + return fvs.get('transition_type') != TRANSITION_TYPE_RECOVERY + except Exception: + pass + return True - def _power_cycle_dpu(self, module_name, module_index): - """Power-cycle a DPU and increment reset_count.""" + def _enter_power_cycle_or_unrecoverable(self, module_name, module_index): + """Attempt power-cycle; mark unrecoverable if reset_limit is reached.""" recovery = self.dpu_recovery_state[module_name] + + # Check limit before issuing power-cycle + if recovery['reset_count'] >= self.reset_limit: + recovery['state'] = DPU_STATE_UNRECOVERABLE + self._set_recovery_status(module_name, RECOVERY_UNRECOVERABLE) + self.log_error("{}: Reached reset_limit ({}). Marking as unrecoverable.".format( + module_name, self.reset_limit)) + return + recovery['reset_count'] += 1 self._set_reset_count(module_name, recovery['reset_count']) - self.log_warning("{}: Initiating power-cycle (reset_count={})".format( - module_name, recovery['reset_count'])) + self.log_warning("{}: Initiating power-cycle (reset_count={}/{})".format( + module_name, recovery['reset_count'], self.reset_limit)) + + module = self.chassis.get_module(module_index) + + # Acquire state transition lock to prevent concurrent planned + # shutdown/reboot from operating on this DPU during power-cycle. + # If the lock cannot be acquired (another operation holds it), + # abort the power-cycle — the other operation owns this DPU. + lock_acquired = try_get(module.set_module_state_transition, module_name, TRANSITION_TYPE_RECOVERY, default=False) + if not lock_acquired: + self.log_warning("{}: Cannot acquire state transition lock; " + "another operation in progress — skipping power-cycle".format(module_name)) + recovery['reset_count'] -= 1 + self._set_reset_count(module_name, recovery['reset_count']) + recovery['state'] = DPU_STATE_POWER_CYCLE + recovery['boot_start_time'] = time.time() + return try: - module = self.chassis.get_module(module_index) + # Keep the same graceful sequence as module_base: + # pre-shutdown -> admin down -> admin up -> post-startup. + # Treat NotImplemented pre/post hooks as no-op success. + if not try_get(module.module_pre_shutdown, default=True): + self.log_warning("{}: module_pre_shutdown() failed".format(module_name)) + try_get(module.set_admin_state, MODULE_ADMIN_DOWN, default=False) try_get(module.set_admin_state, MODULE_ADMIN_UP, default=False) + + if not try_get(module.module_post_startup, default=True): + self.log_warning("{}: module_post_startup() failed".format(module_name)) except Exception as e: self.log_error("{}: Power-cycle failed: {}".format(module_name, e)) - - if recovery['reset_count'] >= self.reset_limit: - recovery['state'] = DPU_STATE_UNRECOVERABLE - self._set_recovery_status(module_name, RECOVERY_UNRECOVERABLE) - self.log_error("{}: Reached reset_limit ({}). Marking as unrecoverable.".format( - module_name, self.reset_limit)) + finally: + try_get(module.clear_module_state_transition, module_name, default=False) + + recovery['state'] = DPU_STATE_POWER_CYCLE + recovery['boot_start_time'] = time.time() + + def _enter_booting(self, name, recovery): + """Transition a DPU into the Booting state.""" + recovery['state'] = DPU_STATE_BOOTING + recovery['boot_start_time'] = time.time() + recovery.pop('boot_dp_warning_logged', None) + + def _enter_ready(self, name, recovery, context_msg): + """Transition a DPU into the Ready state.""" + recovery['state'] = DPU_STATE_READY + self._set_ready_status(name, 'true') + self._set_last_ready_time(name) + self.log_info("{}: {}".format(name, context_msg)) + + def _enter_wait_for_self_recovery(self, name, recovery): + """Transition a DPU into the WaitForSelfRecovery state.""" + recovery['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY + recovery['self_recovery_start_time'] = time.time() + recovery['self_recovery_poll_count'] = 0 + + def _should_handle_hw_offline(self, current_state): + """Return True only if a previously-Ready DPU should treat + oper_status Offline as a new hardware failure. + + Allow-list (Ready only) is fail-safe for future states and avoids a + `module startup` race where a stale Offline snapshot in AdminDown + would power-cycle instead of cleanly restarting. + """ + return current_state == DPU_STATE_READY + + def _handle_boot_timeout_expired(self, name, module_index, recovery, + auto_recovery_enabled, context): + """Handle boot timeout expiry — power-cycle or manual intervention.""" + self.log_warning("{}: Boot timeout ({}s) expired {}".format( + name, self.dpu_boot_timeout, context)) + self._set_ready_status(name, 'false') + if auto_recovery_enabled: + self._enter_power_cycle_or_unrecoverable(name, module_index) else: - recovery['state'] = DPU_STATE_POWER_CYCLE - recovery['boot_start_time'] = time.time() + recovery['state'] = DPU_STATE_MANUAL_INTERVENTION def init_dpu_recovery_state(self): """Initialize recovery DB fields for all DPUs on chassisd startup. @@ -1300,32 +1394,36 @@ class SmartSwitchModuleUpdater(ModuleUpdater): self._set_recovery_status(name, RECOVERY_RECOVERABLE) self._set_reset_count(name, 0) self._set_last_down_time(name) - self.dpu_recovery_state[name]['state'] = DPU_STATE_BOOTING + self._enter_booting(name, self.dpu_recovery_state[name]) self.dpu_recovery_state[name]['reset_count'] = 0 - self.dpu_recovery_state[name]['boot_start_time'] = time.time() if not npu_crash: continue # NPU kernel crash recovery: unconditionally power-cycle # admin-up DPUs to guarantee a known-good starting state. - oper_status = self.get_module_current_status(name) - if oper_status == str(ModuleBase.MODULE_STATUS_OFFLINE): - # Admin-down DPU; leave powered off + admin_status = self.get_module_admin_status(name) + if admin_status == 'down': + # Admin-down DPU; leave powered off. continue if not self._is_auto_recovery_enabled(): self.dpu_recovery_state[name]['state'] = DPU_STATE_MANUAL_INTERVENTION continue - self._power_cycle_dpu(name, module_index) + self._enter_power_cycle_or_unrecoverable(name, module_index) def _npu_crash_on_last_boot(self): - """Return True if the last NPU reboot cause looks like a crash/OOM/unknown.""" + """Return True if the last NPU reboot cause looks like a kernel crash.""" try: if not os.path.isfile(REBOOT_CAUSE_FILE): + # No reboot-cause file means this is likely a fresh boot or the + # file was not written — assume no crash to avoid false positives. return False with open(REBOOT_CAUSE_FILE, 'r') as f: cause = f.read().strip().lower() - return any(kw in cause for kw in NPU_CRASH_REBOOT_CAUSE_KEYWORDS) + + # Match kernel crashes only. "unknown" is reported on first boot + # and must not trigger a power-cycle. + return 'kernel panic' in cause except Exception as e: self.log_warning("Failed to read {}: {}".format(REBOOT_CAUSE_FILE, e)) return False @@ -1340,7 +1438,7 @@ class SmartSwitchModuleUpdater(ModuleUpdater): DPU transitions to ManualIntervention and waits for operator action. Recovery is suppressed while a planned operation (shutdown/reboot) is - in progress for a DPU (state_transition_in_progress == True). + in progress for a DPU (transition_in_progress == True). """ auto_recovery_enabled = self._is_auto_recovery_enabled() @@ -1348,181 +1446,170 @@ class SmartSwitchModuleUpdater(ModuleUpdater): name = try_get(self.chassis.get_module(module_index).get_name) if name not in self.dpu_recovery_state: continue + self._process_single_dpu_recovery(name, module_index, auto_recovery_enabled) + + def _process_single_dpu_recovery(self, name, module_index, auto_recovery_enabled): + """Process recovery state machine for a single DPU.""" + recovery = self.dpu_recovery_state[name] + current_state = recovery['state'] + states = self._get_dpu_states(name) + oper_status = self.get_module_current_status(name) + admin_status = self.get_module_admin_status(name) + + all_up = (states.mp_state == 'up' and states.cp_state == 'up' and states.dp_state == 'up') + + # DPU is administratively down — no automatic recovery; just + # reflect the offline state and wait for `module startup`. + # This check must precede the Unrecoverable check so that an + # operator can shut down an unrecoverable DPU and then restart it. + if admin_status == 'down': + if current_state != DPU_STATE_ADMIN_DOWN: + if current_state == DPU_STATE_UNRECOVERABLE: + recovery[WAS_UNRECOVERABLE_KEY] = True + recovery['state'] = DPU_STATE_ADMIN_DOWN + self._set_ready_status(name, 'false') + return - recovery = self.dpu_recovery_state[name] - current_state = recovery['state'] - mp_state, cp_state, dp_state = self._get_dpu_states(name) - oper_status = self.get_module_current_status(name) - admin_status = self.get_module_admin_status(name) + # Terminal: requires operator to set admin-down then admin-up, + # which triggers the admin-down check above and transitions to + # AdminDown state, clearing the Unrecoverable condition. + if current_state == DPU_STATE_UNRECOVERABLE: + return - all_up = (mp_state == 'up' and cp_state == 'up' and dp_state == 'up') + # Suppress recovery while a planned operation (shutdown/reboot) + # is in progress. The control plane goes down during planned + # operations; we must not misinterpret that as a failure. + if self._is_planned_transition_in_progress(name, module_index): + return - # DPU is administratively down — no automatic recovery; just - # reflect the offline state and wait for `module startup`. - # This check must precede the Unrecoverable check so that an - # operator can shut down an unrecoverable DPU and then restart it. - if admin_status == 'down': - if current_state != DPU_STATE_OFFLINE: - if current_state == DPU_STATE_UNRECOVERABLE: - recovery['was_unrecoverable'] = True - recovery['state'] = DPU_STATE_OFFLINE - self._set_ready_status(name, 'false') - continue + # Admin-up DPU went Offline at the hardware level — treat as a + # hardware failure. + if oper_status == str(ModuleBase.MODULE_STATUS_OFFLINE) and \ + self._should_handle_hw_offline(current_state): + self.log_warning( + "{}: Hardware failure detected (oper_status: Offline)".format(name)) + self._set_ready_status(name, 'false') + self._set_last_down_time(name) + if auto_recovery_enabled: + self._enter_power_cycle_or_unrecoverable(name, module_index) + else: + recovery['state'] = DPU_STATE_MANUAL_INTERVENTION + return - # Terminal: requires chassisd reset or operator action - if current_state == DPU_STATE_UNRECOVERABLE: - continue + # Admin transitioned from down -> up (DPU must be admin-up if + # execution reaches here); restart the recovery FSM. + # If DPU was previously Unrecoverable, operator module startup + # resets recovery_status and reset_count. + if current_state == DPU_STATE_ADMIN_DOWN: + if recovery.pop(WAS_UNRECOVERABLE_KEY, False): + recovery['reset_count'] = 0 + self._set_reset_count(name, 0) + self._set_recovery_status(name, RECOVERY_RECOVERABLE) + self.log_notice("{}: Operator module startup — resetting recovery state".format(name)) + self._enter_booting(name, recovery) + return - # Suppress recovery while a planned operation (shutdown/reboot) - # is in progress. The control plane goes down during planned - # operations; we must not misinterpret that as a failure. - if self._is_planned_transition_in_progress(name): - continue + if current_state == DPU_STATE_BOOTING: + if all_up: + self._enter_ready(name, recovery, "DPU is ready (all states up)") + elif time.time() - recovery.get('boot_start_time', time.time()) >= self.dpu_boot_timeout: + # Boot timeout expired. Per HLD, only trigger power-cycle + # if CP or midplane is still down. If CP is up but DP + # hasn't converged, just log a warning — the DPU SONiC + # stack is running but the data plane pipeline hasn't come up. + if states.cp_state == 'up' and states.mp_state == 'up': + # Data plane not ready but control plane is up — warning only. + if not recovery.get('boot_dp_warning_logged'): + self.log_warning("{}: data plane not up after {}s".format( + name, self.dpu_boot_timeout)) + recovery['boot_dp_warning_logged'] = True + else: + # CP or midplane still down — genuine boot failure. + self._handle_boot_timeout_expired( + name, module_index, recovery, + auto_recovery_enabled, "without reaching Ready") + return - # Admin-up DPU went Offline at the hardware level — treat as a - # hardware failure. - if oper_status == str(ModuleBase.MODULE_STATUS_OFFLINE) and \ - current_state not in (DPU_STATE_BOOTING, DPU_STATE_POWER_CYCLE, - DPU_STATE_WAIT_FOR_SELF_RECOVERY, - DPU_STATE_MANUAL_INTERVENTION): - self.log_warning( - "{}: Hardware failure detected (oper_status: Offline)".format(name)) + if current_state == DPU_STATE_READY: + # Per HLD: midplane down or control-plane down triggers + # recovery. Data-plane-down alone does not trigger recovery. + if states.mp_state == 'down' or states.cp_state == 'down': + self.log_warning("{}: failure detected (mp={}, cp={})".format( + name, states.mp_state, states.cp_state)) self._set_ready_status(name, 'false') self._set_last_down_time(name) if auto_recovery_enabled: - self._power_cycle_dpu(name, module_index) + # Enter WaitForSelfRecovery with a grace period. + self._enter_wait_for_self_recovery(name, recovery) else: + # Per HLD: skip self-recovery wait when auto-recovery + # is disabled; go directly to ManualIntervention. recovery['state'] = DPU_STATE_MANUAL_INTERVENTION - continue - - # Admin transitioned from down -> up; restart the recovery FSM. - # If DPU was previously Unrecoverable, operator module startup - # resets recovery_status and reset_count. - if current_state == DPU_STATE_OFFLINE: - if recovery.pop('was_unrecoverable', False): - recovery['reset_count'] = 0 - self._set_reset_count(name, 0) - self._set_recovery_status(name, RECOVERY_RECOVERABLE) - self.log_notice("{}: Operator module startup — resetting recovery state".format(name)) - recovery['state'] = DPU_STATE_BOOTING - recovery['boot_start_time'] = time.time() - continue - - if current_state == DPU_STATE_BOOTING: - if all_up: - recovery['state'] = DPU_STATE_READY + elif states.dp_state == 'down': + # Data plane down only: mark not ready but no recovery. + # Only write to DB on transition to avoid redundant writes. + if recovery.get('dp_was_down') is not True: + self._set_ready_status(name, 'false') + self._set_last_down_time(name) + recovery['dp_was_down'] = True + else: + # All states up — steady state. Only write on transition + # back from dp-down to avoid redundant writes every cycle. + if recovery.get('dp_was_down') is True: self._set_ready_status(name, 'true') self._set_last_ready_time(name) - self.log_info("{}: DPU is ready (all states up)".format(name)) - elif time.time() - recovery.get('boot_start_time', time.time()) >= self.dpu_boot_timeout: - # Boot timeout expired. Per HLD, only trigger power-cycle - # if CP or midplane is still down. If CP is up but DP - # hasn't converged, just log a warning — the DPU SONiC - # stack is running but the data plane pipeline hasn't come up. - if cp_state == 'up' and mp_state == 'up': - # Data plane not ready but control plane is up — warning only. - if not recovery.get('boot_dp_warning_logged'): - self.log_warning("{}: data plane not up after {}s".format( - name, self.dpu_boot_timeout)) - recovery['boot_dp_warning_logged'] = True - else: - # CP or midplane still down — genuine boot failure. - self.log_warning("{}: Boot timeout ({}s) expired without reaching Ready".format( - name, self.dpu_boot_timeout)) - self._set_ready_status(name, 'false') - self._set_last_down_time(name) - if auto_recovery_enabled: - self._power_cycle_dpu(name, module_index) - else: - recovery['state'] = DPU_STATE_MANUAL_INTERVENTION - continue + recovery['dp_was_down'] = False + return - if current_state == DPU_STATE_READY: - # Per HLD: midplane down or control-plane down triggers - # entry into WaitForSelfRecovery with a grace period. - # Data-plane-down alone does not trigger recovery. - if mp_state == 'down' or cp_state == 'down': - self.log_warning("{}: failure detected (mp={}, cp={}); entering WaitForSelfRecovery".format( - name, mp_state, cp_state)) - recovery['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY - recovery['self_recovery_start_time'] = time.time() - recovery['self_recovery_poll_count'] = 0 - self._set_ready_status(name, 'false') - self._set_last_down_time(name) - elif dp_state == 'down': - # Data plane down only: mark not ready but no recovery. - # Only write to DB on transition to avoid redundant writes. - if recovery.get('dp_was_down') is not True: - self._set_ready_status(name, 'false') - self._set_last_down_time(name) - recovery['dp_was_down'] = True + if current_state == DPU_STATE_WAIT_FOR_SELF_RECOVERY: + # Grace period: allow the DPU to self-recover from transient + # failures (process restart, HW watchdog reboot) without + # external intervention. + # Minimal grace: wait at least 30s or 3 polling intervals + # before evaluating recovery conditions. + elapsed = time.time() - recovery.get('self_recovery_start_time', time.time()) + recovery['self_recovery_poll_count'] = recovery.get('self_recovery_poll_count', 0) + 1 + if elapsed < MINIMUM_SELF_RECOVERY_GRACE_PERIOD and \ + recovery['self_recovery_poll_count'] < MINIMUM_SELF_RECOVERY_POLL_COUNT: + return + if states.mp_state == 'up' or states.cp_state == 'up': + # DPU is self-recovering — transition to Booting and wait + # for full readiness (all planes up) within dpu_boot_timeout. + self.log_notice("{}: self-recovering (mp={}, cp={}); entering Booting".format( + name, states.mp_state, states.cp_state)) + self._enter_booting(name, recovery) + elif elapsed >= self.dpu_self_recovery_timeout: + # Timeout expired and both CP and midplane still down. + self.log_warning("{}: self-recovery timeout ({}s) expired; " + "both cp and midplane still down".format( + name, self.dpu_self_recovery_timeout)) + if auto_recovery_enabled: + self._enter_power_cycle_or_unrecoverable(name, module_index) else: - # All states up — steady state. Only write on transition - # back from dp-down to avoid redundant writes every cycle. - if recovery.get('dp_was_down') is True: - self._set_ready_status(name, 'true') - self._set_last_ready_time(name) - recovery['dp_was_down'] = False - continue - - if current_state == DPU_STATE_WAIT_FOR_SELF_RECOVERY: - # Grace period: allow the DPU to self-recover from transient - # failures (process restart, HW watchdog reboot) without - # external intervention. - # Minimal grace: wait at least 30s or 3 polling intervals - # before evaluating recovery conditions. - elapsed = time.time() - recovery.get('self_recovery_start_time', time.time()) - recovery['self_recovery_poll_count'] = recovery.get('self_recovery_poll_count', 0) + 1 - if elapsed < MINIMUM_SELF_RECOVERY_GRACE_PERIOD and \ - recovery['self_recovery_poll_count'] < MINIMUM_SELF_RECOVERY_POLL_COUNT: - continue - if mp_state == 'up' or cp_state == 'up': - # DPU is self-recovering — transition to Booting and wait - # for full readiness (all planes up) within dpu_boot_timeout. - self.log_notice("{}: self-recovering (mp={}, cp={}); entering Booting".format( - name, mp_state, cp_state)) - recovery['state'] = DPU_STATE_BOOTING - recovery['boot_start_time'] = time.time() - recovery.pop('boot_dp_warning_logged', None) - elif elapsed >= self.dpu_self_recovery_timeout: - # Timeout expired and both CP and midplane still down. - self.log_warning("{}: self-recovery timeout ({}s) expired; " - "both cp and midplane still down".format( - name, self.dpu_self_recovery_timeout)) - if auto_recovery_enabled: - self._power_cycle_dpu(name, module_index) - else: - recovery['state'] = DPU_STATE_MANUAL_INTERVENTION - continue + recovery['state'] = DPU_STATE_MANUAL_INTERVENTION + return - if current_state == DPU_STATE_POWER_CYCLE: - if all_up: - recovery['state'] = DPU_STATE_READY - self._set_ready_status(name, 'true') - self._set_last_ready_time(name) - self.log_info("{}: DPU recovered after power-cycle".format(name)) - elif time.time() - recovery.get('boot_start_time', time.time()) >= self.dpu_boot_timeout: - # DPU did not recover within dpu_boot_timeout after power-cycle - self.log_warning("{}: Boot timeout ({}s) expired after power-cycle".format( - name, self.dpu_boot_timeout)) - if auto_recovery_enabled: - self._power_cycle_dpu(name, module_index) - else: - recovery['state'] = DPU_STATE_MANUAL_INTERVENTION - continue + if current_state == DPU_STATE_POWER_CYCLE: + if all_up: + self._enter_ready(name, recovery, "DPU recovered after power-cycle") + elif time.time() - recovery.get('boot_start_time', time.time()) >= self.dpu_boot_timeout: + # DPU did not recover within dpu_boot_timeout after power-cycle + self._handle_boot_timeout_expired( + name, module_index, recovery, + auto_recovery_enabled, "after power-cycle") + return - if current_state == DPU_STATE_MANUAL_INTERVENTION: - # Operator may have recovered the DPU manually. - if all_up: - recovery['state'] = DPU_STATE_READY - self._set_ready_status(name, 'true') - self._set_last_ready_time(name) - self.log_info("{}: DPU recovered (manual intervention)".format(name)) - elif auto_recovery_enabled: - # Auto-recovery was re-enabled while the DPU is still - # down — start a power-cycle. - self._power_cycle_dpu(name, module_index) - continue + if current_state == DPU_STATE_MANUAL_INTERVENTION: + # Operator may have recovered the DPU manually, or can + # set admin-down (caught by admin-down check above) to + # transition to AdminDown state. + if all_up: + self._enter_ready(name, recovery, "DPU recovered (manual intervention)") + elif auto_recovery_enabled: + # Auto-recovery was re-enabled while the DPU is still + # down — start a power-cycle. + self._enter_power_cycle_or_unrecoverable(name, module_index) + return # @@ -1661,7 +1748,7 @@ class DpuStateUpdater(logger.Logger): self.id = self.chassis.get_dpu_id() self.name = f'DPU{self.id}' - self.dpu_state_table = swsscommon.Table(self.chassis_state_db, 'DPU_STATE') + self.dpu_state_table = swsscommon.Table(self.chassis_state_db, DPU_STATE_TABLE) def _get_data_plane_state_common(self): port_table = swsscommon.Table(self.app_db, 'PORT_TABLE') @@ -1782,7 +1869,7 @@ class ChassisdDaemon(daemon_base.DaemonBase): op = MODULE_ADMIN_DOWN # Initialize DPU_STATE DB table on bootup - dpu_state_key = "DPU_STATE|" + module_name + dpu_state_key = DPU_STATE_TABLE + "|" + module_name if operational_state == ModuleBase.MODULE_STATUS_ONLINE: op_state = 'up' else: diff --git a/sonic-chassisd/tests/mock_platform.py b/sonic-chassisd/tests/mock_platform.py index 1f7b68f69..fb157f21d 100644 --- a/sonic-chassisd/tests/mock_platform.py +++ b/sonic-chassisd/tests/mock_platform.py @@ -73,10 +73,10 @@ def set_midplane_ip(self): self.midplane_ip = '192.168.1.{}'.format(self.get_slot()) def module_pre_shutdown(self): - pass + return self.pci_detach() def module_post_startup(self): - pass + return self.pci_reattach() def set_admin_state_gracefully(self, up): """Mock implementation of set_admin_state_gracefully""" @@ -86,10 +86,26 @@ def clear_module_state_transition(self, module_name): """Mock implementation of clear_module_state_transition""" return True + def set_module_state_transition(self, module_name, transition_type): + """Mock implementation of set_module_state_transition""" + return True + + def get_module_state_transition(self, module_name): + """Mock implementation of get_module_state_transition""" + return False + def clear_module_gnoi_halt_in_progress(self): """Mock implementation of clear_module_gnoi_halt_in_progress""" return True + def pci_detach(self): + """Mock implementation of pci_detach""" + return True + + def pci_reattach(self): + """Mock implementation of pci_reattach""" + return True + def is_midplane_reachable(self): return self.midplane_access @@ -225,7 +241,7 @@ def get_revision(self): def is_smartswitch(self): return self._is_smartswitch - + def get_dataplane_state(self): raise NotImplementedError diff --git a/sonic-chassisd/tests/mock_swsscommon.py b/sonic-chassisd/tests/mock_swsscommon.py index 8b76a7ef0..dde4a8a82 100644 --- a/sonic-chassisd/tests/mock_swsscommon.py +++ b/sonic-chassisd/tests/mock_swsscommon.py @@ -100,3 +100,17 @@ def connect(*args, **kwargs): def get_table(*args, **kwargs): pass + +class SonicV2Connector: + + def __init__(self, *args, **kwargs): + pass + + def connect(self, *args, **kwargs): + pass + + def get(self, *args, **kwargs): + pass + + def keys(self, *args, **kwargs): + return [] diff --git a/sonic-chassisd/tests/test_dpu_auto_recovery.py b/sonic-chassisd/tests/test_dpu_auto_recovery.py index b80ea84c6..12536d50a 100644 --- a/sonic-chassisd/tests/test_dpu_auto_recovery.py +++ b/sonic-chassisd/tests/test_dpu_auto_recovery.py @@ -6,7 +6,7 @@ Offline, ManualIntervention, Unrecoverable) - DPU_STATE DB fields: ready_status, recovery_status, reset_count, last_down_time, last_ready_time -- Auto-recovery feature flag gating (enabled/disabled/always_disabled) +- Auto-recovery DEVICE_METADATA flag gating (enabled/disabled) - Recovery initialization on chassisd startup (init_dpu_recovery_state) - NPU crash detection and forced power-cycle on boot - Reset limit enforcement and unrecoverable marking @@ -39,7 +39,7 @@ DPU_STATE_READY, DPU_STATE_WAIT_FOR_SELF_RECOVERY, DPU_STATE_POWER_CYCLE, - DPU_STATE_OFFLINE, + DPU_STATE_ADMIN_DOWN, DPU_STATE_MANUAL_INTERVENTION, DPU_STATE_UNRECOVERABLE, READY_STATUS, @@ -59,6 +59,8 @@ MODULE_REBOOT_CAUSE_DIR, MAX_HISTORY_FILES, REBOOT_CAUSE_FILE, + WAS_UNRECOVERABLE_KEY, + TRANSITION_TYPE_RECOVERY, ) @@ -155,8 +157,9 @@ def test_init_npu_crash_power_cycles_admin_up_dpus(self): updater.module_table.hset("DPU1", "oper_status", str(ModuleBase.MODULE_STATUS_OFFLINE)) with patch.object(updater, '_npu_crash_on_last_boot', return_value=True), \ + patch.object(updater, 'get_module_admin_status', side_effect=lambda name: 'up' if name == 'DPU0' else 'down'), \ patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ - patch.object(updater, '_power_cycle_dpu') as mock_pc: + patch.object(updater, '_enter_power_cycle_or_unrecoverable') as mock_pc: updater.init_dpu_recovery_state() # DPU0 (online) should be power-cycled, DPU1 (offline) should not @@ -191,13 +194,15 @@ def test_detects_kernel_panic(self): patch("builtins.open", mock_open(read_data="Kernel Panic - not syncing")): assert updater._npu_crash_on_last_boot() is True - def test_detects_unknown_cause(self): + def test_unknown_cause_not_crash(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) + # An "unknown" reboot cause is reported on first boot and must NOT be + # classified as a crash, otherwise every fresh boot would power-cycle. with patch("os.path.isfile", return_value=True), \ patch("builtins.open", mock_open(read_data="Unknown")): - assert updater._npu_crash_on_last_boot() is True + assert updater._npu_crash_on_last_boot() is False def test_normal_reboot_not_crash(self): chassis = create_chassis_with_dpus(1) @@ -216,62 +221,51 @@ def test_missing_file_not_crash(self): # ============================================================================ -# Test: Auto-recovery feature flag +# Test: Auto-recovery DEVICE_METADATA flag # ============================================================================ class TestAutoRecoveryFeatureFlag: - """Test _is_auto_recovery_enabled() feature flag checking.""" + """Test _is_auto_recovery_enabled() reading from DEVICE_METADATA.""" def test_enabled_returns_true(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - mock_table = MagicMock() - mock_table.get.return_value = [True, (('state', 'enabled'),)] - - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_table): - assert updater._is_auto_recovery_enabled() is True + updater.device_metadata_table = MagicMock() + updater.device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + assert updater._is_auto_recovery_enabled() is True def test_disabled_returns_false(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - mock_table = MagicMock() - mock_table.get.return_value = [True, (('state', 'disabled'),)] - - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_table): - assert updater._is_auto_recovery_enabled() is False + updater.device_metadata_table = MagicMock() + updater.device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'disable'),)] + assert updater._is_auto_recovery_enabled() is False - def test_always_disabled_returns_false(self): + def test_missing_field_defaults_to_disabled(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - mock_table = MagicMock() - mock_table.get.return_value = [True, (('state', 'always_disabled'),)] + updater.device_metadata_table = MagicMock() + updater.device_metadata_table.get.return_value = [True, (('hostname', 'sonic'),)] + assert updater._is_auto_recovery_enabled() is False - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_table): - assert updater._is_auto_recovery_enabled() is False - - def test_missing_feature_returns_false(self): + def test_missing_localhost_defaults_to_disabled(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - mock_table = MagicMock() - mock_table.get.return_value = None + updater.device_metadata_table = MagicMock() + updater.device_metadata_table.get.return_value = None + assert updater._is_auto_recovery_enabled() is False - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_table): - assert updater._is_auto_recovery_enabled() is False - - def test_db_exception_returns_false(self): + def test_db_exception_defaults_to_disabled(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - with patch("chassisd.daemon_base.db_connect", side_effect=Exception("DB error")): - assert updater._is_auto_recovery_enabled() is False + updater.device_metadata_table = MagicMock() + updater.device_metadata_table.get.side_effect = Exception("DB error") + assert updater._is_auto_recovery_enabled() is False # ============================================================================ @@ -290,8 +284,10 @@ def test_planned_transition_in_progress_suppresses_recovery(self): set_dpu_states(updater, "DPU0", mp='up', cp='down', dp='up') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_ONLINE) updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - # Mark a planned transition as in progress - updater.module_table.hset("DPU0", "state_transition_in_progress", "True") + # Mark a planned transition as in progress via platform API mock + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=True) + updater.module_table.hset("DPU0", "transition_in_progress", "True") + updater.module_table.hset("DPU0", "transition_type", "shutdown") with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'): @@ -309,8 +305,8 @@ def test_no_planned_transition_allows_recovery(self): set_dpu_states(updater, "DPU0", mp='up', cp='down', dp='up') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_ONLINE) updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - # No state_transition_in_progress set (or set to False) - updater.module_table.hset("DPU0", "state_transition_in_progress", "False") + # No transition_in_progress set (or set to False) + updater.module_table.hset("DPU0", "transition_in_progress", "False") with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'): @@ -329,14 +325,17 @@ def test_planned_transition_completed_allows_recovery(self): updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) # First poll: transition in progress — suppressed - updater.module_table.hset("DPU0", "state_transition_in_progress", "True") + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=True) + updater.module_table.hset("DPU0", "transition_in_progress", "True") + updater.module_table.hset("DPU0", "transition_type", "shutdown") with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_READY # Second poll: transition done — recovery proceeds - updater.module_table.hset("DPU0", "state_transition_in_progress", "False") + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=False) + updater.module_table.hset("DPU0", "transition_in_progress", "False") with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() @@ -351,7 +350,9 @@ def test_planned_transition_midplane_down_suppressed(self): set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_ONLINE) updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - updater.module_table.hset("DPU0", "state_transition_in_progress", "True") + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=True) + updater.module_table.hset("DPU0", "transition_in_progress", "True") + updater.module_table.hset("DPU0", "transition_type", "shutdown") with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'): @@ -360,11 +361,11 @@ def test_planned_transition_midplane_down_suppressed(self): assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_READY def test_is_planned_transition_missing_field_returns_false(self): - """Missing state_transition_in_progress field returns False (no suppression).""" + """Missing transition_in_progress field returns False (no suppression).""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - # Don't set the field at all — module_table.get returns empty - assert updater._is_planned_transition_in_progress("DPU0") is False + # Don't set the field at all — get_module_state_transition returns False + assert updater._is_planned_transition_in_progress("DPU0", 0) is False # ============================================================================ @@ -426,11 +427,30 @@ def test_booting_timeout_triggers_power_cycle(self): with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'), \ - patch.object(updater, '_power_cycle_dpu') as mock_pc: + patch.object(updater, '_enter_power_cycle_or_unrecoverable') as mock_pc: updater.update_dpu_recovery_state() mock_pc.assert_called_once_with("DPU0", 0) + def test_booting_timeout_does_not_refresh_last_down_time(self): + """Boot timeout retry should not overwrite initial outage timestamp.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_BOOTING + import time as time_module + updater.dpu_recovery_state["DPU0"]['boot_start_time'] = time_module.time() - 700 + + set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') + chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_ONLINE) + updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) + + with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ + patch.object(updater, 'get_module_admin_status', return_value='up'), \ + patch.object(updater, '_set_last_down_time') as mock_set_down: + updater.update_dpu_recovery_state() + + mock_set_down.assert_not_called() + def test_booting_timeout_manual_intervention_when_disabled(self): """Booting timeout + auto-recovery disabled → ManualIntervention.""" chassis = create_chassis_with_dpus(1) @@ -483,11 +503,31 @@ def test_power_cycle_timeout_triggers_another_cycle(self): with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'), \ - patch.object(updater, '_power_cycle_dpu') as mock_pc: + patch.object(updater, '_enter_power_cycle_or_unrecoverable') as mock_pc: updater.update_dpu_recovery_state() mock_pc.assert_called_once_with("DPU0", 0) + def test_power_cycle_timeout_does_not_refresh_last_down_time(self): + """PowerCycle timeout retry should not overwrite initial outage timestamp.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_POWER_CYCLE + updater.dpu_recovery_state["DPU0"]['reset_count'] = 1 + import time as time_module + updater.dpu_recovery_state["DPU0"]['boot_start_time'] = time_module.time() - 700 + + set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') + chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_ONLINE) + updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) + + with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ + patch.object(updater, 'get_module_admin_status', return_value='up'), \ + patch.object(updater, '_set_last_down_time') as mock_set_down: + updater.update_dpu_recovery_state() + + mock_set_down.assert_not_called() + def test_power_cycle_timeout_manual_intervention_when_disabled(self): """PowerCycle state exceeds dpu_boot_timeout + auto-recovery disabled → ManualIntervention.""" chassis = create_chassis_with_dpus(1) @@ -533,7 +573,7 @@ def test_booting_timeout_cp_up_dp_down_warning_only(self): with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'), \ - patch.object(updater, '_power_cycle_dpu') as mock_pc: + patch.object(updater, '_enter_power_cycle_or_unrecoverable') as mock_pc: updater.update_dpu_recovery_state() # Should NOT power-cycle — only log warning @@ -579,7 +619,7 @@ def test_booting_timeout_midplane_down_triggers_power_cycle(self): with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'), \ - patch.object(updater, '_power_cycle_dpu') as mock_pc: + patch.object(updater, '_enter_power_cycle_or_unrecoverable') as mock_pc: updater.update_dpu_recovery_state() mock_pc.assert_called_once_with("DPU0", 0) @@ -610,11 +650,11 @@ def test_ready_midplane_down_enters_wait_for_self_recovery(self): assert get_dpu_state_field(updater, "DPU0", READY_STATUS) == 'false' assert 'self_recovery_start_time' in updater.dpu_recovery_state["DPU0"] - def test_ready_midplane_down_wait_for_self_recovery_regardless_of_flag(self): - """Ready + midplane down + auto-recovery disabled → still WaitForSelfRecovery. + def test_ready_midplane_down_auto_recovery_disabled_goes_to_manual_intervention(self): + """Ready + midplane down + auto-recovery disabled → ManualIntervention. - The auto-recovery flag only gates the power-cycle action after timeout, - not the initial transition to WaitForSelfRecovery.""" + Per HLD: when auto-recovery is disabled, skip WaitForSelfRecovery and + go directly to ManualIntervention.""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_READY @@ -627,7 +667,7 @@ def test_ready_midplane_down_wait_for_self_recovery_regardless_of_flag(self): patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_WAIT_FOR_SELF_RECOVERY + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_MANUAL_INTERVENTION def test_ready_cp_down_enters_wait_for_self_recovery(self): """Ready + CP down (midplane up) → WaitForSelfRecovery.""" @@ -940,7 +980,7 @@ def test_reaches_reset_limit_becomes_unrecoverable(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis, platform_json={"dpu_reset_limit": 3}) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY - updater.dpu_recovery_state["DPU0"]['reset_count'] = 2 # one below limit + updater.dpu_recovery_state["DPU0"]['reset_count'] = 3 # at limit import time as time_module updater.dpu_recovery_state["DPU0"]['self_recovery_start_time'] = time_module.time() - 400 @@ -954,7 +994,6 @@ def test_reaches_reset_limit_becomes_unrecoverable(self): assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_UNRECOVERABLE assert get_dpu_state_field(updater, "DPU0", RECOVERY_STATUS) == RECOVERY_UNRECOVERABLE - assert get_dpu_state_field(updater, "DPU0", RESET_COUNT) == '3' def test_unrecoverable_is_terminal(self): """Once unrecoverable, state doesn't change even if DPU comes back.""" @@ -987,8 +1026,8 @@ def test_unrecoverable_admin_down_transitions_to_offline(self): patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE - assert updater.dpu_recovery_state["DPU0"].get('was_unrecoverable') is True + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN + assert updater.dpu_recovery_state["DPU0"].get(WAS_UNRECOVERABLE_KEY) is True def test_unrecoverable_operator_startup_resets_recovery(self): """Unrecoverable → Offline → Booting resets reset_count and recovery_status. @@ -999,8 +1038,8 @@ def test_unrecoverable_operator_startup_resets_recovery(self): """ chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_OFFLINE - updater.dpu_recovery_state["DPU0"]['was_unrecoverable'] = True + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_ADMIN_DOWN + updater.dpu_recovery_state["DPU0"][WAS_UNRECOVERABLE_KEY] = True updater.dpu_recovery_state["DPU0"]['reset_count'] = 2 updater.dpu_state_table.hset("DPU0", RESET_COUNT, '2') updater.dpu_state_table.hset("DPU0", RECOVERY_STATUS, RECOVERY_UNRECOVERABLE) @@ -1053,7 +1092,7 @@ def test_multiple_failures_increment_reset_count(self): patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() - # 5th failure: Ready → WaitForSelfRecovery → timeout → Unrecoverable + # 5th failure: Ready → WaitForSelfRecovery → timeout → PowerCycle (count=5) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_READY set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ @@ -1065,6 +1104,16 @@ def test_multiple_failures_increment_reset_count(self): patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() + assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 5 + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_POWER_CYCLE + + # 6th attempt: PowerCycle timeout → Unrecoverable (count=5 >= limit=5) + import time as time_module + updater.dpu_recovery_state["DPU0"]['boot_start_time'] = time_module.time() - 700 + with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ + patch.object(updater, 'get_module_admin_status', return_value='up'): + updater.update_dpu_recovery_state() + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_UNRECOVERABLE assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 5 @@ -1090,14 +1139,14 @@ def test_admin_down_transitions_to_offline(self): patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN assert get_dpu_state_field(updater, "DPU0", READY_STATUS) == 'false' def test_admin_up_from_offline_transitions_to_booting(self): """Admin-up from Offline → Booting.""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_OFFLINE + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_ADMIN_DOWN set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_PRESENT) @@ -1197,10 +1246,10 @@ def test_hardware_offline_manual_intervention_when_disabled(self): # ============================================================================ class TestPowerCycleDpu: - """Test _power_cycle_dpu() method.""" + """Test _enter_power_cycle_or_unrecoverable() method.""" def test_power_cycle_calls_admin_state(self): - """_power_cycle_dpu should call set_admin_state(DOWN) then set_admin_state(UP).""" + """_enter_power_cycle_or_unrecoverable should call set_admin_state(DOWN) then set_admin_state(UP).""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY @@ -1209,7 +1258,7 @@ def test_power_cycle_calls_admin_state(self): module = chassis.module_list[0] module.set_admin_state = MagicMock() - updater._power_cycle_dpu("DPU0", 0) + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) # Should be called twice: once with DOWN, once with UP calls = module.set_admin_state.call_args_list @@ -1218,13 +1267,13 @@ def test_power_cycle_calls_admin_state(self): assert calls[1][0][0] == MODULE_ADMIN_UP def test_power_cycle_increments_reset_count(self): - """Each _power_cycle_dpu call increments reset_count.""" + """Each _enter_power_cycle_or_unrecoverable call increments reset_count.""" chassis = create_chassis_with_dpus(1) - updater = create_updater(chassis) + updater = create_updater(chassis, platform_json={"dpu_reset_limit": 5}) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY updater.dpu_recovery_state["DPU0"]['reset_count'] = 2 - updater._power_cycle_dpu("DPU0", 0) + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 3 assert get_dpu_state_field(updater, "DPU0", RESET_COUNT) == '3' @@ -1234,9 +1283,9 @@ def test_power_cycle_at_limit_marks_unrecoverable(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis, platform_json={"dpu_reset_limit": 3}) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY - updater.dpu_recovery_state["DPU0"]['reset_count'] = 2 # limit is 3 + updater.dpu_recovery_state["DPU0"]['reset_count'] = 3 # at limit - updater._power_cycle_dpu("DPU0", 0) + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_UNRECOVERABLE assert get_dpu_state_field(updater, "DPU0", RECOVERY_STATUS) == RECOVERY_UNRECOVERABLE @@ -1370,7 +1419,7 @@ def test_one_dpu_unrecoverable_others_continue(self): # DPU0: at limit, in WaitForSelfRecovery with expired timeout → will become unrecoverable updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY - updater.dpu_recovery_state["DPU0"]['reset_count'] = 1 + updater.dpu_recovery_state["DPU0"]['reset_count'] = 2 updater.dpu_recovery_state["DPU0"]['self_recovery_start_time'] = time.time() - 400 set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_ONLINE) @@ -1491,7 +1540,7 @@ def test_full_cycle_admin_down_up(self): with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN # Phase 2: Admin-up (DPU starts booting) with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ @@ -1535,7 +1584,7 @@ def test_admin_down_from_any_state_transitions_to_offline(self, initial_state): patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN assert get_dpu_state_field(updater, "DPU0", READY_STATUS) == 'false' def test_admin_down_from_unrecoverable_goes_offline(self): @@ -1552,14 +1601,14 @@ def test_admin_down_from_unrecoverable_goes_offline(self): patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE - assert updater.dpu_recovery_state["DPU0"].get('was_unrecoverable') is True + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN + assert updater.dpu_recovery_state["DPU0"].get(WAS_UNRECOVERABLE_KEY) is True def test_admin_down_already_offline_stays_offline(self): """Already Offline + admin-down → stays Offline (no-op).""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_OFFLINE + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_ADMIN_DOWN set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_OFFLINE) @@ -1569,7 +1618,7 @@ def test_admin_down_already_offline_stays_offline(self): patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN # ============================================================================ @@ -1601,7 +1650,7 @@ def test_hardware_offline_while_in_recovery_no_duplicate(self, recovery_state): with patch.object(updater, '_is_auto_recovery_enabled', return_value=True), \ patch.object(updater, 'get_module_admin_status', return_value='up'), \ - patch.object(updater, '_power_cycle_dpu') as mock_pc: + patch.object(updater, '_enter_power_cycle_or_unrecoverable') as mock_pc: updater.update_dpu_recovery_state() # The hardware-offline code path should be skipped for these states @@ -1648,12 +1697,16 @@ def test_hardware_offline_from_ready_triggers_power_cycle(self): assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_POWER_CYCLE - def test_hardware_offline_from_offline_state_triggers_recovery(self): - """oper_status=Offline from Offline state + admin-up → hardware-offline - branch fires (Offline is NOT in the exclusion list), triggering power-cycle.""" + def test_hardware_offline_from_admin_down_restarts_booting(self): + """admin-up + current_state still AdminDown + stale oper_status=Offline + must take the clean AdminDown -> Booting restart path, NOT the + hardware-offline power-cycle path. Only a previously-Ready DPU treats + Offline as a new hardware failure, so this avoids the `module startup` + race that would otherwise power-cycle (and bump reset_count).""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_OFFLINE + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_ADMIN_DOWN + updater.dpu_recovery_state["DPU0"]['reset_count'] = 0 set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') chassis.module_list[0].set_oper_status(ModuleBase.MODULE_STATUS_OFFLINE) @@ -1663,9 +1716,10 @@ def test_hardware_offline_from_offline_state_triggers_recovery(self): patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() - # Offline is not in the hardware-offline exclusion list, so the - # hardware-offline branch fires and triggers a power-cycle - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_POWER_CYCLE + # AdminDown falls through to the clean restart path instead of being + # treated as a hardware failure; reset_count is left untouched. + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_BOOTING + assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 0 # ============================================================================ @@ -1759,7 +1813,7 @@ def test_feature_toggle_cycle_disabled_then_reenabled(self): # ============================================================================ class TestPowerCycleFailure: - """Test _power_cycle_dpu() behavior when set_admin_state raises exceptions.""" + """Test _enter_power_cycle_or_unrecoverable() behavior when set_admin_state raises exceptions.""" def test_power_cycle_set_admin_state_exception(self): """If set_admin_state raises, state should still update (best-effort).""" @@ -1771,7 +1825,7 @@ def test_power_cycle_set_admin_state_exception(self): module = chassis.module_list[0] module.set_admin_state = MagicMock(side_effect=Exception("I2C bus error")) - updater._power_cycle_dpu("DPU0", 0) + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) # Despite the exception, reset_count should still increment assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 1 @@ -1780,16 +1834,16 @@ def test_power_cycle_set_admin_state_exception(self): assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_POWER_CYCLE def test_power_cycle_exception_at_limit_still_marks_unrecoverable(self): - """Exception during power-cycle at reset_limit still marks unrecoverable.""" + """At reset_limit, DPU is marked unrecoverable without attempting power-cycle.""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis, platform_json={"dpu_reset_limit": 2}) updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY - updater.dpu_recovery_state["DPU0"]['reset_count'] = 1 + updater.dpu_recovery_state["DPU0"]['reset_count'] = 2 module = chassis.module_list[0] module.set_admin_state = MagicMock(side_effect=RuntimeError("HW error")) - updater._power_cycle_dpu("DPU0", 0) + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_UNRECOVERABLE assert get_dpu_state_field(updater, "DPU0", RECOVERY_STATUS) == RECOVERY_UNRECOVERABLE @@ -1812,11 +1866,92 @@ def side_effect_fn(state): module = chassis.module_list[0] module.set_admin_state = MagicMock(side_effect=side_effect_fn) - updater._power_cycle_dpu("DPU0", 0) + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) # Should still transition (try_get catches the exception) assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 1 + def test_power_cycle_calls_pci_detach_and_reattach(self): + """Power-cycle sequence calls pci_detach before admin_down and pci_reattach after admin_up.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY + updater.dpu_recovery_state["DPU0"]['reset_count'] = 0 + + module = chassis.module_list[0] + call_order = [] + module.pci_detach = MagicMock(side_effect=lambda: call_order.append('pci_detach')) + module.set_admin_state = MagicMock(side_effect=lambda s: call_order.append(f'set_admin_state({s})')) + module.pci_reattach = MagicMock(side_effect=lambda: call_order.append('pci_reattach')) + + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) + + # Verify pci_detach and pci_reattach are called + module.pci_detach.assert_called_once() + module.pci_reattach.assert_called_once() + # Verify order: pci_detach → admin_down → admin_up → pci_reattach + assert call_order == ['pci_detach', 'set_admin_state(0)', 'set_admin_state(1)', 'pci_reattach'] + + def test_power_cycle_acquires_and_releases_state_transition(self): + """Power-cycle calls set_module_state_transition before and clear after.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY + updater.dpu_recovery_state["DPU0"]['reset_count'] = 0 + + module = chassis.module_list[0] + module.set_module_state_transition = MagicMock(return_value=True) + module.clear_module_state_transition = MagicMock(return_value=True) + + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) + + module.set_module_state_transition.assert_called_once_with("DPU0", "recovery") + module.clear_module_state_transition.assert_called_once_with("DPU0") + + def test_power_cycle_clears_state_transition_on_exception(self): + """clear_module_state_transition is called even if power-cycle raises.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY + updater.dpu_recovery_state["DPU0"]['reset_count'] = 0 + + module = chassis.module_list[0] + module.set_module_state_transition = MagicMock(return_value=True) + module.clear_module_state_transition = MagicMock(return_value=True) + module.set_admin_state = MagicMock(side_effect=Exception("HW fault")) + + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) + + module.set_module_state_transition.assert_called_once_with("DPU0", "recovery") + module.clear_module_state_transition.assert_called_once_with("DPU0") + + def test_power_cycle_skipped_when_lock_not_acquired(self): + """If set_module_state_transition returns False, power-cycle is aborted.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY + updater.dpu_recovery_state["DPU0"]['reset_count'] = 0 + + module = chassis.module_list[0] + module.set_module_state_transition = MagicMock(return_value=False) + module.clear_module_state_transition = MagicMock() + module.set_admin_state = MagicMock() + module.pci_detach = MagicMock() + module.pci_reattach = MagicMock() + + updater._enter_power_cycle_or_unrecoverable("DPU0", 0) + + # Power-cycle should NOT have been executed + module.pci_detach.assert_not_called() + module.set_admin_state.assert_not_called() + module.pci_reattach.assert_not_called() + module.clear_module_state_transition.assert_not_called() + # reset_count should be rolled back to 0 + assert updater.dpu_recovery_state["DPU0"]['reset_count'] == 0 + # State should have entered POWER_CYCLE to wait for the in-progress + # operation (gnoi shutdown/reboot) to complete. + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_POWER_CYCLE + # ============================================================================ # Test: DB setter methods @@ -2223,7 +2358,7 @@ def test_all_dpus_fail_simultaneously(self): def test_reset_count_independent_per_dpu(self): """Each DPU's reset_count increments independently.""" chassis = create_chassis_with_dpus(2) - updater = create_updater(chassis) + updater = create_updater(chassis, platform_json={"dpu_reset_limit": 10}) # DPU0: WaitForSelfRecovery with expired timeout, reset_count=3 updater.dpu_recovery_state["DPU0"]['state'] = DPU_STATE_WAIT_FOR_SELF_RECOVERY @@ -2327,25 +2462,37 @@ def test_is_planned_transition_exception_returns_false(self): chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - # Make module_table.get raise an exception - updater.module_table.get = MagicMock(side_effect=Exception("DB error")) - assert updater._is_planned_transition_in_progress("DPU0") is False + # Make get_module_state_transition raise NotImplementedError — try_get returns default=False + chassis.module_list[0].get_module_state_transition = MagicMock(side_effect=NotImplementedError("not supported")) + assert updater._is_planned_transition_in_progress("DPU0", 0) is False def test_is_planned_transition_true(self): - """_is_planned_transition_in_progress returns True when field is 'True'.""" + """_is_planned_transition_in_progress returns True when field is 'True' and type != recovery.""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - updater.module_table.hset("DPU0", "state_transition_in_progress", "True") - assert updater._is_planned_transition_in_progress("DPU0") is True + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=True) + updater.module_table.hset("DPU0", "transition_in_progress", "True") + updater.module_table.hset("DPU0", "transition_type", "shutdown") + assert updater._is_planned_transition_in_progress("DPU0", 0) is True def test_is_planned_transition_false_value(self): - """_is_planned_transition_in_progress returns False when field is 'False'.""" + """_is_planned_transition_in_progress returns False when get_module_state_transition returns False.""" + chassis = create_chassis_with_dpus(1) + updater = create_updater(chassis) + + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=False) + assert updater._is_planned_transition_in_progress("DPU0", 0) is False + + def test_is_planned_transition_recovery_type_returns_false(self): + """_is_planned_transition_in_progress returns False when transition_type is 'recovery'.""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - updater.module_table.hset("DPU0", "state_transition_in_progress", "False") - assert updater._is_planned_transition_in_progress("DPU0") is False + chassis.module_list[0].get_module_state_transition = MagicMock(return_value=True) + updater.module_table.hset("DPU0", "transition_in_progress", "True") + updater.module_table.hset("DPU0", "transition_type", "recovery") + assert updater._is_planned_transition_in_progress("DPU0", 0) is False def test_npu_crash_file_read_exception(self): """Exception reading reboot cause file returns False.""" @@ -2392,18 +2539,15 @@ def test_auto_recovery_feature_not_found_in_table(self): patch("chassisd.swsscommon.Table", return_value=mock_table): assert updater._is_auto_recovery_enabled() is False - def test_auto_recovery_missing_state_field_defaults_enabled(self): - """_is_auto_recovery_enabled with no 'state' field defaults to enabled.""" + def test_auto_recovery_missing_state_field_defaults_disabled(self): + """_is_auto_recovery_enabled with no 'dpu_auto_recovery' field defaults to disabled.""" chassis = create_chassis_with_dpus(1) updater = create_updater(chassis) - mock_table = MagicMock() - # Feature exists but has no 'state' field - mock_table.get.return_value = [True, (('description', 'DPU Auto Recovery'),)] - - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_table): - assert updater._is_auto_recovery_enabled() is True + updater.device_metadata_table = MagicMock() + # Feature exists but has no 'dpu_auto_recovery' field + updater.device_metadata_table.get.return_value = [True, (('description', 'DPU Auto Recovery'),)] + assert updater._is_auto_recovery_enabled() is False class TestGetModuleAdminStatus: @@ -2453,14 +2597,12 @@ def test_booting_to_ready_full_path(self): set_dpu_states(updater, "DPU0", mp='up', cp='up', dp='up') updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - # Mock only the DB access for _is_auto_recovery_enabled - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'enabled'),)] + # Set device_metadata_table directly for _is_auto_recovery_enabled + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + updater.device_metadata_table = mock_device_metadata_table - # Mock get_module_admin_status since it creates a new DB connection - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table), \ - patch.object(updater, 'get_module_admin_status', return_value='up'): + with patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_READY @@ -2473,14 +2615,13 @@ def test_ready_cp_down_enters_wait_for_self_recovery(self): set_dpu_states(updater, "DPU0", mp='up', cp='down', dp='up') updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - updater.module_table.hset("DPU0", "state_transition_in_progress", "False") + updater.module_table.hset("DPU0", "transition_in_progress", "False") - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'enabled'),)] + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + updater.device_metadata_table = mock_device_metadata_table - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table), \ - patch.object(updater, 'get_module_admin_status', return_value='up'): + with patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_WAIT_FOR_SELF_RECOVERY @@ -2494,15 +2635,14 @@ def test_admin_down_to_offline_full_path(self): set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_OFFLINE)) - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'enabled'),)] + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + updater.device_metadata_table = mock_device_metadata_table - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table), \ - patch.object(updater, 'get_module_admin_status', return_value='down'): + with patch.object(updater, 'get_module_admin_status', return_value='down'): updater.update_dpu_recovery_state() - assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_OFFLINE + assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_ADMIN_DOWN def test_hardware_offline_triggers_power_cycle_full_path(self): """Hardware offline with auto-recovery enabled triggers power cycle (full path).""" @@ -2513,17 +2653,16 @@ def test_hardware_offline_triggers_power_cycle_full_path(self): set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_OFFLINE)) - updater.module_table.hset("DPU0", "state_transition_in_progress", "False") + updater.module_table.hset("DPU0", "transition_in_progress", "False") - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'enabled'),)] + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + updater.device_metadata_table = mock_device_metadata_table module = chassis.module_list[0] module.set_admin_state = MagicMock() - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table), \ - patch.object(updater, 'get_module_admin_status', return_value='up'): + with patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() # Should have power-cycled (set_admin_state called) @@ -2542,17 +2681,16 @@ def test_wait_for_self_recovery_timeout_power_cycle_full_path(self): set_dpu_states(updater, "DPU0", mp='down', cp='down', dp='down') updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - updater.module_table.hset("DPU0", "state_transition_in_progress", "False") + updater.module_table.hset("DPU0", "transition_in_progress", "False") - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'enabled'),)] + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + updater.device_metadata_table = mock_device_metadata_table module = chassis.module_list[0] module.set_admin_state = MagicMock() - with patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table), \ - patch.object(updater, 'get_module_admin_status', return_value='up'): + with patch.object(updater, 'get_module_admin_status', return_value='up'): updater.update_dpu_recovery_state() # Should have power-cycled @@ -2567,8 +2705,9 @@ def test_init_recovery_state_npu_crash_full_path(self): updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) updater.module_table.hset("DPU1", "oper_status", str(ModuleBase.MODULE_STATUS_OFFLINE)) - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'enabled'),)] + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'enable'),)] + updater.device_metadata_table = mock_device_metadata_table module = chassis.module_list[0] module.set_admin_state = MagicMock() @@ -2576,9 +2715,8 @@ def test_init_recovery_state_npu_crash_full_path(self): module1.set_admin_state = MagicMock() with patch("os.path.isfile", return_value=True), \ - patch("builtins.open", mock_open(read_data="kernel panic - not syncing")), \ - patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table): + patch("builtins.open", mock_open(read_data="kernel panic - not syncing")), \ + patch.object(updater, 'get_module_admin_status', side_effect=lambda name: 'up' if name == 'DPU0' else 'down'): updater.init_dpu_recovery_state() # DPU0 should be power-cycled (online + NPU crash + auto-recovery enabled) @@ -2593,13 +2731,12 @@ def test_init_recovery_state_npu_crash_disabled_full_path(self): updater.module_table.hset("DPU0", "oper_status", str(ModuleBase.MODULE_STATUS_ONLINE)) - mock_feature_table = MagicMock() - mock_feature_table.get.return_value = [True, (('state', 'disabled'),)] + mock_device_metadata_table = MagicMock() + mock_device_metadata_table.get.return_value = [True, (('dpu_auto_recovery', 'disable'),)] + updater.device_metadata_table = mock_device_metadata_table with patch("os.path.isfile", return_value=True), \ - patch("builtins.open", mock_open(read_data="Unknown")), \ - patch("chassisd.daemon_base.db_connect") as mock_db, \ - patch("chassisd.swsscommon.Table", return_value=mock_feature_table): + patch("builtins.open", mock_open(read_data="kernel panic - not syncing")): updater.init_dpu_recovery_state() assert updater.dpu_recovery_state["DPU0"]['state'] == DPU_STATE_MANUAL_INTERVENTION diff --git a/sonic-chassisd/tests/testbed/test_dpu_auto_recovery.py b/sonic-chassisd/tests/testbed/test_dpu_auto_recovery.py index 4019fbbc1..df0f67643 100644 --- a/sonic-chassisd/tests/testbed/test_dpu_auto_recovery.py +++ b/sonic-chassisd/tests/testbed/test_dpu_auto_recovery.py @@ -57,8 +57,8 @@ RECOVERY_RECOVERABLE = 'recoverable' RECOVERY_UNRECOVERABLE = 'unrecoverable' -# Feature flag -DPU_AUTO_RECOVERY_FEATURE = 'dpu-auto-recovery' +# DEVICE_METADATA field for auto-recovery +DPU_AUTO_RECOVERY_FIELD = 'dpu_auto_recovery' # Timeouts DPU_BOOT_TIMEOUT = 420 # 7 minutes for DPU to come up after power-cycle @@ -134,19 +134,25 @@ def get_module_oper_status(dpu_name): return result if result else None -def get_feature_state(feature_name): - """Get feature state from CONFIG_DB FEATURE table.""" +def set_module_oper_status(dpu_name, status): + """Set oper_status in STATE_DB CHASSIS_MODULE_TABLE (test-only fault injection).""" + sonic_db_cli("STATE_DB", f"HSET 'CHASSIS_MODULE_TABLE|{dpu_name}' 'oper_status' '{status}'") + logger.info(f"Set {dpu_name} oper_status to '{status}'") + + +def get_auto_recovery_state(): + """Get dpu_auto_recovery from CONFIG_DB DEVICE_METADATA|localhost.""" result = run_cmd( - f"sonic-db-cli CONFIG_DB HGET 'FEATURE|{feature_name}' 'state'", + "sonic-db-cli CONFIG_DB HGET 'DEVICE_METADATA|localhost' 'dpu_auto_recovery'", check=False ) return result if result else None -def set_feature_state(feature_name, state): - """Set feature state in CONFIG_DB FEATURE table.""" - sonic_db_cli("CONFIG_DB", f"HSET 'FEATURE|{feature_name}' 'state' '{state}'") - logger.info(f"Set feature {feature_name} state to '{state}'") +def set_auto_recovery_state(state): + """Set dpu_auto_recovery in CONFIG_DB DEVICE_METADATA|localhost.""" + sonic_db_cli("CONFIG_DB", f"HSET 'DEVICE_METADATA|localhost' 'dpu_auto_recovery' '{state}'") + logger.info(f"Set dpu_auto_recovery to '{state}'") def get_chassis_module_admin_status(dpu_name): @@ -361,15 +367,15 @@ def test_reset_count_field(results, dpu_name): def test_auto_recovery_feature_flag(results): - """Verify dpu-auto-recovery feature exists in CONFIG_DB (or defaults to enabled).""" - logger.info("Test: dpu-auto-recovery feature flag") - state = get_feature_state(DPU_AUTO_RECOVERY_FEATURE) - valid_states = ('enabled', 'disabled', 'always_disabled') - # Feature may not be explicitly configured (defaults to enabled in chassisd) + """Verify dpu_auto_recovery exists in CONFIG_DB DEVICE_METADATA (or defaults to disable).""" + logger.info("Test: dpu_auto_recovery in DEVICE_METADATA") + state = get_auto_recovery_state() + valid_states = ('enable', 'disable') + # Field may not be explicitly configured (defaults to disable in chassisd) results.record( "auto_recovery_feature_exists", state is None or state in valid_states, - f"Feature state: '{state}' (expected None/default or one of {valid_states})" + f"dpu_auto_recovery: '{state}' (expected None/default or one of {valid_states})" ) @@ -425,12 +431,12 @@ def test_disable_auto_recovery_no_power_cycle(results, dpu_name): """With auto-recovery disabled, DPU failure should not trigger power-cycle.""" logger.info(f"Test: disabled auto-recovery prevents power-cycle on {dpu_name}") - original_feature_state = get_feature_state(DPU_AUTO_RECOVERY_FEATURE) + original_feature_state = get_auto_recovery_state() original_reset_count = get_dpu_state_field(dpu_name, RESET_COUNT) try: # Disable auto-recovery - set_feature_state(DPU_AUTO_RECOVERY_FEATURE, 'disabled') + set_auto_recovery_state('disable') time.sleep(POLL_INTERVAL) # Record current reset count @@ -451,9 +457,9 @@ def test_disable_auto_recovery_no_power_cycle(results, dpu_name): f"even with auto-recovery disabled" ) finally: - # Restore original feature state + # Restore original state if original_feature_state: - set_feature_state(DPU_AUTO_RECOVERY_FEATURE, original_feature_state) + set_auto_recovery_state(original_feature_state) # Ensure DPU comes back set_module_admin_status(dpu_name, 'up') wait_for_dpu_ready(dpu_name) @@ -491,18 +497,18 @@ def test_enable_auto_recovery_feature(results, dpu_name): """Enable auto-recovery and verify DPU state reflects it.""" logger.info(f"Test: enable auto-recovery feature for {dpu_name}") - original_state = get_feature_state(DPU_AUTO_RECOVERY_FEATURE) + original_state = get_auto_recovery_state() try: - set_feature_state(DPU_AUTO_RECOVERY_FEATURE, 'enabled') + set_auto_recovery_state('enable') time.sleep(POLL_INTERVAL) - # Verify the feature is reflected (chassisd should read it next poll) - state = get_feature_state(DPU_AUTO_RECOVERY_FEATURE) + # Verify the field is reflected (chassisd should read it next poll) + state = get_auto_recovery_state() results.record( f"auto_recovery_enabled_{dpu_name}", - state == 'enabled', - f"Expected state=enabled, got '{state}'" + state == 'enable', + f"Expected state=enable, got '{state}'" ) # If DPU is healthy, it should be ready @@ -516,7 +522,7 @@ def test_enable_auto_recovery_feature(results, dpu_name): ) finally: if original_state: - set_feature_state(DPU_AUTO_RECOVERY_FEATURE, original_state) + set_auto_recovery_state(original_state) def test_dpu_power_cycle_recovery(results, dpu_name): @@ -527,8 +533,8 @@ def test_dpu_power_cycle_recovery(results, dpu_name): logger.info(f"Test: DPU power-cycle recovery for {dpu_name}") # Ensure auto-recovery is enabled - original_state = get_feature_state(DPU_AUTO_RECOVERY_FEATURE) - set_feature_state(DPU_AUTO_RECOVERY_FEATURE, 'enabled') + original_state = get_auto_recovery_state() + set_auto_recovery_state('enable') try: # Record initial reset count @@ -567,7 +573,7 @@ def test_dpu_power_cycle_recovery(results, dpu_name): ) finally: if original_state: - set_feature_state(DPU_AUTO_RECOVERY_FEATURE, original_state) + set_auto_recovery_state(original_state) # Ensure DPU is admin-up set_module_admin_status(dpu_name, 'up') @@ -601,6 +607,197 @@ def test_chassisd_stop_marks_dpus_not_ready(results, dpu_name): logger.info("chassisd restarted after deinit test") +# --------------------------------------------------------------------------- +# Race condition tests +# --------------------------------------------------------------------------- + +def get_transition_in_progress(dpu_name): + """Read transition_in_progress from STATE_DB CHASSIS_MODULE_TABLE.""" + result = run_cmd( + f"sonic-db-cli STATE_DB HGET 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_in_progress'", + check=False + ) + return result if result else None + + +def get_transition_type(dpu_name): + """Read transition_type from STATE_DB CHASSIS_MODULE_TABLE.""" + result = run_cmd( + f"sonic-db-cli STATE_DB HGET 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_type'", + check=False + ) + return result if result else None + + +def set_transition_in_progress(dpu_name, transition_type): + """Simulate a planned operation by setting transition_in_progress in STATE_DB.""" + sonic_db_cli("STATE_DB", + f"HSET 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_in_progress' 'True'") + sonic_db_cli("STATE_DB", + f"HSET 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_type' '{transition_type}'") + sonic_db_cli("STATE_DB", + f"HSET 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_start_time' '{int(time.time())}'") + logger.info(f"Set transition_in_progress=True, type={transition_type} for {dpu_name}") + + +def clear_transition_in_progress(dpu_name): + """Clear transition_in_progress fields from STATE_DB.""" + sonic_db_cli("STATE_DB", + f"HDEL 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_in_progress'") + sonic_db_cli("STATE_DB", + f"HDEL 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_type'") + sonic_db_cli("STATE_DB", + f"HDEL 'CHASSIS_MODULE_TABLE|{dpu_name}' 'transition_start_time'") + logger.info(f"Cleared transition_in_progress for {dpu_name}") + + +def test_transition_lock_suppresses_recovery(results, dpu_name): + """Setting transition_in_progress=True should suppress power-cycle recovery. + + Race condition coverage: chassisd must not initiate power-cycle when + another operation (gnoi shutdown/reboot) holds the transition lock. + """ + logger.info(f"Test: transition lock suppresses recovery on {dpu_name}") + + original_state = get_auto_recovery_state() + set_auto_recovery_state('enable') + original_oper_status = get_module_oper_status(dpu_name) + + try: + # Wait for DPU to be ready first + wait_for_dpu_ready(dpu_name, timeout=120) + initial_reset_count = get_dpu_state_field(dpu_name, RESET_COUNT) + + # Simulate a planned shutdown (gnoi sets the lock) + set_transition_in_progress(dpu_name, "shutdown") + time.sleep(POLL_INTERVAL * 2) + + # Inject hardware-offline while admin stays up. + set_module_oper_status(dpu_name, 'Offline') + time.sleep(POLL_INTERVAL * 3) + + # Recovery should be suppressed — reset_count should not increase + reset_count = get_dpu_state_field(dpu_name, RESET_COUNT) + results.record( + f"lock_suppresses_recovery_{dpu_name}", + reset_count == initial_reset_count, + f"Reset count changed from {initial_reset_count} to {reset_count} " + f"despite transition lock being held" + ) + finally: + clear_transition_in_progress(dpu_name) + if original_oper_status is not None: + set_module_oper_status(dpu_name, original_oper_status) + set_module_admin_status(dpu_name, 'up') + if original_state: + set_auto_recovery_state(original_state) + wait_for_dpu_ready(dpu_name, timeout=DPU_BOOT_TIMEOUT) + + +def test_recovery_type_does_not_suppress(results, dpu_name): + """transition_type='recovery' should NOT suppress chassisd recovery. + + Race condition coverage: chassisd sets its own lock with type='recovery' + during power-cycle. On the next poll, this must not self-suppress. + """ + logger.info(f"Test: recovery type does not suppress on {dpu_name}") + + try: + wait_for_dpu_ready(dpu_name, timeout=120) + + # Set transition_in_progress with type='recovery' (simulating chassisd's own lock) + set_transition_in_progress(dpu_name, "recovery") + time.sleep(POLL_INTERVAL * 2) + + # DPU should still be tracked as ready (not suppressed) + ready = get_dpu_state_field(dpu_name, READY_STATUS) + results.record( + f"recovery_type_no_suppress_{dpu_name}", + ready == 'true', + f"Expected ready_status=true (not suppressed), got '{ready}'" + ) + finally: + clear_transition_in_progress(dpu_name) + + +def test_lock_cleared_after_power_cycle(results, dpu_name): + """After chassisd power-cycles a DPU, the transition lock must be cleared. + + Race condition coverage: stale locks would permanently suppress + future planned operations (gnoi shutdown/reboot). + """ + logger.info(f"Test: lock cleared after power-cycle on {dpu_name}") + + original_state = get_auto_recovery_state() + set_auto_recovery_state('enable') + + try: + wait_for_dpu_ready(dpu_name, timeout=120) + + # Force a power-cycle by admin-down/up (which triggers boot → ready) + set_module_admin_status(dpu_name, 'down') + time.sleep(30) + set_module_admin_status(dpu_name, 'up') + + # Wait for DPU to recover + wait_for_dpu_ready(dpu_name, timeout=DPU_BOOT_TIMEOUT) + time.sleep(POLL_INTERVAL) + + # After recovery, transition_in_progress should NOT be set + flag = get_transition_in_progress(dpu_name) + results.record( + f"lock_cleared_after_cycle_{dpu_name}", + flag is None or flag != 'True', + f"transition_in_progress still set to '{flag}' after recovery" + ) + finally: + clear_transition_in_progress(dpu_name) + set_module_admin_status(dpu_name, 'up') + if original_state: + set_auto_recovery_state(original_state) + + +def test_concurrent_shutdown_during_recovery(results, dpu_name): + """Verify chassisd aborts power-cycle if a planned operation starts concurrently. + + Race condition coverage: TOCTOU between _is_planned_transition_in_progress() + check and the actual power-cycle. Chassisd should detect the lock conflict + and back off. + """ + logger.info(f"Test: concurrent shutdown during recovery on {dpu_name}") + + original_state = get_auto_recovery_state() + set_auto_recovery_state('enable') + original_oper_status = get_module_oper_status(dpu_name) + + try: + wait_for_dpu_ready(dpu_name, timeout=120) + initial_reset_count = int(get_dpu_state_field(dpu_name, RESET_COUNT) or '0') + + # Inject a hardware-offline failure while keeping admin up. + set_module_oper_status(dpu_name, 'Offline') + time.sleep(5) # Short wait, then set lock before chassisd can react + set_transition_in_progress(dpu_name, "shutdown") + time.sleep(POLL_INTERVAL * 3) + + # Chassisd should have been suppressed by the lock + reset_count = int(get_dpu_state_field(dpu_name, RESET_COUNT) or '0') + results.record( + f"concurrent_shutdown_aborts_recovery_{dpu_name}", + reset_count == initial_reset_count, + f"Reset count changed ({initial_reset_count} → {reset_count}); " + f"chassisd did not back off from concurrent shutdown" + ) + finally: + clear_transition_in_progress(dpu_name) + if original_oper_status is not None: + set_module_oper_status(dpu_name, original_oper_status) + set_module_admin_status(dpu_name, 'up') + if original_state: + set_auto_recovery_state(original_state) + wait_for_dpu_ready(dpu_name, timeout=DPU_BOOT_TIMEOUT) + + # --------------------------------------------------------------------------- # Main # --------------------------------------------------------------------------- @@ -696,6 +893,15 @@ def main(): test_chassisd_restart_reinitializes_state(results, dpu) test_dpu_power_cycle_recovery(results, dpu) test_chassisd_stop_marks_dpus_not_ready(results, dpu) + + # --- Race condition tests --- + logger.info("\n=== RACE CONDITION TESTS ===\n") + for dpu in target_dpus: + logger.info(f"\n--- Race condition tests for {dpu} ---") + test_transition_lock_suppresses_recovery(results, dpu) + test_recovery_type_does_not_suppress(results, dpu) + test_lock_cleared_after_power_cycle(results, dpu) + test_concurrent_shutdown_during_recovery(results, dpu) else: logger.info("\n(Skipping destructive tests as requested)\n")