NTP stats getter to replace NTP peers getter

[mirceaulinic]

The actual method get_ntp_peers() returns a dictionary with the statistics of the configured NTP peers. It would feel more natural to be called get_ntp_stats(). For the reasons presented below, we need to implement a different method get_ntp_peers() to retrieve the NTP config.

Unfortunately the current implementation of get_ntp_peers() cannot be used to determine the NTP peers configured on the device (at least on JunOS which returns the raw output of the ntpq Unix daemon), where the columns have a max width of 15 characters:

-2400:cb00:22:10 162.158.5.4      5 u  886 1024  377  139.385  -14.783   4.655

2400:cb00:22:10 obviously is not a valid IPv6 address so, right now, get_ntp_peers() cannot be completely reliable in terms of remote NTP address.

Actions needed:

• since the addresses might start with the same prefix, might have key conflicts in the main dictionary (on the device can configure NTP peers 2400:cb00:22:10::1 and 2400:cb00:22:10::2 or whatever, the device will display two entries, one for each, but because of column width limit, will shrink the remote details column to 2400:cb00:22:10. Thus, in the current implementation, will appear only one key, the value having the sync stats of the latest entry). Solution: transform the dict into list.
• implement get_ntp_peers() to get the config from the device and return a list of NTP peers

[dbarrosop]

The problem with this PR is that we are going to break the API and that might be painful. Do you think it's strictly necessary?

If yes, we should coordinate so we fix or disable the method on all the drivers. We have the following drivers supporting that method:

1. EOS (I can take care of this one)
2. JunOS
3. IOS-XR
4. NXOS
5. IOS

Which ones can you fix?

[mirceaulinic]

What concerns you the most, the name change or the structure?
The name is not a must (although the naming will not reflect completely the functionality) and can remain unchanged. But the return structure is an important improvement. Right now, as I tried to describe previously, on JunOS devices fails.

I can change it for EOS, JunOS, IOS-XR and NXOS. As usually, unfortunately I don't have an IOS to test.

[dbarrosop]

The only concern is that changing the API and tests is going to make automated tests fail while this is being updated but if you can take care of most of the drivers we can proceed. That was actually my only concern.

IOS should be easy to fix, it's probably similar to EOS and the benefit of having mocked data is that we can fix it even without access to a real device :P If you or @ktbyers don't have time to fix IOS I will try to find some time to fix it (traveling right now and for two more weeks)

So feel free to proceed, I guess you understand I will be approving this PR once the drivers have been updated as well. Don't mind the errors when you send the PRs, I will re-trigger the tests myself when this one is merged.

[mirceaulinic]

On most of them will be easy to update -- JunOS is already done: napalm-automation/napalm-junos#10. I will let you know when I finish with all of them.

[mirceaulinic]

@dbarrosop Except IOS, all are prepared.

[dbarrosop]

Ok, I will start merging this. Then I will fix IOS and update the documentation.

[mirceaulinic]

OK, I will rebase the others and make sure everything is fine.

[dbarrosop]

Ok, this is complete. We refactored everything. I also published a new version of each driver with only this fix.

[mirceaulinic]

Great! Thanks! :)

[ktbyers]

@dbarrosop @mirceaulinic I haven't been following this. If there is an work still outstanding for me, just assign it to me so I keep track of it. Thanks.

[mirceaulinic]

@ktbyers: @dbarrosop changed the old get_ntp_peers() to get_ntp_stats() on IOS already. Now, could you please add the method get_ntp_peers() to return the list of NTP peers as configured on the device? You can have a look in #13 for further details.