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

nxos_l3_interfaces: Exclude mgmt0 *Fixes testbed breakage* #61788

Open
wants to merge 5 commits into
base: devel
from

Conversation

@chrisvanheuveln
Copy link
Contributor

commented Sep 4, 2019

SUMMARY
  • This change is needed for 2.9

  • This fix excludes mgmt0 from the list of manageable interfaces. See new method remove_rsvd_interfaces.

    • The current code includes an overridden integration test that results in removing the ip address from mgmt0, which breaks the test along with subsequent tests.
    • Users should not modify mgmt0 settings with this module; if necessary they can use the nxos_config module.
  • I also included a simple unit test that verifies basic functionality and checks for mgmt0 usage. This test should be expanded upon with more detailed tests at some point.

  • Note: I think mgmt0 should be excluded from all of the interface modules. I'll add my checks to those modules if necessary but I wanted to get this initial fix out now since it's breaking our regression testbeds.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nxos nxos_l3_interfaces

ADDITIONAL INFORMATION

Tested with unit test and N9K hardware.

edit:
Note that my interface list cleanup occurs in the module instead of the facts method. This seemed to make more sense to me and Mike. But it does cause another side effect when asserting against ansible_facts.network_resources.l3_interfaces since mgmt0 will still appear there.

nxos_l3_interfaces: Exclude mgmt0 *Fixes testbed breakage*
- This fix excludes `mgmt0` from the list of manageable interfaces. See new method `sanitize_interface_facts`.
- Users should not modify mgmt0 settings with this module; if necessary they can use the nxos_config module.
- The current code includes an `overridden` integration test that results in removing the ip address from `mgmt0`, which breaks the test along with subsequent tests.
- I also included a simple unit test that verifies basic functionality and checks for `mgmt0` usage. This test should be expanded upon with more detailed tests at some point.
@ansibot

This comment has been minimized.

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/module_utils/network/nxos/config/l3_interfaces/l3_interfaces.py:20:161: E501: line too long (183 > 160 characters)

click here for bot help

@chrisvanheuveln

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

affects_2.9

@ansibot ansibot added core_review and removed needs_revision labels Sep 4, 2019

@mikewiebe

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2019

shipit

@ansibot ansibot added shipit and removed core_review labels Sep 4, 2019

@ansibot ansibot removed the shipit label Sep 6, 2019

@ansibot ansibot added the core_review label Sep 6, 2019

@chrisvanheuveln

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

needed in 2.9

@@ -92,6 +97,8 @@ def set_config(self, existing_l3_interfaces_facts):
if config:
for w in config:
w.update({'name': normalize_interface(w['name'])})
if w['name'] == 'mgmt0':

This comment has been minimized.

Copy link
@trishnaguha

trishnaguha Sep 12, 2019

Member

Can you check interface type instead with get_interface_type?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.