napalm-automation / napalm

Network Automation and Programmability Abstraction Layer with Multivendor support
Apache License 2.0
2.24k stars 552 forks source link

Review IOSDriver documentation #171

Closed dbarrosop closed 8 years ago

dbarrosop commented 8 years ago

Could you take a look and make sure the caveats are properly documented and that all optional_args are documented? At least the ones you want to expose : )

dbarrosop commented 8 years ago

@ktbyers I can't assign tasks to you because you are not part of the organization. Do you want me to add you as a contributor?

ktbyers commented 8 years ago

@dbarrosop This is fine. I see the issue.

ktbyers commented 8 years ago

This should be done.

dbarrosop commented 8 years ago

I don't think this is done, we keep having users having issues with the same errors all the time, would be nice to have all those issues explained in the caveats section of IOS so we can point them there instead of having to explain those issues over and over. Before closing this I would like to see how to deal with the common scp problems: how to use the dest_file_system and how to do some basic troubleshooting (trying to upload the file and apply the config manually).

ktbyers commented 8 years ago

Yes, agreed.

Step one is they need to test it with straight Python and not Ansible. Ansible is too problematic when initially trying to get things working. They also need to start with a get_* method and not the file operations.

From memory, recent issues people have reported are:

  1. Not having archive configured.
  2. Using TACACS+ AAA and not allowed to SCP.
  3. Invalid configuration file.
  4. Not using connection: local

We also need to improve the error message handling so that Ansible gets passed back better error messages from underlying driver.

Agreed need to document how to handle alternate dest_file_system.

dbarrosop commented 8 years ago

As the dest_file_system seems to be the main cause of concern. How hard would it be to autodiscover it? Something like:

  1. If the user doesn't specify dest_file_system the open() executes dir and grabs the filesystem from the output.
  2. If the user specifies dest_file_system honor it and do nothing.

Thoughts?

ktbyers commented 8 years ago

Yes, good idea. We should just auto-discover it.

I will write this into the Netmiko SCP handler (since it is a more general issue than NAPALM). I probably can do this today or tomorrow.

Agreed if user specifies 'dest_file_system', then that is used (otherwise autodiscover).

We should also probably add a check whether archive is configured and generate an error if it is not (only for 'replace' operations).

ktbyers commented 8 years ago

@dbarrosop Here is a proposed fix for dest_file_system autodetect:

https://github.com/ktbyers/napalm/commit/8e0fcd72d35b9d86a26dcc2183a76f49a8701ec8 https://github.com/ktbyers/napalm/commit/db88f329d7f39e4325f69dad81a9f6becae29556

I haven't pushed at as a PR yet as I want to test it on IOS-XE. Do you have an IOS-XE device we can test it on?

I tested it on an IOS device.

Note, I still submitted the change to my napalm repo instead of napalm-ios as I was running into too many issues with napalm-ansible.

dbarrosop commented 8 years ago

Checking the commits, what you are doing is trying a few predefined fs and get the first one to succeeds, aren't you? Wouldn't be easier to just dir and parse the output to extract which one is the default? Mainly to avoid having to do a new release just to support autodiscovering a new device that happens to have bootflash0:...

I have a csr1000v I use for testing napalm-ios, would that work? In that case just let me know exactly what you want me to do and I will do it : )

ktbyers commented 8 years ago

Yes, dir is fine. It is probably better than what I did. I looked at 'dir ?', but didn't see that just 'dir' put the default file system at top. I can convert it to that.

I did put a similar fix into Netmiko, but it was going to break napalm (i.e. napalm users using the new napalm, but an older Netmiko would break).

Also the napalm ios.py code had dependencies on self.dest_file_system that were possibly outside the cases where Netmiko would have dynamically determined the file system.

Will resubmit using just 'dir'.

ktbyers commented 8 years ago

@dbarrosop Here is a new proposed fix for this.

https://github.com/ktbyers/napalm-ios/commits/master

I ended up putting the detection code in Netmiko (so _autodetect_fs is now a Netmiko method). If napalm ios.py calls this and it doesn't exist, then an AttributeError with an appropriate error message will be generated (basically need to upgrade Netmiko or pass in dest_file_system in optional_args).

I am going to release a new version of Netmiko (0.4.3) to pypi and increase the Napalm ios requiresments to be Netmiko >= 0.4.3. There are quite a few other reasons I want to get onto this Netmiko release anyways.

I will submit PR once I get that done.

I have tested it on Cisco IOS. Haven't tested on IOS-XE.

dbarrosop commented 8 years ago

That's great! Don't worry, I will test IOS-XE.

Thanks!

dbarrosop commented 8 years ago

Seems to work fine with IOS-XE:

>>> from netmiko import ConnectHandler
>>>
>>> conn = {
...     'device_type': 'cisco_ios',
...     'ip':   '127.0.0.1',
...     'username': 'vagrant',
...     'password': 'vagrant',
...     'port' : 12204,
... }
>>>
>>> net_connect = ConnectHandler(**conn)
>>> print net_connect.send_command('show ver')
Cisco IOS XE Software, Version 03.13.04.S - Extended Support Release
Cisco IOS Software, CSR1000V Software (X86_64_LINUX_IOSD-UNIVERSALK9-M), Version 15.4(3)S4, RELEASE SOFTWARE (fc3)
Technical Support: http://www.cisco.com/techsupport
Copyright (c) 1986-2015 by Cisco Systems, Inc.
Compiled Mon 05-Oct-15 08:35 by mcpre

Cisco IOS-XE software, Copyright (c) 2005-2015 by cisco Systems, Inc.
All rights reserved.  Certain components of Cisco IOS-XE software are
licensed under the GNU General Public License ("GPL") Version 2.0.  The
software code licensed under GPL Version 2.0 is free software that comes
with ABSOLUTELY NO WARRANTY.  You can redistribute and/or modify such
GPL code under the terms of GPL Version 2.0.  For more details, see the
documentation or "License Notice" file accompanying the IOS-XE software,
or the applicable URL provided on the flyer accompanying the IOS-XE
software.

ROM: IOS-XE ROMMON

vagrant_box uptime is 2 minutes
Uptime for this control processor is 3 minutes
System returned to ROM by reload
System image file is "bootflash:packages.conf"
Last reload reason: <NULL>

This product contains cryptographic features and is subject to United
States and local country laws governing import, export, transfer and
use. Delivery of Cisco cryptographic products does not imply
third-party authority to import, export, distribute or use encryption.
Importers, exporters, distributors and users are responsible for
compliance with U.S. and local country laws. By using this product you
agree to comply with applicable laws and regulations. If you are unable
to comply with U.S. and local laws, return this product immediately.

A summary of U.S. laws governing Cisco cryptographic products may be found at:
http://www.cisco.com/wwl/export/crypto/tool/stqrg.html

If you require further assistance please contact us by sending email to
export@cisco.com.

License Level: ax
License Type: Default. No valid license found.
Next reload license Level: ax

cisco CSR1000V (VXE) processor (revision VXE) with 2160071K/6147K bytes of memory.
Processor board ID 9SWF3FSODOU
3 Gigabit Ethernet interfaces
32768K bytes of non-volatile configuration memory.
3988440K bytes of physical memory.
7774207K bytes of virtual hard disk at bootflash:.

Configuration register is 0x2102

>>> net_connect._autodetect_fs()
u'bootflash:'
>>> print net_connect.send_command('dir')
Directory of bootflash:/

   11  drwx            16384  Jan 11 2016 14:24:56 +00:00  lost+found
194689  drwx             4096  Jan 11 2016 14:25:52 +00:00  .super.iso.dir
   12  -rw-               34  Apr 11 2016 09:48:46 +00:00  .CsrLxc_LastInstall
   13  -rw-               72  Jan 11 2016 14:30:36 +00:00  virtual-instance.conf
113569  drwx             4096  Jan 11 2016 14:27:21 +00:00  .ssh
   15  -rw-        171622400  Jan 11 2016 14:25:52 +00:00  csrmgmt.1_4_1.20150814_005127.ova
892324  -rw-        244665088  Jan 11 2016 14:26:19 +00:00  csr1000v-mono-universalk9.03.13.04.S.154-3.S4-ext.SPA.pkg
892322  -rw-             4892  Jan 11 2016 14:26:18 +00:00  csr1000v-packages-universalk9.03.13.04.S.154-3.S4-ext.conf
892323  -rw-             5682  Jan 11 2016 14:26:19 +00:00  packages.conf
502945  drwx             4096  Jan 11 2016 14:27:21 +00:00  core
48673  drwx             4096  Jan 11 2016 14:27:22 +00:00  .prst_sync
243361  drwx             4096  Jan 11 2016 14:27:25 +00:00  .rollback_timer
   16  -rw-                0  Jan 11 2016 14:27:28 +00:00  tracelogs.865
162241  drwx             8192  Apr 11 2016 09:49:02 +00:00  tracelogs
584065  drwx             4096  Jan 11 2016 14:27:35 +00:00  .installer
   17  -rw-                0  Apr 11 2016 09:49:07 +00:00  cvac.log
   18  -rw-               30  Apr 11 2016 09:49:10 +00:00  throughput_monitor_params
   19  -rw-             1520  Apr 11 2016 09:49:21 +00:00  csrlxc-cfg.log
940993  drwx             4096  Jan 11 2016 14:30:46 +00:00  onep

7835619328 bytes total (6603956224 bytes free)
ktbyers commented 8 years ago

@dbarrosop Okay, thanks. Only gating item now is I need to release new version of Netmiko to pypi probably today or tomorrow. I also need to test the 'write mem' on commit change.

dbarrosop commented 8 years ago

I think we can close this for now. With the fs_autodiscover I am pretty sure we are covering most of the problems people have when starting with napalm-ios.

Thanks!