Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat(eos_designs): Add platform match criteria for network_ports #4798

Open
wants to merge 16 commits into
base: devel
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,11 @@ interface Ethernet17
no shutdown
switchport
!
interface Ethernet24
description Matched on only 24 port platform
no shutdown
switchport
!
interface Ethernet51
no shutdown
channel-group 43 mode active
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,11 @@ interface Ethernet14
no shutdown
switchport
!
interface Ethernet48
description Matched on all hostnames and 48 port platform
no shutdown
switchport
!
interface Ethernet51
shutdown
switchport
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,12 @@ ethernet_interfaces:
channel_group:
id: 43
mode: active
- name: Ethernet24
peer_type: network_port
description: Matched on only 24 port platform
shutdown: false
switchport:
enabled: true
mlag_configuration:
domain_id: mlag
local_interface: Vlan4094
Expand All @@ -833,3 +839,5 @@ monitor_sessions:
destinations:
- Ethernet17
encapsulation_gre_metadata_tx: true
metadata:
platform: 720XPM-24Y6
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,12 @@ ethernet_interfaces:
shutdown: true
switchport:
enabled: true
- name: Ethernet48
peer_type: network_port
description: Matched on all hostnames and 48 port platform
shutdown: false
switchport:
enabled: true
mlag_configuration:
domain_id: mlag
local_interface: Vlan4094
Expand All @@ -760,3 +766,5 @@ mlag_configuration:
reload_delay_non_mlag: '330'
ip_igmp_snooping:
globally_enabled: true
metadata:
platform: 720XPM-48Y6
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,37 @@ network_ports:
channel_id: 43
mode: "active"

# Tests for issue #4786
# Test regex match all switches and specific platform match criteria.
- switches:
- network-ports-tests.*
laxmikantchintakindi marked this conversation as resolved.
Show resolved Hide resolved
platforms:
- 720XPM-48Y6
switch_ports:
- Ethernet48
description: Matched on all hostnames and 48 port platform

# Test only platform match.
- platforms:
- 720\w+-24Y6
switch_ports:
- Ethernet24
description: Matched on only 24 port platform

# Test criteria must match both hostname/platform.
- switches:
- network-ports-no-match
platforms:
- 720XPM-48Y6
switch_ports:
- Ethernet49
description: Should not match hostname and platform

# Test no match criteria
- switch_ports:
- Ethernet25
description: No criteria, no config

servers:
- name: CONNECTED_ENDPOINT_OVERWRITING_NETWORK_PORT
adapters:
Expand Down Expand Up @@ -171,7 +202,23 @@ l2leaf:
nodes:
- name: network-ports-tests.1
id: 1
platform: 720XPM-48Y6
- name: network-ports-tests-2
filter:
only_vlans_in_use: true
id: 2
platform: 720XPM-24Y6

# Used for platform filtering tests
custom_platform_settings:
- platforms:
- 720XPM-48Y6
- 720XPM-24Y6
feature_support:
poe: true
queue_monitor_length_notify: false
reload_delay:
mlag: 300
non_mlag: 330
trident_forwarding_table_partition: flexible exact-match 16000 l2-shared 18000 l3-shared
22000
Original file line number Diff line number Diff line change
Expand Up @@ -1139,12 +1139,62 @@ roles/eos_designs/docs/tables/default-connected-endpoints-description.md
The `network_ports` data model is intended to be used with `port_profiles` and `parent_profiles` to keep the configuration generic and compact,
but all features and keys supported under `connected_endpoints.adapters` are also supported directly under `network_ports`.

To filter what switches to configure, match on a switch's full hostname or platform type using regex patterns. When both criteria are used together, the switch must match both in order to generate the assigned port configuration.

All ranges defined under `switch_ports` will be expanded to individual port configuration which leads to a some behavioral differences to `connected_endpoints`:

- By default each port will be configured in a port-channel with one member when leveraging automatic channel-id generation.
To configure multiple ports as member of the same port-channel set the channel-id key (see the example below).
- Inconsistent configurations when used with `short_esi: auto` or `designated_forwarder_algorithm: auto`, since those rely on information from multiple switches and interfaces.

