fix(check-devices): Update --timeout management + add logging managem…

…ent (#117) * fix(check-devices): Update --timeout management + add logging management * fix(check-devices): Remove constraint on logging * fix(check-devices): Update logging management * fix(anta.inventory): Move from multiprocessing to multithreading * make: Add vscode debugger for check-devices * fix(anta.inventory): Rollback logging for create_device_session * fix: update from review * Update code as per gmuloc review
aristanetworks · Sep 23, 2022 · c010bae · c010bae
1 parent 9888c49
commit c010bae
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 51 deletions.
diff --git a/.gitignore b/.gitignore
@@ -94,5 +94,6 @@ venv.bak/
 # mkdocs documentation
 /site
 
-# vscode settings
-.vscode/
+# VScode settings
+.vscode/settings.json
+.vscode/
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -0,0 +1,27 @@
+{
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "check-devices debug",
+            "type": "python",
+            "request": "launch",
+            "program": "${workspaceFolder}/scripts/check-devices.py",
+            "args": [
+                "-u",
+                "admin",
+                "-p",
+                "admin123",
+                "-log",
+                "INFO",
+                "-i",
+                "${workspaceFolder}/.personal/avd-lab.yml",
+                "-c",
+                "${workspaceFolder}/.personal/ceos-catalog.yml",
+                "--table",
+                "--timeout",
+                "1"
+            ],
+            "console": "integratedTerminal"
+        }
+    ]
+}
diff --git a/anta/inventory/__init__.py b/anta/inventory/__init__.py
@@ -7,9 +7,9 @@
 
 import logging
 import ssl
-from multiprocessing import cpu_count, Pool
+from concurrent.futures import ThreadPoolExecutor
 from socket import setdefaulttimeout
-from typing import List, Optional, Union
+from typing import List, Optional, Union, Any, Iterator
 
 import yaml
 from jinja2 import Template
@@ -110,6 +110,7 @@ class AntaInventory():
     def __init__(self, inventory_file: str, username: str, password: str,
                  enable_password: str = None, auto_connect: bool = True,
                  timeout: float = 5) -> None:
+        # sourcery skip: remove-unnecessary-cast, simplify-len-comparison
         """Class constructor.
 
         Args:
@@ -123,9 +124,6 @@ def __init__(self, inventory_file: str, username: str, password: str,
         self.timeout = timeout
         self._inventory = InventoryDevices()
 
-        # Max number of thread to launch for discovery
-        self.max_multiprocessing_thread = cpu_count() + 30
-
         with open(inventory_file, 'r', encoding='utf8') as fd:
             data = yaml.load(fd, Loader=SafeLoader)
 
@@ -186,12 +184,12 @@ def _is_device_online(self, device: InventoryDevice, timeout: float = 5) -> bool
         logger.debug(f'Checking if device {device.host} is online')
         connection = Server(device.url)
         # Check connectivity
+        setdefaulttimeout(timeout)
         try:
-            setdefaulttimeout(timeout)
             connection.runCmds(1, ['show version'])
         # pylint: disable=W0703
-        except Exception:
-            logger.warning(f'Service not running on device {device.host}')
+        except Exception as exp:
+            logger.warning(f'Service not running on device {device.host} with: f{exp}')
             return False
         else:
             return True
@@ -353,7 +351,7 @@ def _inventory_read_ranges(self) -> None:
                     host_ip=str(range_increment), tags=range_def.tags)
                 range_increment += 1
 
-    def _inventory_rebuild(self, list_devices: List[InventoryDevice]) -> InventoryDevices:
+    def _inventory_rebuild(self, list_devices: Iterator[Any]) -> InventoryDevices:
         """
         _inventory_rebuild Transform a list of InventoryDevice into a InventoryDevices object.
 
@@ -494,19 +492,16 @@ def create_device_session(self, host_ip: str) -> bool:
         Returns:
             bool: True if update succeed, False if not
         """
-        logger.debug(
-            f'Searching for device {host_ip} in {[str(dev.host) for dev in self._inventory]}')
-        if len([dev for dev in self._inventory if str(dev.host) == str(host_ip)]) > 0:
-            device = [dev for dev in self._inventory if str(
-                dev.host) == str(host_ip)][0]
-            logger.debug(f'Search result is: {device}')
-            if device.is_online and not device.established and self._is_ip_exist(host_ip):
-                logger.debug(f'Trying to connect to device {str(device.host)}')
-                device = self._build_device_session(
-                    device=device, timeout=self.timeout)
-                # pylint: disable=W0104
-                [device if dev.host == device.host else dev for dev in self._inventory]
-                return True
+        logger.debug(f'Searching for device {host_ip} in {self._inventory}')
+        device = [dev for dev in self._inventory if str(dev.host) == host_ip][:1][0] or None
+        if device is None:
+            return False
+        logger.debug(f'Search result is: {device}')
+        if device.is_online and not device.established and self._is_ip_exist(host_ip):
+            logger.debug(f'Trying to connect to device {str(device.host)}')
+            device = self._build_device_session(
+                device=device, timeout=self.timeout)
+            return True
         return False
 
     ###########################################################################
@@ -535,9 +530,7 @@ def refresh_device_facts(self) -> None:
         Execute in parallel a call to _refresh_online_flag_device to test device connectivity.
         """
         logger.debug('Refreshing facts for current inventory')
