-
Notifications
You must be signed in to change notification settings - Fork 555
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
arista ssh access. #1502
arista ssh access. #1502
Conversation
…. Only get_facts supported at this point.
…l additional get commands now work as well.
I've now updated as per 2ff4e21 the get functions that do not try to parse the This still excludes the following functions on Arista that use them:
I also need to look at the following functions for further testing:
In |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @thomasbridge74! This looks good to me, @ktbyers any opinion?
napalm/eos/eos.py
Outdated
return {"is_alive": True} # always true as eAPI is HTTP-based | ||
|
||
def _run_commands(self, commands, **kwargs): | ||
if self.transport == "ssh": | ||
fn0039_transform = kwargs.pop("fn0039_transform", True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have an optional argument, eos_fn0039_config
, whose value you can access it from self.fn0039_config
- doesn't have to be per-method. Or could you explain what are you trying to do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So obviously the main goal is to be able to convert commands between the old and new syntax.
Digging into how this was done for EAPI connections, I found inside the run_commands
method of the Node
inside the pyeapi_syntax_wrapper.py
file the above code, and basically copied it to maintain the logic.
I assume your recommendation here is drop the line
fn0039_transform = kwargs.pop("fn0039_transform", True)
and replace the following if
statement with
if self.fn0039_config
Happy to do that - it looks like I missed the significance of that setting as I poked around the code.
While I've done the get functions, I still need to do the following:
UPDATE: these are now done! - Oct 19th as well as update the neccessary functions for the configuration support matrix |
…commit_config, rollback, has_pending_commit, compare_config, confirm_commit)
I've completed the commands for configuration management. One caveat: in order to get the SSH version of the session management to work, I've changed the logic in
to
(ie a oneline command instead of two lines). This is because it's a little messy using napalm with two lines considering that effectively enters configuration mode - which would also need to be quit but changes the prompt as well. This may have issues. I'll raise a question with my companies Arista Sales Architect. |
I had confirmation from the Arista account contact who advised that
So I'm happy leaving this modified approach if that's okay? |
I'm not a big fan of this, as this means only people running EOS 4.18.0F or newer will be able to use this. But otoh, I can't see a better workaround, so I'm fine to proceed. I'd like to request however to document this caveat. Thanks @thomasbridge74! |
just checking in - is there anything you're waiting on from me to merge this? Two things with regard to the issue around 4.18.0F or newer
|
Hi @thomasbridge74 - yes, I think we want a documented caveat for this.
True, but I wouldn't assume there aren't people still EOS older than 4.20F (in fact, I'm absolutely sure there are). Here in NAPALM, is not our job to limit this.
No so sure about that: in the past, I've personally used NAPALM with EOS versions earlier than 4.18 which made use of the configue sessions (this behaviour didn't change over the last 6 years at least). |
Hello @thomasbridge74! I have an interest in getting napalm to work with arista over ssh as well. I wanted to try out your extensions. I've noticed that I have a problem with opening connections to devices with optional args, specifically an alternative port. I believe the issue is the transport_class being a string doesn't allow all of the optional args to make it to the netmiko_optional_args dict that is used. I can share the full traceback if necessary and happy to have a chat or take a look my self if necessary. Thanks! |
raise ConnectionException("Unknown transport: {}".format(self.transport)) | ||
self.transport = transport | ||
if transport == "ssh": | ||
self.transport_class = "ssh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transport_class as a string runs into an issue on line 150 and again in line 155 where a string only has self, so no optional args get passed into netmiko_optional_args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've merged the changes to the napalm/eos/eos.py file.
This PR: Will affect these changes. |
ret.append({"output": cmd_output}) | ||
continue | ||
|
||
cmd_pipe = command + " | json" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might have to be | json version 2
. I can't remember exactly which version/reversion of JSON output EAPI uses. Might have to ask Arista for exact clarification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC @thomasbridge74 - I let you investigate this, feel free to come up with a subsequent PR if needed.
@mirceaulinic I've updated the pull request with the update to the driver file from @scetron but not including his (very substantial) suite of tests. I've also made a couple of minor updates to the documentation. With regard to the comment about 4.18.0F versus earlier versions, it would appear it was specifically the config timer that was introduced with 4.18.0F - I've included a comment about that in the documentation updates. I can also be pinged on the NTC Slack if you think that might be a quicker path to resolving the issues here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
Thanks for your work @thomasbridge74! This will be included in release 4.0.0, which I target to release very soon.
@mirceaulinic, with this can we use SSH for EOS like NXOS_SSH , what should be driver name for this |
@AshikKuppili Try setting the transport in optional_args to be "ssh": https://github.com/napalm-automation/napalm/blob/develop/napalm/eos/eos.py#L144 |
Hello, I am trying to use transport SSH for Arista with napalm-ansible, passing optional_args ssh, it looks like working, but I keep receiving an error
example of my task
|
As discussed in #1014 I'm starting work on allowing the EOS module to use ssh as a transport rather than HTTP/HTTPS. This does necessitate some additional if statements within the code base.
So far I've managed to get this working to the extent we can connect to an Arista device using the ssh as the underlying transport. I've updated a number of getter functions (not all yet) as well as the is_alive() function to cover the case where we're using ssh as the transport. This PR is very much a WIP and (in my view) is not ready for committing to the develop branch but given there's still an amount of effort involved (the rest of the getters, ping and traceroutes, not to mention the configuration support options I've not even started to look at), I wanted to submit something for feedback!