From 841b083c55c5fce8c6d77dafcc78b7333ab94493 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Tue, 4 Nov 2025 18:11:21 +0000 Subject: [PATCH] [Smartswitch] Host container differentiation for sensor commands #### Description Differentiate commands being executed in host and container seperately for some smartswitch specific functions, sensord commands can only be executed inside hte pmon docker container, so these commands need to differentiate between host and container execution #### Motivation and Context This is done because these functions are called from inside the pmon docker during chassis command execution and from the host directly during reboot command execution #### How Has This Been Tested? ``` tests/module_base_test.py::TestModuleBase::test_module_base PASSED [ 2%] tests/module_base_test.py::TestModuleBase::test_is_container_detection PASSED [ 2%] tests/module_base_test.py::TestModuleBase::test_sensors PASSED [ 2%] tests/module_base_test.py::TestModuleBase::test_pci_entry_state_db PASSED [ 2%] tests/module_base_test.py::TestModuleBase::test_file_operation_lock PASSED [ 2%] tests/module_base_test.py::TestModuleBase::test_pci_operation_lock PASSED [ 2%] tests/module_base_test.py::TestModuleBase::test_sensord_operation_lock PASSED [ 3%] tests/module_base_test.py::TestModuleBase::test_handle_pci_removal PASSED [ 3%] tests/module_base_test.py::TestModuleBase::test_handle_pci_rescan PASSED [ 3%] tests/module_base_test.py::TestModuleBase::test_handle_sensor_removal PASSED [ 3%] tests/module_base_test.py::TestModuleBase::test_handle_sensor_addition PASSED [ 3%] tests/module_base_test.py::TestModuleBase::test_module_pre_shutdown PASSED [ 3%] tests/module_base_test.py::TestModuleBase::test_module_post_startup PASSED [ 3%] ``` Manual test with: `reboot -d DPU0` and `config chassis modules startup DPU0` #### Additional Information (Optional) --- sonic_platform_base/module_base.py | 39 ++++++-- tests/module_base_test.py | 143 ++++++++++++++++++++++------- 2 files changed, 142 insertions(+), 40 deletions(-) diff --git a/sonic_platform_base/module_base.py b/sonic_platform_base/module_base.py index bd33357..e8fb6af 100644 --- a/sonic_platform_base/module_base.py +++ b/sonic_platform_base/module_base.py @@ -13,6 +13,8 @@ import threading import contextlib import shutil +import subprocess +import os # PCI state database constants PCIE_DETACH_INFO_TABLE = "PCIE_DETACH_INFO" @@ -95,6 +97,9 @@ def __init__(self): # List of ASIC-derived objects representing all ASICs # visibile in PCI domain on the module self._asic_list = [] + + # Flag to indicate if the module is running on the host/container + self.is_host = self._is_host() @contextlib.contextmanager def _file_operation_lock(self, lock_file_path): @@ -614,6 +619,10 @@ def get_voltage_sensor(self, index): return voltage_sensor + def _is_host(self): + docker_env_file = '/.dockerenv' + return not os.path.exists(docker_env_file) + ############################################## # Current sensor methods ############################################## @@ -798,16 +807,25 @@ def handle_sensor_removal(self): module_name = self.get_name() source_file = f"/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_{module_name}.conf" target_file = f"/etc/sensors.d/ignore_sensors_{module_name}.conf" + file_exists_command = ['test', '-f', source_file] + copy_command = ['cp', source_file, target_file] + container_command = ['docker', 'exec', 'pmon'] + restart_command = ['service', 'sensord', 'restart'] + + if self.is_host: + file_exists_command = container_command + file_exists_command + copy_command = container_command + copy_command + restart_command = container_command + restart_command # If source file does not exist, we dont need to copy it and restart sensord - if not os.path.exists(source_file): + if subprocess.call(file_exists_command, stdout=subprocess.DEVNULL) != 0: return True - shutil.copy2(source_file, target_file) + subprocess.call(copy_command, stdout=subprocess.DEVNULL) # Restart sensord with lock with self._sensord_operation_lock(): - os.system("service sensord restart") + subprocess.call(restart_command, stdout=subprocess.DEVNULL) return True except Exception as e: @@ -825,17 +843,26 @@ def handle_sensor_addition(self): try: module_name = self.get_name() target_file = f"/etc/sensors.d/ignore_sensors_{module_name}.conf" + remove_command = ['rm', target_file] + restart_command = ['service', 'sensord', 'restart'] + container_command = ['docker', 'exec', 'pmon'] + file_exists_command = ['test', '-f', target_file] + + if self.is_host: + file_exists_command = container_command + file_exists_command + remove_command = container_command + remove_command + restart_command = container_command + restart_command # If target file does not exist, we dont need to remove it and restart sensord - if not os.path.exists(target_file): + if subprocess.call(file_exists_command, stdout=subprocess.DEVNULL) != 0: return True # Remove the file - os.remove(target_file) + subprocess.call(remove_command, stdout=subprocess.DEVNULL) # Restart sensord with lock with self._sensord_operation_lock(): - os.system("service sensord restart") + subprocess.call(restart_command, stdout=subprocess.DEVNULL) return True except Exception as e: diff --git a/tests/module_base_test.py b/tests/module_base_test.py index 025849e..ec8d842 100644 --- a/tests/module_base_test.py +++ b/tests/module_base_test.py @@ -5,7 +5,7 @@ import fcntl from unittest.mock import patch, MagicMock, call from io import StringIO -import shutil +import subprocess class MockFile: def __init__(self, data=None): @@ -55,6 +55,17 @@ def test_module_base(self): assert exception_raised + def test_is_host_detection(self): + # Test when /.dockerenv does not exist - running on host + with patch('os.path.exists', return_value=False): + module = ModuleBase() + assert module.is_host is True + + # Test when /.dockerenv exists - running in container (inside pmon) + with patch('os.path.exists', return_value=True): + module = ModuleBase() + assert module.is_host is False + def test_sensors(self): module = ModuleBase() assert(module.get_num_voltage_sensors() == 0) @@ -173,76 +184,140 @@ def test_handle_pci_rescan(self): def test_handle_sensor_removal(self): module = ModuleBase() + # Test successful case on host - commands run via docker exec pmon to access container + with patch.object(module, 'get_name', return_value="DPU0"), \ + patch('subprocess.call') as mock_call, \ + patch.object(module, '_sensord_operation_lock') as mock_lock: + module.is_host = True + # First call to test -f (file exists) returns 0, second call is cp, third is service restart + mock_call.side_effect = [0, 0, 0] + assert module.handle_sensor_removal() is True + assert mock_call.call_count == 3 + # When on host, commands are prefixed with docker exec pmon to run inside container + mock_call.assert_any_call(['docker', 'exec', 'pmon', 'test', '-f', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['docker', 'exec', 'pmon', 'cp', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf', + '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['docker', 'exec', 'pmon', 'service', 'sensord', 'restart'], + stdout=subprocess.DEVNULL) + mock_lock.assert_called_once() + + # Test successful case inside container - commands run directly without docker exec with patch.object(module, 'get_name', return_value="DPU0"), \ - patch('os.path.exists', return_value=True), \ - patch('shutil.copy2') as mock_copy, \ - patch('os.system') as mock_system, \ + patch('subprocess.call') as mock_call, \ patch.object(module, '_sensord_operation_lock') as mock_lock: + module.is_host = False + # First call to test -f (file exists) returns 0, second call is cp, third is service restart + mock_call.side_effect = [0, 0, 0] assert module.handle_sensor_removal() is True - mock_copy.assert_called_once_with("/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf", - "/etc/sensors.d/ignore_sensors_DPU0.conf") - mock_system.assert_called_once_with("service sensord restart") + assert mock_call.call_count == 3 + # When inside container, commands run directly without docker exec prefix + mock_call.assert_any_call(['test', '-f', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['cp', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf', + '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['service', 'sensord', 'restart'], + stdout=subprocess.DEVNULL) mock_lock.assert_called_once() + # Test file does not exist - should return True but not call copy or restart with patch.object(module, 'get_name', return_value="DPU0"), \ - patch('os.path.exists', return_value=False), \ - patch('shutil.copy2') as mock_copy, \ - patch('os.system') as mock_system, \ + patch('subprocess.call') as mock_call, \ patch.object(module, '_sensord_operation_lock') as mock_lock: + module.is_host = True + # Return 1 to indicate file doesn't exist + mock_call.return_value = 1 assert module.handle_sensor_removal() is True - mock_copy.assert_not_called() - mock_system.assert_not_called() + # Only the file existence check should be called (with docker exec when on host) + mock_call.assert_called_once_with(['docker', 'exec', 'pmon', 'test', '-f', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) mock_lock.assert_not_called() + # Test exception handling with patch.object(module, 'get_name', return_value="DPU0"), \ - patch('os.path.exists', return_value=True), \ - patch('shutil.copy2', side_effect=Exception("Copy failed")): + patch('subprocess.call', side_effect=Exception("Copy failed")): + module.is_host = True assert module.handle_sensor_removal() is False def test_handle_sensor_addition(self): module = ModuleBase() + # Test successful case on host - commands run via docker exec pmon to access container + with patch.object(module, 'get_name', return_value="DPU0"), \ + patch('subprocess.call') as mock_call, \ + patch.object(module, '_sensord_operation_lock') as mock_lock: + module.is_host = True + # First call to test -f (file exists) returns 0, second call is rm, third is service restart + mock_call.side_effect = [0, 0, 0] + assert module.handle_sensor_addition() is True + assert mock_call.call_count == 3 + # When on host, commands are prefixed with docker exec pmon to run inside container + mock_call.assert_any_call(['docker', 'exec', 'pmon', 'test', '-f', '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['docker', 'exec', 'pmon', 'rm', '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['docker', 'exec', 'pmon', 'service', 'sensord', 'restart'], + stdout=subprocess.DEVNULL) + mock_lock.assert_called_once() + + # Test successful case inside container - commands run directly without docker exec with patch.object(module, 'get_name', return_value="DPU0"), \ - patch('os.path.exists', return_value=True), \ - patch('os.remove') as mock_remove, \ - patch('os.system') as mock_system, \ + patch('subprocess.call') as mock_call, \ patch.object(module, '_sensord_operation_lock') as mock_lock: + module.is_host = False + # First call to test -f (file exists) returns 0, second call is rm, third is service restart + mock_call.side_effect = [0, 0, 0] assert module.handle_sensor_addition() is True - mock_remove.assert_called_once_with("/etc/sensors.d/ignore_sensors_DPU0.conf") - mock_system.assert_called_once_with("service sensord restart") + assert mock_call.call_count == 3 + # When inside container, commands run directly without docker exec prefix + mock_call.assert_any_call(['test', '-f', '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['rm', '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) + mock_call.assert_any_call(['service', 'sensord', 'restart'], + stdout=subprocess.DEVNULL) mock_lock.assert_called_once() + # Test file does not exist - should return True but not call remove or restart with patch.object(module, 'get_name', return_value="DPU0"), \ - patch('os.path.exists', return_value=False), \ - patch('os.remove') as mock_remove, \ - patch('os.system') as mock_system, \ + patch('subprocess.call') as mock_call, \ patch.object(module, '_sensord_operation_lock') as mock_lock: + module.is_host = True + # Return 1 to indicate file doesn't exist + mock_call.return_value = 1 assert module.handle_sensor_addition() is True - mock_remove.assert_not_called() - mock_system.assert_not_called() + # Only the file existence check should be called (with docker exec when on host) + mock_call.assert_called_once_with(['docker', 'exec', 'pmon', 'test', '-f', '/etc/sensors.d/ignore_sensors_DPU0.conf'], + stdout=subprocess.DEVNULL) mock_lock.assert_not_called() + # Test exception handling with patch.object(module, 'get_name', return_value="DPU0"), \ - patch('os.path.exists', return_value=True), \ - patch('os.remove', side_effect=Exception("Remove failed")): + patch('subprocess.call', side_effect=Exception("Remove failed")): + module.is_host = True assert module.handle_sensor_addition() is False def test_module_pre_shutdown(self): module = ModuleBase() # Test successful case - with patch.object(module, 'handle_pci_removal', return_value=True), \ - patch.object(module, 'handle_sensor_removal', return_value=True): + with patch.object(module, 'handle_sensor_removal', return_value=True) as mock_sensor, \ + patch.object(module, 'handle_pci_removal', return_value=True) as mock_pci: assert module.module_pre_shutdown() is True + # Verify sensor removal is called before PCI removal + mock_sensor.assert_called_once() + mock_pci.assert_called_once() - # Test PCI removal failure - with patch.object(module, 'handle_pci_removal', return_value=False), \ - patch.object(module, 'handle_sensor_removal', return_value=True): + # Test sensor removal failure + with patch.object(module, 'handle_sensor_removal', return_value=False), \ + patch.object(module, 'handle_pci_removal', return_value=True): assert module.module_pre_shutdown() is False - # Test sensor removal failure - with patch.object(module, 'handle_pci_removal', return_value=True), \ - patch.object(module, 'handle_sensor_removal', return_value=False): + # Test PCI removal failure + with patch.object(module, 'handle_sensor_removal', return_value=True), \ + patch.object(module, 'handle_pci_removal', return_value=False): assert module.module_pre_shutdown() is False def test_module_post_startup(self):