Improve Configuration Handling and Runtime Validation

framework/python/src/api/api.py

@@ -550,8 +550,7 @@ async def delete_device(self, request: Request, response: Response):
       response.status_code = 200
       return self._generate_msg(True, "Successfully deleted the device")
 
-    # TODO: Find specific exception to catch
-    except Exception as e:  # pylint: disable=W0703
+    except (OSError, ValueError) as e:
       LOGGER.error(e)
       response.status_code = 500
       return self._generate_msg(

framework/python/src/common/docker_util.py

@@ -14,17 +14,16 @@
 """Utility for common docker methods"""
 import docker
 
-def create_private_net(network_name):
+def create_private_net(network_name, subnet='100.100.0.0/16', iprange='100.100.100.0/24'):
   client = docker.from_env()
   try:
     network = client.networks.get(network_name)
     network.remove()
   except docker.errors.NotFound:
     pass
 
-  # TODO: These should be made into variables
-  ipam_pool = docker.types.IPAMPool(subnet='100.100.0.0/16',
-                                    iprange='100.100.100.0/24')
+  ipam_pool = docker.types.IPAMPool(subnet=subnet,
+                                    iprange=iprange)
 
   ipam_config = docker.types.IPAMConfig(pool_configs=[ipam_pool])
 

framework/python/src/common/logger.py

@@ -36,9 +36,12 @@
   if 'log_level' in system_conf_json:
     log_level_str = system_conf_json['log_level']
     log_level = logging.getLevelName(log_level_str)
-except OSError:
-  # TODO: Print out warning that log level is incorrect or missing
-  log_level = _DEFAULT_LEVEL
+  else:
+    print(f'Warning: log_level not specified in {_CONF_FILE_NAME}. '
+          f'Using default: {logging.getLevelName(_DEFAULT_LEVEL)}')
+except (OSError, ValueError) as e:
+  print(f'Warning: Could not load {_CONF_FILE_NAME} ({str(e)}). '
+        f'Using default log level: {logging.getLevelName(_DEFAULT_LEVEL)}')
 
 log_format = logging.Formatter(fmt=_LOG_FORMAT, datefmt=_DATE_FORMAT)
 

framework/python/src/core/session.py

@@ -40,6 +40,7 @@
 ORG_NAME_KEY = 'org_name'
 TEST_CONFIG_KEY = 'test_modules'
 ALLOW_DISCONNECT_KEY='allow_disconnect'
+UI_PORT_KEY = 'ui_port'
 CERTS_PATH = 'local/root_certs'
 CONFIG_FILE_PATH = 'local/system.json'
 STATUS_TOPIC = 'status'
@@ -163,9 +164,11 @@ def __init__(self, root_dir):
     self.load_certs()
 
     # Fetch the timezone of the host system
-    tz = util.run_command('cat /etc/timezone')
-    # TODO: Check if timezone is fetched successfully
-    self._timezone = tz[0]
+    tz = util.run_command('cat /etc/timezone', supress_error=True)
+    if tz and tz[0].strip():
+      self._timezone = tz[0].strip()
+    else:
+      self._timezone = 'UTC'
     LOGGER.debug(f'System timezone is {self._timezone}')
 
     # MQTT client
@@ -210,6 +213,7 @@ def _get_default_config(self):
         'max_device_reports': 0,
         'api_url': 'http://localhost',
         'api_port': 8000,
+        'ui_port': 8080,
         'org_name': '',
         'single_intf': False,
     }
@@ -272,6 +276,9 @@ def _load_config(self):
           TEST_CONFIG_KEY
         )
 
+      if UI_PORT_KEY in config_file_json:
+        self._config[UI_PORT_KEY] = config_file_json.get(UI_PORT_KEY)
+
   def _load_version(self):
     version_cmd = util.run_command(
         'dpkg-query --showformat=\'${Version}\' --show testrun')
@@ -356,6 +363,9 @@ def get_api_url(self):
   def get_api_port(self):
     return self._config.get(API_PORT_KEY)
 
+  def get_ui_port(self):
+    return self._config.get(UI_PORT_KEY)
+
   def get_max_device_reports(self):
     return self._config.get(MAX_DEVICE_REPORTS_KEY)
 

framework/python/src/core/testrun.py

@@ -513,21 +513,21 @@ def start_ui(self):
 
     client = docker.from_env()
 
