napalm-automation / napalm-ios

Apache License 2.0
31 stars 40 forks source link

MAC address formatting not matching convention #106

Closed ebeahan closed 7 years ago

ebeahan commented 7 years ago

Description of Issue/Question

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

Setup

napalm-ios version

napalm-ios==0.5.1

IOS version

Cisco IOS Software, Catalyst 4500 L3 Switch Software (cat4500e-IPBASEK9-M), Version 15.1(2)SG7, RELEASE SOFTWARE (fc1)

Steps to Reproduce the Issue

Per discussions on Slack, napalm drivers should normalize MAC addresses to the 11:AA:22:BB:33:CC convention. Currently napalm-ios is returning MAC addresses in the 11aa.22bb.33cc format.

A helper function called mac already exists in napalm-base helpers to handle the formatting.

Example:

Current formatting for get_arp_table method:

    {   'age': 0.0,
        'interface': 'Vlan3055',
        'ip': '10.226.65.242',
        'mac': '8843.e17d.197f'}

Expected formatting:

    {   'age': 0.0,
        'interface': 'Vlan3055',
        'ip': '10.226.65.242',
        'mac': '88:43:E1:7D:19:7F'}
ebeahan commented 7 years ago

Am working on a PR to address.

ktbyers commented 7 years ago

Okay, if the format is not matching the expected format this should fail the unit tests...so it also means there is a NAPALM unit test issue that needs corrected.

ebeahan commented 7 years ago

Would that mean adjusting the existing model here with a more stringent check? Still getting familiar with the napalm testing structure.

https://github.com/napalm-automation/napalm-base/blob/develop/napalm_base/test/models.py#L172

mirceaulinic commented 7 years ago

As per doc of the mac helper, it should transform this anyway: https://github.com/napalm-automation/napalm-base/blob/develop/napalm_base/helpers.py#L205-L211

Otherwise, we need to address it.

ktbyers commented 7 years ago

It is probably an error in the formatting of the expected_result.json files in:

https://github.com/ktbyers/napalm-ios/tree/devel/test/unit/mocked_data/test_get_mac_address_table

We will just need to update these expected_result.json files to be the proper format.

ebeahan commented 7 years ago

Fix merged in https://github.com/napalm-automation/napalm-ios/pull/110.