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_vpc:Fix multiple idempotency issues, add missing commands #55735

Merged
merged 3 commits into from
May 10, 2019

Conversation

chrisvanheuveln
Copy link
Contributor

SUMMARY

Several attributes were causing idempotency problems on various platforms:

  • auto_recovery

    • This command can be disabled on certain platforms and will nvgen as no auto-recovery
    • When enabled it has an additional optional-keyword for changing the reload-delay timer value
      • This was addressed by adding a new attribute auto_recovery_reload_delay to handle setting the timer value
      • This new attribute is mutually exclusive with auto_recovery
  • /show run vpc/show run vpc all/

    • Changed the command that gets state to all so that it could differentiate between auto-recovery and auto-recovery reload-delay
    • This change resulted in also changing some attribute handling withing get_vpc, since some attributes like peer_gw relied on presence of the config to determine state true or false. With all the config is always there so these attrs must specifically check for 'no ' in the string.
  • delay_restore

    • This command has two additional, optional keywords that exist on some platforms and not others.
    • New attrs:
      • delay_restore_interface_vlan
      • delay_restore_orphan_port
  • Modified the sanity test to include the new attributes and to fix the platform issues.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

modules/network/nxos/nxos_vpc.py

ADDITIONAL INFORMATION
  • Validated nxos_vpc sanity test on these platforms, all are now 100% Pass: N35, N3K, N3K-F, N6K, N7K, N9K, N9K-F

  • TBD: Future work is needed to add support for peer_gw_exclude_gw timers. This could be addressed in the same way as the auto_recovery_reload_delay changes included here.

Several attributes were causing idempotency problems on various platforms:

- `auto_recovery`
 - This command can be disabled on certain platforms and will nvgen as `no auto-recovery`
 - When enabled it has an additional optional-keyword for changing the `reload-delay` timer value
  - This was addressed by adding a new attribute `auto_recovery_reload_delay` to handle setting the timer value
  - This new attribute is mutually exclusive with `auto_recovery`

- `/show run vpc/show run vpc all/`
 - Changed the command that gets state to `all` so that it could differentiate between `auto-recovery` and `auto-recovery reload-delay`
 - This change resulted in also changing some attribute handling withing `get_vpc`, since some attributes like `peer_gw` relied on presence of the config to determine state true or false. With `all` the config is always there so these attrs must specifically check for `'no '` in the string.

- `delay_restore`
 - This command has two additional, optional keywords that exist on some platforms and not others.
 - New attrs:
  - `delay_restore_interface_vlan`
  - `delay_restore_orphan_port`

- Modified the `sanity` test to include the new attributes and to fix the platform issues.

- Bugfix Pull Request

`modules/network/nxos/nxos_vpc.py`

- Validated `nxos_vpc` `sanity` test on these platforms, all are now 100% Pass: N35, N3K, N3K-F, N6K, N7K, N9K, N9K-F

- TBD: Future work is needed to add support for `peer_gw_exclude_gw` timers. This could be addressed in the same way as the `auto_recovery_reload_delay` changes included here.
@ansibot
Copy link
Contributor

ansibot commented Apr 24, 2019

@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:network This issue/PR relates to code supported by the Ansible Network Team. test This PR relates to tests. 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 Apr 24, 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 Apr 25, 2019
@chrisvanheuveln
Copy link
Contributor Author

Please cherry-pick to stable2.8

@trishnaguha trishnaguha self-assigned this May 2, 2019
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_triage Needs a first human triage before being processed. labels May 2, 2019
Copy link
Member

@trishnaguha trishnaguha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR

@trishnaguha trishnaguha merged commit 1554bef into ansible:devel May 10, 2019
@trishnaguha
Copy link
Member

@chrisvanheuveln This cannot be cherry-picked to 2.8 as it introduced new arguments.

ndclt pushed a commit to ndclt/ansible that referenced this pull request Jun 13, 2019
…le#55735)

* nxos_vpc:Fix idempotency issues with multiple attributes

Several attributes were causing idempotency problems on various platforms:

- `auto_recovery`
 - This command can be disabled on certain platforms and will nvgen as `no auto-recovery`
 - When enabled it has an additional optional-keyword for changing the `reload-delay` timer value
  - This was addressed by adding a new attribute `auto_recovery_reload_delay` to handle setting the timer value
  - This new attribute is mutually exclusive with `auto_recovery`

- `/show run vpc/show run vpc all/`
 - Changed the command that gets state to `all` so that it could differentiate between `auto-recovery` and `auto-recovery reload-delay`
 - This change resulted in also changing some attribute handling withing `get_vpc`, since some attributes like `peer_gw` relied on presence of the config to determine state true or false. With `all` the config is always there so these attrs must specifically check for `'no '` in the string.

- `delay_restore`
 - This command has two additional, optional keywords that exist on some platforms and not others.
 - New attrs:
  - `delay_restore_interface_vlan`
  - `delay_restore_orphan_port`

- Modified the `sanity` test to include the new attributes and to fix the platform issues.

- Bugfix Pull Request

`modules/network/nxos/nxos_vpc.py`

- Validated `nxos_vpc` `sanity` test on these platforms, all are now 100% Pass: N35, N3K, N3K-F, N6K, N7K, N9K, N9K-F

- TBD: Future work is needed to add support for `peer_gw_exclude_gw` timers. This could be addressed in the same way as the `auto_recovery_reload_delay` changes included here.

* lint fix

* Add 'version_added' tags for new options
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
@chrisvanheuveln chrisvanheuveln deleted the devel-nxos_vpc 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 stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. 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.

None yet

3 participants