From f4e54b76920d1455dd43ddd5ca81d7d3c6aee786 Mon Sep 17 00:00:00 2001 From: Neda Moeini <neda.moeini@geant.org> Date: Mon, 22 Jan 2024 10:13:40 +0100 Subject: [PATCH] Improvements from NAT-408 and NAT-412. --- gso/services/crm.py | 11 ----------- gso/services/netbox_client.py | 2 +- gso/utils/helpers.py | 2 +- gso/workflows/iptrunk/create_iptrunk.py | 19 +++++++++++-------- gso/workflows/router/create_router.py | 9 +++++---- gso/workflows/site/create_site.py | 9 +++++---- test/services/conftest.py | 2 +- test/services/test_netbox_client.py | 6 +++--- test/workflows/iptrunk/test_create_iptrunk.py | 16 +++++++++++----- .../workflows/iptrunk/test_migrate_iptrunk.py | 4 ++-- test/workflows/router/test_create_router.py | 2 -- test/workflows/site/test_create_site.py | 1 - 12 files changed, 40 insertions(+), 43 deletions(-) diff --git a/gso/services/crm.py b/gso/services/crm.py index e0b8c61c..810df588 100644 --- a/gso/services/crm.py +++ b/gso/services/crm.py @@ -6,8 +6,6 @@ For the time being, it's hardcoded to only contain GÉANT as a customer, since t from typing import Any -from pydantic_forms.validators import Choice - class CustomerNotFoundError(Exception): """Exception raised when a customer is not found.""" @@ -31,12 +29,3 @@ def get_customer_by_name(name: str) -> dict[str, Any]: msg = f"Customer {name} not found" raise CustomerNotFoundError(msg) - - -def customer_selector() -> Choice: - """GUI input field for selecting a customer.""" - customers = {} - for customer in all_customers(): - customers[customer["id"]] = customer["name"] - - return Choice("Select a customer", zip(customers.keys(), customers.items(), strict=True)) # type: ignore[arg-type] diff --git a/gso/services/netbox_client.py b/gso/services/netbox_client.py index 6209fa9a..74f56982 100644 --- a/gso/services/netbox_client.py +++ b/gso/services/netbox_client.py @@ -300,7 +300,7 @@ class NetboxClient: ] # Generate all feasible LAGs - all_feasible_lags = [f"LAG-{i}" for i in FEASIBLE_IP_TRUNK_LAG_RANGE] + all_feasible_lags = [f"lag-{i}" for i in FEASIBLE_IP_TRUNK_LAG_RANGE] # Return available LAGs not assigned to the device return [lag for lag in all_feasible_lags if lag not in lag_interface_names] diff --git a/gso/utils/helpers.py b/gso/utils/helpers.py index 9c08bb15..8c28dd25 100644 --- a/gso/utils/helpers.py +++ b/gso/utils/helpers.py @@ -47,7 +47,7 @@ def available_interfaces_choices(router_id: UUID, speed: str) -> Choice | None: if get_router_vendor(router_id) != RouterVendor.NOKIA: return None interfaces = { - interface["name"]: f"{interface['name']} - {interface['module']['display']} - {interface['description']}" + interface["name"]: f"{interface['name']} - {interface['description']}" for interface in NetboxClient().get_available_interfaces(router_id, speed) } return Choice("ae member", zip(interfaces.keys(), interfaces.items(), strict=True)) # type: ignore[arg-type] diff --git a/gso/workflows/iptrunk/create_iptrunk.py b/gso/workflows/iptrunk/create_iptrunk.py index 32834486..acbfe375 100644 --- a/gso/workflows/iptrunk/create_iptrunk.py +++ b/gso/workflows/iptrunk/create_iptrunk.py @@ -12,6 +12,7 @@ from orchestrator.workflow import StepList, conditional, done, init, step, workf from orchestrator.workflows.steps import resync, set_status, store_process_subscription from orchestrator.workflows.utils import wrap_create_initial_input_form from pydantic import validator +from pydantic_forms.core import ReadOnlyField from pynetbox.models.dcim import Interfaces from gso.products.product_blocks.iptrunk import ( @@ -24,7 +25,7 @@ from gso.products.product_blocks.router import RouterVendor from gso.products.product_types.iptrunk import IptrunkInactive, IptrunkProvisioning from gso.products.product_types.router import Router from gso.services import infoblox, subscriptions -from gso.services.crm import customer_selector +from gso.services.crm import get_customer_by_name from gso.services.netbox_client import NetboxClient from gso.services.provisioning_proxy import execute_playbook, pp_interaction from gso.utils.helpers import ( @@ -49,7 +50,7 @@ def initial_input_form_generator(product_name: str) -> FormGenerator: title = product_name tt_number: str - customer: customer_selector() # type: ignore[valid-type] + customer: str = ReadOnlyField("GÉANT") geant_s_sid: str iptrunk_description: str iptrunk_type: IptrunkType @@ -172,9 +173,9 @@ def initial_input_form_generator(product_name: str) -> FormGenerator: @step("Create subscription") -def create_subscription(product: UUIDstr, customer: UUIDstr) -> State: +def create_subscription(product: UUIDstr, customer: str) -> State: """Create a new subscription object in the database.""" - subscription = IptrunkInactive.from_product_id(product, customer) + subscription = IptrunkInactive.from_product_id(product, get_customer_by_name(customer)["id"]) return { "subscription": subscription, @@ -215,6 +216,8 @@ def initialize_subscription( side_b_ae_members: list[dict], ) -> State: """Take all input from the user, and store it in the database.""" + side_a = Router.from_subscription(side_a_node_id).router + side_b = Router.from_subscription(side_b_node_id).router subscription.iptrunk.geant_s_sid = geant_s_sid subscription.iptrunk.iptrunk_description = iptrunk_description subscription.iptrunk.iptrunk_type = iptrunk_type @@ -222,7 +225,7 @@ def initialize_subscription( subscription.iptrunk.iptrunk_isis_metric = 90000 subscription.iptrunk.iptrunk_minimum_links = iptrunk_minimum_links - subscription.iptrunk.iptrunk_sides[0].iptrunk_side_node = Router.from_subscription(side_a_node_id).router + subscription.iptrunk.iptrunk_sides[0].iptrunk_side_node = side_a subscription.iptrunk.iptrunk_sides[0].iptrunk_side_ae_iface = side_a_ae_iface subscription.iptrunk.iptrunk_sides[0].iptrunk_side_ae_geant_a_sid = side_a_ae_geant_a_sid for member in side_a_ae_members: @@ -230,15 +233,15 @@ def initialize_subscription( IptrunkInterfaceBlockInactive.new(subscription_id=uuid4(), **member), ) - subscription.iptrunk.iptrunk_sides[1].iptrunk_side_node = Router.from_subscription(side_b_node_id).router + subscription.iptrunk.iptrunk_sides[1].iptrunk_side_node = side_b subscription.iptrunk.iptrunk_sides[1].iptrunk_side_ae_iface = side_b_ae_iface subscription.iptrunk.iptrunk_sides[1].iptrunk_side_ae_geant_a_sid = side_b_ae_geant_a_sid for member in side_b_ae_members: subscription.iptrunk.iptrunk_sides[1].iptrunk_side_ae_members.append( IptrunkInterfaceBlockInactive.new(subscription_id=uuid4(), **member), ) - - subscription.description = f"IP trunk, geant_s_sid:{geant_s_sid}" + side_names = sorted([side_a.router_site.site_name, side_b.router_site.site_name]) + subscription.description = f"IP trunk {side_names[0]} {side_names[1]}, geant_s_sid:{geant_s_sid}" subscription = IptrunkProvisioning.from_other_lifecycle(subscription, SubscriptionLifecycle.PROVISIONING) return {"subscription": subscription} diff --git a/gso/workflows/router/create_router.py b/gso/workflows/router/create_router.py index f07cec4e..6bec84a6 100644 --- a/gso/workflows/router/create_router.py +++ b/gso/workflows/router/create_router.py @@ -10,6 +10,7 @@ from orchestrator.workflow import StepList, conditional, done, init, step, workf from orchestrator.workflows.steps import resync, set_status, store_process_subscription from orchestrator.workflows.utils import wrap_create_initial_input_form from pydantic import validator +from pydantic_forms.core import ReadOnlyField from gso.products.product_blocks.router import ( PortNumber, @@ -20,7 +21,7 @@ from gso.products.product_blocks.router import ( from gso.products.product_types.router import RouterInactive, RouterProvisioning from gso.products.product_types.site import Site from gso.services import infoblox, subscriptions -from gso.services.crm import customer_selector +from gso.services.crm import get_customer_by_name from gso.services.netbox_client import NetboxClient from gso.services.provisioning_proxy import pp_interaction from gso.utils.helpers import iso_from_ipv4 @@ -44,7 +45,7 @@ def initial_input_form_generator(product_name: str) -> FormGenerator: title = product_name tt_number: str - customer: customer_selector() # type: ignore[valid-type] + customer: str = ReadOnlyField("GÉANT") vendor: RouterVendor router_site: _site_selector() # type: ignore[valid-type] hostname: str @@ -72,9 +73,9 @@ def initial_input_form_generator(product_name: str) -> FormGenerator: @step("Create subscription") -def create_subscription(product: UUIDstr, customer: UUIDstr) -> State: +def create_subscription(product: UUIDstr, customer: str) -> State: """Create a new subscription object.""" - subscription = RouterInactive.from_product_id(product, customer) + subscription = RouterInactive.from_product_id(product, get_customer_by_name(customer)["id"]) return { "subscription": subscription, diff --git a/gso/workflows/site/create_site.py b/gso/workflows/site/create_site.py index d94c1196..8e6e13f8 100644 --- a/gso/workflows/site/create_site.py +++ b/gso/workflows/site/create_site.py @@ -6,11 +6,12 @@ from orchestrator.types import FormGenerator, State, SubscriptionLifecycle, UUID from orchestrator.workflow import StepList, done, init, step, workflow from orchestrator.workflows.steps import resync, set_status, store_process_subscription from orchestrator.workflows.utils import wrap_create_initial_input_form +from pydantic_forms.core import ReadOnlyField from gso.products.product_blocks import site as site_pb from gso.products.product_blocks.site import LatitudeCoordinate, LongitudeCoordinate from gso.products.product_types import site -from gso.services.crm import customer_selector +from gso.services.crm import get_customer_by_name from gso.utils.helpers import BaseSiteValidatorModel @@ -21,7 +22,7 @@ def initial_input_form_generator(product_name: str) -> FormGenerator: class Config: title = product_name - customer: customer_selector() # type: ignore[valid-type] + customer: str = ReadOnlyField("GÉANT") site_name: str site_city: str site_country: str @@ -39,9 +40,9 @@ def initial_input_form_generator(product_name: str) -> FormGenerator: @step("Create subscription") -def create_subscription(product: UUIDstr, customer: UUIDstr) -> State: +def create_subscription(product: UUIDstr, customer: str) -> State: """Create a new subscription object in the service database.""" - subscription = site.SiteInactive.from_product_id(product, customer) + subscription = site.SiteInactive.from_product_id(product, get_customer_by_name(customer)["id"]) return { "subscription": subscription, diff --git a/test/services/conftest.py b/test/services/conftest.py index ce6c6c45..9fc3d191 100644 --- a/test/services/conftest.py +++ b/test/services/conftest.py @@ -9,7 +9,7 @@ class MockedNetboxClient: @staticmethod def get_available_lags() -> list[str]: - return [f"LAG{lag}" for lag in range(1, 5)] + return [f"lag-{lag}" for lag in range(1, 5)] @staticmethod def get_available_interfaces(): diff --git a/test/services/test_netbox_client.py b/test/services/test_netbox_client.py index c4dc6bdd..d03bf828 100644 --- a/test/services/test_netbox_client.py +++ b/test/services/test_netbox_client.py @@ -107,12 +107,12 @@ def test_create_device( @patch("gso.services.netbox_client.pynetbox.api") def test_get_available_lags(mock_api, mock_from_subscription, data_config_filename: PathLike): router_id = uuid.uuid4() - feasible_lags = [f"LAG-{i}" for i in range(1, 11)] + feasible_lags = [f"lag-{i}" for i in range(1, 11)] # Mock the pynetbox API instance mock_netbox = mock_api.return_value mock_filter = mock_netbox.dcim.interfaces.filter - mock_filter.return_value = [{"name": f"LAG-{i}", "type": "lag"} for i in range(1, 4)] + mock_filter.return_value = [{"name": f"lag-{i}", "type": "lag"} for i in range(1, 4)] # Mock the Router.from_subscription method mock_subscription = mock_from_subscription.return_value @@ -123,7 +123,7 @@ def test_get_available_lags(mock_api, mock_from_subscription, data_config_filena result = netbox_client.get_available_lags(router_id) # Check the result of the function - assert result == [lag for lag in feasible_lags if lag not in [f"LAG-{i}" for i in range(1, 4)]] + assert result == [lag for lag in feasible_lags if lag not in [f"lag-{i}" for i in range(1, 4)]] @patch("gso.services.netbox_client.pynetbox.api") diff --git a/test/workflows/iptrunk/test_create_iptrunk.py b/test/workflows/iptrunk/test_create_iptrunk.py index 773dabb3..59ecc85e 100644 --- a/test/workflows/iptrunk/test_create_iptrunk.py +++ b/test/workflows/iptrunk/test_create_iptrunk.py @@ -6,7 +6,6 @@ import pytest from gso.products import Iptrunk, ProductType from gso.products.product_blocks.iptrunk import IptrunkType, PhyPortCapacity from gso.products.product_blocks.router import RouterVendor -from gso.services.crm import customer_selector, get_customer_by_name from gso.services.subscriptions import get_product_id_by_name from gso.utils.helpers import LAGMember from test.services.conftest import MockedNetboxClient @@ -60,7 +59,6 @@ def input_form_wizard_data(request, juniper_router_subscription_factory, nokia_r create_ip_trunk_step = { "tt_number": faker.tt_number(), - "customer": getattr(customer_selector(), get_customer_by_name("GÉANT")["id"]), "geant_s_sid": faker.geant_sid(), "iptrunk_type": IptrunkType.DARK_FIBER, "iptrunk_description": faker.sentence(), @@ -69,7 +67,7 @@ def input_form_wizard_data(request, juniper_router_subscription_factory, nokia_r } create_ip_trunk_side_a_router_name = {"side_a_node_id": router_side_a} create_ip_trunk_side_a_step = { - "side_a_ae_iface": "LAG1", + "side_a_ae_iface": "lag-1", "side_a_ae_geant_a_sid": faker.geant_sid(), "side_a_ae_members": [ LAGMember( @@ -81,7 +79,7 @@ def input_form_wizard_data(request, juniper_router_subscription_factory, nokia_r } create_ip_trunk_side_b_router_name = {"side_b_node_id": router_side_b} create_ip_trunk_side_b_step = { - "side_b_ae_iface": "LAG4", + "side_b_ae_iface": "lag-4", "side_b_ae_geant_a_sid": faker.geant_sid(), "side_b_ae_members": side_b_members, } @@ -125,8 +123,16 @@ def test_successful_iptrunk_creation_with_standard_lso_result( subscription_id = state["subscription_id"] subscription = Iptrunk.from_subscription(subscription_id) + sorted_sides = sorted( + [ + subscription.iptrunk.iptrunk_sides[0].iptrunk_side_node.router_site.site_name, + subscription.iptrunk.iptrunk_sides[1].iptrunk_side_node.router_site.site_name, + ] + ) assert subscription.status == "active" - assert subscription.description == f"IP trunk, geant_s_sid:{input_form_wizard_data[0]['geant_s_sid']}" + assert subscription.description == ( + f"IP trunk {sorted_sides[0]} {sorted_sides[1]}, geant_s_sid:{input_form_wizard_data[0]['geant_s_sid']}" + ) assert mock_execute_playbook.call_count == 6 diff --git a/test/workflows/iptrunk/test_migrate_iptrunk.py b/test/workflows/iptrunk/test_migrate_iptrunk.py index e7852bc1..8dc3acf0 100644 --- a/test/workflows/iptrunk/test_migrate_iptrunk.py +++ b/test/workflows/iptrunk/test_migrate_iptrunk.py @@ -49,7 +49,7 @@ def migrate_form_input( new_router = nokia_router_subscription_factory() replace_side = str(old_subscription.iptrunk.iptrunk_sides[0].iptrunk_side_node.subscription.subscription_id) new_side_ae_members = faker.link_members_nokia()[0:2] - lag_name = "LAG1" + lag_name = "lag-1" elif use_juniper == UseJuniperSide.SIDE_BOTH: # Juniper -> Juniper old_side_a_node = juniper_router_subscription_factory() @@ -69,7 +69,7 @@ def migrate_form_input( new_router = nokia_router_subscription_factory() replace_side = str(old_subscription.iptrunk.iptrunk_sides[0].iptrunk_side_node.subscription.subscription_id) new_side_ae_members = faker.link_members_nokia()[0:2] - lag_name = "LAG1" + lag_name = "lag-1" return [ {"subscription_id": product_id}, diff --git a/test/workflows/router/test_create_router.py b/test/workflows/router/test_create_router.py index 6c297606..957c3bda 100644 --- a/test/workflows/router/test_create_router.py +++ b/test/workflows/router/test_create_router.py @@ -6,7 +6,6 @@ from infoblox_client import objects from gso.products import ProductType, Site from gso.products.product_blocks.router import RouterRole, RouterVendor from gso.products.product_types.router import Router -from gso.services.crm import customer_selector, get_customer_by_name from gso.services.subscriptions import get_product_id_by_name from test.workflows import ( assert_complete, @@ -23,7 +22,6 @@ def router_creation_input_form_data(site_subscription_factory, faker): return { "tt_number": faker.tt_number(), - "customer": getattr(customer_selector(), get_customer_by_name("GÉANT")["id"]), "router_site": router_site, "hostname": faker.pystr(), "ts_port": faker.pyint(), diff --git a/test/workflows/site/test_create_site.py b/test/workflows/site/test_create_site.py index a1122f59..6a260bc1 100644 --- a/test/workflows/site/test_create_site.py +++ b/test/workflows/site/test_create_site.py @@ -25,7 +25,6 @@ def test_create_site(responses, faker): "site_internal_id": faker.pyint(), "site_tier": SiteTier.TIER1, "site_ts_address": faker.ipv4(), - "customer": get_customer_by_name("GÉANT")["id"], }, ] result, _, _ = run_workflow("create_site", initial_site_data) -- GitLab