First commit for SSH support

[GGabriele]

[mirceaulinic]

Very nice, that was really fast! I'm looking forward to give it a try!

I have just one question: is there any particular reason why send_command_timing instead of send_command?

[ktbyers]

I queried in the Cisco Slack Channel about NXOS versions. Here are some answers:

the recommended code for 7Ks is 6.2.(16) IIRC. and i guess it depends 
on the platform too most of our N3Ks and 9Ks are on 
7.X for example (edited)

Above correlates with my environment. 7710s on 7.2(2)D1(2), 
5ks on 7.1(3)N1(2)

[GGabriele]

@mirceaulinic I've seen send_command_timing behaving better with slow connection sometimes. I can definitely revert it to send_command if it doesn't makes sense / seems required.

Same thing for json/xml, I can revert it to xml. Anybody knows if 6.2.(16) IIRC supports both?

[coveralls]

Coverage decreased (-6.2%) to 61.048% when pulling 5cdb5a24bc27031d00e703a82112d786c53b36f0 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-6.2%) to 61.048% when pulling 5cdb5a24bc27031d00e703a82112d786c53b36f0 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-6.2%) to 61.048% when pulling 5cdb5a24bc27031d00e703a82112d786c53b36f0 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[GGabriele]

@ktbyers are you working on this too? I'm asking just to be sure we don't overlap

[ktbyers]

@GGabriele No, I will just try to review what you have done.

[dbarrosop]

Awesome! (edited)

[coveralls]

Coverage decreased (-9.2%) to 57.973% when pulling 0821653bc12fe0efe35881c2c8ef4fb0ad001be6 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-9.2%) to 58.051% when pulling 0821653bc12fe0efe35881c2c8ef4fb0ad001be6 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-9.2%) to 58.051% when pulling 0821653bc12fe0efe35881c2c8ef4fb0ad001be6 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-11.9%) to 55.344% when pulling 096681efb686d5004ef15874a9d578d14bb391e3 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[GGabriele]

Not there yet, but we're getting closer...it's always fun to deal with non-buggy platforms like this :D :sarcasm:

[coveralls]

Coverage decreased (-11.9%) to 55.344% when pulling 3c86b7908e1c8b51508bb940f6d44560d8262771 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-11.9%) to 55.344% when pulling 3c86b7908e1c8b51508bb940f6d44560d8262771 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-12.02%) to 55.203% when pulling 363f0802df45024f82c7c10a072b408a74f4fe40 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[coveralls]

Coverage decreased (-12.02%) to 55.203% when pulling 363f0802df45024f82c7c10a072b408a74f4fe40 on GGabriele:ssh into 7b81e9c20779cfe1c28a3d3f1a05217717f1df12 on napalm-automation:ssh.

[GGabriele]

Tests are passing. Let me know if any of you get a chance to test it :) @dbarrosop @mirceaulinic @jedelman8 @ktbyers

[ktbyers]

@GGabriele Did we decide which NXOS version we want to support. I think we should say we support NXOS version X.X and later.

Otherwise, I think we might have problems as people keep bringing up older and older NXOS versions.

I can do some more digging on which version that should be (if it helps).

[mirceaulinic]

From my discussion with an engineer from Cisco: the | json has been introduced together with the NX-API, starting with the following releases

• 7.2 on Nexus 5k & 7k
• 6.x on Nexus 9k

In other words, there's no device unable to run the NX-API, but able to execute the | json.

[dbarrosop]

If other people is good with this, I am good with this :)

[ktbyers]

@GGabriele It looks like we are requiring the | json. In doing this, are we accomplishing what we want?

In other words, if you have | json...you probably could just enable NX-API. We also don't get NXOS 6.2 support here. That being said, adding the SSH support we can try to expand on text methods to support earlier NXOS versions.

Could you also describe how the config methods on NXOS work? In other words, how are you accomplishing merge, replace, diff, rollback, and commit operations.

Thanks for all your work here...

[GGabriele]

That being said, adding the SSH support we can try to expand on text methods to support earlier NXOS versions.

Are you suggesting to migrate to | xml or more to screen scraping/templating?

Regarding how config methods work, they work just like they already do with the current implementation but using netmiko and actual commands instead of pynxos library:

• merge: send config line by line (not atomic).
• replace: copy a checkpoint-like file to the device and use rollback running file X for committing
• diff: for merge, it compares the commands on the buffer with the running configuration...for replace, it uses show diff rollback-patch file sot_file file X
• rollback: rollback running-config file X on a backup file

