diff --git a/brian_polling_manager/error_report/cli.py b/brian_polling_manager/error_report/cli.py index 3f1d52a236c7ae8d2f068a6bfaea4fb156e1dcf2..eb3f1c775a5eb0aaadc5443d442d8d56e667d4f4 100644 --- a/brian_polling_manager/error_report/cli.py +++ b/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...") diff --git a/brian_polling_manager/error_report/config-example.json b/brian_polling_manager/error_report/config-example.json index b9cc65d8bc8908cf43a714d1b0116359490630b4..705c961a222141f12479f155c8cf534e3d3dbb9f 100644 --- a/brian_polling_manager/error_report/config-example.json +++ b/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", diff --git a/brian_polling_manager/error_report/report.py b/brian_polling_manager/error_report/report.py index 946d4594491216b47efe499ce6ff6968b4051cff..a7086382868b7d8fba3855e8f8c36ed79fecef23 100644 --- a/brian_polling_manager/error_report/report.py +++ b/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")) diff --git a/test/error_report/data/small-inventory.json b/test/error_report/data/small-inventory.json index d0fede63bb3130be3a4acce5f5e8472d3ca489b5..dd7fae2b3762375fafe5cff089f31fa9c1711d41 100644 --- a/test/error_report/data/small-inventory.json +++ b/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", diff --git a/test/error_report/test_error_report.py b/test/error_report/test_error_report.py index 21532c68d9055aecb7f3599dfed1e37d317e8623..b1d286283142f3ab693f62dd056409d0311d0399 100644 --- a/test/error_report/test_error_report.py +++ b/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, ): diff --git a/test/test_api.py b/test/test_api.py index 0a647feed8c14f664ae78a5074f68bdd1835a8d1..2b753906c03b91e333a41de1e9fb009f8538e0d4 100644 --- a/test/test_api.py +++ b/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