workfloworchestrator / orchestrator-core

The workflow orchestrator core repository
Apache License 2.0
41 stars 15 forks source link

[Bug]: Referring more than once to a product block type #677

Closed Mark90 closed 2 months ago

Mark90 commented 4 months ago

Problem

Nesting product blocks is the recommended way to separate configuration into reusable blocks.

Pseudo code example:

class InterfaceBlock(ProductBlockModel): ...

class NodeBlock(ProductBlockModel): ...

class PortBlock(ProductBlockModel):
    node: NodeBlock
    interface: InterfaceBlock

However, referring to the same product block type more than once raises a ValueError (see below). For example:

class PortBlock(ProductBlockModel):
    node: NodeBlock
    node2: NodeBlock:
    interface: InterfaceBlock

A workaround is to change the field to a node: list[NodeBlock].

Reproduce

When using this example-orchestrator branch https://github.com/workfloworchestrator/example-orchestrator/pull/19/files

With the following patch

diff --git a/docker-compose.yml b/docker-compose.yml
index 60753c7..cee5f98 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -116,7 +116,7 @@ services:

   orchestrator:
     &orchestrator
-    image: "ghcr.io/workfloworchestrator/orchestrator-core:latest"
+    image: "ghcr.io/workfloworchestrator/orchestrator-core:edge"
     env_file: ./docker/orchestrator/orchestrator.env
     environment:
       LSO_ENABLED: ${COMPOSE_PROFILES:+True}
diff --git a/products/product_blocks/port.py b/products/product_blocks/port.py
index 7decb2f..68e055f 100644
--- a/products/product_blocks/port.py
+++ b/products/product_blocks/port.py
@@ -38,6 +38,7 @@ class PortBlockInactive(ProductBlockModel, product_block_name="Port"):
     lldp: bool | None = None
     enabled: bool | None = None
     node: NodeBlockInactive | None = None
+    node2: NodeBlockInactive | None = None
     ims_id: int | None = None
     nrm_id: int | None = None

@@ -51,6 +52,7 @@ class PortBlockProvisioning(PortBlockInactive, lifecycle=[SubscriptionLifecycle.
     lldp: bool
     enabled: bool
     node: NodeBlockProvisioning
+    node2: NodeBlockProvisioning
     ims_id: int
     nrm_id: int | None = None

@@ -91,5 +93,6 @@ class PortBlock(PortBlockProvisioning, lifecycle=[SubscriptionLifecycle.ACTIVE])
     lldp: bool
     enabled: bool
     node: NodeBlock
+    node2: NodeBlock
     ims_id: int
     nrm_id: int
diff --git a/workflows/port/create_port.py b/workflows/port/create_port.py
index a3bf230..a771017 100644
--- a/workflows/port/create_port.py
+++ b/workflows/port/create_port.py
@@ -44,7 +44,7 @@ def initial_input_form_generator(product: UUIDstr, product_name: str) -> FormGen
     NodeChoice: TypeAlias = cast(type[Choice], node_selector())

     class SelectNodeForm(FormPage):
-        model_config = ConfigDict(title=product_name)
+        model_config = ConfigDict(title=f"{product_name} Select node 1 for ports")

         # organisation: OrganisationId

@@ -54,6 +54,18 @@ def initial_input_form_generator(product: UUIDstr, product_name: str) -> FormGen
     select_node_dict = select_node.model_dump()
     node_subscription_id = select_node_dict["node_subscription_id"]

+
+    class SelectNode2Form(FormPage):
+        model_config = ConfigDict(title=f"{product_name} Select node 2 just to test")
+
+        # organisation: OrganisationId
+
+        node2_subscription_id: NodeChoice
+
+    select_node2 = yield SelectNode2Form
+    select_node2_dict = select_node2.model_dump()
+    node2_subscription_id = select_node2_dict["node2_subscription_id"]
+
     _product = get_product_by_id(product)
     speed = int(_product.fixed_input_value("speed"))
     FreePortChoice: TypeAlias = cast(type[Choice], free_port_selector(node_subscription_id, speed))
@@ -77,13 +89,14 @@ def initial_input_form_generator(product: UUIDstr, product_name: str) -> FormGen
     summary_fields = ["port_ims_id", "port_description", "port_mode", "auto_negotiation", "lldp"]
     yield from create_summary_form(user_input_dict, product_name, summary_fields)

-    return user_input_dict | {"node_subscription_id": node_subscription_id}
+    return user_input_dict | {"node_subscription_id": node_subscription_id, "node2_subscription_id": node2_subscription_id, }

 @step("Construct Subscription model")
 def construct_port_model(
     product: UUIDstr,
     node_subscription_id: UUIDstr,
+    node2_subscription_id: UUIDstr,
     port_ims_id: int,
     port_description: str | None,
     port_mode: PortMode,
@@ -96,8 +109,10 @@ def construct_port_model(
         status=SubscriptionLifecycle.INITIAL,
     )
     node = Node.from_subscription(node_subscription_id)
+    node2 = Node.from_subscription(node2_subscription_id)
     interface = netbox.get_interface(id=port_ims_id)
     subscription.port.node = node.node
+    subscription.port.node2 = node2.node
     subscription.port.port_name = interface.name
     subscription.port.port_type = interface.type.value
     subscription.port.port_description = port_description

Try to create a port with 2 nodes attached. This results in:

image

Trying to access the subscription in the faulty state shows the following traceback:

2024-06-11 15:14:01 [error    ] Expected exactly one item in iterable, but got SubscriptionInstanceTable(subscription_instance_id=c8efb7f3-b59d-4b5e-9fee-6c34e3d21174, subscription_id=7a61250e-2582-4d9c-9498-863fbf0e39c6, product_block_id=ed0d0e4d-6188-4b1d-b511-1087e9c96a29, label=None), SubscriptionInstanceTable(subscription_instance_id=3c306fd4-11ba-4bbc-a6b8-6854611a70fe, subscription_id=89a01545-b990-4d35-8f49-1bcd569e7787, product_block_id=ed0d0e4d-6188-4b1d-b511-1087e9c96a29, label=None), and perhaps more.

GraphQL request:37:17
36 |                 }
37 |                 productBlockInstances {
   |                 ^
38 |                     id [strawberry.execution]
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/graphql/execution/execute.py", line 528, in await_result
    return_type, field_nodes, info, path, await result
                                          ^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/strawberry/schema/schema_converter.py", line 704, in _async_resolver
    return await await_maybe(
           ^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/strawberry/utils/await_maybe.py", line 12, in await_maybe
    return await value
           ^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/graphql/schemas/subscription.py", line 68, in product_block_instances
    return await get_subscription_product_blocks(self.subscription_id, tags, resource_types)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/graphql/utils/get_subscription_product_blocks.py", line 66, in get_subscription_product_blocks
    subscription, _ = await get_subscription_dict(subscription_id)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/utils/get_subscription_dict.py", line 14, in get_subscription_dict
    subscription_model = SubscriptionModel.from_subscription(subscription_id)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/domain/base.py", line 1272, in from_subscription
    instances = cls._load_instances(subscription.instances, status, match_domain_attr=False)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/domain/base.py", line 345, in _load_instances
    ].from_db(subscription_instance=instance, status=status)
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/domain/base.py", line 744, in from_db
    sub_instances = cls._load_instances(subscription_instance.depends_on, status)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/orchestrator/domain/base.py", line 333, in _load_instances
    instance = only(instance_list)
               ^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/more_itertools/more.py", line 3294, in only
    raise too_long or ValueError(msg)
ValueError: Expected exactly one item in iterable, but got SubscriptionInstanceTable(subscription_instance_id=c8efb7f3-b59d-4b5e-9fee-6c34e3d21174, subscription_id=7a61250e-2582-4d9c-9498-863fbf0e39c6, product_block_id=ed0d0e4d-6188-4b1d-b511-1087e9c96a29, label=None), SubscriptionInstanceTable(subscription_instance_id=3c306fd4-11ba-4bbc-a6b8-6854611a70fe, subscription_id=89a01545-b990-4d35-8f49-1bcd569e7787, product_block_id=ed0d0e4d-6188-4b1d-b511-1087e9c96a29, label=None), and perhaps more.

Affected versions

2.1.2 - 2.3.0rc5

tjeerddie commented 4 months ago

its somehow not using the domain_model_attr to correctly connect the block relations, in the test test_migrate_domain_models_new_product_block_on_product_block it does show it correctly in the DB, here a print of the relation table inside that test:

[
  SubscriptionInstanceRelationTable(
    in_use_by_id=be45114a-befc-4874-a883-cc4e5012ff82,
    depends_on_id=4200db96-9fd0-4911-a040-0bd82dae5c5a,
    order_id=0,
    domain_model_attr=sub_block
  ),
  SubscriptionInstanceRelationTable(
    in_use_by_id=be45114a-befc-4874-a883-cc4e5012ff82,
    depends_on_id=fd98ba9c-1343-489a-8811-63e478275453, 
    order_id=0, 
    domain_model_attr=sub_block_2
  ), 
  SubscriptionInstanceRelationTable(
    in_use_by_id=be45114a-befc-4874-a883-cc4e5012ff82, 
    depends_on_id=a91ba570-ef61-4644-9971-5c1baaece405, 
    order_id=0, 
    domain_model_attr=None
  )
]