Skip to content

Commit

Permalink
[psud] Fix PSU log issue (#235)
Browse files Browse the repository at this point in the history
Description
Set PSU status default to True
Add PSU index to PsuStatus to help print the log with PSU id

Motivation and Context
Issue flow:
1. PSU is in bad state (e.g. PSU is not present, PSU is powered off, PSU overheat and so on) psud startup
2. There is no warning logs in syslog
3. This PR is aimed to fix the issue

How Has This Been Tested?
1. Manual test
2. Adjusted unit test
  • Loading branch information
Junchao-Mellanox authored Jan 17, 2022
1 parent 07542cb commit c4127c2
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 65 deletions.
27 changes: 15 additions & 12 deletions sonic-psud/scripts/psud
Original file line number Diff line number Diff line change
Expand Up @@ -272,13 +272,13 @@ class PsuChassisInfo(logger.Logger):


class PsuStatus(object):
def __init__(self, logger, psu):

def __init__(self, logger, psu, psu_index):
self.psu = psu
self.presence = False
self.power_good = False
self.voltage_good = False
self.temperature_good = False
self.psu_index = psu_index
self.presence = True
self.power_good = True
self.voltage_good = True
self.temperature_good = True
self.logger = logger

def set_presence(self, presence):
Expand Down Expand Up @@ -308,8 +308,8 @@ class PsuStatus(object):
def set_voltage(self, voltage, high_threshold, low_threshold):
if voltage == NOT_AVAILABLE or high_threshold == NOT_AVAILABLE or low_threshold == NOT_AVAILABLE:
if self.voltage_good is not True:
self.logger.log_warning('PSU voltage or high_threshold or low_threshold become unavailable, '
'voltage={}, high_threshold={}, low_threshold={}'.format(voltage, high_threshold, low_threshold))
self.logger.log_warning('PSU {} voltage or high_threshold or low_threshold become unavailable, '
'voltage={}, high_threshold={}, low_threshold={}'.format(self.psu_index, voltage, high_threshold, low_threshold))
self.voltage_good = True
return False

Expand All @@ -323,8 +323,8 @@ class PsuStatus(object):
def set_temperature(self, temperature, high_threshold):
if temperature == NOT_AVAILABLE or high_threshold == NOT_AVAILABLE:
if self.temperature_good is not True:
self.logger.log_warning('PSU temperature or high_threshold become unavailable, '
'temperature={}, high_threshold={}'.format(temperature, high_threshold))
self.logger.log_warning('PSU {} temperature or high_threshold become unavailable, '
'temperature={}, high_threshold={}'.format(self.psu_index, temperature, high_threshold))
self.temperature_good = True
return False

Expand Down Expand Up @@ -465,16 +465,19 @@ class DaemonPsud(daemon_base.DaemonBase):
power = try_get(psu.get_power, NOT_AVAILABLE)

if index not in self.psu_status_dict:
self.psu_status_dict[index] = PsuStatus(self, psu)
self.psu_status_dict[index] = PsuStatus(self, psu, index)

psu_status = self.psu_status_dict[index]
set_led = self.first_run
if psu_status.set_presence(presence):
presence_changed = psu_status.set_presence(presence)
if presence_changed:
set_led = True
log_on_status_changed(self, psu_status.presence,
'PSU absence warning cleared: {} is inserted back.'.format(name),
'PSU absence warning: {} is not present.'.format(name)
)

if presence_changed or self.first_run:
# Have to update PSU fan data here because PSU presence status changed. If we don't
# update PSU fan data here, there might be an inconsistent output between "show platform psustatus"
# and "show platform fan". For example, say PSU 1 is removed, and psud query PSU status every 3 seconds,
Expand Down
8 changes: 2 additions & 6 deletions sonic-psud/tests/test_DaemonPsud.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,10 @@ def test_update_single_psu_data(self):
def test_set_psu_led(self):
mock_logger = mock.MagicMock()
mock_psu = MockPsu("PSU 1", 0, True, True)
psu_status = psud.PsuStatus(mock_logger, mock_psu)
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)

daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER)

psu_status.presence = True
psu_status.power_good = True
psu_status.voltage_good = True
psu_status.temperature_good = True
daemon_psud._set_psu_led(mock_psu, psu_status)
assert mock_psu.get_status_led() == mock_psu.STATUS_LED_COLOR_GREEN

Expand Down Expand Up @@ -228,7 +224,7 @@ def test_set_psu_led(self):
def test_update_led_color(self):
mock_psu = MockPsu("PSU 1", 0, True, True)
mock_logger = mock.MagicMock()
psu_status = psud.PsuStatus(mock_logger, mock_psu)
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)

