Merge branch 'develop' into POL1-487-fixes

Changelog.md

@@ -2,6 +2,9 @@
 
 All notable changes to this project will be documented in this file.
 
+## [0.81] - 2022-02-17
+- POL1-521: handle RPC timeout error properly, and log errors 
+
 ## [0.80] - 2022-02-16
 - POL1-487: Adjusted filtering for COPERNICUS dashboard (no longer overrides customer names)
 

inventory_provider/juniper.py

+import contextlib
 import logging
 import re
 import ipaddress
@@ -114,6 +115,7 @@ class NetconfHandlingError(Exception):
     pass
 
 
+@contextlib.contextmanager
 def _rpc(hostname, ssh):
     dev = Device(
         host=hostname,
@@ -121,9 +123,9 @@ def _rpc(hostname, ssh):
         ssh_private_key_file=ssh['private-key'])
     try:
         dev.open()
-    except (EzErrors.ConnectError, EzErrors.RpcError) as e:
-        raise ConnectionError(str(e))
-    return dev.rpc
+        yield dev.rpc
+    finally:
+        dev.close()
 
 
 def validate_netconf_config(config_doc):
@@ -166,14 +168,18 @@ def load_config(hostname, ssh_params, validate=True):
     :param ssh_params: 'ssh' config element(cf. config.py:CONFIG_SCHEMA)
     :param validate: whether or not to validate netconf data (default True)
     :return:
-    :raises: NetconfHandlingError from validate_netconf_config
+    :raises: NetconfHandlingError or ConnectionError
     """
     logger = logging.getLogger(__name__)
     logger.info("capturing netconf data for '%s'" % hostname)
-    config = _rpc(hostname, ssh_params).get_config()
-    if validate:
-        validate_netconf_config(config)
-    return config
+    try:
+        with _rpc(hostname, ssh_params) as router:
+            config = router.get_config()
+        if validate:
+            validate_netconf_config(config)
+        return config
+    except (EzErrors.ConnectError, EzErrors.RpcError) as e:
+        raise ConnectionError(str(e))
 
 
 def list_interfaces(netconf_config):

inventory_provider/tasks/worker.py

@@ -562,9 +562,13 @@ def reload_lab_router_config_chorded(self, hostname):
                 hostname, community, logical_systems, self.log_info)
 
         self.log_info(f'updated configuration for lab {hostname}')
-    except Exception as e:
-        logger.error(e)
+    except Exception:
+        errmsg = f'unhandled exception loading {hostname} info'
+        logger.exception(errmsg)
         update_latch_status(InventoryTask.config, pending=True, failure=True)
+        self.log_error(errmsg)
+        # TODO: re-raise and handle in some common way for all tasks
+        # raise
 
 
 @app.task(base=InventoryTask, bind=True, name='reload_router_config')
@@ -598,9 +602,13 @@ def reload_router_config_chorded(self, hostname):
             snmp_refresh_peerings_chorded(hostname, community, logical_systems)
 
         logger.info(f'updated configuration for {hostname}')
-    except Exception as e:
-        logger.error(e)
+    except Exception:
+        errmsg = f'unhandled exception loading {hostname} info'
+        logger.exception(errmsg)
         update_latch_status(InventoryTask.config, pending=True, failure=True)
+        self.log_error(errmsg)
+        # TODO: re-raise and handle in some common way for all tasks
+        # raise
 
 
 def retrieve_and_persist_netconf_config(

setup.py

@@ -2,7 +2,7 @@ from setuptools import setup, find_packages
 
 setup(
     name='inventory-provider',
-    version="0.81",
+    version="0.82",
     author='GEANT',
     author_email='swd@geant.org',
     description='Dashboard inventory provider',

test/per_router/conftest.py

@@ -54,6 +54,9 @@ class MockedJunosDevice(object):
     def open(self):
         pass
 
+    def close(self):
+        pass
+
 
 @pytest.fixture
 def netconf_doc(mocker, router, data_config):

test/per_router/test_celery_worker.py

@@ -3,6 +3,9 @@ just checks that the worker methods call the right functions
 and some data ends up in the right place ... otherwise not very detailed
 """
 import re
+
+import pytest
+
 from inventory_provider.tasks import worker
 from inventory_provider.tasks.common import _get_redis
 
@@ -18,6 +21,9 @@ def backend_db():
 
 
 def test_netconf_refresh_config(mocked_worker_module, router):
+    if router in ['qfx.par.fr.geant.net', 'qfx.fra.de.geant.net']:
+        # expected to fail
+        pytest.skip(f'test data has no community string for {router}')
     del backend_db()['netconf:' + router]
     worker.reload_router_config_chorded(router)
     assert backend_db()['netconf:' + router]
@@ -94,7 +100,7 @@ def test_reload_router_config(mocked_worker_module, router, mocker):
         _mocked_snmp_refresh_interfaces_chorded)
 
     def _mocked_snmp_refresh_peerings_chorded(*args, **kwargs):
-        assert len(args) == 4
+        assert len(args) == 3
         backend_db().update(saved_peerings)
     mocker.patch(
         'inventory_provider.tasks.worker.snmp_refresh_peerings_chorded',

tox.ini

@@ -17,8 +17,7 @@ commands =
     coverage run --source inventory_provider -m py.test {posargs}
     coverage xml
     coverage html
-    coverage report --fail-under 75
-    # coverage report --fail-under 80
+    coverage report --fail-under 80
     flake8
     sphinx-build -M html docs/source docs/build