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_vlan: fix broken purge behavior (issue #57101) #57229

Merged
merged 5 commits into from
Jun 4, 2019

Conversation

chrisvanheuveln
Copy link
Contributor

@chrisvanheuveln chrisvanheuveln commented May 31, 2019

SUMMARY

Fixes #57101

Symptoms/Analysis

Problem 1:

  • nxos_vlan purge: true would fail when purge was trying to delete all unspecified vlans, including vlan 1.
  • nxos devices do not allow removing vlan 1 and raise a cli exception error

Problem 2:

  • Previous fix fix nxos_vlan mode idempotence bug #55144 caused a side effect when purge was used: vlan changes specified by aggregate were ignored; e.g.
    • vlan 4 is not present; playbook specifies aggregate: { vlan: 4 }, purge: true
    • results in proper purging but vlan 4 is not created

Solution

  • ignore vlan 1 when purging
  • remove the not purge check from state present logic

Added additional unit tests and integration tests.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nxos_vlan

ADDITIONAL INFORMATION

Tested against all regression platforms: N3K,N6K,N7K,N9K,N3K-F,N9K-F

Symptoms/Analysis:
- `nxos_vlan` `purge: true` would fail when `purge` was trying to delete all unspecified vlans, including vlan 1.
- `nxos` devices do not allow removing vlan 1 and raise a cli exception error
- Previous fix ansible#55144 caused a side effect when `purge` was used: vlan changes specified by `aggregate` were ignored; e.g.
 - vlan 4 is not present; playbook specifies `aggregate: { vlan: 4 }, purge: true`
 - results in proper purging but vlan 4 is not created

Solutions:
- ignore vlan 1 when purging
- remove the `not purge` check from state present logic

Added additional unit tests and integration tests.
Tested against all regression platforms.
@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 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. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. networking Network category nxos Cisco NXOS community 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. test This PR relates to tests. labels May 31, 2019
@ansibot
Copy link
Contributor

ansibot commented May 31, 2019

The test ansible-test sanity --test pylint [explain] failed with 16 errors:

test/units/modules/network/nxos/test_nxos_vlan.py:86:12: bad-whitespace No space allowed after bracket             { 'name': '_5_', 'vlan_id': 5 },             ^
test/units/modules/network/nxos/test_nxos_vlan.py:86:42: bad-whitespace No space allowed before bracket             { 'name': '_5_', 'vlan_id': 5 },                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:87:12: bad-whitespace No space allowed after bracket             { 'name': '_6_', 'vlan_id': 6 }             ^
test/units/modules/network/nxos/test_nxos_vlan.py:87:42: bad-whitespace No space allowed before bracket             { 'name': '_6_', 'vlan_id': 6 }                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:101:12: bad-whitespace No space allowed after bracket             { 'name': '_5_', 'vlan_id': 5 },             ^
test/units/modules/network/nxos/test_nxos_vlan.py:101:42: bad-whitespace No space allowed before bracket             { 'name': '_5_', 'vlan_id': 5 },                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:102:12: bad-whitespace No space allowed after bracket             { 'name': '_4_', 'vlan_id': 4 }             ^
test/units/modules/network/nxos/test_nxos_vlan.py:102:42: bad-whitespace No space allowed before bracket             { 'name': '_4_', 'vlan_id': 4 }                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:109:12: bad-whitespace No space allowed after bracket             { 'name': '_5_', 'vlan_id': 5 },             ^
test/units/modules/network/nxos/test_nxos_vlan.py:109:42: bad-whitespace No space allowed before bracket             { 'name': '_5_', 'vlan_id': 5 },                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:110:12: bad-whitespace No space allowed after bracket             { 'name': '_6_', 'vlan_id': 6 }             ^
test/units/modules/network/nxos/test_nxos_vlan.py:110:42: bad-whitespace No space allowed before bracket             { 'name': '_6_', 'vlan_id': 6 }                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:124:12: bad-whitespace No space allowed after bracket             { 'name': '_5_', 'vlan_id': 5 },             ^
test/units/modules/network/nxos/test_nxos_vlan.py:124:42: bad-whitespace No space allowed before bracket             { 'name': '_5_', 'vlan_id': 5 },                                           ^
test/units/modules/network/nxos/test_nxos_vlan.py:125:12: bad-whitespace No space allowed after bracket             { 'name': '_4_', 'vlan_id': 4 }             ^
test/units/modules/network/nxos/test_nxos_vlan.py:125:42: bad-whitespace No space allowed before bracket             { 'name': '_4_', 'vlan_id': 4 }                                           ^

The test ansible-test sanity --test pep8 [explain] failed with 26 errors:

test/units/modules/network/nxos/test_nxos_vlan.py:86:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:86:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:87:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:87:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:88:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:95:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:98:5: E303 too many blank lines (2)
test/units/modules/network/nxos/test_nxos_vlan.py:101:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:101:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:102:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:102:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:103:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:109:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:109:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:110:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:110:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:111:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:119:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:124:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:124:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:125:14: E201 whitespace after '{'
test/units/modules/network/nxos/test_nxos_vlan.py:125:42: E202 whitespace before '}'
test/units/modules/network/nxos/test_nxos_vlan.py:126:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:140:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:148:13: E123 closing bracket does not match indentation of opening bracket's line
test/units/modules/network/nxos/test_nxos_vlan.py:151:5: E303 too many blank lines (2)

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels May 31, 2019
@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label May 31, 2019
@trishnaguha trishnaguha self-assigned this Jun 3, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 3, 2019
@trishnaguha
Copy link
Member

Thanks for the fix!

@trishnaguha trishnaguha merged commit 6bb13bb into ansible:devel Jun 4, 2019
@navisidhu
Copy link

@chrisvanheuveln & @trishnaguha : Thanks 👍

trishnaguha pushed a commit to trishnaguha/ansible that referenced this pull request Jun 6, 2019
…7229)

* nxos_vlan: fix broken purge behavior (issue ansible#57101)

Symptoms/Analysis:
- `nxos_vlan` `purge: true` would fail when `purge` was trying to delete all unspecified vlans, including vlan 1.
- `nxos` devices do not allow removing vlan 1 and raise a cli exception error
- Previous fix ansible#55144 caused a side effect when `purge` was used: vlan changes specified by `aggregate` were ignored; e.g.
 - vlan 4 is not present; playbook specifies `aggregate: { vlan: 4 }, purge: true`
 - results in proper purging but vlan 4 is not created

Solutions:
- ignore vlan 1 when purging
- remove the `not purge` check from state present logic

Added additional unit tests and integration tests.
Tested against all regression platforms.

* PEP fixes

* Add agg_show_vlan_brief.txt fixture

* Add warning for removing vlan 1

* change method name check

(cherry picked from commit 6bb13bb)
abadger pushed a commit that referenced this pull request Jun 12, 2019
* nxos_vlan: fix broken purge behavior (issue #57101) (#57229)

* nxos_vlan: fix broken purge behavior (issue #57101)

Symptoms/Analysis:
- `nxos_vlan` `purge: true` would fail when `purge` was trying to delete all unspecified vlans, including vlan 1.
- `nxos` devices do not allow removing vlan 1 and raise a cli exception error
- Previous fix #55144 caused a side effect when `purge` was used: vlan changes specified by `aggregate` were ignored; e.g.
 - vlan 4 is not present; playbook specifies `aggregate: { vlan: 4 }, purge: true`
 - results in proper purging but vlan 4 is not created

Solutions:
- ignore vlan 1 when purging
- remove the `not purge` check from state present logic

Added additional unit tests and integration tests.
Tested against all regression platforms.

* PEP fixes

* Add agg_show_vlan_brief.txt fixture

* Add warning for removing vlan 1

* change method name check

(cherry picked from commit 6bb13bb)

* changelog

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
@chrisvanheuveln chrisvanheuveln deleted the nxos_vlan-fix-57101 branch October 30, 2019 12:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 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. module This issue/PR relates to a module. networking Network category nxos Cisco NXOS community 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. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nxos_vlan: purge param not working as documented
5 participants