[psud] Increase unit test coverage; Refactor mock platform (sonic-net…

…#154)

- Refactor psud such that the `run()` method does not contain an infinite loop, thus allowing us to unit test it
- Refactor mock_platform.py such that it inherits from sonic-platform-common in order to ensure it is aligned with the current API definitions (this introduces a test-time dependency on the sonic-platform-common package)
- Eliminate the need to check for a `PSUD_UNIT_TESTING` environment variable in daemon code
- Increase overall unit test unit test coverage from 78% to 97%

sonic-psud/pytest.ini

@@ -1,2 +1,2 @@
 [pytest]
-addopts = --cov=scripts --cov-report html --cov-report term --cov-report xml --junitxml=test-results.xml -v
+addopts = --cov=scripts --cov-report html --cov-report term --cov-report xml --junitxml=test-results.xml -vv

sonic-psud/scripts/psud

@@ -9,21 +9,14 @@
  The loop interval is PSU_INFO_UPDATE_PERIOD_SECS in seconds.
 """
 
-import os
 import signal
 import sys
 import threading
 from datetime import datetime
 
+from sonic_platform.psu import Psu
 from sonic_py_common import daemon_base, logger
-
-# If unit testing is occurring, mock swsscommon and module_base
-if os.getenv("PSUD_UNIT_TESTING") == "1":
- from tests.mock_platform import MockPsu as Psu
- from tests import mock_swsscommon as swsscommon
-else:
- from sonic_platform.psu import Psu
- from swsscommon import swsscommon
+from swsscommon import swsscommon
 
 
 #
@@ -32,8 +25,8 @@ else:
 
 # TODO: Once we no longer support Python 2, we can eliminate this and get the
 # name using the 'name' field (e.g., `signal.SIGINT.name`) starting with Python 3.5
-SIGNALS_TO_NAMES_DICT = dict((getattr(signal, n), n) \
- for n in dir(signal) if n.startswith('SIG') and '_' not in n )
+SIGNALS_TO_NAMES_DICT = dict((getattr(signal, n), n)
+  for n in dir(signal) if n.startswith('SIG') and '_' not in n)
 
 SYSLOG_IDENTIFIER = "psud"
 
@@ -85,8 +78,11 @@ PSUUTIL_LOAD_ERROR = 1
 platform_psuutil = None
 platform_chassis = None
 
+exit_code = 0
 
 # temporary wrappers that are compliable with both new platform api and old-style plugin mode
+
+
 def _wrapper_get_num_psus():
  if platform_chassis is not None:
  try:
@@ -131,7 +127,7 @@ def psu_db_update(psu_tbl, psu_num):
  psu_tbl.set(get_psu_key(psu_index), fvs)
 
 
-# try get information from platform API and return a default value if caught NotImplementedError
+# try get information from platform API and return a default value if we catch NotImplementedError
 def try_get(callback, default=None):
  """
  Handy function to invoke the callback and catch NotImplementedError
@@ -257,11 +253,10 @@ class PsuChassisInfo(logger.Logger):
  self.master_status_good = master_status_good
 
  # Update the PSU master status LED
- try:
- color = Psu.STATUS_LED_COLOR_GREEN if master_status_good else Psu.STATUS_LED_COLOR_RED
- Psu.set_status_master_led(color)
- except NotImplementedError:
- self.log_warning("set_status_master_led() not implemented")
+ # set_status_master_led() is a class method implemented in PsuBase
+ # so we do not need to catch NotImplementedError here
+ color = Psu.STATUS_LED_COLOR_GREEN if master_status_good else Psu.STATUS_LED_COLOR_RED
+ Psu.set_status_master_led(color)
 
  log_on_status_changed(self, self.master_status_good,
  'PSU supplied power warning cleared: supplied power is back to normal.',
@@ -351,32 +346,21 @@ class DaemonPsud(daemon_base.DaemonBase):
  def __init__(self, log_identifier):
  super(DaemonPsud, self).__init__(log_identifier)
 
- self.stop = threading.Event()
+ # Set minimum logging level to INFO
+ self.set_min_log_priority_info()
+
+ self.stop_event = threading.Event()
+ self.num_psus = 0
  self.psu_status_dict = {}
+ self.chassis_tbl = None
  self.fan_tbl = None
+ self.psu_tbl = None
  self.psu_chassis_info = None
  self.first_run = True
 
- # Signal handler
- def signal_handler(self, sig, frame):
- if sig == signal.SIGHUP:
- self.log_info("Caught SIGHUP - ignoring...")
- elif sig == signal.SIGINT:
- self.log_info("Caught SIGINT - exiting...")
- self.stop.set()
- elif sig == signal.SIGTERM:
- self.log_info("Caught SIGTERM - exiting...")
- self.stop.set()
- else:
- self.log_warning("Caught unhandled signal '{}'".format(SIGNALS_TO_NAMES_DICT[sig]))
-
- # Run daemon
- def run(self):
  global platform_psuutil
  global platform_chassis
 
- self.log_info("Starting up...")
-
  # Load new platform api class
  try:
  import sonic_platform.platform
@@ -394,52 +378,70 @@ class DaemonPsud(daemon_base.DaemonBase):
 
  # Connect to STATE_DB and create psu/chassis info tables
  state_db = daemon_base.db_connect("STATE_DB")
- chassis_tbl = swsscommon.Table(state_db, CHASSIS_INFO_TABLE)
- psu_tbl = swsscommon.Table(state_db, PSU_INFO_TABLE)
+ self.chassis_tbl = swsscommon.Table(state_db, CHASSIS_INFO_TABLE)
+ self.psu_tbl = swsscommon.Table(state_db, PSU_INFO_TABLE)
  self.fan_tbl = swsscommon.Table(state_db, FAN_INFO_TABLE)
  self.phy_entity_tbl = swsscommon.Table(state_db, PHYSICAL_ENTITY_INFO_TABLE)
 
  # Post psu number info to STATE_DB
- psu_num = _wrapper_get_num_psus()
- fvs = swsscommon.FieldValuePairs([(CHASSIS_INFO_PSU_NUM_FIELD, str(psu_num))])
- chassis_tbl.set(CHASSIS_INFO_KEY, fvs)
+ self.num_psus = _wrapper_get_num_psus()
+ fvs = swsscommon.FieldValuePairs([(CHASSIS_INFO_PSU_NUM_FIELD, str(self.num_psus))])
+ self.chassis_tbl.set(CHASSIS_INFO_KEY, fvs)
 
- # Start main loop
- self.log_info("Start daemon main loop")
+ def __del__(self):
+ # Delete all the information from DB and then exit
+ for psu_index in range(1, self.num_psus + 1):
+ self.psu_tbl._del(get_psu_key(psu_index))
 
- while not self.stop.wait(PSU_INFO_UPDATE_PERIOD_SECS):
- self._update_psu_entity_info()
- psu_db_update(psu_tbl, psu_num)
- self.update_psu_data(psu_tbl)
- self._update_led_color(psu_tbl)
+ self.chassis_tbl._del(CHASSIS_INFO_KEY)
+ self.chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1))
 
- if platform_chassis and platform_chassis.is_modular_chassis():
- self.update_psu_chassis_info(chassis_tbl)
+ # Override signal handler from DaemonBase
+ def signal_handler(self, sig, frame):
+ FATAL_SIGNALS = [signal.SIGINT, signal.SIGTERM]
+ NONFATAL_SIGNALS = [signal.SIGHUP]
 
-  self.first_run = False
+ global exit_code
 
- self.log_info("Stop daemon main loop")
+ if sig in FATAL_SIGNALS:
+ self.log_info("Caught signal '{}' - exiting...".format(SIGNALS_TO_NAMES_DICT[sig]))
+ exit_code = 128 + sig # Make sure we exit with a non-zero code so that supervisor will try to restart us
+ self.stop_event.set()
+ elif sig in NONFATAL_SIGNALS:
+ self.log_info("Caught signal '{}' - ignoring...".format(SIGNALS_TO_NAMES_DICT[sig]))
+ else:
+ self.log_warning("Caught unhandled signal '{}' - ignoring...".format(SIGNALS_TO_NAMES_DICT[sig]))
 
- # Delete all the information from DB and then exit
- for psu_index in range(1, psu_num + 1):
- psu_tbl._del(get_psu_key(psu_index))
+ # Main daemon logic
+ def run(self):
+ if self.stop_event.wait(PSU_INFO_UPDATE_PERIOD_SECS):
+ # We received a fatal signal
+ return False
 
