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

Junos ping form routing instance #633

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

MrPaulAR
Copy link
Contributor

@MrPaulAR MrPaulAR commented Mar 4, 2022

Description

Added routing-instance support to the ping command. No changes to returned dictionary.

Motivation and Context

The ping command in the current release doesn't work if run from a routing-instance.

Impact (If any)

No impact.

Screenshots:

image

Checklist:

  • [x ] I have updated the changelog.
  • I have updated the documentation (If applicable).
  • [ x] I have added tests to cover my changes (If applicable).
  • [ x] All new and existing tests passed.
  • [ x] All new code passed compilation.

@MrPaulAR MrPaulAR requested a review from a team as a code owner March 4, 2022 23:53
@MrPaulAR MrPaulAR requested review from GerriorL and d-the-iii March 4, 2022 23:53
Comment on lines +10 to +12
* junos
* Modified ping.py
* Added routing instance support
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put this changelog in the 2022/undistributed folder, the release engineer will handle putting it in this file.

Comment on lines +97 to +105
"ping {addr}",
"ping {addr} count {count}",
"ping {addr} ttl {ttl} count {count} wait {wait}",
"ping {addr} source {source} count {count}",
"ping {addr} source {source} size {size} do-not-fragment count {count}",
"ping {addr} source {source} size {size} count {count} tos {tos} rapid",
"ping {addr} size {size} count {count} do-not-fragment",
"ping {addr} routing-instance {routing_instance} count {count}",
"ping {addr} routing-instance {routing_instance} source {source} count {count}"
Copy link
Contributor

Choose a reason for hiding this comment

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

please stop changing all the quoting, it's fine the way it is

Comment on lines +156 to +157
r"^PING +(?P<address>\S+) +\((?P<source>\S+)\): +"
r"(?P<data_bytes>\d+) +data +bytes$"
Copy link
Contributor

Choose a reason for hiding this comment

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

change the quoting back on all the patterns

@@ -198,45 +231,59 @@ def cli(self, addr, count=None, ttl=None,
m = p2.match(line) or p2_2.match(line)
if m:
group = m.groupdict()
result_list = ping_dict.setdefault('result', [])
result_list = ping_dict.setdefault("result", [])
Copy link
Contributor

Choose a reason for hiding this comment

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

revert

continue

# round-trip min/avg/max/stddev = 1.823/2.175/2.399/0.191 ms
m = p4.match(line)
if m:
group = m.groupdict()
round_trip_dict = ping_statistics_dict.setdefault('round-trip', {})
round_trip_dict.update({k.replace('_', '-'):v for k, v in group.items() if v is not None})
round_trip_dict = ping_statistics_dict.setdefault("round-trip", {})
Copy link
Contributor

Choose a reason for hiding this comment

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

fix quoting

Comment on lines +276 to +279
"lsping-statistics": {
"send": int,
"received": int,
"loss-rate": int,
Copy link
Contributor

Choose a reason for hiding this comment

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

revert

class PingMplsRsvp(PingMplsRsvpSchema):

cli_command = 'ping mpls rsvp {rsvp}'
cli_command = "ping mpls rsvp {rsvp}"
Copy link
Contributor

Choose a reason for hiding this comment

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

revert

Comment on lines +104 to +105
"ping {addr} routing-instance {routing_instance} count {count}",
"ping {addr} routing-instance {routing_instance} source {source} count {count}"
Copy link
Contributor

Choose a reason for hiding this comment

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

You haven't added any section to the parser cli command caller that will run entry 7 or 8, making this do nothing.

@MrPaulAR MrPaulAR closed this Mar 7, 2022
@MrPaulAR MrPaulAR reopened this Mar 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants