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

Fix EOS get_arp_table for static ARP records #1272

Merged

Conversation

praniq
Copy link
Contributor

@praniq praniq commented Aug 18, 2020

For static ARP entries EOS eAPI gives records without "age" key. Therefore NAPALM throws an exception in get_arp_table during:
age = float(neighbor.get('age'))

Fixing this bug.

@coveralls
Copy link

coveralls commented Aug 18, 2020

Coverage Status

Coverage remained the same at 0.0% when pulling 9208837 on praniq:eos_get_arp_table_static into 96269d6 on napalm-automation:develop.

@@ -966,7 +966,7 @@ def get_arp_table(self, vrf=""):
interface = str(neighbor.get("interface"))
mac_raw = neighbor.get("hwAddress")
ip = str(neighbor.get("address"))
age = float(neighbor.get("age"))
age = float(neighbor.get("age", 0.0))
Copy link
Member

Choose a reason for hiding this comment

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

Should we use -1.0 to mark static entries? I notice nxos uses this approach, but IOS doesn't. Perhaps worth standardizing on?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would vote for -1.0 for all OSes, as looks like "-1" is a standard value for N/A in Napalm.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed. @praniq - please feel free to update this branch, then I can merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated EOS, IOS and IOS XR. Please have a look. Thanks

@mirceaulinic mirceaulinic added this to the 3.2.0 milestone Aug 19, 2020
praniq and others added 2 commits August 20, 2020 18:06
…device didn't return any value. Usually for static ARP entries or local IPs. NX-OS already does this
Copy link
Member

@mirceaulinic mirceaulinic left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @praniq

@mirceaulinic mirceaulinic merged commit 910143f into napalm-automation:develop Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants