Refactor: Better error messages for missing keys

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/converge.yml

@@ -208,6 +208,27 @@
               - ansible_failed_result is defined
               - ansible_failed_result.msg == expected_error_message
 
+- name: Converge Negative tests for 'eos_designs_facts'
+  hosts: FAILURE_CONNECTED_ENDPOINT_PORT_PROFILE_DOES_NOT_EXIST_IN_FACTS
+  connection: local
+  tasks:
+    - name: Run failure scenario Test
+      block:
+        - name: Trigger Error
+          ansible.builtin.import_role:
+            name: arista.avd.eos_designs
+      rescue:
+        - name: Error message
+          run_once: true
+          ansible.builtin.debug:
+            var: ansible_failed_result.msg
+        - name: Assert eos_designs failed with the expected error message
+          run_once: true
+          ansible.builtin.assert:
+            that:
+              - ansible_failed_result is defined
+              - ansible_failed_result.msg == expected_error_message
+
 - name: Converge Negative tests for 'eos_designs_structured_config'
   hosts: EOS_DESIGNS_FAILURES_INCLUDED
   gather_facts: false

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/group_vars/FABRIC_DOWNLINK_POOLS_MISSING.yml

@@ -31,4 +31,4 @@ l3leaf:
 
 expected_error_message: >-
   Unable to assign IPs for uplinks. Either 'uplink_ipv4_pool' on this switch or 'downlink_pools' on
-  all the uplink switches is required but was not found for host 'downlink-pools-missing-l3leaf'
+  all the uplink switches must be set for host 'downlink-pools-missing-l3leaf'.

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/host_vars/failure-connected-endpoint-port-profile-does-not-exist-in-facts.yml

@@ -0,0 +1,29 @@
+---
+type: l3leaf
+fabric_name: FAILURE_CONNECTED_ENDPOINT_PORT_PROFILE_DOES_NOT_EXIST_IN_FACTS
+
+port_profiles:
+  - profile: PROFILE-1
+
+l3leaf:
+  nodes:
+    - name: failure-connected-endpoint-port-profile-does-not-exist-in-facts
+      loopback_ipv4_pool: 192.168.42.0/24
+      vtep_loopback_ipv4_pool: 192.168.255.0/24
+      bgp_as: 65042
+      id: 1
+      filter:
+        only_vlans_in_use: True
+
+servers:
+  - name: TEST-ENDPOINT
+    adapters:
+      - switches: [failure-connected-endpoint-port-profile-does-not-exist-in-facts]
+        switch_ports: [Ethernet9]
+        profile: THIS-PROFILE-DOES-NOT-EXIST
+        # Setting VLANs to trigger merging of adapters in facts phase
+        vlans: :42,666"
+
+expected_error_message: >-
+  Profile 'THIS-PROFILE-DOES-NOT-EXIST' applied under 'servers[TEST-ENDPOINT].adapters[0]'
+  does not exist in `port_profiles` for host 'failure-connected-endpoint-port-profile-does-not-exist-in-facts'.

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/host_vars/failure-svi-parent-profile-does-not-exist.yml

@@ -24,5 +24,4 @@ tenants:
               - node: failure-svi-parent-profile-does-not-exist
 
 expected_error_message: >-
-  Profile 'THIS-PROFILE-DOES-NOT-EXIST' applied under SVI 'TEST-SVI' does
-  not exist in `svi_profiles`.
+  Profile 'THIS-PROFILE-DOES-NOT-EXIST' applied under SVI 'TEST-SVI' does not exist in `svi_profiles`.

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/host_vars/missing-avt-id-cv-pathfinder.yml

@@ -95,4 +95,4 @@ application_classification:
     - name: TEST
 
 expected_error_message: >-
-  Missing mandatory `id` in `wan_virtual_topologies.policies[DEFAULT-POLICY].application_virtual_topologies[TEST]` when `wan_mode` is 'cv-pathfinder
+  Missing mandatory `id` in `wan_virtual_topologies.policies[DEFAULT-POLICY].application_virtual_topologies[TEST]` when `wan_mode` is 'cv-pathfinder.

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/host_vars/missing-mlag-peer.yml

@@ -15,4 +15,4 @@ l2leaf:
 expected_error_message: >-
   Facts not found for node 'some-missing-device'. Something in the input vars is pointing to this node.
   Check that 'some-missing-device' is in the inventory and is part of the group set by 'fabric_name'.
-  Node is required but was not found for host 'missing-mlag-peer'
+  Node is required for host 'missing-mlag-peer'.

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/host_vars/missing-prefix-list-definition-cv-pathfinder.yml

@@ -85,4 +85,5 @@ wan_carriers:
     path_group: INET
     trusted: true
 
-expected_error_message: "ipv4_prefix_list_catalog[name=PL1]"
+expected_error_message: >-
+  'ipv4_prefix_list_catalog[name=PL1]' is required but was not found.

ansible_collections/arista/avd/molecule/eos_designs_negative_unit_tests/inventory/hosts.yml

@@ -56,6 +56,9 @@ all:
           hosts:
             downlink-pools-duplicate-spine:
             downlink-pools-duplicate-l3leaf:
+        FAILURE_CONNECTED_ENDPOINT_PORT_PROFILE_DOES_NOT_EXIST_IN_FACTS:
+          hosts:
+            failure-connected-endpoint-port-profile-does-not-exist-in-facts:
     EOS_DESIGNS_FAILURES: # Add cases that fail during 'eos_designs_structured_config' phase
       children:
         EOS_DESIGNS_FAILURES_INCLUDED:
@@ -89,18 +92,18 @@ all:
             duplicate-interfaces-underlay:
             duplicate-ip-address-uplink-switch-router-bgp:
             duplicate-tunnel-interface-internet-exit:
+            failure-connected-endpoint-parent-port-profile-does-not-exist:
+            failure-connected-endpoint-port-profile-does-not-exist:
+            failure-duplicate-evpn-vlan-bundle-name:
+            failure-l3-interface-profile-does-not-exist:
             failure-missing-evpn-vlan-bundle:
             failure-missing-evpn-vlan-bundle_svi:
             failure-missing-evpn-multicast-l3-with-pim:
             failure-missing-evpn-multicast-peg-rps:
             failure-missing-evpn-multicast-with-pim:
-            failure-duplicate-evpn-vlan-bundle-name:
             failure-no-local-path-group-in-default-policy:
-            failure-l3-interface-profile-does-not-exist:
-            failure-svi-parent-profile-does-not-exist:
             failure-svi-grandparent-profile-does-not-exist:
-            failure-connected-endpoint-port-profile-does-not-exist:
-            failure-connected-endpoint-parent-port-profile-does-not-exist:
+            failure-svi-parent-profile-does-not-exist:
             ipv4-acl-in-missing-on-wan-interface:
             ipv4-acls:
             isis-system-id-format-missing-node-id:

ansible_collections/arista/avd/plugins/action/eos_designs_facts.py

@@ -195,8 +195,8 @@ def render_avd_switch_facts(self, avd_switch_facts_instances: dict) -> dict:
             try:
                 rendered_facts[host] = {"switch": avd_switch_facts_instances[host]["switch"].render()}
             except AristaAvdMissingVariableError as e:
-                msg = f"{e} is required but was not found for host '{host}'"
-                raise AnsibleActionFail(msg) from e
+                e.host = host
+                raise AnsibleActionFail(message=str(e)) from e
 
             # If the argument 'template_output' is set, run the output data through jinja2 rendering.
             # This is to resolve any input values with inline jinja using variables/facts set by eos_designs_facts.

ansible_collections/arista/avd/plugins/action/eos_designs_structured_config.py

@@ -20,6 +20,7 @@
 PLUGIN_NAME = "arista.avd.eos_designs_structured_config"
 try:
     from pyavd._eos_designs.structured_config import get_structured_config
+    from pyavd._errors import AristaAvdMissingVariableError
     from pyavd._utils import get, merge, strip_null_from_data
     from pyavd._utils import template as templater
 except ImportError as e:
@@ -96,6 +97,8 @@ def run(self, tmp: Any = None, task_vars: dict | None = None) -> dict:
                 result=result,
                 templar=self.templar,
             )
+        except AristaAvdMissingVariableError as e:
+            raise AnsibleActionFail(message=str(e)) from e
         except Exception as error:
             raise AnsibleActionFail(message=str(error)) from error
 

python-avd/pyavd/_eos_designs/eos_designs_facts/vlans.py

@@ -86,9 +86,9 @@ def _local_endpoint_vlans_and_trunk_groups(self: EosDesignsFacts) -> tuple[set,
         for connected_endpoints_key in self.shared_utils.connected_endpoints_keys:
             connected_endpoints = get(self._hostvars, connected_endpoints_key["key"], default=[])
             for connected_endpoint in connected_endpoints:
-                for adapter in connected_endpoint.get("adapters", []):
+                for index, adapter in enumerate(connected_endpoint.get("adapters", [])):
                     adapter_settings = self.shared_utils.get_merged_adapter_settings(
-                        adapter, context=f"{connected_endpoints_key['key']}[{connected_endpoint['name']}].adapters"
+                        adapter, context=f"{connected_endpoints_key['key']}[{connected_endpoint['name']}].adapters[{index}]"
                     )
                     if self.shared_utils.hostname not in adapter_settings.get("switches", []):
                         # This switch is not connected to this endpoint. Skipping.
@@ -104,7 +104,7 @@ def _local_endpoint_vlans_and_trunk_groups(self: EosDesignsFacts) -> tuple[set,
                         return vlans, trunk_groups
 
         network_ports = get(self._hostvars, "network_ports", default=[])
-        for network_port_item in network_ports:
+        for index, network_port_item in enumerate(network_ports):
             for switch_regex in network_port_item.get("switches", []):
                 # The match test is built on Python re.match which tests from the beginning of the string #}
                 # Since the user would not expect "DC1-LEAF1" to also match "DC-LEAF11" we will force ^ and $ around the regex
@@ -113,7 +113,7 @@ def _local_endpoint_vlans_and_trunk_groups(self: EosDesignsFacts) -> tuple[set,
                     # Skip entry if no match
                     continue
 
-                adapter_settings = self.shared_utils.get_merged_adapter_settings(network_port_item, context="network_ports")
+                adapter_settings = self.shared_utils.get_merged_adapter_settings(network_port_item, context=f"network_ports[{index}]")
                 adapter_vlans, adapter_trunk_groups = self._parse_adapter_settings(adapter_settings)
                 vlans.update(adapter_vlans)
                 trunk_groups.update(adapter_trunk_groups)

python-avd/pyavd/_eos_designs/shared_utils/cv_topology.py

@@ -45,7 +45,7 @@ def cv_topology(self: SharedUtils) -> dict | None:
             self.hostvars,
             "cv_topology",
             required=True,
-            org_key="Found 'use_cv_topology:true' so 'cv_topology'",
+            custom_error_msg="Found 'use_cv_topology:true' so 'cv_topology' is required.",
         )
 
         return get_item(cv_topology, "hostname", self.hostname)
@@ -79,7 +79,7 @@ def cv_topology_config(self: SharedUtils) -> dict:
                 self.default_interfaces,
                 "uplink_interfaces",
                 required=True,
-                org_key="Found 'use_cv_topology:true' so 'default_interfaces.[].uplink_interfaces'",
+                custom_error_msg="Found 'use_cv_topology:true' so 'default_interfaces.[].uplink_interfaces' is required.",
             ),
         )
         config = {}
@@ -97,7 +97,7 @@ def cv_topology_config(self: SharedUtils) -> dict:
                 self.default_interfaces,
                 "mlag_interfaces",
                 required=True,
-                org_key="Found 'use_cv_topology:true' so 'default_interfaces.[].mlag_interfaces'",
+                custom_error_msg="Found 'use_cv_topology:true' so 'default_interfaces.[].mlag_interfaces' is required.",
             ),
         )
         for mlag_interface in default_mlag_interfaces:

python-avd/pyavd/_eos_designs/shared_utils/filtered_tenants.py

@@ -230,7 +230,7 @@ def filtered_vrfs(self: SharedUtils, tenant: dict) -> list[dict]:
                                 rp_entry,
                                 "rps",
                                 required=True,
-                                org_key=f"pim_rp_addresses.rps under VRF '{vrf['name']}' in Tenant '{tenant['name']}'",
+                                custom_error_msg=f"'pim_rp_addresses.rps' under VRF '{vrf['name']}' in Tenant '{tenant['name']}' is required.",
                             ):
                                 rp_address = {"address": rp_ip}
                                 if (rp_groups := get(rp_entry, "groups")) is not None:
@@ -370,7 +370,7 @@ def get_vrf_id(vrf: dict, required: bool = True) -> int | None:
         if vrf_id is None:
             if required:
                 msg = f"'vrf_id' or 'vrf_vni' for VRF '{vrf['name']} must be set."
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
             return None
         return int(vrf_id)
 
@@ -379,7 +379,7 @@ def get_vrf_vni(vrf: dict) -> int:
         vrf_vni = default(vrf.get("vrf_vni"), vrf.get("vrf_id"))
         if vrf_vni is None:
             msg = f"'vrf_vni' or 'vrf_id' for VRF '{vrf['name']} must be set."
-            raise AristaAvdMissingVariableError(msg)
+            raise AristaAvdMissingVariableError(message=msg)
         return int(vrf_vni)
 
     @cached_property

python-avd/pyavd/_eos_designs/shared_utils/inband_management.py

@@ -130,7 +130,7 @@ def inband_mgmt_ip(self: SharedUtils) -> str | None:
 
         if self.id is None:
             msg = f"'id' is not set on '{self.hostname}' and is required to set inband_mgmt_ip from inband_mgmt_subnet"
-            raise AristaAvdMissingVariableError(msg)
+            raise AristaAvdMissingVariableError(message=msg)
 
         subnet = ip_network(self.inband_mgmt_subnet, strict=False)
         inband_mgmt_ip = str(subnet[3 + self.id])
@@ -154,7 +154,7 @@ def inband_mgmt_ipv6_address(self: SharedUtils) -> str | None:
 
         if self.id is None:
             msg = f"'id' is not set on '{self.hostname}' and is required to set inband_mgmt_ipv6_address from inband_mgmt_ipv6_subnet"
-            raise AristaAvdMissingVariableError(msg)
+            raise AristaAvdMissingVariableError(message=msg)
 
         subnet = ip_network(self.inband_mgmt_ipv6_subnet, strict=False)
         inband_mgmt_ipv6_address = str(subnet[3 + self.id])

python-avd/pyavd/_eos_designs/shared_utils/l3_interfaces.py

@@ -71,14 +71,14 @@ def l3_interfaces_bgp_neighbors(self: SharedUtils) -> list:
             peer_as = get(bgp, "peer_as")
             if peer_as is None:
                 msg = f"'l3_interfaces[{interface['name']}].bgp.peer_as' needs to be set to enable BGP."
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
 
             is_intf_wan = get(interface, "wan_carrier") is not None
 
             prefix_list_in = get(bgp, "ipv4_prefix_list_in")
             if prefix_list_in is None and is_intf_wan:
                 msg = f"BGP is enabled but 'bgp.ipv4_prefix_list_in' is not configured for l3_interfaces[{interface['name']}]"
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
 
             description = interface.get("description")
             if not description:

python-avd/pyavd/_eos_designs/shared_utils/mgmt.py

@@ -70,15 +70,15 @@ def default_mgmt_method(self: SharedUtils) -> str | None:
         if default_mgmt_method == "oob":
             if (self.mgmt_ip is None) and (self.ipv6_mgmt_ip is None):
                 msg = "'default_mgmt_method: oob' requires either 'mgmt_ip' or 'ipv6_mgmt_ip' to bet set."
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
 
             return default_mgmt_method
 
         if default_mgmt_method == "inband":
             # Check for missing interface
             if self.inband_mgmt_interface is None:
                 msg = "'default_mgmt_method: inband' requires 'inband_mgmt_interface' to be set."
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
 
             return default_mgmt_method
 

python-avd/pyavd/_eos_designs/shared_utils/misc.py

@@ -136,7 +136,7 @@ def uplink_switch_interfaces(self: SharedUtils) -> list:
 
         if self.id is None:
             msg = f"'id' is not set on '{self.hostname}'"