??? example "Example using match criteria"

```yaml
# Port Profiles
# Common settings inherited to network_ports
port_profiles:
- profile: common
mode: access
vlans: "999"
spanning_tree_portfast: edge
spanning_tree_bpdufilter: enabled

# Network Ports
# Switches are matched with regex matching the full hostname and platform type.
network_ports:
- switches:
- network-ports-[est]{5}-.*
platforms:
- 720XPM-48Y6
switch_ports:
- Ethernet1-48
profile: common

# Switches are matched on platform type, regardless of hostname.
- platforms:
- 720XPM-24Y6
switch_ports:
- Ethernet1-24
profile: common

# Custom Platform Settings
# Copied default 720XP platform settings, adding more specific platform names for match.
# These platform types can be assigned to devices as part of nodes/node_group settings.
custom_platform_settings:
- platforms:
- 720XPM-48Y6
- 720XPM-24Y6
feature_support:
poe: true
queue_monitor_length_notify: false
reload_delay:
mlag: 300
non_mlag: 330
trident_forwarding_table_partition: flexible exact-match 16000 l2-shared 18000 l3-shared
22000

```

??? example "Example using network ports and profiles"

```yaml
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 45 additions & 0 deletions python-avd/pyavd/_eos_designs/schema/__init__.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions python-avd/pyavd/_eos_designs/schema/eos_designs.schema.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,29 @@ keys:
$ref: "eos_designs#/$defs/adapter_config"
# These keys will take precedence over $ref
keys:
# TODO: AVD 6.0.0 add min_length: 1 for switches
switches:
type: list
description: |
Regex matching the full hostname of one or more switches.
The regular expression must match the full hostname.
items:
type: str
platforms:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a description on the network_ports stating that when switches and platforms are both set, then only devices matching both criteria will be configured.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added clarification in schema docs and in markdown above clarifying match criteria expectations.

type: list
kpbush30 marked this conversation as resolved.
Show resolved Hide resolved
min_length: 1
description: |
Regex matching the full platform name of one or more switches. If used in combination with switch hostname matching, both criteria must match for configuration.
If you need to add custom platforms to match specific port quantities, create them under `custom_platform_settings`. Entries under `custom_platform_settings` should
match the platform match criteria.

For example, `720XP-48Y6` would require a custom platform type:

platforms:
- 720XP
- 720XP-48Y6
items:
type: str
switch_ports:
type: list
description: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ def _filtered_network_ports(self: AvdStructuredConfigConnectedEndpoints) -> list
for index, network_port in enumerate(self.inputs.network_ports):
network_port._context = f"network_ports[{index}]"
network_port_settings = self.shared_utils.get_merged_adapter_settings(network_port)
if not self._match_regexes(network_port_settings.switches, self.shared_utils.hostname):

if not network_port_settings.switches and not network_port_settings.platforms:
continue
if network_port_settings.switches and not self._match_regexes(network_port_settings.switches, self.shared_utils.hostname):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is changing the behavior. In the current implementation, if switches is not set, then no network_port are configured on any device. The change, if neither switches, nor platforms are set would configure the network_ports on ALL devices.

you would need an extra if to make sure that either switches or platforms are set + add a test case with network_ports without switches not platforms set to validate the behavior

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed behavior to ensure match criteria is required, added test to ensure configuration is not generated if missing both switch hostname and platform match criteria.

continue
if network_port_settings.platforms and not self._match_regexes(network_port_settings.platforms, self.shared_utils.platform):
continue

filtered_network_ports.append(network_port_settings)
Expand All @@ -85,7 +90,7 @@ def _match_regexes(self: AvdStructuredConfigConnectedEndpoints, regexes: list, v

Regex must match the full value to pass, so regex is wrapped in ^$.
"""
return any(re.match(rf"^{regex}$", value) for regex in regexes)
return any(re.fullmatch(regex, value) for regex in regexes)

def _get_short_esi(
self: AvdStructuredConfigConnectedEndpoints,
Expand Down
Loading