search

napalm-automation / napalm-junos

Apache License 2.0
22 stars 42 forks source link

Add ssh_keyfile as an optional argument. #145

Closed stiltzkin10 closed 7 years ago

stiltzkin10 commented 7 years ago

Also change optional_args to default to an empty dict.

This is for issue/FR: #143

ktbyers commented 7 years ago

I added a couple of comments about fixing the optional_args=None change. Also unit test failures need fixed...but I think that will fixed by the optional_args=None change. Also leave the password argument in __init__ as being required. I think that change probably needs to wait until we can make it uniform across all of the drivers.

Did you test validate that this change works using SSH keys.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.06%) to 83.898% when pulling 9a2ff79110a78e805b21d11346525162dad5e4a9 on stiltzkin10:feature/junos_ssh_key into ebc02218348c21d8e8beaf46c3f14beff1d556cf on napalm-automation:develop.

stiltzkin10 commented 7 years ago

Got it. Reverted back to optional_args=None. Thanks!

I have tested this and it works fine on the JUNOS devices I have access to.

To my knowledge the API does not support fall back to username/password.

ktbyers commented 7 years ago

Okay, I added one more comment about self.port missing at one point. I also think we should make the optional argument be key_file for consistency with the other drivers.

stiltzkin10 commented 7 years ago

Agree. Updated.

ktbyers commented 7 years ago

@stiltzkin10 This looks good to me.

Let me get @dbarrosop or @mirceaulinic to sanity check since they use Juniper driver more than I do.

stiltzkin10 commented 7 years ago

Yep, no worries. Thanks!

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.06%) to 83.898% when pulling 7c8d9670774d528dd50283f03968b767a7696c31 on stiltzkin10:feature/junos_ssh_key into ebc02218348c21d8e8beaf46c3f14beff1d556cf on napalm-automation:develop.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.06%) to 83.898% when pulling 7c8d9670774d528dd50283f03968b767a7696c31 on stiltzkin10:feature/junos_ssh_key into ebc02218348c21d8e8beaf46c3f14beff1d556cf on napalm-automation:develop.

dbarrosop commented 7 years ago

I don't use juniper devices anymore but I could confirm with TestJunOSDriver.py::TestConfigJunOSDriver that this PR works fine. If you have the key in the ssh-agent works as usual and if you don't you can pass the key_file arg. and work as promised.

Thanks! :)

dbarrosop commented 7 years ago

Oh, just one thing. Would you mind updating the optional_args documentation in the napalm docs, please? Once that PR is sent link it here and I will merge both. Thanks again!

stiltzkin10 commented 7 years ago

Sure, I created napalm-automation/napalm#368. I actually created sub bullets since vyos/junos are slightly different.

stiltzkin10 commented 7 years ago

Awesome, thanks!