napalm-automation / napalm-base

Apache License 2.0
32 stars 48 forks source link

update napalm ping response keys #240

Open jedelman8 opened 7 years ago

jedelman8 commented 7 years ago

Reference: https://github.com/napalm-automation/napalm-ansible/pull/53

one thing I'm thinking about within the ansible module and @dbarrosop sugessted maybe we just update the ping method within NAPALM so we can keep the ansible module a wrapper.

Normalizing the key used regardless if success or error.

Today it's this:

# when echo request succeeds
  "{"success": {"packet_loss": 0, "probes_sent": 2,
           "results": [{"ip_address": "10.0.0.5:", "rtt": 1.71},
            {"ip_address": "10.0.0.5:", "rtt": 0.733}],
            "rtt_avg": 1.225, "rtt_max": 1.718, "rtt_min": 0.733,
            "rtt_stddev": 0.493}}
   # when echo request fails
   {"error": "connect: Network is unreachable\n"}}

I'd rather see a status key that has success or error as values. Then a msg key for the error message and results for the dict of results. What are your thoughts?

mirceaulinic commented 7 years ago

I would just say to wait till OC will provide the models for action commands and provide a proper implementation. These methods have been designed improperly from the beginning (the same, I have it) and I think we'd only continue on the same line doing so.

jedelman8 commented 7 years ago

Fair enough if that's the consensus...module works and my use-case has been accomplished...so for now, it'll remain a custom module!

dbarrosop commented 7 years ago

@mirceaulinic are there any OC models for actions that you know of? I haven't seen them (haven't looked for them either)

mirceaulinic commented 7 years ago

Currently there aren't any models available yet, but there will be something in the public repo soon-ish :-)

dbarrosop commented 7 years ago

Something tells me the model is going to be even messier than the platforms model...

jemcek commented 7 years ago

Are there any news regarding this issue?

Currently I get completely different answers from the same unreachable destination using Junos and IOS what makes further processing more complicated. Also is the "probes_sent":0 and "packet_loss": -2 on IOS expected result in case 2 echo request were sent and no reply was received?

Using napalm_ping module, count:2, destination: 4.4.4.4

Result on IOS: ... "item": "4.4.4.4", "results": { "success": { "packet_loss": -2, "probes_sent": 0, "results": [], "rtt_avg": 0.0, "rtt_max": 0.0, "rtt_min": 0.0, "rtt_stddev": 0.0 } }

Resutl on Junos: ... "item": "4.4.4.4", "results": { "error": "Packet loss 100" }

dbarrosop commented 7 years ago

@jemcek would you mind opening an issue on napalm-ios. Looks like the output returned by IOS is not compliant with the model we specified so we need to fix that. Thanks!

@mirceaulinic do you have any news about the "action" model?

jemcek commented 7 years ago

done.

mirceaulinic commented 7 years ago

No, I don't have any news yet. I guess is going to take longer than I thought...