Skip to content

Commit

Permalink
Fix dict merge issue and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
ClausHolbechArista committed Nov 22, 2024
1 parent dfd9fea commit b10d1d6
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ interface Ethernet10
description P2P_LINK_TO_CORE-2-OSPF-LDP_Ethernet10
no shutdown
mtu 1500
l2 mtu 2222
l2 mru 2222
speed forced 1000full
no switchport
ip address 100.64.48.12/31
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ interface Ethernet10
description P2P_LINK_TO_CORE-1-ISIS-SR-LDP_Ethernet10
no shutdown
mtu 1500
l2 mtu 2222
l2 mru 2222
speed forced 1000full
no switchport
ip address 100.64.48.13/31
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ ethernet_interfaces:
igp_sync: true
description: P2P_LINK_TO_CORE-2-OSPF-LDP_Ethernet10
speed: forced 1000full
l2_mru: 2222
l2_mtu: 2222
- name: Ethernet12
peer: core-2-ospf-ldp
peer_interface: Ethernet12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ ethernet_interfaces:
igp_sync: true
description: P2P_LINK_TO_CORE-1-ISIS-SR-LDP_Ethernet10
speed: forced 1000full
l2_mru: 2222
l2_mtu: 2222
- name: Ethernet12
peer: core-1-isis-sr-ldp
peer_interface: Ethernet12
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ core_interfaces:
speed: "forced 1000full"
mtu: 1500
ip_pool: underlay_pool
# Testing merge of structured config on profile and link.
structured_config:
l2_mtu: 2222

p2p_links:

Expand Down Expand Up @@ -139,6 +142,9 @@ core_interfaces:
id: 7
interfaces: [ Ethernet10, Ethernet10 ]
profile: ospf_bb_profile
# Testing merge of structured config on profile and link.
structured_config:
l2_mru: 2222

# core Port-Channel
- nodes: [ core-1-isis-sr-ldp, core-2-ospf-ldp ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,22 @@ l2leaf:
level: 25
unknown_unicast:
level: 25
switchport:
trunk:
native_vlan_tag: true
mlag_interfaces: [ Ethernet3, Ethernet4 ]
mlag_peer_ipv4_pool: 10.255.252.0/24
mlag_port_channel_structured_config: # Setting structured config here but it is overridden on group level and applied again on node level.
description: "Port-channel description not being used because it is overridden on node-group level."
mlag_port_channel_structured_config: # Setting structured config here but it is overridden by 'null' on group level and applied again on node level.
description: "Port-channel description not being used because it is overridden by 'null' on node-group level."

spanning_tree_mode: mstp
spanning_tree_priority: 16384
node_groups:
- group: UPLINK-MLAG-STRUCTURED-CONFIG-L2LEAF1
# Testing the inheritance works correctly when nullifying a dict in the group and then applying a new one per node.
mlag_port_channel_structured_config: null
# Testing merging of uplink_structured_config which is a dict type with no schema.
uplink_structured_config:
switchport:
trunk:
native_vlan_tag: true
nodes:
- name: UPLINK-MLAG-STRUCTURED-CONFIG-L2LEAF1A
id: 1
Expand Down
20 changes: 17 additions & 3 deletions python-avd/pyavd/_schema/models/avd_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing import TYPE_CHECKING, Any, ClassVar, Literal

from pyavd._schema.coerce_type import coerce_type
from pyavd._utils import Undefined, UndefinedType
from pyavd._utils import Undefined, UndefinedType, merge

from .avd_base import AvdBase
from .avd_indexed_list import AvdIndexedList
Expand Down Expand Up @@ -256,8 +256,15 @@ def _deepmerge(self, other: Self, list_merge: Literal["append", "replace"] = "ap
if issubclass(field_type, AvdBase) and isinstance(old_value, field_type):
# Merge in to the existing object
old_value._deepmerge(new_value, list_merge=list_merge)
else:
setattr(self, field, new_value)
continue

if field_type is dict:
# In-place deepmerge in to the existing dict without schema.
# Deepcopying since merge() does not copy.
merge(old_value, deepcopy(new_value), list_merge=list_merge)
continue

setattr(self, field, new_value)

def _inherit(self, other: Self) -> None:
"""Update unset fields on this instance with fields from other instance. No merging."""
Expand Down Expand Up @@ -291,12 +298,19 @@ def _deepinherit(self, other: Self) -> None:
# Inherit the field only if the old value is Undefined.
if old_value is Undefined:
setattr(self, field, deepcopy(new_value))
continue

# Merge new value if it is a class with inheritance support.
field_type = field_info["type"]
if issubclass(field_type, (AvdModel, AvdIndexedList)) and isinstance(old_value, field_type):
# Inherit into the existing object.
old_value._deepinherit(new_value)
continue

if field_type is dict:
# In-place deepmerge in to the existing dict without schema.
# Deepcopying since merge() does not copy.
merge(old_value, deepcopy(new_value), list_merge="replace")

def _deepinherited(self, other: Self) -> Self:
"""Return new instance with the result of recursively inheriting unset fields from other instance. Lists are not merged."""
Expand Down
2 changes: 1 addition & 1 deletion python-avd/pyavd/_utils/merge/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def _strategy_must_match(_config: object, path: list, base: Any, nxt: Any) -> An

def merge(
base: Any,
*nxt_list: list[Any],
*nxt_list: Any,
recursive: bool = True,
list_merge: str = "append",
same_key_strategy: str = "override",
Expand Down

0 comments on commit b10d1d6

Please sign in to comment.