-
Notifications
You must be signed in to change notification settings - Fork 234
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
Refactor(eos_designs): structured_config for network_services router_ospf #4981
Open
Vibhu-gslab
wants to merge
6
commits into
aristanetworks:devel
Choose a base branch
from
Vibhu-gslab:refactor_router_ospf
base: devel
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+73
−65
Open
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
743852a
Refactor(eos_designs): structured_config for network_services router_…
Vibhu-gslab 7e041d1
Fixing sonar lint issue
Vibhu-gslab 96712ad
addressing comments on improving code 1
Vibhu-gslab 6b3f0fa
Merge branch 'devel' into refactor_router_ospf
Vibhu-gslab 5fc22e7
Addressing comments
Vibhu-gslab ffc052f
Merge branch 'devel' into refactor_router_ospf
Vibhu-gslab File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,16 @@ | |
# that can be found in the LICENSE file. | ||
from __future__ import annotations | ||
|
||
from functools import cached_property | ||
from typing import TYPE_CHECKING, Protocol | ||
|
||
from pyavd._eos_cli_config_gen.schema import EosCliConfigGen | ||
from pyavd._eos_designs.structured_config.structured_config_generator import structured_config_contributor | ||
from pyavd._errors import AristaAvdInvalidInputsError | ||
from pyavd._utils import append_if_not_duplicate, default | ||
from pyavd._utils import default | ||
|
||
if TYPE_CHECKING: | ||
from pyavd._eos_designs.schema import EosDesigns | ||
|
||
from . import AvdStructuredConfigNetworkServicesProtocol | ||
|
||
|
||
|
@@ -20,85 +23,110 @@ class RouterOspfMixin(Protocol): | |
Class should only be used as Mixin to a AvdStructuredConfig class. | ||
""" | ||
|
||
@cached_property | ||
def router_ospf(self: AvdStructuredConfigNetworkServicesProtocol) -> dict | None: | ||
@structured_config_contributor | ||
def router_ospf(self: AvdStructuredConfigNetworkServicesProtocol) -> None: | ||
""" | ||
Return structured config for router_ospf. | ||
Set structured config for router_ospf. | ||
|
||
If we have static_routes in default VRF and not EPVN, and underlay is OSPF | ||
Then add redistribute static to the underlay OSPF process. | ||
""" | ||
if not self.shared_utils.network_services_l3: | ||
return None | ||
return | ||
|
||
ospf_processes = [] | ||
for tenant in self.shared_utils.filtered_tenants: | ||
for vrf in tenant.vrfs: | ||
if not vrf.ospf.enabled: | ||
continue | ||
|
||
if vrf.ospf.nodes and self.shared_utils.hostname not in vrf.ospf.nodes: | ||
if not vrf.ospf.enabled or (vrf.ospf.nodes and self.shared_utils.hostname not in vrf.ospf.nodes): | ||
continue | ||
|
||
ospf_interfaces = [] | ||
for l3_interface in vrf.l3_interfaces: | ||
if l3_interface.ospf.enabled: | ||
for node_index, node in enumerate(l3_interface.nodes): | ||
if node != self.shared_utils.hostname: | ||
continue | ||
|
||
ospf_interfaces.append(l3_interface.interfaces[node_index]) | ||
|
||
for svi in vrf.svis: | ||
if svi.ospf.enabled: | ||
interface_name = f"Vlan{svi.id}" | ||
ospf_interfaces.append(interface_name) | ||
process = EosCliConfigGen.RouterOspf.ProcessIdsItem() | ||
self._set_ospf_interface(process, vrf) | ||
|
||
process_id = default(vrf.ospf.process_id, vrf.vrf_id) | ||
if not process_id: | ||
msg = f"Missing or invalid 'ospf.process_id' or 'vrf_id' under vrf '{vrf.name}" | ||
raise AristaAvdInvalidInputsError(msg) | ||
process._update(id=process_id, passive_interface_default=True) | ||
|
||
process = { | ||
"id": process_id, | ||
"vrf": vrf.name if vrf.name != "default" else None, | ||
"passive_interface_default": True, | ||
"router_id": self.get_vrf_router_id(vrf, vrf.ospf.router_id, tenant.name), | ||
"no_passive_interfaces": ospf_interfaces, | ||
"bfd_enable": vrf.ospf.bfd or None, # Historic behavior is to only output if True. | ||
"max_lsa": vrf.ospf.max_lsa, | ||
} | ||
|
||
process_redistribute = {} | ||
|
||
if vrf.ospf.redistribute_bgp.enabled: | ||
process_redistribute["bgp"] = {"enabled": True} | ||
if route_map := vrf.ospf.redistribute_bgp.route_map: | ||
process_redistribute["bgp"]["route_map"] = route_map | ||
|
||
if vrf.ospf.redistribute_connected.enabled: | ||
process_redistribute["connected"] = {"enabled": True} | ||
if route_map := vrf.ospf.redistribute_connected.route_map: | ||
process_redistribute["connected"]["route_map"] = route_map | ||
|
||
process["redistribute"] = process_redistribute or None | ||
|
||
# Strip None values from process before adding to list | ||
process = {key: value for key, value in process.items() if value is not None} | ||
|
||
append_if_not_duplicate( | ||
list_of_dicts=ospf_processes, | ||
primary_key="id", | ||
new_dict=process, | ||
context="OSPF Processes defined under network services", | ||
context_keys=["id"], | ||
) | ||
self._set_ospf_vrf(process, vrf, tenant.name) | ||
self._set_ospf_redistribute(process, vrf) | ||
|
||
self.structured_config.router_ospf.process_ids.append(process) | ||
# If we have static_routes in default VRF and not EPVN, and underlay is OSPF | ||
# Then add redistribute static to the underlay OSPF process. | ||
if self._vrf_default_ipv4_static_routes["redistribute_in_underlay"] and self.shared_utils.underlay_routing_protocol in ["ospf", "ospf-ldp"]: | ||
ospf_processes.append({"id": self.inputs.underlay_ospf_process_id, "redistribute": {"static": {"enabled": True}}}) | ||
if ospf_processes: | ||
return {"process_ids": ospf_processes} | ||
process = EosCliConfigGen.RouterOspf.ProcessIdsItem(id=self.inputs.underlay_ospf_process_id) | ||
process.redistribute.static.enabled = True | ||
self.structured_config.router_ospf.process_ids.append(process) | ||
ClausHolbechArista marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def _set_ospf_redistribute( | ||
ClausHolbechArista marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self, process: EosCliConfigGen.RouterOspf.ProcessIdsItem, vrf: EosDesigns._DynamicKeys.DynamicNetworkServicesItem.NetworkServicesItem.VrfsItem | ||
) -> None: | ||
""" | ||
Configures OSPF route redistribution settings for the given VRF. | ||
|
||
This method enables redistribution of BGP and connected routes into OSPF, | ||
setting the associated route maps if specified. | ||
|
||
return None | ||
Args: | ||
process: The OSPF process configuration object. | ||
vrf: The VRF object containing OSPF redistribution settings. | ||
""" | ||
if vrf.ospf.redistribute_bgp.enabled: | ||
process.redistribute.bgp.enabled = True | ||
if route_map := vrf.ospf.redistribute_bgp.route_map: | ||
process.redistribute.bgp.route_map = route_map | ||
Comment on lines
+78
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could possibly be done with a _cast_as since the keys are 1:1. If you do that, it ends up with two lines and no "if` so it would be easier done directly in the process creation so remove this function. |
||
|
||
if vrf.ospf.redistribute_connected.enabled: | ||
process.redistribute.connected.enabled = True | ||
if route_map := vrf.ospf.redistribute_connected.route_map: | ||
process.redistribute.connected.route_map = route_map | ||
|
||
def _set_ospf_interface( | ||
self, | ||
process: EosCliConfigGen.RouterOspf.ProcessIdsItem, | ||
vrf: EosDesigns._DynamicKeys.DynamicNetworkServicesItem.NetworkServicesItem.VrfsItem, | ||
) -> None: | ||
""" | ||
Populates the list of OSPF-enabled interfaces for the given VRF. | ||
|
||
This method iterates through L3 interfaces and SVIs, adding those that have OSPF enabled. | ||
|
||
Args: | ||
process: The OSPF process configuration object. | ||
vrf: The VRF object containing interface OSPF settings. | ||
""" | ||
for l3_interface in vrf.l3_interfaces: | ||
if l3_interface.ospf.enabled: | ||
for node_index, node in enumerate(l3_interface.nodes): | ||
if node != self.shared_utils.hostname: | ||
continue | ||
process.no_passive_interfaces.append(l3_interface.interfaces[node_index]) | ||
|
||
for svi in vrf.svis: | ||
if svi.ospf.enabled: | ||
interface_name = f"Vlan{svi.id}" | ||
process.no_passive_interfaces.append(interface_name) | ||
|
||
def _set_ospf_vrf( | ||
ClausHolbechArista marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self, | ||
process: EosCliConfigGen.RouterOspf.ProcessIdsItem, | ||
laxmikantchintakindi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
vrf: EosDesigns._DynamicKeys.DynamicNetworkServicesItem.NetworkServicesItem.VrfsItem, | ||
name: str, | ||
) -> None: | ||
""" | ||
Configures VRF-specific OSPF settings. | ||
|
||
This method sets the VRF name, router ID, BFD, and max LSA limit for the OSPF process. | ||
|
||
Args: | ||
process: The OSPF process configuration object. | ||
vrf: The VRF object containing OSPF settings. | ||
name: The name of the tenant or associated network service. | ||
""" | ||
if vrf.name != "default": | ||
process.vrf = vrf.name | ||
if vrf_router_id := self.get_vrf_router_id(vrf, vrf.ospf.router_id, name): | ||
process.router_id = vrf_router_id | ||
if vrf.ospf.bfd: | ||
process.bfd_enable = vrf.ospf.bfd | ||
process.max_lsa = vrf.ospf.max_lsa |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the process creation here so you can set these directly, and then call the
_update_ospf_interface
after.