Merge branch 'feature/use-error-report-interfaces' into 'develop'

Use special inprov endpoint for error report

See merge request live-projects/brian-polling-manager!11

brian_polling_manager/error_report/cli.py

@@ -47,7 +47,10 @@ import pathlib
 import sys
 from typing import Sequence, Tuple
 from brian_polling_manager.influx import influx_client
-from brian_polling_manager.inventory import load_interfaces
+from brian_polling_manager.inventory import (
+    INVENTORY_INTERFACES_SCHEMA,
+    load_inventory_json,
+)
 import click
 from influxdb import InfluxDBClient
 from brian_polling_manager.error_report.config import load
@@ -59,6 +62,7 @@ from brian_polling_manager.error_report.report import (
 
 logger = logging.getLogger(__name__)
 
+DEFAULT_INTERFACES_URL = "/poller/error-report-interfaces"
 
 # The error field names in the influx query vs their reporting name
 ERROR_FIELDS = {
@@ -303,35 +307,20 @@ def is_excluded_interface(ifc, exclusions: Sequence[Tuple[str, str]]):
     )
 
 
-def get_relevant_interfaces(hosts):
+def get_relevant_interfaces(config):
     """Get interface info from inventory provider. Some interfaces are considered
     irrelevant based on their description
     """
-    return _filter_and_sort_interfaces(load_interfaces(hosts))
-
-
-def _filter_and_sort_interfaces(interfaces):
-    # We may want to put this logic inside inventory provider and serve from a new
-    # endpoint
-    return dict(
-        sorted(
-            ((i["router"], i["name"]), i)
-            for i in interfaces
-            if all(
-                (
-                    "PHY" in i["description"].upper(),
-                    "SPARE" not in i["description"].upper(),
-                    "NON-OPERATIONAL" not in i["description"].upper(),
-                    "RESERVED" not in i["description"].upper(),
-                    "TEST" not in i["description"].upper(),
-                    "dsc." not in i["name"].lower(),
-                    "fxp" not in i["name"].lower(),
-                )
-            )
-        )
+    url = config.get("inventory-url") or DEFAULT_INTERFACES_URL
+    return _sort_interfaces(
+        load_inventory_json(url, config["inventory"], INVENTORY_INTERFACES_SCHEMA)
     )
 
 
+def _sort_interfaces(interfaces):
+    return dict(sorted(((i["router"], i["name"]), i) for i in interfaces))
+
+
 def main(config: dict, send_mail: bool):
     """Main function for the error reporting script
 
@@ -339,7 +328,7 @@ def main(config: dict, send_mail: bool):
     """
     logger.info(f"Retrieving interfaces from inventory provider: {config['inventory']}")
 
-    all_interfaces = get_relevant_interfaces(config["inventory"])
+    all_interfaces = get_relevant_interfaces(config)
     client = influx_client(config["influx"])
     with client:
         logger.info("Retrieving error points from influxdb...")

brian_polling_manager/error_report/config-example.json

@@ -10,6 +10,7 @@
     "starttls": false
   },
   "inventory": ["blah"],
+  "inventory-url": "/poller/interfaces",
   "influx": {
     "hostname": "hostname",
     "database": "dbname",

brian_polling_manager/error_report/report.py

@@ -36,7 +36,8 @@ def render_email(
     message["To"] = "; ".join(email_config["to"])
     if email_config.get("cc"):
         message["Cc"] = "; ".join(email_config["cc"])
-
+    if email_config.get("reply_to"):
+        message["Reply-To"] = email_config["reply_to"]
     message["Subject"] = subject
 
     message.attach(MIMEText(body, "plain"))

test/error_report/data/small-inventory.json

@@ -31,22 +31,6 @@
       ],
       "snmp-index": 1006
   },
-  {
-      "router": "mx1.fra.de.geant.net",
-      "name": "ae99.1",
-      "bundle": [],
-      "bundle-parents": [],
-      "description": "SRV blah blah bar",
-      "circuits": [
-          {
-              "id": 50028,
-              "name": "something",
-              "type": "SERVICE",
-              "status": "operational"
-          }
-      ],
-      "snmp-index": 9999
-  },
   {
     "router": "rt0.ams.nl.geant.net",
     "name": "lag-1",

test/error_report/test_error_report.py

@@ -19,9 +19,8 @@ from brian_polling_manager.error_report.cli import (
     INFLUX_TIME_WINDOW_YESTERDAY,
     PROCESSED_ERROR_COUNTERS_SCHEMA,
     MessageCountingLogHandler,
-    _filter_and_sort_interfaces,
+    _sort_interfaces,
     get_error_points,
-    get_relevant_interfaces,
     interface_errors,
     is_excluded_interface,
     select_error_fields,
@@ -32,11 +31,6 @@ from click.testing import CliRunner
 DATA_DIR = pathlib.Path(__file__).parent / "data"
 
 
-@pytest.fixture(scope="session")
-def full_inventory():
-    return json.loads(((DATA_DIR / "full-inventory.json").read_text()))
-
-
 @pytest.fixture(scope="session")
 def small_inventory():
     return json.loads(((DATA_DIR / "small-inventory.json").read_text()))
@@ -123,7 +117,7 @@ def create_error_point(mock_influx_client):
 
 @pytest.fixture
 def get_interface_errors(small_inventory, mock_influx_client):
-    interfaces = _filter_and_sort_interfaces(small_inventory)
+    interfaces = _sort_interfaces(small_inventory)
 
     def _get_interface_errors(**kwargs):
         defaults = {
@@ -188,23 +182,6 @@ def test_validate_config(tmp_path):
     }
 
 
-def test_get_relevant_interfaces(full_inventory):
-    with patch.object(cli, "load_interfaces", return_value=full_inventory) as mock:
-        result = get_relevant_interfaces("some-host")
-        assert mock.call_args == call("some-host")
-        assert 0 < len(result) < len(full_inventory)
-        assert all(
-            "PHY" in i["description"].upper()
-            and "SPARE" not in i["description"].upper()
-            and "NON-OPERATIONAL" not in i["description"].upper()
-            and "RESERVED" not in i["description"].upper()
-            and "TEST" not in i["description"].upper()
-            and "dsc." not in i["name"].lower()
-            and "fxp" not in i["name"].lower()
-            for i in result.values()
-        )
-
-
 @pytest.mark.parametrize(
     "router, interface, exclusions, is_excluded",
     [
@@ -552,6 +529,18 @@ def test_render_email_for_multiple_recipients():
     assert "Cc: cc1@geant.org; cc2@geant.org" in result
 
 
+def test_render_email_with_reply_to():
+    body = "<SOME_BODY>"
+    config = {
+        "from": "someone@geant.org",
+        "to": ["someone.else@geant.org"],
+        "reply_to": "replyto@geant.org",
+    }
+    result = render_email(config, html=body, subject="<subject>")
+
+    assert "Reply-To: replyto@geant.org" in result
+
+
 @patch.object(smtplib, "SMTP")
 def test_send_email(SMTP):
     config = {
@@ -667,7 +656,7 @@ def test_e2e(
 ):
     create_error_point("mx1.ams.nl.geant.net", "ae1", "today", input_drops=1)
     with patch.object(
-        cli, "load_interfaces", return_value=small_inventory
+        cli, "load_inventory_json", return_value=small_inventory
     ), patch.object(cli, "influx_client", return_value=mock_influx_client):
         runner = CliRunner()
         result = runner.invoke(cli.cli, ["--config", str(config_file)])
@@ -691,7 +680,7 @@ def test_e2e(
 @patch.object(cli, "setup_logging", return_value=MessageCountingLogHandler())
 @patch.object(cli, "main")
 def test_e2e_with_errors(
-    unused_mainmain,
+    unused_main,
     setup_logging,
     config_file,
 ):

test/test_api.py

@@ -12,8 +12,10 @@ from brian_polling_manager.main import REFRESH_RESULT_SCHEMA
 
 @pytest.fixture
 def client(config_filename, mocked_sensu, mocked_inventory):
-    os.environ['CONFIG_FILENAME'] = config_filename
-    with brian_polling_manager.create_app().test_client() as c:
+    os.environ["CONFIG_FILENAME"] = config_filename
+    app = brian_polling_manager.create_app()
+    app.testing = True
+    with app.test_client() as c:
         yield c