Skip to content

Commit

Permalink
Combine psu presence/status update with data update (sonic-net#424)
Browse files Browse the repository at this point in the history
When sysfs is simultaneously changing while psud updating, some data could be outdated immediately once read, so delay the psu presence and status updating to the final step, insure we get the latest info.

Description
PSU Model Serial HW Rev Voltage (V) Current (A) Power (W) Status LED
PSU 1 N/A N/A N/A 12.00 N/A N/A OK green
PSU 2 XXXXXXX XXXXXXXX A3 12.01 16.25 194.75 OK green

The status should be NOTOK as the psu had been turned off and other fields had became N/A as expected.

Motivation and Context
The failure flow are showing below:

    psu_db_update(self.psu_tbl, self.num_psus) // here we read and update the old data of psu status. 
    self.update_psu_data() // now psu status had been changed to NOTOK, but we won’t read it until next psu update cycle , other filed becomes N/A as expected.
How Has This Been Tested?
platform_tests/api/test_psu.py

Signed-off-by: Yuanzhe, Liu <[email protected]>
  • Loading branch information
yuazhe authored Mar 5, 2024
1 parent b9e6ba5 commit 55b5805
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 74 deletions.
12 changes: 2 additions & 10 deletions sonic-psud/scripts/psud
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,6 @@ def get_psu_key(psu_index):
return PSU_INFO_KEY_TEMPLATE.format(psu_index)


def psu_db_update(psu_tbl, psu_num):
for psu_index in range(1, psu_num + 1):
fvs = swsscommon.FieldValuePairs([(PSU_INFO_PRESENCE_FIELD,
'true' if _wrapper_get_psu_presence(psu_index) else 'false'),
(PSU_INFO_STATUS_FIELD,
'true' if _wrapper_get_psu_status(psu_index) else 'false')])
psu_tbl.set(get_psu_key(psu_index), fvs)


# try get information from platform API and return a default value if we catch NotImplementedError
def try_get(callback, default=None):
"""
Expand Down Expand Up @@ -443,7 +434,6 @@ class DaemonPsud(daemon_base.DaemonBase):
return False

self._update_psu_entity_info()
psu_db_update(self.psu_tbl, self.num_psus)
self.update_psu_data()
self._update_led_color()

Expand Down Expand Up @@ -606,6 +596,8 @@ class DaemonPsud(daemon_base.DaemonBase):
(PSU_INFO_IN_CURRENT_FIELD, str(in_current)),
(PSU_INFO_IN_VOLTAGE_FIELD, str(in_voltage)),
(PSU_INFO_POWER_MAX_FIELD, str(max_power)),
(PSU_INFO_PRESENCE_FIELD, 'true' if _wrapper_get_psu_presence(index) else 'false'),
(PSU_INFO_STATUS_FIELD, 'true' if _wrapper_get_psu_status(index) else 'false'),
])
self.psu_tbl.set(name, fvs)

Expand Down
2 changes: 2 additions & 0 deletions sonic-psud/tests/test_DaemonPsud.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ def _construct_expected_fvp(self, power=100.0, power_warning_suppress_threshold=
(psud.PSU_INFO_IN_VOLTAGE_FIELD, '220.25'),
(psud.PSU_INFO_IN_CURRENT_FIELD, '0.72'),
(psud.PSU_INFO_POWER_MAX_FIELD, 'N/A'),
(psud.PSU_INFO_PRESENCE_FIELD, 'true'),
(psud.PSU_INFO_STATUS_FIELD, 'true'),
])
return expected_fvp

Expand Down
64 changes: 0 additions & 64 deletions sonic-psud/tests/test_psud.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,70 +96,6 @@ def test_wrapper_get_psu_status():
psud.platform_psuutil.get_psu_status.assert_called_with(1)


@mock.patch('psud._wrapper_get_psu_presence', mock.MagicMock())
@mock.patch('psud._wrapper_get_psu_status', mock.MagicMock())
def test_psu_db_update():
psu_tbl = mock.MagicMock()

psud._wrapper_get_psu_presence.return_value = True
psud._wrapper_get_psu_status.return_value = True
expected_fvp = psud.swsscommon.FieldValuePairs(
[(psud.PSU_INFO_PRESENCE_FIELD, 'true'),
(psud.PSU_INFO_STATUS_FIELD, 'true'),
])
psud.psu_db_update(psu_tbl, 1)
assert psu_tbl.set.call_count == 1
psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

psu_tbl.set.reset_mock()

psud._wrapper_get_psu_presence.return_value = False
psud._wrapper_get_psu_status.return_value = True
expected_fvp = psud.swsscommon.FieldValuePairs(
[(psud.PSU_INFO_PRESENCE_FIELD, 'false'),
(psud.PSU_INFO_STATUS_FIELD, 'true'),
])
psud.psu_db_update(psu_tbl, 1)
assert psu_tbl.set.call_count == 1
psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

psu_tbl.set.reset_mock()

psud._wrapper_get_psu_presence.return_value = True
psud._wrapper_get_psu_status.return_value = False
expected_fvp = psud.swsscommon.FieldValuePairs(
[(psud.PSU_INFO_PRESENCE_FIELD, 'true'),
(psud.PSU_INFO_STATUS_FIELD, 'false'),
])
psud.psu_db_update(psu_tbl, 1)
assert psu_tbl.set.call_count == 1
psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

psu_tbl.set.reset_mock()

psud._wrapper_get_psu_presence.return_value = False
psud._wrapper_get_psu_status.return_value = False
expected_fvp = psud.swsscommon.FieldValuePairs(
[(psud.PSU_INFO_PRESENCE_FIELD, 'false'),
(psud.PSU_INFO_STATUS_FIELD, 'false'),
])
psud.psu_db_update(psu_tbl, 1)
assert psu_tbl.set.call_count == 1
psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(1), expected_fvp)

psu_tbl.set.reset_mock()

psud._wrapper_get_psu_presence.return_value = True
psud._wrapper_get_psu_status.return_value = True
expected_fvp = psud.swsscommon.FieldValuePairs(
[(psud.PSU_INFO_PRESENCE_FIELD, 'true'),
(psud.PSU_INFO_STATUS_FIELD, 'true'),
])
psud.psu_db_update(psu_tbl, 32)
assert psu_tbl.set.call_count == 32
psu_tbl.set.assert_called_with(psud.PSU_INFO_KEY_TEMPLATE.format(32), expected_fvp)


def test_log_on_status_changed():
normal_log = "Normal log message"
abnormal_log = "Abnormal log message"
Expand Down

0 comments on commit 55b5805

Please sign in to comment.