From 1e84483613b03b8a5db3b8c6e5084844824ad264 Mon Sep 17 00:00:00 2001 From: Milan Fencik Date: Mon, 25 Nov 2024 17:30:48 +0000 Subject: [PATCH] review fixes --- .../neutron_understack/config.py | 2 +- .../neutron_understack/nautobot.py | 34 +++++++++++-------- .../neutron_understack_mech.py | 14 +++----- .../neutron_understack/undersync.py | 23 ++++++++----- 4 files changed, 38 insertions(+), 35 deletions(-) diff --git a/python/neutron-understack/neutron_understack/config.py b/python/neutron-understack/neutron_understack/config.py index 02d5338c8..fa2479f69 100644 --- a/python/neutron-understack/neutron_understack/config.py +++ b/python/neutron-understack/neutron_understack/config.py @@ -27,7 +27,7 @@ ), cfg.StrOpt( "undersync_token", - help="Undersync API token", + help="Undersync API token. If not provided, the '/etc/undersync/token' will be read instead.", ), cfg.BoolOpt( "undersync_dry_run", default=True, help="Call Undersync with dry-run mode" diff --git a/python/neutron-understack/neutron_understack/nautobot.py b/python/neutron-understack/neutron_understack/nautobot.py index d925a6fdc..ebb1833a0 100644 --- a/python/neutron-understack/neutron_understack/nautobot.py +++ b/python/neutron-understack/neutron_understack/nautobot.py @@ -9,26 +9,31 @@ LOG = log.getLogger(__name__) +class NautobotError(Exception): + pass + + class Nautobot: CALLER_FRAME = 1 def __init__(self, nb_url: str | None = None, nb_token: str | None = None): """Basic Nautobot wrapper because pynautobot doesn't expose plugin APIs.""" self.base_url = nb_url or "http://nautobot-default.nautobot.svc.cluster.local" - self.token = nb_token or self.fetch_nb_token() + self.token = nb_token or self._fetch_nb_token() self.s = requests.Session() self.s.headers.update({"Authorization": f"Token {self.token}"}) - def fetch_nb_token(self): + def _fetch_nb_token(self): file = pathlib.Path("/etc/nb-token/token") with file.open() as f: return f.read().strip() - def log_for_status(self, response): + def _log_and_raise_for_status(self, response): try: response.raise_for_status() except HTTPError as error: - LOG.debug("Nautobot error: %(error)s", {"error": error}) + LOG.error("Nautobot error: %(error)s", {"error": error}) + raise NautobotError() from error def make_api_request( self, url: str, method: str, payload: dict | None = None @@ -52,7 +57,7 @@ def make_api_request( "%(caller_function)s resp: %(resp)s", {"resp": resp_data, "caller_function": caller_function}, ) - self.log_for_status(resp) + self._log_and_raise_for_status(resp) return resp_data def ucvni_create( @@ -78,10 +83,8 @@ def ucvni_create( return resp_data def ucvni_delete(self, network_id): - payload = {"id": network_id} url = f"/api/plugins/undercloud-vni/ucvnis/{network_id}/" - resp_data = self.make_api_request(url, "delete", payload) - return resp_data + return self.make_api_request(url, "delete") def prep_switch_interface( self, connected_interface_id: str, ucvni_uuid: str @@ -112,13 +115,14 @@ def fetch_vlan_group_uuid(self, device_uuid: str) -> str: url = f"/api/dcim/devices/{device_uuid}/?include=relationships" resp_data = self.make_api_request(url, "get") - vlan_group_uuid = ( - resp_data.get("relationships") - .get("vlan_group_to_devices") - .get("source") - .get("objects")[0] - .get("id") - ) + try: + vlan_group_uuid = resp_data["relationships"]["vlan_group_to_devices"][ + "source" + ]["objects"][0]["id"] + except (KeyError, IndexError, TypeError) as error: + LOG.error("vlan_group_uuid_error: %(error)s", {"error": error}) + raise NautobotError() from error + LOG.debug( "vlan_group_uuid: %(vlan_group_uuid)s", {"vlan_group_uuid": vlan_group_uuid} ) diff --git a/python/neutron-understack/neutron_understack/neutron_understack_mech.py b/python/neutron-understack/neutron_understack/neutron_understack_mech.py index a5e53fe0d..a5773b085 100644 --- a/python/neutron-understack/neutron_understack/neutron_understack_mech.py +++ b/python/neutron-understack/neutron_understack/neutron_understack_mech.py @@ -217,9 +217,6 @@ def update_port_postcommit(self, context): network_id = context.current["network_id"] connected_interface_uuid = self.fetch_connected_interface_uuid(context) - if not connected_interface_uuid: - return - nb_vlan_group_id = self.update_nautobot(network_id, connected_interface_uuid) self.undersync.sync_devices( @@ -290,7 +287,7 @@ def fetch_connected_interface_uuid(self, context): " port_id: %(connected_interface_uuid)s", {"connected_interface_uuid": connected_interface_uuid}, ) - return + raise return connected_interface_uuid def update_nautobot(self, network_id, connected_interface_uuid): @@ -299,10 +296,7 @@ def update_nautobot(self, network_id, connected_interface_uuid): configure_port_status_data = self.nb.configure_port_status( connected_interface_uuid, port_status ) - switch_uuid = configure_port_status_data.get("device").get("id") - nb_vlan_group_id = self.nb.fetch_vlan_group_uuid(switch_uuid) + switch_uuid = configure_port_status_data.get("device", {}).get("id") + return self.nb.fetch_vlan_group_uuid(switch_uuid) else: - nb_vlan_group_id = self.nb.prep_switch_interface( - connected_interface_uuid, network_id - ) - return nb_vlan_group_id + return self.nb.prep_switch_interface(connected_interface_uuid, network_id) diff --git a/python/neutron-understack/neutron_understack/undersync.py b/python/neutron-understack/neutron_understack/undersync.py index a2e55258b..4c0beb7bf 100644 --- a/python/neutron-understack/neutron_understack/undersync.py +++ b/python/neutron-understack/neutron_understack/undersync.py @@ -8,6 +8,10 @@ LOG = log.getLogger(__name__) +class UndersyncError(Exception): + pass + + class Undersync: def __init__( self, @@ -16,21 +20,22 @@ def __init__( timeout: int = 90, ) -> None: """Simple client for Undersync.""" - self.token = auth_token or self.fetch_undersync_token() + self.token = auth_token or self._fetch_undersync_token() self.url = "http://undersync-service.undersync.svc.cluster.local:8080" self.api_url = api_url or self.url self.timeout = timeout - def fetch_undersync_token(self): + def _fetch_undersync_token(self): file = pathlib.Path("/etc/undersync/token") with file.open() as f: return f.read().strip() - def log_for_status(self, response): + def _log_and_raise_for_status(self, response): try: response.raise_for_status() except HTTPError as error: - LOG.debug("Undersync error: %(error)s", {"error": error}) + LOG.error("Undersync error: %(error)s", {"error": error}) + raise UndersyncError() from error def sync_devices( self, vlan_group_uuids: str | list[str], force=False, dry_run=False @@ -54,7 +59,7 @@ def client(self): } return session - def undersync_post(self, action: str, uuids: str) -> requests.Response: + def _undersync_post(self, action: str, uuids: str) -> requests.Response: response = self.client.post( f"{self.api_url}/v1/vlan-group/{uuids}/{action}", timeout=self.timeout ) @@ -62,14 +67,14 @@ def undersync_post(self, action: str, uuids: str) -> requests.Response: "undersync %(action)s resp: %(resp)s", {"resp": response.json(), "action": action}, ) - self.log_for_status(response) + self._log_and_raise_for_status(response) return response def sync(self, uuids: str) -> requests.Response: - return self.undersync_post("sync", uuids) + return self._undersync_post("sync", uuids) def dry_run(self, uuids: str) -> requests.Response: - return self.undersync_post("dry-run", uuids) + return self._undersync_post("dry-run", uuids) def force(self, uuids: str) -> requests.Response: - return self.undersync_post("force", uuids) + return self._undersync_post("force", uuids)