daemon_psud = psud.DaemonPsud(SYSLOG_IDENTIFIER)
daemon_psud.psu_tbl = mock.MagicMock()
Expand Down
81 changes: 34 additions & 47 deletions sonic-psud/tests/test_PsuStatus.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,29 +33,39 @@ def test_set_presence(self):
mock_logger = mock.MagicMock()
mock_psu = MockPsu("PSU 1", 0, True, True)

psu_status = psud.PsuStatus(mock_logger, mock_psu)
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)
assert psu_status.presence is True

# Test toggling presence to False
ret = psu_status.set_presence(False)
assert ret == True
assert psu_status.presence == False

# Test toggling presence to True
ret = psu_status.set_presence(True)
assert ret == True
assert psu_status.presence == True

# Test toggling presence to False
ret = psu_status.set_presence(False)
assert ret == True
assert psu_status.presence == False

# Test attempting to set presence to the same as the current value
ret = psu_status.set_presence(False)
ret = psu_status.set_presence(True)
assert ret == False
assert psu_status.presence == False
assert psu_status.presence == True

def test_set_power_good(self):
mock_logger = mock.MagicMock()
mock_psu = MockPsu("PSU 1", 0, True, True)

psu_status = psud.PsuStatus(mock_logger, mock_psu)
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)
assert psu_status.power_good is True

# Test toggling power_good to False
ret = psu_status.set_power_good(False)
assert ret == True
assert psu_status.power_good == False

# Test attempting to set power_good to the same as the current value (return value should be False)
ret = psu_status.set_power_good(False)
assert ret == False
assert psu_status.power_good == False

# Test toggling power_good to True
Expand All @@ -68,32 +78,13 @@ def test_set_power_good(self):
assert ret == False
assert psu_status.power_good == True

# Test toggling power_good to False
ret = psu_status.set_power_good(False)
assert ret == True
assert psu_status.power_good == False

# Test attempting to set power_good to the same as the current value (return value should be False)
ret = psu_status.set_power_good(False)
assert ret == False
assert psu_status.power_good == False

def test_set_voltage(self):
mock_logger = mock.MagicMock()
mock_psu = MockPsu("PSU 1", 0, True, True)

psu_status = psud.PsuStatus(mock_logger, mock_psu)
assert psu_status.voltage_good == False

# Pass in a good voltage
ret = psu_status.set_voltage(12.0, 12.5, 11.5)
assert ret == True
assert psu_status.voltage_good == True

# Pass in a another good voltage successively (return value should be False)
ret = psu_status.set_voltage(11.9, 12.5, 11.5)
assert ret == False
assert psu_status.voltage_good == True
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)
assert psu_status.voltage_good is True

# Pass in a high voltage
ret = psu_status.set_voltage(12.6, 12.5, 11.5)
Expand All @@ -110,6 +101,11 @@ def test_set_voltage(self):
assert ret == True
assert psu_status.voltage_good == True

# Pass in a another good voltage successively (return value should be False)
ret = psu_status.set_voltage(11.9, 12.5, 11.5)
assert ret == False
assert psu_status.voltage_good == True

# Pass in a low voltage
ret = psu_status.set_voltage(11.4, 12.5, 11.5)
assert ret == True
Expand Down Expand Up @@ -149,18 +145,8 @@ def test_set_temperature(self):
mock_logger = mock.MagicMock()
mock_psu = MockPsu("PSU 1", 0, True, True)

psu_status = psud.PsuStatus(mock_logger, mock_psu)
assert psu_status.temperature_good == False

# Pass in a good temperature
ret = psu_status.set_temperature(20.123, 50.0)
assert ret == True
assert psu_status.temperature_good == True

# Pass in a another good temperature successively (return value should be False)
ret = psu_status.set_temperature(31.456, 50.0)
assert ret == False
assert psu_status.temperature_good == True
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)
assert psu_status.temperature_good is True

# Pass in a high temperature
ret = psu_status.set_temperature(50.001, 50.0)
Expand All @@ -177,6 +163,11 @@ def test_set_temperature(self):
assert ret == True
assert psu_status.temperature_good == True

# Pass in a another good temperature successively (return value should be False)
ret = psu_status.set_temperature(31.456, 50.0)
assert ret == False
assert psu_status.temperature_good == True

# Test passing parameters as None when temperature_good == True
ret = psu_status.set_temperature(psud.NOT_AVAILABLE, 50.0)
assert ret == False
Expand All @@ -199,11 +190,7 @@ def test_is_ok(self):
mock_logger = mock.MagicMock()
mock_psu = MockPsu("PSU 1", 0, True, True)

psu_status = psud.PsuStatus(mock_logger, mock_psu)
psu_status.presence = True
psu_status.power_good = True
psu_status.voltage_good = True
psu_status.temperature_good = True
psu_status = psud.PsuStatus(mock_logger, mock_psu, 1)
ret = psu_status.is_ok()
assert ret == True

Expand Down

0 comments on commit c4127c2

Please sign in to comment.