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

Add nxos_lldp module #34453

Merged
merged 2 commits into from Jan 4, 2018

Conversation

Projects
None yet
5 participants
@ganeshrn
Member

ganeshrn commented Jan 4, 2018

SUMMARY
  • Implementation for nxos_lldp module
  • Integration test for nxos_lldp module
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

nxos_lldp

ANSIBLE VERSION
2.5
ADDITIONAL INFORMATION

Add nxos_lldp module
*  Implemetation for nxos_lldp module
*  Integration test for nxos_lldp module
@ansibot

This comment has been minimized.

Show comment
Hide comment
@ansibot

This comment has been minimized.

Show comment
Hide comment
@ansibot

ansibot Jan 4, 2018

Contributor

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/network/nxos/nxos_lldp.py:0:0: E305 DOCUMENTATION.module: not a valid value for dictionary value @ data['module']. Got 'ios_lldp'

click here for bot help

Contributor

ansibot commented Jan 4, 2018

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/network/nxos/nxos_lldp.py:0:0: E305 DOCUMENTATION.module: not a valid value for dictionary value @ data['module']. Got 'ios_lldp'

click here for bot help

@ganeshrn ganeshrn merged commit ab67539 into ansible:devel Jan 4, 2018

1 check passed

Shippable Run 49108 status is SUCCESS.
Details

@ganeshrn ganeshrn deleted the ganeshrn:nxos_lldp_declarative branch Jan 4, 2018

@mkrizek mkrizek removed the needs_triage label Jan 5, 2018

@mikewiebe

This comment has been minimized.

Show comment
Hide comment
@mikewiebe

mikewiebe Jan 11, 2018

Contributor

@trishnaguha and @ganeshrn Why did we add a module just for enabling feature lldp? We already have nxos_feature to enable nxos features. Does this provide something that nxos_feature does not?

https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/nxos/nxos_feature.py

Contributor

mikewiebe commented Jan 11, 2018

@trishnaguha and @ganeshrn Why did we add a module just for enabling feature lldp? We already have nxos_feature to enable nxos features. Does this provide something that nxos_feature does not?

https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/nxos/nxos_feature.py

@trishnaguha

This comment has been minimized.

Show comment
Hide comment
@trishnaguha

trishnaguha Jan 12, 2018

Member

@mikewiebe net_lldp enables/disables lldp service on network devices. http://docs.ansible.com/ansible/latest/net_lldp_module.html. nxos_lldp is implemented so that net_lldp can be used in platform agnostic way. We plan to implement net*_ modules for all platforms.

Member

trishnaguha commented Jan 12, 2018

@mikewiebe net_lldp enables/disables lldp service on network devices. http://docs.ansible.com/ansible/latest/net_lldp_module.html. nxos_lldp is implemented so that net_lldp can be used in platform agnostic way. We plan to implement net*_ modules for all platforms.

@mikewiebe

This comment has been minimized.

Show comment
Hide comment
@mikewiebe

mikewiebe Jan 12, 2018

Contributor

@trishnaguha Interesting. Thanks for the response. Do you have a writeup on the strategy and plan to implement these net_* modules? Will net_lldp simply make use of nxos_lldp for nxos platforms? If this is the case, I would like to understand why the existing nxos_feature module could not be used instead.

Contributor

mikewiebe commented Jan 12, 2018

@trishnaguha Interesting. Thanks for the response. Do you have a writeup on the strategy and plan to implement these net_* modules? Will net_lldp simply make use of nxos_lldp for nxos platforms? If this is the case, I would like to understand why the existing nxos_feature module could not be used instead.

@trishnaguha

This comment has been minimized.

Show comment
Hide comment
@trishnaguha

trishnaguha Jan 15, 2018

Member

@mikewiebe So net_* modules are simply the plugins which calls the underlying implementation modules for specific platform.
https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/action/net_lldp.py is just a plugin. The module doesn't have any code https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/protocol/net_lldp.py in there. It simply triggers {{ platform }}_lldp module, the platform that you specify in network_os or ansible_network_os.

For example:

- name: del user for eos
  net_user: &eos_task
    name: test1
    state: absent
    provider: "{{ eapi }}"
  network_os: eos

- name: enable lldp for nxos
  net_lldp: &nxos_task
    state: present
  network_os: nxos

Here &eos_task triggers eos_user module since the network_os is eos.
&nxos_tasks triggers nxos_lldp module since the network_os is nxos.

Member

trishnaguha commented Jan 15, 2018

@mikewiebe So net_* modules are simply the plugins which calls the underlying implementation modules for specific platform.
https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/action/net_lldp.py is just a plugin. The module doesn't have any code https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/protocol/net_lldp.py in there. It simply triggers {{ platform }}_lldp module, the platform that you specify in network_os or ansible_network_os.

For example:

- name: del user for eos
  net_user: &eos_task
    name: test1
    state: absent
    provider: "{{ eapi }}"
  network_os: eos

- name: enable lldp for nxos
  net_lldp: &nxos_task
    state: present
  network_os: nxos

Here &eos_task triggers eos_user module since the network_os is eos.
&nxos_tasks triggers nxos_lldp module since the network_os is nxos.

@ansibot ansibot added feature and removed feature_pull_request labels Mar 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment