Skip to content
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

ios_facts: Add check to skip lldp neighbors if lldp doesn't exist or isn't enabled. #40109

Merged
merged 2 commits into from
May 16, 2018

Conversation

tyler-8
Copy link
Contributor

@tyler-8 tyler-8 commented May 14, 2018

SUMMARY

Fixes #37507

Older IOS devices that don't support LLDP fail when including interfaces as part of ios_facts. This commit adds a check for any IOS errors when running the command show lldp and skips the LLDP neighbor parsing if LLDP isn't available.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ios_facts

ANSIBLE VERSION
ansible 2.6.0 (devel ddf7873d6e) last updated 2018/05/14 16:49:20 (GMT -400)
  config file = /Users/tyler8/projects/pycon18/projfiles/ansible.cfg
  configured module search path = ['/Users/tyler8/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/tyler8/projects/pycon18/ansible/lib/ansible
  executable location = /Users/tyler8/.local/share/virtualenvs/pycon18-CTYMJGfK/bin/ansible
  python version = 3.6.5 (default, Mar 30 2018, 06:41:53) [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.39.2)]
ADDITIONAL INFORMATION
- hosts: csr
  gather_facts: no
  connection: network_cli

  tasks:

  - name: IOS | Get the facts
    ios_facts:
      gather_subset: all
    register: thefacts

  - debug: msg="{{ thefacts }}"

With LLDP enabled with no neighbors:

            "ansible_net_model": "CSR1000V",
            "ansible_net_neighbors": {
                "null": [
                    {
                        "host": null,
                        "port": null
                    }
                ]
            },
            "ansible_net_serialnum": "1234QWERTY",

With no LLDP enabled (should be same for no LLDP support:

            "ansible_net_model": "CSR1000V",
            "ansible_net_serialnum": "1234QWERTY",

I don't have the ability to test on a device that doesn't support LLDP at all - however the check logic should be the same regardless if the error string is present.

@ansibot
Copy link
Contributor

ansibot commented May 14, 2018

@ansibot ansibot added bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. networking Network category support:network This issue/PR relates to code supported by the Ansible Network Team. labels May 14, 2018
@Qalthos Qalthos self-requested a review May 14, 2018 21:03
@Qalthos Qalthos self-assigned this May 14, 2018
@tyler-8
Copy link
Contributor Author

tyler-8 commented May 14, 2018

Closing for now as the issue is higher up in the IOS logic and was actually a regression since ansible/ansible-modules-core#5491

@tyler-8 tyler-8 closed this May 14, 2018
@Qalthos
Copy link
Contributor

Qalthos commented May 14, 2018

Hey, this actually is almost right, give me a minute

@Qalthos Qalthos reopened this May 14, 2018
@ansibot
Copy link
Contributor

ansibot commented May 14, 2018

@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed needs_triage Needs a first human triage before being processed. labels May 14, 2018
@ansibot ansibot removed the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label May 15, 2018
@Qalthos
Copy link
Contributor

Qalthos commented May 15, 2018

Oh no, you rebased and removed my commit. I can add it back on, but you'll need to pull afterwards.

@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label May 15, 2018
@Qalthos Qalthos merged commit 39bed45 into ansible:devel May 16, 2018
Qalthos pushed a commit to Qalthos/ansible that referenced this pull request May 16, 2018
…isn't enabled. (ansible#40109)

* Add check to skip lldp neighbors if lldp doesn't exist or isn't enabled.

* Re-enable check_rc on ios' run_commands

(cherry picked from commit 39bed45)
@Qalthos Qalthos moved this from In Review to Done in zzz NOT USED: Networking Bugs May 16, 2018
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
…isn't enabled. (ansible#40109)

* Add check to skip lldp neighbors if lldp doesn't exist or isn't enabled.

* Re-enable check_rc on ios' run_commands
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
…isn't enabled. (ansible#40109)

* Add check to skip lldp neighbors if lldp doesn't exist or isn't enabled.

* Re-enable check_rc on ios' run_commands
@dagwieers dagwieers added ios Cisco IOS community cisco Cisco technologies labels Feb 27, 2019
@ansible ansible locked and limited conversation to collaborators May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. cisco Cisco technologies core_review In order to be merged, this PR must follow the core review workflow. ios Cisco IOS community module This issue/PR relates to a module. networking Network category support:core This issue/PR relates to code supported by the Ansible Engineering Team. support:network This issue/PR relates to code supported by the Ansible Network Team.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

ios modules failing with network_cli execution being skipped
4 participants