374084009: Fix error on restoring DHCP server configuration if device has no DHCP lease

modules/network/dhcp-1/python/src/grpc_server/dhcp_leases.py

@@ -68,8 +68,8 @@ def get_leases(self):
         leases.append(lease)
       except Exception as e:  # pylint: disable=W0718
         # Let non lease lines file without extra checks
-        LOGGER.error('Making Lease Error: ' + str(e))
-        LOGGER.error('Not a valid lease line: ' + line)
+        LOGGER.info('Not a valid lease line: ' + line)
+        LOGGER.error('Get lease error: ' + str(e))
     return leases
 
   def delete_lease(self, ip_addr):

modules/network/dhcp-2/python/src/grpc_server/dhcp_leases.py

@@ -58,18 +58,18 @@ def get_leases(self):
     leases = []
     lease_list_raw = self._get_lease_list()
     LOGGER.info('Raw Leases:\n' + str(lease_list_raw) + '\n')
-    lease_list_start = lease_list_raw.find('=========',0)
-    lease_list_start = lease_list_raw.find('\n',lease_list_start)
-    lease_list = lease_list_raw[lease_list_start+1:]
+    lease_list_start = lease_list_raw.find('=========', 0)
+    lease_list_start = lease_list_raw.find('\n', lease_list_start)
+    lease_list = lease_list_raw[lease_list_start + 1:]
     lines = lease_list.split('\n')
     for line in lines:
       try:
         lease = DHCPLease(line)
         leases.append(lease)
       except Exception as e:  # pylint: disable=W0718
         # Let non lease lines file without extra checks
-        LOGGER.error('Making Lease Error: ' + str(e))
-        LOGGER.error('Not a valid lease line: ' + line)
+        LOGGER.info('Not a valid lease line: ' + line)
+        LOGGER.error('Get lease error: ' + str(e))
     return leases
 
   def delete_lease(self, ip_addr):

modules/network/radius/bin/start_network_service

@@ -27,7 +27,7 @@ cp $CONF_DIR/ca.crt /etc/ssl/certs/ca-certificates.crt
 
 python3 -u $PYTHON_SRC_DIR/authenticator.py &
 
-#Create and set permissions on the log file
+# Create and set permissions on the log file
 touch $LOG_FILE
 chown $HOST_USER $LOG_FILE
 

modules/test/base/python/src/test_module.py

@@ -32,7 +32,6 @@ class TestModule:
   def __init__(self,
                module_name,
                log_name,
-               log_dir=None,
                conf_file=CONF_FILE,
                results_dir=RESULTS_DIR):
     self._module_name = module_name
