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

NXOS_SSH/IOS XE IPv6 Regex update #2016

Merged
merged 5 commits into from
Oct 23, 2023
Merged

Conversation

ubaumann
Copy link
Contributor

  • The regex did not match for IPv6 addresses ending or starting with :: (IOS XE and NXOS_SSH)
  • On NXOS, the show bgp all summary vrf all with IPv6 addresses and long AS numbers has two newlines.

@ubaumann
Copy link
Contributor Author

I tested the long AS number with IPv4 and the same problem occurs. Therefore and for readability I have changed the code to make two re.sub

@ktbyers
Copy link
Contributor

ktbyers commented Oct 22, 2023

@ubaumann Are there tests for the long as-number case?

@ubaumann
Copy link
Contributor Author

@ktbyers The long as-number is combined with the IPv6 test case

2001:db8:e0:df::
                4    12345678
                         327465  327268      145    0    0     3w1d 4

https://github.com/napalm-automation/napalm/pull/2016/files#diff-cea37f4186bd9fbcbb95c704fda5ea3b17ecad55d1d50ee605d55aab70ef84fbR29

I did not write test for IPv4 with long AS number but because the two re.sub the case with ipv4 and ipv6 is the same.
I also did not write tests in the IOS XE for the IPv6 addresses ending or starting with ::

Should I add some more testing?

@ktbyers
Copy link
Contributor

ktbyers commented Oct 23, 2023

That is probably fine...I missed the long-number AS earlier in the test output.

@ktbyers ktbyers merged commit 1858e72 into napalm-automation:develop Oct 23, 2023
6 checks passed
@mirceaulinic mirceaulinic added this to the 5.0.0 milestone Mar 21, 2024
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.

3 participants