-            raise AristaAvdMissingVariableError(msg)
+            raise AristaAvdMissingVariableError(message=msg)
 
         uplink_switch_interfaces = []
         uplink_switch_counter = {}

python-avd/pyavd/_eos_designs/shared_utils/mlag.py

@@ -168,20 +168,20 @@ def mlag_switch_ids(self: SharedUtils) -> dict | None:
         if self.mlag_role == "primary":
             if self.id is None:
                 msg = f"'id' is not set on '{self.hostname}' and is required to compute MLAG ids"
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
             return {"primary": self.id, "secondary": self.mlag_peer_id}
         if self.mlag_role == "secondary":
             if self.id is None:
                 msg = f"'id' is not set on '{self.hostname}' and is required to compute MLAG ids"
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
             return {"primary": self.mlag_peer_id, "secondary": self.id}
         return None
 
     @cached_property
     def mlag_port_channel_id(self: SharedUtils) -> int:
         if not self.mlag_interfaces:
             msg = f"'mlag_interfaces' not set on '{self.hostname}."
-            raise AristaAvdMissingVariableError(msg)
+            raise AristaAvdMissingVariableError(message=msg)
         default_mlag_port_channel_id = int("".join(findall(r"\d", self.mlag_interfaces[0])))
         return get(self.switch_data_combined, "mlag_port_channel_id", default_mlag_port_channel_id)
 

python-avd/pyavd/_eos_designs/shared_utils/node_type.py

@@ -31,7 +31,7 @@ def type(self: SharedUtils) -> str:
             return self.default_node_type
 
         msg = f"'type' for host {self.hostname}"
-        raise AristaAvdMissingVariableError(msg)
+        raise AristaAvdMissingVariableError(message=msg)
 
     @cached_property
     def default_node_type(self: SharedUtils) -> str | None:

python-avd/pyavd/_eos_designs/shared_utils/node_type_keys.py

@@ -214,4 +214,4 @@ def node_type_key_data(self: SharedUtils) -> dict:
 
         # Not found
         msg = f"node_type_keys.[type=={self.type}]"
-        raise AristaAvdMissingVariableError(msg)
+        raise AristaAvdMissingVariableError(message=msg)

python-avd/pyavd/_eos_designs/shared_utils/overlay.py

@@ -91,7 +91,7 @@ def get_rd_admin_subfield_value(self: SharedUtils, admin_subfield: str, admin_su
         if admin_subfield == "switch_id":
             if self.id is None:
                 msg = f"'id' is not set on '{self.hostname}' and 'overlay_rd_type_admin_subfield' is set to 'switch_id'"
-                raise AristaAvdMissingVariableError(msg)
+                raise AristaAvdMissingVariableError(message=msg)
             return self.id + admin_subfield_offset
 
         if fullmatch(r"\d+", str(admin_subfield)):

python-avd/pyavd/_eos_designs/shared_utils/routing.py

@@ -103,7 +103,7 @@ def bgp_as(self: SharedUtils) -> str | None:
 
                 if self.id is None:
                     msg = f"'id' is not set on '{self.hostname}' and is required when expanding 'bgp_as'"
-                    raise AristaAvdMissingVariableError(msg)
+                    raise AristaAvdMissingVariableError(message=msg)
                 return bgp_as_range_expanded[self.id - 1]
             except IndexError as exc:
                 msg = f"Unable to allocate BGP AS: bgp_as range is too small ({len(bgp_as_range_expanded)}) for the id of the device"

python-avd/pyavd/_eos_designs/shared_utils/utils.py

@@ -37,9 +37,9 @@ def get_peer_facts(self: SharedUtils, peer_name: str, required: bool = True) ->
             f"avd_switch_facts..{peer_name}..switch",
             separator="..",
             required=required,
-            org_key=(
+            custom_error_msg=(
                 f"Facts not found for node '{peer_name}'. Something in the input vars is pointing to this node. "
-                f"Check that '{peer_name}' is in the inventory and is part of the group set by 'fabric_name'. Node"
+                f"Check that '{peer_name}' is in the inventory and is part of the group set by 'fabric_name'. Node is required."
             ),
         )