@@ -42,19 +41,15 @@ def __init__(self,
     self._ipv4_subnet = os.environ.get('IPV4_SUBNET', '')
     self._ipv6_subnet = os.environ.get('IPV6_SUBNET', '')
     self._dev_iface_mac = os.environ.get('DEV_IFACE_MAC', '')
-    self._add_logger(log_name=log_name,
-                     module_name=module_name,
-                     log_dir=log_dir)
+    self._add_logger(log_name=log_name)
     self._config = self._read_config(
         conf_file=conf_file if conf_file is not None else CONF_FILE)
     self._device_ipv4_addr = None
     self._device_ipv6_addr = None
 
-  def _add_logger(self, log_name, module_name, log_dir=None):
+  def _add_logger(self, log_name):
     global LOGGER
-    LOGGER = logger.get_logger(name=log_name, # pylint: disable=E1123
-                               log_file=module_name,
-                               log_dir=log_dir)
+    LOGGER = logger.get_logger(name=log_name)
 
   def generate_module_report(self):
     pass

modules/test/baseline/bin/start_test_module

@@ -41,11 +41,8 @@ else
 fi
 
 # Create and set permissions on the log files
-LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python scrip that will execute the tests for this module

modules/test/conn/bin/start_test_module

@@ -40,9 +40,7 @@ fi
 # Create and set permissions on the log files
 LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python script that will execute the tests for this module

modules/test/conn/conf/module_config.json

@@ -44,7 +44,8 @@
         "expected_behavior": "The device is not setup with a static IP address.  The device accepts an IP address from a DHCP server (RFC 2131) and responds succesfully to an ICMP echo (ping) request.",
         "recommendations": [
           "Enable DHCP",
-          "Install a DHCP client"
+          "Install a DHCP client",
+          "Ensure that your DHCP client renews its lease at the correct time"
         ]
       },
       {

modules/test/conn/python/src/connection_module.py

@@ -43,15 +43,13 @@ class ConnectionModule(TestModule):
 
   def __init__(self,
                module,
-               log_dir=None,
                conf_file=None,
                results_dir=None,
                startup_capture_file=STARTUP_CAPTURE_FILE,
                monitor_capture_file=MONITOR_CAPTURE_FILE):
 
     super().__init__(module_name=module,
                      log_name=LOG_NAME,
-                     log_dir=log_dir,
                      conf_file=conf_file,
                      results_dir=results_dir)
     global LOGGER
@@ -146,7 +144,7 @@ def _connection_switch_arp_inspection(self):
     if no_arp:
       return None, 'No ARP packets from the device found'
 
-    return True, 'Device uses ARP'
+    return True, 'Device uses ARP correctly'
 
   def _connection_switch_dhcp_snooping(self):
     LOGGER.info('Running connection.switch.dhcp_snooping')
@@ -204,8 +202,10 @@ def _connection_dhcp_address(self):
         LOGGER.info('No IP information found in lease: ' + self._device_mac)
         return False, 'No IP information found in lease: ' + self._device_mac
     else:
-      LOGGER.info('No DHCP lease could be found: ' + self._device_mac)
-      return False, 'No DHCP lease could be found: ' + self._device_mac
+      LOGGER.info('No DHCP lease could be found for MAC ' + self._device_mac +
+        ' at the time of this test')
+      return (False, 'No DHCP lease could be found for MAC ' +
+        self._device_mac + ' at the time of this test')
 
   def _connection_mac_address(self):
     LOGGER.info('Running connection.mac_address')
@@ -323,8 +323,10 @@ def _connection_ipaddr_ip_change(self, config):
         else:
           result = None, 'Failed to create reserved lease for device'
       else:
-        LOGGER.info('Device has no current DHCP lease')
-        result = None, 'Device has no current DHCP lease'
+        LOGGER.info('Device has no current DHCP lease so ' +
+          'this test could not be run')
+        result = None, ('Device has no current DHCP lease so ' +
+          'this test could not be run')
       # Restore the network
       self._dhcp_util.restore_failover_dhcp_server()
       LOGGER.info('Waiting 30 seconds for reserved lease to expire')
@@ -377,7 +379,8 @@ def _connection_ipaddr_dhcp_failover(self, config):
         else:
           result = False, 'Device did not respond to ping'
       else:
-        result = None, 'Device has no current DHCP lease'
+        result = (None,
+          'Device has no current DHCP lease so this test could not be run')
     else:
       LOGGER.error('Network is not ready for this test. Skipping')
       result = None, 'Network is not ready for this test'
@@ -688,6 +691,7 @@ def is_ip_in_range(self, ip, start_ip, end_ip):
     return start_int <= ip_int <= end_int
 
   def _run_subnet_test(self, config):
+
     # Resolve the configured dhcp subnet ranges
     ranges = None
     if 'ranges' in config:
@@ -702,6 +706,7 @@ def _run_subnet_test(self, config):
 
     response = self.dhcp1_client.get_dhcp_range()
     cur_range = {}
+
     if response.code == 200:
       cur_range['start'] = response.start
       cur_range['end'] = response.end
@@ -714,16 +719,20 @@ def _run_subnet_test(self, config):
 
     results = []
     dhcp_setup = self.setup_single_dhcp_server()
+
     if dhcp_setup[0]:
       LOGGER.info(dhcp_setup[1])
       lease = self._dhcp_util.get_cur_lease(mac_address=self._device_mac,
                                             timeout=self._lease_wait_time_sec)
+
       if lease is not None:
         if self._dhcp_util.is_lease_active(lease):
           results = self.test_subnets(ranges)
       else:
-        LOGGER.info('Failed to confirm a valid active lease for the device')
-        return None, 'Failed to confirm a valid active lease for the device'
+        LOGGER.info('Device has no current DHCP lease ' +
+          'so this test could not be run')
+        return (None,
+          'Device has no current DHCP lease so this test could not be run')
     else:
       LOGGER.error(dhcp_setup[1])
       return None, 'Failed to setup DHCP server for test'
@@ -750,10 +759,17 @@ def _run_subnet_test(self, config):
       # Wait for the current lease to expire
       lease = self._dhcp_util.get_cur_lease(mac_address=self._device_mac,
                                             timeout=self._lease_wait_time_sec)
-      self._dhcp_util.wait_for_lease_expire(lease, self._lease_wait_time_sec)
+
+      # Check if lease is active
+      if lease is not None:
+        self._dhcp_util.wait_for_lease_expire(lease, self._lease_wait_time_sec)
+      else:
+        # If not, wait for 30 seconds as a fallback
+        time.sleep(30)
 
       # Wait for a new lease to be provided before exiting test
       # to prevent other test modules from failing
+
       LOGGER.info('Checking for new lease')
       # Subnet changes tend to take longer to pick up so we'll allow
       # for twice the lease wait time

modules/test/conn/python/src/dhcp_util.py

@@ -256,16 +256,20 @@ def setup_single_dhcp_server(self):
       return False
 
   def wait_for_lease_expire(self, lease, max_wait_time=30):
+
     expiration_utc = datetime.strptime(lease['expires'], '%Y-%m-%d %H:%M:%S')
-    # lease information stored in UTC so we need to convert to local time
+
+    # Lease information stored in UTC so we need to convert to local time
     expiration = self.utc_to_local(expiration_utc)
     time_to_expire = expiration - datetime.now(tz=tz.tzlocal())
+
     # Wait until the expiration time and padd 5 seconds
     # If wait time is longer than max_wait_time, only wait
     # for the max wait time
     wait_time = min(max_wait_time,
                     time_to_expire.total_seconds() +
                     5) if time_to_expire.total_seconds() > 0 else 0
+
     LOGGER.info('Time until lease expiration: ' + str(wait_time))
     LOGGER.info('Waiting for current lease to expire: ' + str(expiration))
     if wait_time > 0:

modules/test/dns/bin/start_test_module

@@ -41,11 +41,8 @@ else
 fi
 
 # Create and set permissions on the log files
-LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python scrip that will execute the tests for this module

modules/test/dns/python/src/dns_module.py

@@ -31,15 +31,13 @@ class DNSModule(TestModule):
 
   def __init__(self,
                module,
-               log_dir=None,
                conf_file=None,
                results_dir=None,
                dns_server_capture_file=DNS_SERVER_CAPTURE_FILE,
                startup_capture_file=STARTUP_CAPTURE_FILE,
                monitor_capture_file=MONITOR_CAPTURE_FILE):
     super().__init__(module_name=module,
                      log_name=LOG_NAME,
-                     log_dir=log_dir,
                      conf_file=conf_file,
                      results_dir=results_dir)
     self.dns_server_capture_file = dns_server_capture_file

modules/test/ntp/bin/start_test_module

@@ -27,11 +27,8 @@ else
 fi
 
 # Create and set permissions on the log files
-LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python scrip that will execute the tests for this module

modules/test/ntp/python/src/ntp_module.py

@@ -30,15 +30,13 @@ class NTPModule(TestModule):
 
   def __init__(self,
                module,
-               log_dir=None,
                conf_file=None,
                results_dir=None,
                ntp_server_capture_file=NTP_SERVER_CAPTURE_FILE,
                startup_capture_file=STARTUP_CAPTURE_FILE,
                monitor_capture_file=MONITOR_CAPTURE_FILE):
     super().__init__(module_name=module,
                      log_name=LOG_NAME,
-                     log_dir=log_dir,
                      conf_file=conf_file,
                      results_dir=results_dir)
     self.ntp_server_capture_file = ntp_server_capture_file

modules/test/protocol/bin/start_test_module

@@ -38,11 +38,8 @@ else
 fi
 
 # Create and set permissions on the log files
-LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python script that will execute the tests for this module

modules/test/protocol/python/src/protocol_module.py

@@ -22,7 +22,7 @@
 
 
 class ProtocolModule(TestModule):
-  """Protocol Test module"""
+  """Protocol test module"""
 
   def __init__(self, module):
     self._supports_bacnet = False

modules/test/protocol/python/src/run.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Run Baseline module"""
+"""Run protocol module"""
 import argparse
 import signal
 import sys

modules/test/services/bin/start_test_module

@@ -41,11 +41,8 @@ else
 fi
 
 # Create and set permissions on the log files
-LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python scrip that will execute the tests for this module

modules/test/services/python/src/services_module.py

@@ -31,14 +31,12 @@ class ServicesModule(TestModule):
 
   def __init__(self,
                module,
-               log_dir=None,
                conf_file=None,
                results_dir=None,
                run=True,
                nmap_scan_results_path=None):
     super().__init__(module_name=module,
                      log_name=LOG_NAME,
-                     log_dir=log_dir,
                      conf_file=conf_file,
                      results_dir=results_dir)
     self._scan_tcp_results = None

modules/test/tls/bin/start_test_module

@@ -41,11 +41,8 @@ else
 fi
 
 # Create and set permissions on the log files
-LOG_FILE=/runtime/output/$MODULE_NAME.log
 RESULT_FILE=/runtime/output/$MODULE_NAME-result.json
-touch $LOG_FILE
 touch $RESULT_FILE
-chown $HOST_USER $LOG_FILE
 chown $HOST_USER $RESULT_FILE
 
 # Run the python scrip that will execute the tests for this module

modules/test/tls/python/src/tls_module.py

@@ -32,15 +32,13 @@ class TLSModule(TestModule):
 
   def __init__(self,
                module,
-               log_dir=None,
                conf_file=None,
                results_dir=None,
                startup_capture_file=STARTUP_CAPTURE_FILE,
                monitor_capture_file=MONITOR_CAPTURE_FILE,
                tls_capture_file=TLS_CAPTURE_FILE):
     super().__init__(module_name=module,
                      log_name=LOG_NAME,
-                     log_dir=log_dir,
                      conf_file=conf_file,
                      results_dir=results_dir)
     self.startup_capture_file = startup_capture_file

testing/unit/conn/conn_module_test.py

@@ -133,7 +133,6 @@ def connection_port_speed_autonegotiation_fail_test(self):
   def connection_switch_dhcp_snooping_icmp_test(self):
     LOGGER.info('connection_switch_dhcp_snooping_icmp_test')
     conn_module = ConnectionModule(module=MODULE,
-                           log_dir=OUTPUT_DIR,
                            results_dir=OUTPUT_DIR,
                            startup_capture_file=STARTUP_CAPTURE_FILE,
                            monitor_capture_file=MONITOR_CAPTURE_FILE)