+    ui_port = self._session.get_ui_port()
     try:
       client.containers.run(image='testrun/ui',
                             auto_remove=True,
                             name='tr-ui',
                             hostname='testrun.io',
                             detach=True,
-                            ports={'80': 8080})
+                            ports={'80': ui_port})
     except docker.errors.ImageNotFound as ie:
       LOGGER.error('An error occurred whilst starting the UI. ' +
                    'Please investigate and try again.')
       LOGGER.error(ie)
       sys.exit(1)
 
-    # TODO: Make port configurable
-    LOGGER.info('User interface is ready on http://localhost:8080')
+    LOGGER.info(f'User interface is ready on http://localhost:{ui_port}')
 
   def _stop_ui(self):
     LOGGER.info('Stopping user interface')

framework/python/src/net_orc/ip_control.py

@@ -211,10 +211,9 @@ def configure_container_interface(self,
 
     if container_name is not None:
       # Get PID for running container
-      # TODO: Some error checking around missing PIDs might be required
       container_pid = util.run_command('docker inspect -f {{.State.Pid}} ' +
                                        container_name)[0]
-      if not container_pid.isdigit():
+      if not container_pid or not container_pid.isdigit() or container_pid == '0':
         LOGGER.error(f'Failed to resolve pid for {container_name}')
         return False
 

framework/python/src/net_orc/network_orchestrator.py

@@ -292,7 +292,6 @@ def _dhcp_lease_ack(self, packet):
     if device is None:
       return
 
-    # TODO: Check if device is None
     device.ip_addr = packet[BOOTP].yiaddr
 
   def _start_device_monitor(self, device):
@@ -558,9 +557,11 @@ def attach_test_module_to_network(self, test_module):
     self._ovs.add_port(port=bridge_intf, bridge_name=DEVICE_BRIDGE)
 
     # Get PID for running container
-    # TODO: Some error checking around missing PIDs might be required
     container_pid = util.run_command('docker inspect -f {{.State.Pid}} ' +
                                      test_module.container_name)[0]
+    if not container_pid or not container_pid.isdigit() or container_pid == '0':
+      LOGGER.error(f'Failed to get container PID for {test_module.container_name}')
+      return
 
     # Create symlink for container network namespace
     util.run_command('ln -sf /proc/' + container_pid +

framework/python/src/net_orc/network_validator.py

@@ -259,9 +259,11 @@ def _attach_device_to_network(self, device):
     util.run_command('ovs-vsctl add-port ' + DEVICE_BRIDGE + ' ' + bridge_intf)
 
     # Get PID for running container
-    # TODO: Some error checking around missing PIDs might be required
     container_pid = util.run_command('docker inspect -f {{.State.Pid}} ' +
                                      device.container_name)[0]
+    if not container_pid or not container_pid.isdigit() or container_pid == '0':
+      LOGGER.error(f'Failed to get container PID for {device.container_name}')
+      return
 
     # Create symlink for container network namespace
     util.run_command('ln -sf /proc/' + container_pid +

modules/network/base/python/src/logger.py

@@ -26,16 +26,21 @@
 _LOG_DIR = '/runtime/network/'
 
 # Set log level
+log_level = _DEFAULT_LEVEL
 try:
   with open(os.path.join(_CONF_DIR, _CONF_FILE_NAME),
             encoding='UTF-8') as config_json_file:
     system_conf_json = json.load(config_json_file)
 
-  log_level_str = system_conf_json['log_level']
-  log_level = logging.getLevelName(log_level_str)
-except OSError:
-  # TODO: Print out warning that log level is incorrect or missing
-  log_level = _DEFAULT_LEVEL
+  if 'log_level' in system_conf_json:
+    log_level_str = system_conf_json['log_level']
+    log_level = logging.getLevelName(log_level_str)
+  else:
+    print(f'Warning: log_level not specified in {_CONF_FILE_NAME}. '
+          f'Using default: {logging.getLevelName(_DEFAULT_LEVEL)}')
+except (OSError, KeyError, ValueError) as e:
+  print(f'Warning: Could not load {_CONF_FILE_NAME} ({str(e)}). '
+        f'Using default log level: {logging.getLevelName(_DEFAULT_LEVEL)}')
 
 log_format = logging.Formatter(fmt=_LOG_FORMAT, datefmt=_DATE_FORMAT)