[ktbyers]

@GGabriele Probably screen scraping/textfsm templates, but it depends what version of NXOS we want to support. If we want to support NXOS 6.2, then we would need screen-scraping. If we are just going to support NXOS >7.2 or >7.3, then does it really even make sense to have this?

Personally, I don't want to support it in the case where the organization is too disfunction to enable NX-API (when it exists). I.E. there security organization tells them they can't use it.

[GGabriele]

I thought the initial idea was to add SSH here instead of a new nxos-ssh driver by reusing as most code as possible.

anyway, I see your point here and I agree with you, we have 8 getters to migrate and if everybody is okay with that I'm going to try something this weekend (along with the changes you requested)

[ktbyers]

Yes, I think the purpose of having this all consolidated into NAPALM-NXOS is to avoid the proliferation of drivers (i.e. it doesn't make sense to have a different package for each different transport i.e. -api and -ssh).

That probably implies you won't be able to share much of the getter code since in one case it will be screen scraping.

[dbarrosop]

I think we should be doing the | json here. Otherwise this is going to grow insane and unmaintainable. For a more generic ssh screen scraping approach we should work on a generic napalm-ssh|netmiko driver instead and move the logic to napalm-yang profiles.

[ktbyers]

I think it is pointless to do the | json as any NXOS version that supports | json also supports NX-API (so we are not actually increasing the amount of NAPALM support on NXOS in any meaningful way).

On the generic SSH, we need to have more discussions on this.

[dbarrosop]

My understanding was that we were targeting people without the API enabled. I don't think it makes sense to target screen scrapping in this driver.

On the generic SSH, we need to have more discussions on this.

I agree, we should setup a meeting some day and have a discussion on this.

[ktbyers]

So we are just trying to cover people that could be using NXAPI, but are choosing not to?

Yes, I am against doing that.

IMO that is the worse option (adding complexity without adding meaningful in-field device support).

We probably should just table this PR until the longer term discussions happen.

[dbarrosop]

IMO that is the worse option (adding complexity without adding meaningful in-field device support).

Well, I agree with you here but I guess that @GGabriele and @jedelman8 had some reason so I will defer to them in case they have some particular case they were trying to tackle.

[jedelman8]

Yeah, I agree we shouldn't use | json either. Ansible did this and it makes it confusing and it's like "API is required but we use SSH as transport" since | json was more or less introduced when the API was. So main use case was for non-API (older) devices.

[dbarrosop]

Ok, in that case I'd suggest having a discussion around a generic napalm-{ssh|netmiko}, add the minimum necessary code there to merge configuration and rely on napalm-yang for the getters.

[dbarrosop]

Should we revisit this after our discussions around napalm-ng? I think we should consider merging this one.

[ktbyers]

I think we should try to integrate this in as part of napalm-ng. I will try to incorporate this as part of the SSH support there.

Does that sound good?

[mirceaulinic]

Two things:

• As a reminder, the | json has been introduced together with the NXAPI. So do we target to help those lazy people that don't enable the NXAPI? I thought we want compability with the old OS versions that don't have the NXAPI and implicitly they don't have the | json either. While this PR relies on the | json, I'm not sure about the real benefits, despite @GGabriele great work here.

• @GGabriele @jedelman8 do you know what is the napalm-ng thing @dbarrosop and @ktbyers are talking about above? I think we should discuss more about napalm-ng and publish the RFC, discuss and listen to feedback from the community. Currently, I'm afraid that only @dbarrosop, @ktbyers and me are the only persons in the entire world that know about that, its implications etc...

[ktbyers]

I think we need to target primarily SSH support for NX-OS as there are lots of field devices that can neither support NX-API nor | json.

[mirceaulinic]

Yeah, I think so too, we should focus more on the pure SSH support.

[GGabriele]

@GGabriele @jedelman8 do you know what is the napalm-ng thing @dbarrosop and @ktbyers are talking about above? I think we should discuss more about napalm-ng and publish the RFC, discuss and listen to feedback from the community. Currently, I'm afraid that only @dbarrosop, @ktbyers and me are the only persons in the entire world that know about that, its implications etc...

I've lightly followed the discussion around napalm-ng but couldn't dig any deeper yet, but if we want to target primarily SSH support it wouldn't make sense much sense to use this branch and instead go for napalm-ng

[dbarrosop]

but if we want to target primarily SSH support it wouldn't make sense much sense to use this branch and instead go for napalm-ng

Agreed