From 98c5866d57f1d8a872cd54066c21e594e4ccd9c1 Mon Sep 17 00:00:00 2001
From: Erik Reid <erik.reid@geant.org>
Date: Mon, 18 Jan 2021 10:51:24 +0100
Subject: [PATCH] generate per-router logical-systems lists

---
 inventory_provider/juniper.py               | 12 ++++++++
 inventory_provider/snmp.py                  | 16 +++++-----
 inventory_provider/tasks/worker.py          | 17 +++++------
 test/per_router/conftest.py                 | 31 +++++++++++++++++++
 test/{ => per_router}/test_snmp_handling.py | 33 ++++++++++++++-------
 5 files changed, 83 insertions(+), 26 deletions(-)
 rename test/{ => per_router}/test_snmp_handling.py (71%)

diff --git a/inventory_provider/juniper.py b/inventory_provider/juniper.py
index 013b0afc..4a08c523 100644
--- a/inventory_provider/juniper.py
+++ b/inventory_provider/juniper.py
@@ -370,3 +370,15 @@ def netconf_changed_timestamp(netconf_config):
     logger = logging.getLogger(__name__)
     logger.warning('no valid timestamp found in netconf configuration')
     return None
+
+
+def logical_systems(netconf_config):
+    """
+    Return a list of logical system names for the router.
+
+    It's not an error if a router has no defined logical systems.
+
+    :param netconf_config: netconf lxml etree document
+    :return: a list of strings
+    """
+    return netconf_config.xpath('//configuration/logical-systems/name/text()')
diff --git a/inventory_provider/snmp.py b/inventory_provider/snmp.py
index 6bd56f45..2ffb90f4 100644
--- a/inventory_provider/snmp.py
+++ b/inventory_provider/snmp.py
@@ -13,7 +13,6 @@ from pysnmp.error import PySnmpError
 RFC1213_MIB_IFDESC = '1.3.6.1.2.1.2.2.1.2'
 # BGP4-V2-MIB-JUNIPER::jnxBgpM2PeerState
 JNX_BGP_M2_PEER_STATE = '1.3.6.1.4.1.2636.5.1.1.2.1.1.1.2'
-JNX_LOGICAL_SYSTEMS = ['VRR']
 logger = logging.getLogger(__name__)
 
 
@@ -144,12 +143,13 @@ def _get_router_snmp_indexes(hostname, community):
         }
 
 
-def _walk_util(hostname, default_community, walk_handler):
+def _walk_util(hostname, default_community, logical_systems, walk_handler):
     """
     Run walk_handler for default_community and
 
     :param hostname:
     :param community: base community name
+    :param logical_systems: a list of logical-systems strings, ok if empty
     :param walk_handler: a method that takes params (hostname, community)
         and yields things
     :return: generator yielding whatever walk_handler yields
@@ -157,14 +157,14 @@ def _walk_util(hostname, default_community, walk_handler):
     # do the default community last, in case of duplicates
     communities = [
         f'{ls}/default@{default_community}'
-        for ls in JNX_LOGICAL_SYSTEMS]
+        for ls in logical_systems]
     communities.append(default_community)
 
     for c in communities:
         yield from walk_handler(hostname, c)
 
 
-def get_router_snmp_indexes(hostname, community):
+def get_router_snmp_indexes(hostname, community, logical_systems):
     """
     return interface names and snmp indexes
 
@@ -173,9 +173,10 @@ def get_router_snmp_indexes(hostname, community):
 
     :param hostname:
     :param community: base community name
+    :param logical_systems: a list of logical-systems names
     :return: generator yielding dicts
     """
-    _walk_util(hostname, community, _get_router_snmp_indexes)
+    yield from _walk_util(hostname, community, logical_systems, _get_router_snmp_indexes)
 
 
 def get_peer_state_info(hostname, community):
@@ -240,7 +241,7 @@ def _get_peer_state_info(hostname, community):
         }
 
 
-def get_peer_state_info(hostname, community):
+def get_peer_state_info(hostname, community, logical_systems):
     """
     return peering states from all logical systems
 
@@ -249,9 +250,10 @@ def get_peer_state_info(hostname, community):
 
     :param hostname:
     :param community: base community name
+    :param logical_systems: a list of logical-systems names
     :return: generator yielding dicts
     """
-    _walk_util(hostname, community, _get_peer_state_info)
+    yield from _walk_util(hostname, community, logical_systems, _get_peer_state_info)
 
 # if __name__ == '__main__':
 #
diff --git a/inventory_provider/tasks/worker.py b/inventory_provider/tasks/worker.py
index 2f6edb33..bde4b0b0 100644
--- a/inventory_provider/tasks/worker.py
+++ b/inventory_provider/tasks/worker.py
@@ -87,11 +87,9 @@ class InventoryTask(Task):
 
 @app.task(base=InventoryTask, bind=True, name='snmp_refresh_peerings')
 @log_task_entry_and_exit
-def snmp_refresh_peerings(self, hostname, community):
+def snmp_refresh_peerings(self, hostname, community, logical_systems):
     try:
-        peerings = list(snmp.get_peer_state_info(hostname, community))
-        for p in peerings:
-            p['community'] = community
+        peerings = list(snmp.get_peer_state_info(hostname, community, logical_systems))
     except ConnectionError:
         msg = f'error loading snmp peering data from {hostname}'
         logger.exception(msg)
@@ -121,9 +119,9 @@ def snmp_refresh_peerings(self, hostname, community):
 
 @app.task(base=InventoryTask, bind=True, name='snmp_refresh_interfaces')
 @log_task_entry_and_exit
-def snmp_refresh_interfaces(self, hostname, community):
+def snmp_refresh_interfaces(self, hostname, community, logical_systems):
     try:
-        interfaces = list(snmp.get_router_snmp_indexes(hostname, community))
+        interfaces = list(snmp.get_router_snmp_indexes(hostname, community, logical_systems))
     except ConnectionError:
         msg = f'error loading snmp interface data from {hostname}'
         logger.exception(msg)
@@ -147,7 +145,6 @@ def snmp_refresh_interfaces(self, hostname, community):
     # interfaces is a list of dicts like: {'name': str, 'index': int}
     for ifc in interfaces:
         ifc['hostname'] = hostname
-        ifc['community'] = community
         rp.set(
             f'snmp-interfaces-single:{hostname}:{ifc["name"]}',
             json.dumps(ifc))
@@ -572,9 +569,11 @@ def reload_router_config(self, hostname):
             f'error extracting community string for {hostname}')
     else:
         self.log_info(f'refreshing snmp interface indexes for {hostname}')
+        logical_systems = juniper.logical_systems(netconf_doc)
+
         # load snmp data, in this thread
-        snmp_refresh_interfaces.apply(args=[hostname, community])
-        snmp_refresh_peerings.apply(args=[hostname, community])
+        snmp_refresh_interfaces.apply(args=[hostname, community, logical_systems])
+        snmp_refresh_peerings.apply(args=[hostname, community, logical_systems])
 
     clear_cached_classifier_responses(None)
     self.log_info(f'updated configuration for {hostname}')
diff --git a/test/per_router/conftest.py b/test/per_router/conftest.py
index f8faaf84..9c59af8d 100644
--- a/test/per_router/conftest.py
+++ b/test/per_router/conftest.py
@@ -43,3 +43,34 @@ def pytest_generate_tests(metafunc):
 
     routers = list(set(_junosspace_hosts()) & set(_available_netconf_hosts()))
     metafunc.parametrize("router", routers)
+
+
+from inventory_provider import juniper
+
+
+class MockedJunosRpc(object):
+
+    def __init__(self, hostname):
+        filename = os.path.join(TEST_DATA_DIRNAME, "%s-netconf.xml" % hostname)
+        self.config = etree.parse(filename)
+
+    def get_config(self):
+        return self.config
+
+
+class MockedJunosDevice(object):
+
+    def __init__(self, **kwargs):
+        self.rpc = MockedJunosRpc(kwargs['host'])
+
+    def open(self):
+        pass
+
+
+@pytest.fixture
+def netconf_doc(mocker, router, data_config):
+    mocker.patch(
+        'inventory_provider.juniper.Device',
+        MockedJunosDevice)
+    return juniper.load_config(router, data_config['ssh'])
+
diff --git a/test/test_snmp_handling.py b/test/per_router/test_snmp_handling.py
similarity index 71%
rename from test/test_snmp_handling.py
rename to test/per_router/test_snmp_handling.py
index 38ecb7c9..8ddaad13 100644
--- a/test/test_snmp_handling.py
+++ b/test/per_router/test_snmp_handling.py
@@ -2,7 +2,9 @@ import json
 import os
 
 import jsonschema
+import pytest
 
+from inventory_provider import juniper
 from inventory_provider import snmp
 
 import inventory_provider
@@ -14,6 +16,7 @@ TEST_DATA_DIRNAME = os.path.realpath(os.path.join(
     "data"))
 
 
+
 def interfaces_walk_responses():
     test_data_filename = os.path.join(
         TEST_DATA_DIRNAME, "snmp-walk-interfaces.json")
@@ -41,7 +44,7 @@ def _gen_mocked_nextCmd(mocked_walk_data):
     return _mocked_nextCmd
 
 
-def test_snmp_interfaces(mocker):
+def test_snmp_interfaces(mocker, netconf_doc):
 
     expected_result_schema = {
         "$schema": "http://json-schema.org/draft-07/schema#",
@@ -50,9 +53,10 @@ def test_snmp_interfaces(mocker):
             "type": "object",
             "properties": {
                 "name": {"type": "string"},
-                "index": {"type": "integer"}
+                "index": {"type": "integer"},
+                "community": {"type": "string"}
             },
-            "required": ["name", "index"],
+            "required": ["name", "index", "community"],
             "additionalProperties": False
         }
     }
@@ -64,13 +68,17 @@ def test_snmp_interfaces(mocker):
     mocker.patch('inventory_provider.snmp.nextCmd', _mocked_nextCmd)
     mocker.patch('inventory_provider.snmp.UdpTransportTarget', lambda x: None)
 
-    interfaces = list(snmp.get_router_snmp_indexes('ignored', 'ignored'))
+    logical_systems = juniper.logical_systems(netconf_doc)
+    interfaces = list(snmp.get_router_snmp_indexes('ignored', 'ignored', logical_systems))
 
     jsonschema.validate(interfaces, expected_result_schema)
-    assert len(interfaces) == len(test_data)
+    # we should loop the base community
+    # string and once per logical_system
+    expected_num_loops = 1 + len(logical_systems)
+    assert len(interfaces) == expected_num_loops * len(test_data)
 
 
-def test_peer_info(mocker):
+def test_peer_info(mocker, netconf_doc):
 
     expected_result_schema = {
         "$schema": "http://json-schema.org/draft-07/schema#",
@@ -80,9 +88,10 @@ def test_peer_info(mocker):
             "properties": {
                 "local": {"type": "string"},
                 "remote": {"type": "string"},
-                "oid": {"type": "string"}
+                "oid": {"type": "string"},
+                "community": {"type": "string"}
             },
-            "required": ["local", "remote", "oid"],
+            "required": ["local", "remote", "oid", "community"],
             "additionalProperties": False
         }
     }
@@ -94,7 +103,11 @@ def test_peer_info(mocker):
     mocker.patch('inventory_provider.snmp.nextCmd', _mocked_nextCmd)
     mocker.patch('inventory_provider.snmp.UdpTransportTarget', lambda x: None)
 
-    peerings = list(snmp.get_peer_state_info('ignored', 'ignored'))
+    logical_systems = juniper.logical_systems(netconf_doc)
+    peerings = list(snmp.get_peer_state_info('ignored', 'ignored', logical_systems))
 
     jsonschema.validate(peerings, expected_result_schema)
-    assert len(peerings) == len(test_data)
+    # we should loop the base community
+    # string and once per logical_system
+    expected_num_loops = 1 + len(logical_systems)
+    assert len(peerings) == expected_num_loops * len(test_data)
-- 
GitLab