From 05b156af44353b79dc738293ab0148664e959079 Mon Sep 17 00:00:00 2001 From: mssonicbld <79238446+mssonicbld@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:14:59 +1000 Subject: [PATCH 1/2] Add an inline platform check for ExecCondition of gnoi-shutdown.service (#416) Add an inline platform check for ExecCondition of gnoi-shutdown.service so that the gnoi-shutdown.service is skipped on non-smartswitch NPU platforms. Fix issue: [#27975](https://github.com/sonic-net/sonic-buildimage/issues/27975) Signed-off-by: Sonic Build Admin --- ...c-host-services-data.gnoi-shutdown.service | 1 + scripts/gnoi_shutdown_daemon.py | 9 ----- tests/gnoi_shutdown_daemon_test.py | 35 ++----------------- 3 files changed, 4 insertions(+), 41 deletions(-) diff --git a/data/debian/sonic-host-services-data.gnoi-shutdown.service b/data/debian/sonic-host-services-data.gnoi-shutdown.service index d3bb6b4..b1e17d3 100644 --- a/data/debian/sonic-host-services-data.gnoi-shutdown.service +++ b/data/debian/sonic-host-services-data.gnoi-shutdown.service @@ -7,6 +7,7 @@ After=network-online.target database.service gnmi.service pmon.service [Service] Type=simple ExecStart=/usr/bin/python3 /usr/local/bin/gnoi_shutdown_daemon.py +ExecCondition=/usr/bin/python3 -c "import sys; from sonic_py_common import device_info; sys.exit(0 if (device_info.is_smartswitch() and not device_info.is_dpu()) else 1)" Restart=on-failure RestartSec=5 diff --git a/scripts/gnoi_shutdown_daemon.py b/scripts/gnoi_shutdown_daemon.py index ecd3429..33b5318 100644 --- a/scripts/gnoi_shutdown_daemon.py +++ b/scripts/gnoi_shutdown_daemon.py @@ -293,15 +293,6 @@ def _clear_halt_flag(self, dpu_name: str) -> bool: # ######### def main(): - # Check if this is a SmartSwitch NPU platform - exit gracefully if not - try: - if not (device_info.is_smartswitch() and not is_dpu()): - logger.log_notice("Not a SmartSwitch NPU platform, exiting gracefully") - return - except (ImportError, AttributeError, RuntimeError) as e: - logger.log_notice(f"Platform check failed ({e}), exiting gracefully") - return - # Connect for STATE_DB (for gnoi_halt_in_progress flag) and CONFIG_DB state_db = daemon_base.db_connect("STATE_DB") config_db = daemon_base.db_connect("CONFIG_DB") diff --git a/tests/gnoi_shutdown_daemon_test.py b/tests/gnoi_shutdown_daemon_test.py index cff978f..adba090 100644 --- a/tests/gnoi_shutdown_daemon_test.py +++ b/tests/gnoi_shutdown_daemon_test.py @@ -29,29 +29,6 @@ def setUp(self): # Ensure a clean state for each test gnoi_shutdown_daemon.main = gnoi_shutdown_daemon.__dict__["main"] - @patch('gnoi_shutdown_daemon.device_info.is_smartswitch', return_value=False) - def test_main_exits_on_non_smartswitch(self, mock_is_smartswitch): - """Test main() exits gracefully on non-SmartSwitch platforms.""" - # Should return without doing anything - result = gnoi_shutdown_daemon.main() - self.assertIsNone(result) - mock_is_smartswitch.assert_called_once() - - @patch('gnoi_shutdown_daemon.is_dpu', return_value=True) - @patch('gnoi_shutdown_daemon.device_info.is_smartswitch', return_value=True) - def test_main_exits_on_dpu(self, mock_is_smartswitch, mock_is_dpu): - """Test main() exits gracefully when running on a DPU (not NPU).""" - result = gnoi_shutdown_daemon.main() - self.assertIsNone(result) - mock_is_smartswitch.assert_called_once() - mock_is_dpu.assert_called_once() - - @patch('gnoi_shutdown_daemon.device_info.is_smartswitch', side_effect=ImportError("No module")) - def test_main_exits_on_platform_check_exception(self, mock_is_smartswitch): - """Test main() exits gracefully when platform check raises exception.""" - result = gnoi_shutdown_daemon.main() - self.assertIsNone(result) - def test_execute_command_success(self): """Test successful execution of a gNOI command.""" with patch("gnoi_shutdown_daemon.subprocess.run") as mock_run: @@ -129,13 +106,11 @@ def test_get_halt_timeout_exception(self): timeout = gnoi_shutdown_daemon._get_halt_timeout() self.assertEqual(timeout, gnoi_shutdown_daemon.STATUS_POLL_TIMEOUT_SEC) - @patch('gnoi_shutdown_daemon.is_dpu', return_value=False) - @patch('gnoi_shutdown_daemon.device_info.is_smartswitch', return_value=True) @patch('gnoi_shutdown_daemon.daemon_base.db_connect') @patch('gnoi_shutdown_daemon.GnoiRebootHandler') @patch('gnoi_shutdown_daemon.swsscommon.ConfigDBConnector') @patch('threading.Thread') - def test_main_loop_flow(self, mock_thread, mock_config_db_connector_class, mock_gnoi_reboot_handler, mock_db_connect, mock_is_smartswitch, mock_is_dpu): + def test_main_loop_flow(self, mock_thread, mock_config_db_connector_class, mock_gnoi_reboot_handler, mock_db_connect): """Test the main loop processing of a shutdown event.""" # Mock DB connections mock_state_db = MagicMock() @@ -354,11 +329,9 @@ def test_get_dpu_gnmi_port_variants(self): self.assertEqual(port, "12345") self.assertEqual(mock_config.hget.call_count, 3) - @patch('gnoi_shutdown_daemon.is_dpu', return_value=False) - @patch('gnoi_shutdown_daemon.device_info.is_smartswitch', return_value=True) @patch('gnoi_shutdown_daemon.daemon_base.db_connect') @patch('gnoi_shutdown_daemon.swsscommon.ConfigDBConnector') - def test_main_loop_no_dpu_name(self, mock_config_db_connector_class, mock_db_connect, mock_is_smartswitch, mock_is_dpu): + def test_main_loop_no_dpu_name(self, mock_config_db_connector_class, mock_db_connect): """Test main loop with a malformed key.""" mock_chassis = MagicMock() mock_platform_instance = MagicMock() @@ -398,11 +371,9 @@ def test_main_loop_no_dpu_name(self, mock_config_db_connector_class, mock_db_con with self.assertRaises(KeyboardInterrupt): gnoi_shutdown_daemon.main() - @patch('gnoi_shutdown_daemon.is_dpu', return_value=False) - @patch('gnoi_shutdown_daemon.device_info.is_smartswitch', return_value=True) @patch('gnoi_shutdown_daemon.daemon_base.db_connect') @patch('gnoi_shutdown_daemon.swsscommon.ConfigDBConnector') - def test_main_loop_get_transition_exception(self, mock_config_db_connector_class, mock_db_connect, mock_is_smartswitch, mock_is_dpu): + def test_main_loop_get_transition_exception(self, mock_config_db_connector_class, mock_db_connect): """Test main loop when hget raises an exception.""" mock_chassis = MagicMock() mock_platform_instance = MagicMock() From dd015e195db1278f2ff8e610b7062783b7a85f2f Mon Sep 17 00:00:00 2001 From: mssonicbld <79238446+mssonicbld@users.noreply.github.com> Date: Wed, 29 Jul 2026 16:15:06 +1000 Subject: [PATCH 2/2] [gnoi_shutdown_daemon] Log gNOI Reboot failure details (#417) #### Why I did it When the gNOI Reboot (HALT) call to a DPU failed during graceful shutdown, the daemon only logged "Reboot command failed" and hid stderr. That made failures hard to debug. ##### Work item tracking - Microsoft ADO **(number only)**: N/A #### How I did it - Stopped hiding stderr on the gNOI Reboot call. - Added the return code, target IP:port, and the real error text to the log message. #### How to verify it - Run a graceful shutdown where the gNOI Reboot call fails (for example, an unreachable DPU). - Check syslog and confirm the log now shows rc, target IP:port, and the gNOI error text. #### Description for the changelog Log full gNOI Reboot failure details (rc, target, stderr) in gnoi_shutdown_daemon. Signed-off-by: Sonic Build Admin --- scripts/gnoi_shutdown_daemon.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/gnoi_shutdown_daemon.py b/scripts/gnoi_shutdown_daemon.py index 33b5318..596718b 100644 --- a/scripts/gnoi_shutdown_daemon.py +++ b/scripts/gnoi_shutdown_daemon.py @@ -243,9 +243,9 @@ def _send_reboot_command(self, dpu_name: str, dpu_ip: str, port: str) -> bool: "-rpc", "Reboot", "-jsonin", json.dumps({"method": REBOOT_METHOD_HALT, "message": "Triggered by SmartSwitch graceful shutdown"}) ] - rc, out, err = execute_command(reboot_cmd, timeout_sec=REBOOT_RPC_TIMEOUT_SEC, suppress_stderr=True) + rc, out, err = execute_command(reboot_cmd, timeout_sec=REBOOT_RPC_TIMEOUT_SEC) if rc != 0: - logger.log_error(f"{dpu_name}: Reboot command failed") + logger.log_error(f"{dpu_name}: Reboot command failed (rc={rc}, target={dpu_ip}:{port}): {err}") return False return True