- chassis_tbl._del(CHASSIS_INFO_KEY)
- chassis_tbl._del(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1))
+ self._update_psu_entity_info()
+ psu_db_update(self.psu_tbl, self.num_psus)
+ self.update_psu_data()
+ self._update_led_color()
 
- self.log_info("Shutting down...")
+ if platform_chassis and platform_chassis.is_modular_chassis():
+ self.update_psu_chassis_info()
 
- def update_psu_data(self, psu_tbl):
+ if self.first_run:
+ self.first_run = False
+
+ return True
+
+ def update_psu_data(self):
  if not platform_chassis:
  return
 
  for index, psu in enumerate(platform_chassis.get_all_psus()):
  try:
- self._update_single_psu_data(index + 1, psu, psu_tbl)
+ self._update_single_psu_data(index + 1, psu)
  except Exception as e:
  self.log_warning("Failed to update PSU data - {}".format(e))
 
- def _update_single_psu_data(self, index, psu, psu_tbl):
+ def _update_single_psu_data(self, index, psu):
  name = get_psu_key(index)
  presence = _wrapper_get_psu_presence(index)
  power_good = False
@@ -517,7 +519,7 @@ class DaemonPsud(daemon_base.DaemonBase):
  (PSU_INFO_POWER_FIELD, str(power)),
  (PSU_INFO_FRU_FIELD, str(is_replaceable)),
  ])
- psu_tbl.set(name, fvs)
+ self.psu_tbl.set(name, fvs)
 
  def _update_psu_entity_info(self):
  if not platform_chassis:
@@ -567,15 +569,15 @@ class DaemonPsud(daemon_base.DaemonBase):
  except NotImplementedError:
  self.log_warning("set_status_led() not implemented")
 
- def _update_led_color(self, psu_tbl):
+ def _update_led_color(self):
  if not platform_chassis:
  return
 
  for index, psu_status in self.psu_status_dict.items():
  fvs = swsscommon.FieldValuePairs([
  ('led_status', str(try_get(psu_status.psu.get_status_led, NOT_AVAILABLE)))
-  ])
- psu_tbl.set(get_psu_key(index), fvs)
+ ])
+ self.psu_tbl.set(get_psu_key(index), fvs)
  self._update_psu_fan_led_status(psu_status.psu, index)
 
  def _update_psu_fan_led_status(self, psu, psu_index):
@@ -588,14 +590,14 @@ class DaemonPsud(daemon_base.DaemonBase):
  ])
  self.fan_tbl.set(fan_name, fvs)
 
- def update_psu_chassis_info(self, chassis_tbl):
+ def update_psu_chassis_info(self):
  if not platform_chassis:
  return
 
  if not self.psu_chassis_info:
  self.psu_chassis_info = PsuChassisInfo(SYSLOG_IDENTIFIER, platform_chassis)
 
- self.psu_chassis_info.run_power_budget(chassis_tbl)
+ self.psu_chassis_info.run_power_budget(self.chassis_tbl)
  self.psu_chassis_info.update_master_status()
 
 
@@ -606,8 +608,16 @@ class DaemonPsud(daemon_base.DaemonBase):
 
 def main():
  psud = DaemonPsud(SYSLOG_IDENTIFIER)
- psud.run()
+
+ psud.log_info("Starting up...")
+
+ while psud.run():
+ pass
+
+ psud.log_info("Shutting down...")
+
+ return exit_code
 
 
 if __name__ == '__main__':
- main()
+ sys.exit(main())

sonic-psud/setup.py

@@ -23,7 +23,8 @@
  tests_require=[
  'mock>=2.0.0; python_version < "3.3"',
  'pytest',
- 'pytest-cov'
+ 'pytest-cov',
+ 'sonic_platform_common'
  ],
  classifiers=[
  'Development Status :: 4 - Beta',