From 5c9120e40f676e3149a44290b7831fa44a882b32 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 13:30:07 -0300 Subject: [PATCH 1/9] Parametrized force option and added more logs for errors --- main.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/main.py b/main.py index aa6292b..316fc3c 100644 --- a/main.py +++ b/main.py @@ -128,11 +128,17 @@ def handle_lldp_flows(self, event): if flow: destination = switch.id endpoint = f'{settings.FLOW_MANAGER_URL}/flows/{destination}' - data = {'flows': [flow]} + data = {'force': True, 'flows': [flow]} if event.name == 'kytos/topology.switch.enabled': - requests.post(endpoint, json=data) + res = requests.post(endpoint, json=data) + if res.status_code != 202: + log.error(f"Failed to push flows on {destination} error: " + f"{res.text}, status: {res.status_code}") else: - requests.delete(endpoint, json=data) + res = requests.delete(endpoint, json=data) + if res.status_code != 202: + log.error(f"Failed to delete flows on {destination} error:" + f" {res.text}, status: {res.status_code}") @listen_to('kytos/of_core.v0x0[14].messages.in.ofpt_packet_in') def notify_uplink_detected(self, event): From 00284739af4a15df311f7643a3ea6e7176d0900e Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 14:15:17 -0300 Subject: [PATCH 2/9] Updated status code error handling on handle_lldp_flows --- main.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/main.py b/main.py index 316fc3c..ec039a2 100644 --- a/main.py +++ b/main.py @@ -128,17 +128,29 @@ def handle_lldp_flows(self, event): if flow: destination = switch.id endpoint = f'{settings.FLOW_MANAGER_URL}/flows/{destination}' - data = {'force': True, 'flows': [flow]} + data = {'flows': [flow]} if event.name == 'kytos/topology.switch.enabled': res = requests.post(endpoint, json=data) if res.status_code != 202: - log.error(f"Failed to push flows on {destination} error: " - f"{res.text}, status: {res.status_code}") + log.error(f"Failed to push flows on {destination}," + f" error: {res.text}, " f"status: {res.status_code}") + if res.status_code == 424: + data["force"] = True + res = requests.post(endpoint, json=data) + if res.status_code != 202: + return + log.info(f"Successfully force pushed flows to {destination}") else: res = requests.delete(endpoint, json=data) if res.status_code != 202: - log.error(f"Failed to delete flows on {destination} error:" - f" {res.text}, status: {res.status_code}") + log.error(f"Failed to delete flows on {destination}," + f" error: {res.text}, " f"status: {res.status_code}") + if res.status_code == 424: + data["force"] = True + res = requests.post(endpoint, json=data) + if res.status_code != 202: + return + log.info(f"Successfully force deleted flows to {destination}") @listen_to('kytos/of_core.v0x0[14].messages.in.ofpt_packet_in') def notify_uplink_detected(self, event): From 2dc963c83114f146221d3d403dcc647439496a79 Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 14:37:50 -0300 Subject: [PATCH 3/9] Added _retry_if_status_code --- main.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/main.py b/main.py index ec039a2..62768fb 100644 --- a/main.py +++ b/main.py @@ -124,6 +124,20 @@ def handle_lldp_flows(self, event): except AttributeError: of_version = None + def _retry_if_status_code(response, endpoint, data, status_codes): + """Retry if the response is in the status_codes.""" + if response.status_code not in status_codes: + return + data = dict(data) + data["force"] = True + res = requests.post(endpoint, json=data) + method = res.request.method + if res.status_code != 202: + log.error(f"Failed to retry on {endpoint}, error: {res.text}," + f" status: {res.status_code}, method: {method}") + return + log.info(f"Successfully forced {method} flows to {endpoint}") + flow = self._build_lldp_flow(of_version) if flow: destination = switch.id @@ -134,23 +148,13 @@ def handle_lldp_flows(self, event): if res.status_code != 202: log.error(f"Failed to push flows on {destination}," f" error: {res.text}, " f"status: {res.status_code}") - if res.status_code == 424: - data["force"] = True - res = requests.post(endpoint, json=data) - if res.status_code != 202: - return - log.info(f"Successfully force pushed flows to {destination}") + _retry_if_status_code(res, endpoint, data, [424]) else: res = requests.delete(endpoint, json=data) if res.status_code != 202: log.error(f"Failed to delete flows on {destination}," f" error: {res.text}, " f"status: {res.status_code}") - if res.status_code == 424: - data["force"] = True - res = requests.post(endpoint, json=data) - if res.status_code != 202: - return - log.info(f"Successfully force deleted flows to {destination}") + _retry_if_status_code(res, endpoint, data, [424]) @listen_to('kytos/of_core.v0x0[14].messages.in.ofpt_packet_in') def notify_uplink_detected(self, event): From 6230b8f4a605bbad6ffef2fcc6678c5c02083c6f Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 14:40:11 -0300 Subject: [PATCH 4/9] Linter fixes --- main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.py b/main.py index 62768fb..71dfdd9 100644 --- a/main.py +++ b/main.py @@ -147,13 +147,13 @@ def _retry_if_status_code(response, endpoint, data, status_codes): res = requests.post(endpoint, json=data) if res.status_code != 202: log.error(f"Failed to push flows on {destination}," - f" error: {res.text}, " f"status: {res.status_code}") + f" error: {res.text}, status: {res.status_code}") _retry_if_status_code(res, endpoint, data, [424]) else: res = requests.delete(endpoint, json=data) if res.status_code != 202: log.error(f"Failed to delete flows on {destination}," - f" error: {res.text}, " f"status: {res.status_code}") + f" error: {res.text}, status: {res.status_code}") _retry_if_status_code(res, endpoint, data, [424]) @listen_to('kytos/of_core.v0x0[14].messages.in.ofpt_packet_in') From a4aae2ff4d6ed357209280ad1a8dc0266865628b Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 17:42:14 -0300 Subject: [PATCH 5/9] Parametrized retries, wait and linter fixes --- main.py | 19 ++++++++++++++----- setup.cfg | 2 +- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/main.py b/main.py index 71dfdd9..175cadb 100644 --- a/main.py +++ b/main.py @@ -1,5 +1,6 @@ """NApp responsible to discover new switches and hosts.""" import struct +import time import requests from flask import jsonify, request @@ -124,18 +125,24 @@ def handle_lldp_flows(self, event): except AttributeError: of_version = None - def _retry_if_status_code(response, endpoint, data, status_codes): + def _retry_if_status_code(response, endpoint, data, status_codes, + retries=3, wait=2): """Retry if the response is in the status_codes.""" if response.status_code not in status_codes: return + if retries - 1 <= 0: + return data = dict(data) data["force"] = True res = requests.post(endpoint, json=data) method = res.request.method if res.status_code != 202: log.error(f"Failed to retry on {endpoint}, error: {res.text}," - f" status: {res.status_code}, method: {method}") - return + f" status: {res.status_code}, method: {method}," + f" data: {data}") + time.sleep(wait) + return _retry_if_status_code(response, endpoint, data, + status_codes, retries - 1, wait) log.info(f"Successfully forced {method} flows to {endpoint}") flow = self._build_lldp_flow(of_version) @@ -147,13 +154,15 @@ def _retry_if_status_code(response, endpoint, data, status_codes): res = requests.post(endpoint, json=data) if res.status_code != 202: log.error(f"Failed to push flows on {destination}," - f" error: {res.text}, status: {res.status_code}") + f" error: {res.text}, status: {res.status_code}," + f" data: {data}") _retry_if_status_code(res, endpoint, data, [424]) else: res = requests.delete(endpoint, json=data) if res.status_code != 202: log.error(f"Failed to delete flows on {destination}," - f" error: {res.text}, status: {res.status_code}") + f" error: {res.text}, status: {res.status_code}", + f" data: {data}") _retry_if_status_code(res, endpoint, data, [424]) @listen_to('kytos/of_core.v0x0[14].messages.in.ofpt_packet_in') diff --git a/setup.cfg b/setup.cfg index fd06696..f98c96e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -3,7 +3,7 @@ exclude = .eggs,ENV,build,docs/conf.py,venv [yala] radon mi args = --min C -pylint args = --disable=too-many-locals,too-few-public-methods,too-many-instance-attributes --ignored-modules=napps.kytos.of_lldp +pylint args = --disable=too-many-locals,too-few-public-methods,too-many-instance-attributes,too-many-arguments,inconsistent-return-statements --ignored-modules=napps.kytos.of_lldp [pydocstyle] add-ignore = D105 From 364378cd6efda4c718261f1a71c3084f93ee7aaa Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 17:56:53 -0300 Subject: [PATCH 6/9] Extracted _handle_lldp_flows for testability Parametrized retry for 500 --- main.py | 13 +++++++++++-- tests/unit/test_main.py | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/main.py b/main.py index 175cadb..9abc54b 100644 --- a/main.py +++ b/main.py @@ -116,6 +116,15 @@ def handle_lldp_flows(self, event): event (:class:`~kytos.core.events.KytosEvent`): Event with new switch information. + """ + self._handle_lldp_flows(event) + + def _handle_lldp_flows(self, event): + """Install or remove flows in a switch. + + Install a flow to send LLDP packets to the controller. The proactive + flow is installed whenever a switch is enabled. If the switch is + disabled the flow is removed. """ try: dpid = event.content['dpid'] @@ -156,14 +165,14 @@ def _retry_if_status_code(response, endpoint, data, status_codes, log.error(f"Failed to push flows on {destination}," f" error: {res.text}, status: {res.status_code}," f" data: {data}") - _retry_if_status_code(res, endpoint, data, [424]) + _retry_if_status_code(res, endpoint, data, [424, 500]) else: res = requests.delete(endpoint, json=data) if res.status_code != 202: log.error(f"Failed to delete flows on {destination}," f" error: {res.text}, status: {res.status_code}", f" data: {data}") - _retry_if_status_code(res, endpoint, data, [424]) + _retry_if_status_code(res, endpoint, data, [424, 500]) @listen_to('kytos/of_core.v0x0[14].messages.in.ofpt_packet_in') def notify_uplink_detected(self, event): diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index baaf777..767f312 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -81,6 +81,24 @@ def test_handle_lldp_flows(self, mock_post, mock_delete): self.napp.handle_lldp_flows(event_del) mock_delete.assert_called() + @patch("time.sleep") + @patch("requests.post") + def test_handle_lldp_flows_retries(self, mock_post, _): + """Test handle_lldp_flow method retries.""" + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock("00:00:00:00:00:00:00:01", 0x04) + self.napp.controller.switches = {dpid: switch} + event_post = get_kytos_event_mock(name="kytos/topology.switch.enabled", + content={"dpid": dpid}) + + mock = MagicMock() + mock.request.method = "POST" + mock.status_code = 500 + mock.text = "Switch not connected" + mock_post.return_value = mock + self.napp._handle_lldp_flows(event_post) + self.assertTrue(mock_post.call_count, 3) + @patch('kytos.core.buffers.KytosEventBuffer.put') @patch('napps.kytos.of_lldp.main.KytosEvent') @patch('kytos.core.controller.Controller.get_switch_by_dpid') From 8a985c4d519e79ee72ce8a4a3fc798784c781aae Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 17:57:18 -0300 Subject: [PATCH 7/9] Updated changelog and bumped to version 1.2.0 --- CHANGELOG.rst | 9 +++++++++ kytos.json | 2 +- setup.py | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0693220..52ae12c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -24,6 +24,15 @@ Security ======== +[1.1.1] - 2021-12-13 +******************** +Changed +======= +- Added support for retries when sending a request to ``flow_manager`` +- Parametrized ``force`` option as a fallback +- Added more logs for request errors + + [1.1.1] - 2021-04-22 ******************** Changed diff --git a/kytos.json b/kytos.json index 74e1d0f..1ad35e2 100644 --- a/kytos.json +++ b/kytos.json @@ -3,7 +3,7 @@ "username": "kytos", "name": "of_lldp", "description": "Discover network-to-network interfaces (NNIs) using the LLDP protocol.", - "version": "1.1.1", + "version": "1.2.0", "napp_dependencies": ["kytos/of_core", "kytos/flow_manager", "kytos/topology"], "license": "MIT", "url": "https://github.com/kytos/of_lldp.git", diff --git a/setup.py b/setup.py index e949afd..c2c325d 100644 --- a/setup.py +++ b/setup.py @@ -22,7 +22,7 @@ BASE_ENV = Path(os.environ.get('VIRTUAL_ENV', '/')) NAPP_NAME = 'of_lldp' -NAPP_VERSION = '1.1.1' +NAPP_VERSION = '1.2.0' # Kytos var folder VAR_PATH = BASE_ENV / 'var' / 'lib' / 'kytos' From 35ac2ff03edbfca7711b3d949023325b1ed374ee Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 18:00:14 -0300 Subject: [PATCH 8/9] Updated mock.text --- tests/unit/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 767f312..7da29bf 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -94,7 +94,7 @@ def test_handle_lldp_flows_retries(self, mock_post, _): mock = MagicMock() mock.request.method = "POST" mock.status_code = 500 - mock.text = "Switch not connected" + mock.text = "some_err" mock_post.return_value = mock self.napp._handle_lldp_flows(event_post) self.assertTrue(mock_post.call_count, 3) From cefb5a17572eecf8c5c33fdcf991c32fefab0d3b Mon Sep 17 00:00:00 2001 From: Vinicius Arcanjo Date: Mon, 13 Dec 2021 18:32:29 -0300 Subject: [PATCH 9/9] Fixed changelog version --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 52ae12c..d08f6c7 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -24,7 +24,7 @@ Security ======== -[1.1.1] - 2021-12-13 +[1.2.0] - 2021-12-13 ******************** Changed =======