device.get_lldp_neighbors() does not support spaces in Device ID

Shayneeking commented 7 years ago

Description of Issue/Question

Did you follow the steps from https://github.com/napalm-automation/napalm#faq

[x] Yes
[ ] No

Setup

napalm-ios version

(Paste verbatim output from pip freeze | grep napalm-ios between quotes below)

napalm-ios==0.6.1

IOS version

(Paste verbatim output from show version between quotes below)

Version 12.2(53r)SE

Steps to Reproduce the Issue

device.get_lldp_neighbors() does not support spaces in Device ID device_id, local_int_brief, hold_time, capability, remote_port = lldp_entry.split() will result in except ValueError: with a value of len(lldp_entry.split()) > 5 but this is not expected so it will pass if len(lldp_entry.split()) == 4: move onto local_port = self._expand_interface_name(local_int_brief) which local_int_brief is never defined and result in Traceback (most recent call last): File "hello.py", line 57, in <module> print (local_int_brief) NameError: name 'local_int_brief' is not defined

Example data:

Device ID           Local Intf     Hold-time  Capability      Port ID
Polycom VVX 500     Gi2/0/36       120        T               0004.f263.5b3e

Example out of lldp_entry.split(): ['Polycom', 'VVX', '500', 'Gi2/0/36', '120', 'T', '0004.f263.5b3e']

Error Traceback

(Paste the complete traceback of the exception between quotes below)

Traceback (most recent call last):
  File "hello.py", line 57, in <module>
    print (local_int_brief)
NameError: name 'local_int_brief' is not defined

Shayneeking commented 7 years ago

So i have created a small change looking for regex of two space or more.

ktbyers commented 7 years ago

Splitting on >=2 spaces won't work as that won't work between the "Device ID" field and the "Local Intf" field (that can actually go to 0 spaces).

@Shayneeking Can you test the current develop branch (as there have been fixes here since 0.6.1).

Shayneeking commented 7 years ago

@ktbyers Still running into the issue using https://raw.githubusercontent.com/napalm-automation/napalm-ios/develop/napalm_ios/ios.py My fix should work on anything other than when their is one space between "Device ID" field and the "Local Intf" field

ktbyers commented 7 years ago

Yes, but we know that happens (i.e. I know that the field between Device ID and Local Intf can be one space or zero spaces).

In other words, I have previously had to fix an issue(s) because of Device ID and Local Intf can go down to zero spaces.

Can you post the error that you receive when testing with the develop branch?

Shayneeking commented 7 years ago

Traceback (most recent call last): File "hello.py", line 13, in <module> print (device.get_lldp_neighbors()) File "/var/opt/python/helloworld/testPythonVM/lib64/python3.6/site-packages/napalm_ios/ios.py", line 679, in get_lldp_neighbors local_port = self._expand_interface_name(local_int_brief) UnboundLocalError: local variable 'local_int_brief' referenced before assignment

Is the error i get with no changes on the develop branch.

ktbyers commented 7 years ago

@Shayneeking Can you post the output of show lldp neighbors detail from the device that is failing (just obfuscate any sensitive information)

Shayneeking commented 7 years ago

------------------------------------------------
Chassis id: 10.100.100.100
Port id: 0004.f262.e373
Port Description: 1
System Name: Polycom VVX 500

System Description: 
Polycom;VVX-VVX_500;3111-44500-001,7;SIP/5.0.2.2185/03-Apr-14 03:22;UP/5.2.2.2158/03-Apr-14 03:30;

Time remaining: 115 seconds
System Capabilities: B,T
Enabled Capabilities: T
Management Addresses:
    IP: 10.100.100.100
Auto Negotiation - supported, enabled
Physical media capabilities:
    1000baseT(FD)
    100base-TX(FD)
    100base-TX(HD)
    10base-T(FD)
    10base-T(HD)
Media Attachment Unit type: 30
Vlan ID: - not advertised

MED Information:

    MED Codes:
          (NP) Network Policy, (LI) Location Identification
          (PS) Power Source Entity, (PD) Power Device
          (IN) Inventory

    H/W revision: 3111-44500-001,7
    F/W revision: UP/5.2.2.2158/03-Apr-14 03:30
    S/W revision: SIP/5.0.2.2185/03-Apr-14 03:22
    Serial number: 000000000000
    Manufacturer: Polycom
    Model: VVX-VVX_500
    Capabilities: NP, PD, IN
    Device type: Endpoint Class III
    Network Policy(Voice): VLAN 0, untagged, Layer-2 priority: 5, DSCP: 46
    Network Policy(Voice Signal): VLAN 0, untagged, Layer-2 priority: 5, DSCP: 26
    Network Policy(Video conferencing): VLAN 0, untagged, Layer-2 priority: 5, DSCP: 46
    PD device, Power source: PSE & Local, Power Priority: Unknown, Wattage: 8.0
    Location - not advertised

------------------------------------------------

bgodette commented 7 years ago

FYI Port ID can also contain spaces. Since you're already using some fixed position parsing (long Device ID) and tests (no Capability), why not just extract ranges and strip whitespace?

def _parse_fixed(self, s, *args):
    idx = 0
    for width in args:
        yield s[idx:idx + width].strip()
        idx += width
    yield s[idx:].strip()

device_id, local_int_brief, hold_time, capability, remote_port = self._parse_fixed(lldp_entry, 20, 15, 11, 16)

mirceaulinic commented 6 years ago

Hi @Shayneeking - we are currently in the process of reunification, please check https://napalm-automation.net/reunification/. For the time being, we have moved this issue to https://github.com/napalm-automation/napalm/issues/456 so we can discuss further. Going forward, we'd like to ask you to submit Pull Requests and Issues to the main repository: https://github.com/napalm-automation/napalm

Thanks for understanding!

napalm-automation / napalm-ios