Added ndo_l3out_bgp_peer module to manage L3Out Node/Interface Group Policy - BGP Peer objects (DCNE-182) #611
Conversation
sajagana
requested review from
akinross,
anvitha-jain,
gmicol,
lhercot,
samiib and
shrsr
as code owners
github-actions
bot
changed the title
sajagana
force-pushed
the
535_l3out_bgp_peer
branch
3 times, most recently
from
5ed4f77 to
261d13c
Compare
| mso_values_remove.append("peerAddressV6") | ||
| mso_values.pop("peerAddressV6", None) | ||
|
|
||
| if remote_asn == 0 and "peerAsn" in proposed_payload: |
There was a problem hiding this comment.
If we go by documentation this will never be true. We need to figure out a more effective way to remove int values because a few attributes might have "0" as a valid value.
| mso_values_remove.append("peerAsn") | ||
| mso_values.pop("peerAsn", None) | ||
|
|
||
| if weight == 0 and "weight" in proposed_payload: |
…nterface Group Policy - BGP Peer objects
sajagana
force-pushed
the
535_l3out_bgp_peer
branch
from
261d13c to
a90ec92
Compare
| When the UUID is existing, returns object name -> Str | ||
| When the UUID is not existing -> None | ||
| """ | ||
| response_object = mso.request("templates/objects?type={0}&uuid={1}".format(object_type, uuid), "GET") |
There was a problem hiding this comment.
I have a few concerns with this method of obtaining object names with a UUID.
- The templates/objects API is currently undocumented and potentially not supported yet unless there's been any recent changes.
- This is another API call which can produce slow playbook performance.
- The module and template may already have the name information we need. This would be the case if the reference object needs to be in the same template.
| elif peer_prefix_identifier.get("name") is not None and peer_prefix_identifier.get("name") != "": | ||
| peer_prefix_uuid = mso_template.get_object_by_key_value_pairs( | ||
| "BGP Peer Prefix Policy", | ||
| mso.query_objs(generate_api_endpoint("templates/objects", **{"type": "bgpPeerPrefixPol", "tenant-id": tenant_id})), |
There was a problem hiding this comment.
Does this work when there are objects in different templates with the same name?
The get_object_from_list function will return the first matched object which may not be the intended reference.
There was a problem hiding this comment.
The template objects are unique within the tenant level, so we cannot have the same name object with two different templates. Let me recheck the logic once again.
| - The name of the L3Out template. | ||
| type: str | ||
| aliases: [ l3out_template ] | ||
| template_id: |
There was a problem hiding this comment.
Should we have a task to update the previous ndo_ module with template_id?
| - The local autonomous system number (ASN) of the L3Out BGP Peer. | ||
| - The value must be between 1 and 4294967295. | ||
| type: int | ||
| import_route_map: |
There was a problem hiding this comment.
Should the route_map and peer_prefix parameters be a dictionary with name and template options? How do we specify which template the objects are from?
There was a problem hiding this comment.
The route_map objects are unique within the tenant level, so we can skip the template check for the route_map objects.
The peer_prefix also unique within the tenant level, but it allows to import common tenant peer_prefix value. Let me make changes for both route map and peer prefix.
There was a problem hiding this comment.
see choices made in this discussion where it is a map with name, https://github.com/CiscoDevNet/ansible-mso/pull/577/files#r1970341772. perhaps we should follow same here?
| - The local autonomous system number (ASN) of the L3Out BGP Peer. | ||
| - The value must be between 1 and 4294967295. | ||
| type: int | ||
| import_route_map: |
There was a problem hiding this comment.
see choices made in this discussion where it is a map with name, https://github.com/CiscoDevNet/ansible-mso/pull/577/files#r1970341772. perhaps we should follow same here?
|
|
||
| mso = MSOModule(module) | ||
|
|
||
| template_identifier = get_object_identifier(module.params.get("template_id"), module.params.get("template")) |
There was a problem hiding this comment.
what is the use case for template_identifier instead of just storing in two variables?
No description provided.