diff --git a/Changelog.md b/Changelog.md
index b6cd2534d833a6842d0aad571a8a8a7bd5b3e63b..294dda7c630383e848c2c2c962dd89b9c8ee9c2e 100644
--- a/Changelog.md
+++ b/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)
diff --git a/inventory_provider/juniper.py b/inventory_provider/juniper.py
index 1976176493c60059a3b0a969039da2d102f69117..3f1f94a3860a18dfd7ed0aba68b69d1718421ac5 100644
--- a/inventory_provider/juniper.py
+++ b/inventory_provider/juniper.py
@@ -1,3 +1,4 @@
+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):
diff --git a/inventory_provider/tasks/worker.py b/inventory_provider/tasks/worker.py
index 4d2ee9dbc3e62a8fd81f855f3000317939d37269..aca83241178140922013115cd2727ff337c62074 100644
--- a/inventory_provider/tasks/worker.py
+++ b/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(
diff --git a/setup.py b/setup.py
index c53c286396dd23dc7b41aa4a71245306a4690a59..bb0df08010d2045b0b1c495740ce11d1284bb9f6 100644
--- a/setup.py
+++ b/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',
diff --git a/test/per_router/conftest.py b/test/per_router/conftest.py
index 559167093813de008c2682b7d2cb065029f8cd92..9fc9ac4790cefd26df008c72cfc2a61539ca482e 100644
--- a/test/per_router/conftest.py
+++ b/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):
diff --git a/test/per_router/test_celery_worker.py b/test/per_router/test_celery_worker.py
index 28776edbf5ea5233a47c2e3011bfd95258672df3..fea7c407711f953bdb06fe3ea6074860d191478f 100644
--- a/test/per_router/test_celery_worker.py
+++ b/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',
diff --git a/tox.ini b/tox.ini
index 1f32f60ae56f411df6f492156ae11ebf471692ef..eb2bec7d2f9b8ba10ead57bbc9cf2591a675e095 100644
--- a/tox.ini
+++ b/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