From 16ca04bedddf7c19c6bdc10a028d82c73e91d466 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Fri, 24 Jul 2026 16:51:57 +0000 Subject: [PATCH] [module_base]: Add 'recovery' transition type to _TRANSITION_TIMEOUT_DEFAULTS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### Description Add a `"recovery": 600` (10 min) entry to `ModuleBase._TRANSITION_TIMEOUT_DEFAULTS`. `set_module_state_transition()` validates the requested `transition_type` against the keys of `_TRANSITION_TIMEOUT_DEFAULTS`. chassisd's DPU power-cycle auto-recovery acquires the module state-transition lock via `set_module_state_transition(name, 'recovery')`; because `'recovery'` was missing from the defaults, every recovery acquisition was rejected ("Cannot acquire state transition lock") and the power-cycle was silently skipped — blocking DPU auto-recovery. #### Motivation and Context The `'recovery'` transition type is used by chassisd (sonic-platform-daemons) to power-cycle an unresponsive DPU. Without a matching entry in `_TRANSITION_TIMEOUT_DEFAULTS`, the transition is never accepted, so a DPU that loses its control plane is never power-cycled. This unblocks the DPU auto-recovery path in sonic-net/sonic-platform-daemons#829. `recovery` has no `platform.json` override (only `startup`/`shutdown`/`reboot`/`halt_services` are overridable in `_load_transition_timeouts()`), so the 600 s default applies uniformly. The timeout only bounds the power-cycle critical section (pre_shutdown → admin down → admin up → post_startup), which is cleared in `finally`; 600 s is ample headroom. #### How Has This Been Tested? - Unit tests: updated the timeout-loading tests in `tests/module_base_test.py` to expect the new key. `python3 -m pytest tests/module_base_test.py` → **115 passed**. - On-device with both master and 202605 image: with the fix, a control-plane-down DPU was power-cycled (reboot-cause "Switch rebooted DPU") and recovered to `ready_status=true`. #### Additional Information (Optional) Companion to sonic-net/sonic-platform-daemons#829 (chassisd DPU auto-recovery robustness), which issues the `'recovery'` transition this default enables. Signed-off-by: Sonic Build Admin --- sonic_platform_base/module_base.py | 3 ++- tests/module_base_test.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/sonic_platform_base/module_base.py b/sonic_platform_base/module_base.py index a474000b3..53f862725 100644 --- a/sonic_platform_base/module_base.py +++ b/sonic_platform_base/module_base.py @@ -488,7 +488,8 @@ def set_admin_state_gracefully(self, up): "startup": 300, # 5 mins "shutdown": 180, # 3 mins "reboot": 240, # 4 mins - "halt_services": 60 # 1 min + "halt_services": 60, # 1 min + "recovery": 600 # 10 mins } _TRANSITION_TIMEOUTS_CACHE = None diff --git a/tests/module_base_test.py b/tests/module_base_test.py index 17ec65da5..a57d4b90a 100644 --- a/tests/module_base_test.py +++ b/tests/module_base_test.py @@ -447,6 +447,7 @@ def test_load_transition_timeouts_defaults_host(self): with patch("sonic_py_common.device_info.get_platform", return_value="test_platform"), \ patch("os.path.exists", return_value=False): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 300, "shutdown": 180, "reboot": 240, @@ -459,6 +460,7 @@ def test_load_transition_timeouts_defaults_container(self): self.module.is_host = False with patch("os.path.exists", return_value=False): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 300, "shutdown": 180, "reboot": 240, @@ -480,6 +482,7 @@ def test_load_transition_timeouts_custom_host(self): patch("os.path.exists", return_value=True), \ patch("builtins.open", return_value=mf): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 600, "shutdown": 360, "reboot": 480, @@ -500,6 +503,7 @@ def test_load_transition_timeouts_custom_container(self): with patch("os.path.exists", return_value=True), \ patch("builtins.open", return_value=mf): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 600, "shutdown": 360, "reboot": 480, @@ -514,6 +518,7 @@ def test_load_transition_timeouts_partial(self): with patch("os.path.exists", return_value=True), \ patch("builtins.open", return_value=mf): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 500, "shutdown": 180, "reboot": 240, @@ -528,6 +533,7 @@ def test_load_transition_timeouts_error_host(self, capsys): patch("os.path.exists", return_value=True), \ patch("builtins.open", side_effect=Exception("read error")): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 300, "shutdown": 180, "reboot": 240, @@ -542,6 +548,7 @@ def test_load_transition_timeouts_error_container(self, capsys): with patch("os.path.exists", return_value=True), \ patch("builtins.open", side_effect=Exception("read error")): assert self.module._load_transition_timeouts() == { + "recovery": 600, "startup": 300, "shutdown": 180, "reboot": 240, @@ -832,6 +839,21 @@ def test_set_module_state_transition_happy(self): call(self._key("DPU0"), "transition_start_time", "1000"), ]) + def test_set_module_state_transition_recovery(self): + """The 'recovery' transition type must be accepted (added to defaults).""" + db = MagicMock() + self.module.state_db = db + db.hget.return_value = None + with patch.object(self.module, "get_name", return_value="DPU0"), \ + patch.object(self.module, "_transition_operation_lock"), \ + patch("time.time", return_value=1000): + assert self.module.set_module_state_transition("dpu0", "recovery") is True + db.hset.assert_has_calls([ + call(self._key("DPU0"), "transition_in_progress", "True"), + call(self._key("DPU0"), "transition_type", "recovery"), + call(self._key("DPU0"), "transition_start_time", "1000"), + ]) + def test_set_module_state_transition_within_timeout(self, capsys): db = MagicMock() self.module.state_db = db