-
Notifications
You must be signed in to change notification settings - Fork 52
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
bgp send-community fix #487
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,13 +168,13 @@ route_reflector_client: | |
default_value: false | ||
|
||
send_community: | ||
multiple: | ||
auto_default: false | ||
default_value: 'none' | ||
nexus: | ||
get_value: '/^send-community(?: .*)?/' | ||
set_value: '<state> send-community <attr>' | ||
ios_xr: | ||
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. Why did you remove multiple under xr? 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. I did not remove it. I moved it above so that it is common for both xr and nexus. 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. @saichint Sorry my mistake. |
||
multiple: | ||
# XR three seperate commands: send-community-ebgp send-community-gshut-ebgp | ||
# and send-extended-community-ebgp | ||
# send-community-ebgp' and 'send-extended-community-ebgp' are the equivalents | ||
|
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.
What are the specific contents of
val
whensize > 1
? Does it nvgen bothextended
andstandard
?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.
yes, when we execute the CLI with 'both', it generates 2 lines, one with standard and the 2nd with extended.
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.
I think I would prefer we be more explicit here and inspect
val
forextended
andstandard
then rather then just check the size.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.
Ok. Changed it to test the actual values. Since the order of the nvgen could change in the future without our knowledge, I added sort just in case.