Skip to content

Commit 68aedfd

Browse files
authored
[Smartswitch] Host container differentiation for sensor commands (#111)
<!-- Provide a general summary of your changes in the Title above --> #### Description <!-- Describe your changes in detail --> 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 <!-- Why is this change required? What problem does it solve? If this pull request closes/resolves an open Issue, make sure you include the text "fixes #xxxx", "closes #xxxx" or "resolves #xxxx" here --> 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? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ``` 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)
1 parent 7f01aea commit 68aedfd

2 files changed

Lines changed: 142 additions & 40 deletions

File tree

sonic_platform_base/module_base.py

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
import threading
1414
import contextlib
1515
import shutil
16+
import subprocess
17+
import os
1618

1719
# PCI state database constants
1820
PCIE_DETACH_INFO_TABLE = "PCIE_DETACH_INFO"
@@ -95,6 +97,9 @@ def __init__(self):
9597
# List of ASIC-derived objects representing all ASICs
9698
# visibile in PCI domain on the module
9799
self._asic_list = []
100+
101+
# Flag to indicate if the module is running on the host/container
102+
self.is_host = self._is_host()
98103

99104
@contextlib.contextmanager
100105
def _file_operation_lock(self, lock_file_path):
@@ -614,6 +619,10 @@ def get_voltage_sensor(self, index):
614619

615620
return voltage_sensor
616621

622+
def _is_host(self):
623+
docker_env_file = '/.dockerenv'
624+
return not os.path.exists(docker_env_file)
625+
617626
##############################################
618627
# Current sensor methods
619628
##############################################
@@ -798,16 +807,25 @@ def handle_sensor_removal(self):
798807
module_name = self.get_name()
799808
source_file = f"/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_{module_name}.conf"
800809
target_file = f"/etc/sensors.d/ignore_sensors_{module_name}.conf"
810+
file_exists_command = ['test', '-f', source_file]
811+
copy_command = ['cp', source_file, target_file]
812+
container_command = ['docker', 'exec', 'pmon']
813+
restart_command = ['service', 'sensord', 'restart']
814+
815+
if self.is_host:
816+
file_exists_command = container_command + file_exists_command
817+
copy_command = container_command + copy_command
818+
restart_command = container_command + restart_command
801819

802820
# If source file does not exist, we dont need to copy it and restart sensord
803-
if not os.path.exists(source_file):
821+
if subprocess.call(file_exists_command, stdout=subprocess.DEVNULL) != 0:
804822
return True
805823

806-
shutil.copy2(source_file, target_file)
824+
subprocess.call(copy_command, stdout=subprocess.DEVNULL)
807825

808826
# Restart sensord with lock
809827
with self._sensord_operation_lock():
810-
os.system("service sensord restart")
828+
subprocess.call(restart_command, stdout=subprocess.DEVNULL)
811829

812830
return True
813831
except Exception as e:
@@ -825,17 +843,26 @@ def handle_sensor_addition(self):
825843
try:
826844
module_name = self.get_name()
827845
target_file = f"/etc/sensors.d/ignore_sensors_{module_name}.conf"
846+
remove_command = ['rm', target_file]
847+
restart_command = ['service', 'sensord', 'restart']
848+
container_command = ['docker', 'exec', 'pmon']
849+
file_exists_command = ['test', '-f', target_file]
850+
851+
if self.is_host:
852+
file_exists_command = container_command + file_exists_command
853+
remove_command = container_command + remove_command
854+
restart_command = container_command + restart_command
828855

829856
# If target file does not exist, we dont need to remove it and restart sensord
830-
if not os.path.exists(target_file):
857+
if subprocess.call(file_exists_command, stdout=subprocess.DEVNULL) != 0:
831858
return True
832859

833860
# Remove the file
834-
os.remove(target_file)
861+
subprocess.call(remove_command, stdout=subprocess.DEVNULL)
835862

836863
# Restart sensord with lock
837864
with self._sensord_operation_lock():
838-
os.system("service sensord restart")
865+
subprocess.call(restart_command, stdout=subprocess.DEVNULL)
839866

840867
return True
841868
except Exception as e:

tests/module_base_test.py

Lines changed: 109 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import fcntl
66
from unittest.mock import patch, MagicMock, call
77
from io import StringIO
8-
import shutil
8+
import subprocess
99

1010
class MockFile:
1111
def __init__(self, data=None):
@@ -55,6 +55,17 @@ def test_module_base(self):
5555

5656
assert exception_raised
5757

58+
def test_is_host_detection(self):
59+
# Test when /.dockerenv does not exist - running on host
60+
with patch('os.path.exists', return_value=False):
61+
module = ModuleBase()
62+
assert module.is_host is True
63+
64+
# Test when /.dockerenv exists - running in container (inside pmon)
65+
with patch('os.path.exists', return_value=True):
66+
module = ModuleBase()
67+
assert module.is_host is False
68+
5869
def test_sensors(self):
5970
module = ModuleBase()
6071
assert(module.get_num_voltage_sensors() == 0)
@@ -173,76 +184,140 @@ def test_handle_pci_rescan(self):
173184
def test_handle_sensor_removal(self):
174185
module = ModuleBase()
175186

187+
# Test successful case on host - commands run via docker exec pmon to access container
188+
with patch.object(module, 'get_name', return_value="DPU0"), \
189+
patch('subprocess.call') as mock_call, \
190+
patch.object(module, '_sensord_operation_lock') as mock_lock:
191+
module.is_host = True
192+
# First call to test -f (file exists) returns 0, second call is cp, third is service restart
193+
mock_call.side_effect = [0, 0, 0]
194+
assert module.handle_sensor_removal() is True
195+
assert mock_call.call_count == 3
196+
# When on host, commands are prefixed with docker exec pmon to run inside container
197+
mock_call.assert_any_call(['docker', 'exec', 'pmon', 'test', '-f', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf'],
198+
stdout=subprocess.DEVNULL)
199+
mock_call.assert_any_call(['docker', 'exec', 'pmon', 'cp', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf',
200+
'/etc/sensors.d/ignore_sensors_DPU0.conf'],
201+
stdout=subprocess.DEVNULL)
202+
mock_call.assert_any_call(['docker', 'exec', 'pmon', 'service', 'sensord', 'restart'],
203+
stdout=subprocess.DEVNULL)
204+
mock_lock.assert_called_once()
205+
206+
# Test successful case inside container - commands run directly without docker exec
176207
with patch.object(module, 'get_name', return_value="DPU0"), \
177-
patch('os.path.exists', return_value=True), \
178-
patch('shutil.copy2') as mock_copy, \
179-
patch('os.system') as mock_system, \
208+
patch('subprocess.call') as mock_call, \
180209
patch.object(module, '_sensord_operation_lock') as mock_lock:
210+
module.is_host = False
211+
# First call to test -f (file exists) returns 0, second call is cp, third is service restart
212+
mock_call.side_effect = [0, 0, 0]
181213
assert module.handle_sensor_removal() is True
182-
mock_copy.assert_called_once_with("/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf",
183-
"/etc/sensors.d/ignore_sensors_DPU0.conf")
184-
mock_system.assert_called_once_with("service sensord restart")
214+
assert mock_call.call_count == 3
215+
# When inside container, commands run directly without docker exec prefix
216+
mock_call.assert_any_call(['test', '-f', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf'],
217+
stdout=subprocess.DEVNULL)
218+
mock_call.assert_any_call(['cp', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf',
219+
'/etc/sensors.d/ignore_sensors_DPU0.conf'],
220+
stdout=subprocess.DEVNULL)
221+
mock_call.assert_any_call(['service', 'sensord', 'restart'],
222+
stdout=subprocess.DEVNULL)
185223
mock_lock.assert_called_once()
186224

225+
# Test file does not exist - should return True but not call copy or restart
187226
with patch.object(module, 'get_name', return_value="DPU0"), \
188-
patch('os.path.exists', return_value=False), \
189-
patch('shutil.copy2') as mock_copy, \
190-
patch('os.system') as mock_system, \
227+
patch('subprocess.call') as mock_call, \
191228
patch.object(module, '_sensord_operation_lock') as mock_lock:
229+
module.is_host = True
230+
# Return 1 to indicate file doesn't exist
231+
mock_call.return_value = 1
192232
assert module.handle_sensor_removal() is True
193-
mock_copy.assert_not_called()
194-
mock_system.assert_not_called()
233+
# Only the file existence check should be called (with docker exec when on host)
234+
mock_call.assert_called_once_with(['docker', 'exec', 'pmon', 'test', '-f', '/usr/share/sonic/platform/module_sensors_ignore_conf/ignore_sensors_DPU0.conf'],
235+
stdout=subprocess.DEVNULL)
195236
mock_lock.assert_not_called()
196237

238+
# Test exception handling
197239
with patch.object(module, 'get_name', return_value="DPU0"), \
198-
patch('os.path.exists', return_value=True), \
199-
patch('shutil.copy2', side_effect=Exception("Copy failed")):
240+
patch('subprocess.call', side_effect=Exception("Copy failed")):
241+
module.is_host = True
200242
assert module.handle_sensor_removal() is False
201243

202244
def test_handle_sensor_addition(self):
203245
module = ModuleBase()
204246

247+
# Test successful case on host - commands run via docker exec pmon to access container
248+
with patch.object(module, 'get_name', return_value="DPU0"), \
249+
patch('subprocess.call') as mock_call, \
250+
patch.object(module, '_sensord_operation_lock') as mock_lock:
251+
module.is_host = True
252+
# First call to test -f (file exists) returns 0, second call is rm, third is service restart
253+
mock_call.side_effect = [0, 0, 0]
254+
assert module.handle_sensor_addition() is True
255+
assert mock_call.call_count == 3
256+
# When on host, commands are prefixed with docker exec pmon to run inside container
257+
mock_call.assert_any_call(['docker', 'exec', 'pmon', 'test', '-f', '/etc/sensors.d/ignore_sensors_DPU0.conf'],
258+
stdout=subprocess.DEVNULL)
259+
mock_call.assert_any_call(['docker', 'exec', 'pmon', 'rm', '/etc/sensors.d/ignore_sensors_DPU0.conf'],
260+
stdout=subprocess.DEVNULL)
261+
mock_call.assert_any_call(['docker', 'exec', 'pmon', 'service', 'sensord', 'restart'],
262+
stdout=subprocess.DEVNULL)
263+
mock_lock.assert_called_once()
264+
265+
# Test successful case inside container - commands run directly without docker exec
205266
with patch.object(module, 'get_name', return_value="DPU0"), \
206-
patch('os.path.exists', return_value=True), \
207-
patch('os.remove') as mock_remove, \
208-
patch('os.system') as mock_system, \
267+
patch('subprocess.call') as mock_call, \
209268
patch.object(module, '_sensord_operation_lock') as mock_lock:
269+
module.is_host = False
270+
# First call to test -f (file exists) returns 0, second call is rm, third is service restart
271+
mock_call.side_effect = [0, 0, 0]
210272
assert module.handle_sensor_addition() is True
211-
mock_remove.assert_called_once_with("/etc/sensors.d/ignore_sensors_DPU0.conf")
212-
mock_system.assert_called_once_with("service sensord restart")
273+
assert mock_call.call_count == 3
274+
# When inside container, commands run directly without docker exec prefix
275+
mock_call.assert_any_call(['test', '-f', '/etc/sensors.d/ignore_sensors_DPU0.conf'],
276+
stdout=subprocess.DEVNULL)
277+
mock_call.assert_any_call(['rm', '/etc/sensors.d/ignore_sensors_DPU0.conf'],
278+
stdout=subprocess.DEVNULL)
279+
mock_call.assert_any_call(['service', 'sensord', 'restart'],
280+
stdout=subprocess.DEVNULL)
213281
mock_lock.assert_called_once()
214282

283+
# Test file does not exist - should return True but not call remove or restart
215284
with patch.object(module, 'get_name', return_value="DPU0"), \
216-
patch('os.path.exists', return_value=False), \
217-
patch('os.remove') as mock_remove, \
218-
patch('os.system') as mock_system, \
285+
patch('subprocess.call') as mock_call, \
219286
patch.object(module, '_sensord_operation_lock') as mock_lock:
287+
module.is_host = True
288+
# Return 1 to indicate file doesn't exist
289+
mock_call.return_value = 1
220290
assert module.handle_sensor_addition() is True
221-
mock_remove.assert_not_called()
222-
mock_system.assert_not_called()
291+
# Only the file existence check should be called (with docker exec when on host)
292+
mock_call.assert_called_once_with(['docker', 'exec', 'pmon', 'test', '-f', '/etc/sensors.d/ignore_sensors_DPU0.conf'],
293+
stdout=subprocess.DEVNULL)
223294
mock_lock.assert_not_called()
224295

296+
# Test exception handling
225297
with patch.object(module, 'get_name', return_value="DPU0"), \
226-
patch('os.path.exists', return_value=True), \
227-
patch('os.remove', side_effect=Exception("Remove failed")):
298+
patch('subprocess.call', side_effect=Exception("Remove failed")):
299+
module.is_host = True
228300
assert module.handle_sensor_addition() is False
229301

230302
def test_module_pre_shutdown(self):
231303
module = ModuleBase()
232304

233305
# Test successful case
234-
with patch.object(module, 'handle_pci_removal', return_value=True), \
235-
patch.object(module, 'handle_sensor_removal', return_value=True):
306+
with patch.object(module, 'handle_sensor_removal', return_value=True) as mock_sensor, \
307+
patch.object(module, 'handle_pci_removal', return_value=True) as mock_pci:
236308
assert module.module_pre_shutdown() is True
309+
# Verify sensor removal is called before PCI removal
310+
mock_sensor.assert_called_once()
311+
mock_pci.assert_called_once()
237312

238-
# Test PCI removal failure
239-
with patch.object(module, 'handle_pci_removal', return_value=False), \
240-
patch.object(module, 'handle_sensor_removal', return_value=True):
313+
# Test sensor removal failure
314+
with patch.object(module, 'handle_sensor_removal', return_value=False), \
315+
patch.object(module, 'handle_pci_removal', return_value=True):
241316
assert module.module_pre_shutdown() is False
242317

243-
# Test sensor removal failure
244-
with patch.object(module, 'handle_pci_removal', return_value=True), \
245-
patch.object(module, 'handle_sensor_removal', return_value=False):
318+
# Test PCI removal failure
319+
with patch.object(module, 'handle_sensor_removal', return_value=True), \
320+
patch.object(module, 'handle_pci_removal', return_value=False):
246321
assert module.module_pre_shutdown() is False
247322

248323
def test_module_post_startup(self):

0 commit comments

Comments
 (0)