-        with Pool(processes=self.max_multiprocessing_thread) as pool:
-            logger.debug('Check devices using multiprocessing')
-            results_map = pool.map(
+        with ThreadPoolExecutor() as executor:
+            results_map = executor.map(
                 self._get_from_device,  self._inventory)
-            logger.debug('Update inventory with updated data')
             self._inventory = self._inventory_rebuild(results_map)
diff --git a/scripts/check-devices.py b/scripts/check-devices.py
@@ -26,6 +26,7 @@
 import logging
 import sys
 import itertools
+from typing import Any
 from yaml import safe_load
 
 from rich.console import Console
@@ -41,25 +42,50 @@
 from anta.reporter import ReportTable
 
 
-FORMAT = "%(message)s"
-logging.basicConfig(
-    level=logging.DEBUG, format=FORMAT, datefmt="[%X]", handlers=[RichHandler()]
-)
-logging.getLogger('anta.inventory').setLevel(logging.CRITICAL)
-logging.getLogger('anta.result_manager').setLevel(logging.CRITICAL)
-logging.getLogger('anta.reporter').setLevel(logging.CRITICAL)
-logging.getLogger('anta.tests.configuration').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.hardware').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.interfaces').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.mlag').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.multicast').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.profiles').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.system').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.software').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.vxlan').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.routing.generic').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.routing.bgp').setLevel(logging.ERROR)
-logging.getLogger('anta.tests.routing.ospf').setLevel(logging.ERROR)
+def setup_logging(level: str = 'critical') -> Any:
+    """
+    Configure logging for check-devices execution
+
+    Helpers to set logging for
+    * anta.inventory
+    * anta.result_manager
+    * check-devices
+
+    Args:
+        level (str, optional): level name to configure. Defaults to 'critical'.
+
+    Returns:
+        logging: Logger for script
+    """
+    loglevel = getattr(logging, level.upper())
+
+    # FORMAT = "%(asctime)s:%(levelname)s:%(message)s"
+    FORMAT = "%(message)s"
+    logging.basicConfig(
+        level=logging.DEBUG, format=FORMAT, datefmt="[%X]", handlers=[RichHandler()]
+    )
+    logging.getLogger('anta.inventory').setLevel(loglevel)
+    logging.getLogger('anta.result_manager').setLevel(loglevel)
+
+    logging.getLogger('anta.reporter').setLevel(logging.CRITICAL)
+    logging.getLogger('anta.tests').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.configuration').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.hardware').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.interfaces').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.mlag').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.multicast').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.profiles').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.system').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.software').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.vxlan').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.routing.generic').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.routing.bgp').setLevel(logging.ERROR)
+    logging.getLogger('anta.tests.routing.ospf').setLevel(logging.ERROR)
+
+    # pylint: disable=W0621
+    logger = logging.getLogger(__name__)
+    logger.setLevel(loglevel)
+    return logger
 
 
 def cli_manager() -> argparse.Namespace:
@@ -86,8 +112,8 @@ def cli_manager() -> argparse.Namespace:
     parser.add_argument('--enable_password', '-e', required=False,
                         default='ansible', help='EOS Enable Password')
 
-    parser.add_argument('--timeout', '-t', required=False,
-                        default=0.5, help='eAPI connection timeout')
+    parser.add_argument('--timeout', '-t', required=False, type=float,
+                        default=1, help='eAPI connection timeout')
 
     #############################
     # Search options
@@ -134,13 +160,24 @@ def cli_manager() -> argparse.Namespace:
     parser.add_argument('--by-test', required=False, action='store_true',
                         help='Provides summary of test results per test case (Only valid with --table)')
 
+    # Logging option
+
+    parser.add_argument('-log', '--loglevel', default='critical',
+                        help='Provide logging level. Example --loglevel debug, default=critical')
+
     return parser.parse_args()
 
 
 if __name__ == '__main__':
-    logger = logging.getLogger(__name__)
     console = Console()
     cli_options = cli_manager()
+    logger = setup_logging(level=cli_options.loglevel)
+
+    console.print(
+        Panel.fit(f'Running check-devices with:\n\
+              - Inventory: {cli_options.inventory}\n\
+              - Tests catalog: {cli_options.catalog}', title="[green]Settings")
+    )
 
     inventory_anta = AntaInventory(
         inventory_file=cli_options.inventory,
@@ -188,7 +225,7 @@ def cli_manager() -> argparse.Namespace:
 
     logger.info('testing done !')
     if cli_options.list:
-        console.print(Panel('List results of all tests', style='cyan'))
+        console.print(Panel.fit('List results of all tests', style='cyan'))
         pprint(manager.get_results(output_format="list"))
         if cli_options.save is not None:
             with open(cli_options.save, 'w', encoding='utf-8') as fout: