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 breaks using default vrf #57069

Closed
clammers opened this issue May 28, 2019 · 5 comments

Comments

Projects
None yet
4 participants
@clammers
Copy link

commented May 28, 2019

SUMMARY

When using pkl_vrf": "default" command is missing vrf value

ISSUE TYPE
  • Bug Report
COMPONENT NAME

Module: nxos_vpc

ANSIBLE VERSION
ansible 2.7.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/dist-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.15rc1 (default, Nov 12 2018, 14:31:15) [GCC 7.3.0]
CONFIGURATION

asterisk due privacy

CACHE_PLUGIN(/etc/ansible/ansible.cfg) = jsonfile
CACHE_PLUGIN_CONNECTION(/etc/ansible/ansible.cfg) = /**/config/ansible/facts
DEFAULT_HOST_LIST(/etc/ansible/ansible.cfg) = [u'/**/config/ansible/**/hosts.yml']
DISPLAY_SKIPPED_HOSTS(/etc/ansible/ansible.cfg) = True
HOST_KEY_CHECKING(/etc/ansible/ansible.cfg) = False
RETRY_FILES_ENABLED(/etc/ansible/ansible.cfg) = False
OS / ENVIRONMENT
STEPS TO REPRODUCE
  nxos_vpc:
    domain: 10
    pkl_src: 1.1.1.2
    pkl_dest: 1.1.1.1
    pkl_vrf: default
EXPECTED RESULTS
    "commands": [
        "vpc domain 10",
        "peer-keepalive destination 1.1.1.1 source 1.1.1.2 vrf default",
ACTUAL RESULTS
    "commands": [
        "vpc domain 10",
        "peer-keepalive destination 1.1.1.1 source 1.1.1.2",
@clammers

This comment has been minimized.

Copy link
Author

commented May 28, 2019

BUG in

if str(value).lower() == 'default':

@ansibot

This comment has been minimized.

@clammers

This comment has been minimized.

Copy link
Author

commented Jun 3, 2019

@trishnaguha could you reproduce the issue?

@chrisvanheuveln

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2019

@trishnaguha @clammers I drafted a fix for this. I'll post a PR later today.

chrisvanheuveln added a commit to chrisvanheuveln/ansible that referenced this issue Jun 4, 2019

nxos_vpc: pkl_vrf fixes for ansible#57069
Fixes ansible#57069

- Symptom: When playbooks specify `pkl_vrf: default`, the result is that the cli does not set the `vrf` state.

- Analysis:
 - First issue: 'default' is a reserved word when used with the `peer-keepalive` `vrf` keyword. It refers to the default rib.
   - This is confusing in several ways because `peer-keepalive`'s *default* vrf is the `management` vrf.

 - Second issue: When changing only one optional value (`pkl_vrf`) while other optional values are idempotent (`pkl_src`), the result is that the idempotent values are ignored; unfortunately the device cli *replaces* the entire command, in which case the idempotent values are removed.
   - e.g. playbook specifies this:
     ```
     { pkl_dest: 10.1.1.1, pkl_src: 10.2.2.2, pkl_vrf: my_vrf }
     ```

     ```
     peer-keepalive dest 10.1.1.1 src 10.2.2.2             # original

     peer-keepalive dest 10.1.1.1 src 10.2.2.2 vrf my_vrf  # intended result

     peer-keepalive dest 10.1.1.1 vrf my_vrf               # actual result
     ```

 - Third issue: the `pkl` getter was relying on positional data. This broke when the `udp` keyword nvgen'd where `vrf` used to appear (shifting all keywords to the right).

- Tested on regression platforms: `N3K,N6k,N7K,N9K,N3K-F,N9K-F`
@clammers

This comment has been minimized.

Copy link
Author

commented Jun 7, 2019

👍 Thanks for fixing

chrisvanheuveln added a commit to chrisvanheuveln/ansible that referenced this issue Jun 18, 2019

nxos_vpc: pkl_vrf fixes for ansible#57069
Fixes ansible#57069

- Symptom: When playbooks specify `pkl_vrf: default`, the result is that the cli does not set the `vrf` state.

- Analysis:
 - First issue: 'default' is a reserved word when used with the `peer-keepalive` `vrf` keyword. It refers to the default rib.
   - This is confusing in several ways because `peer-keepalive`'s *default* vrf is the `management` vrf.

 - Second issue: When changing only one optional value (`pkl_vrf`) while other optional values are idempotent (`pkl_src`), the result is that the idempotent values are ignored; unfortunately the device cli *replaces* the entire command, in which case the idempotent values are removed.
   - e.g. playbook specifies this:
     ```
     { pkl_dest: 10.1.1.1, pkl_src: 10.2.2.2, pkl_vrf: my_vrf }
     ```

     ```
     peer-keepalive dest 10.1.1.1 src 10.2.2.2             # original

     peer-keepalive dest 10.1.1.1 src 10.2.2.2 vrf my_vrf  # intended result

     peer-keepalive dest 10.1.1.1 vrf my_vrf               # actual result
     ```

 - Third issue: the `pkl` getter was relying on positional data. This broke when the `udp` keyword nvgen'd where `vrf` used to appear (shifting all keywords to the right).

- Tested on regression platforms: `N3K,N6k,N7K,N9K,N3K-F,N9K-F`

StephenSorriaux pushed a commit to StephenSorriaux/ansible that referenced this issue Jun 28, 2019

nxos_vpc: pkl_vrf fixes for ansible#57069 (ansible#57370)
* nxos_vpc: pkl_vrf fixes for ansible#57069

Fixes ansible#57069

- Symptom: When playbooks specify `pkl_vrf: default`, the result is that the cli does not set the `vrf` state.

- Analysis:
 - First issue: 'default' is a reserved word when used with the `peer-keepalive` `vrf` keyword. It refers to the default rib.
   - This is confusing in several ways because `peer-keepalive`'s *default* vrf is the `management` vrf.

 - Second issue: When changing only one optional value (`pkl_vrf`) while other optional values are idempotent (`pkl_src`), the result is that the idempotent values are ignored; unfortunately the device cli *replaces* the entire command, in which case the idempotent values are removed.
   - e.g. playbook specifies this:
     ```
     { pkl_dest: 10.1.1.1, pkl_src: 10.2.2.2, pkl_vrf: my_vrf }
     ```

     ```
     peer-keepalive dest 10.1.1.1 src 10.2.2.2             # original

     peer-keepalive dest 10.1.1.1 src 10.2.2.2 vrf my_vrf  # intended result

     peer-keepalive dest 10.1.1.1 vrf my_vrf               # actual result
     ```

 - Third issue: the `pkl` getter was relying on positional data. This broke when the `udp` keyword nvgen'd where `vrf` used to appear (shifting all keywords to the right).

- Tested on regression platforms: `N3K,N6k,N7K,N9K,N3K-F,N9K-F`

* PEP fixes

* PEP fix 2

* pkl should merge by default, not override

* rmv debugs

* add mike's tests

* fix comments

agowa338 added a commit to agowa338/ansible-1 that referenced this issue Jun 30, 2019

nxos_vpc: pkl_vrf fixes for ansible#57069 (ansible#57370)
* nxos_vpc: pkl_vrf fixes for ansible#57069

Fixes ansible#57069

- Symptom: When playbooks specify `pkl_vrf: default`, the result is that the cli does not set the `vrf` state.

- Analysis:
 - First issue: 'default' is a reserved word when used with the `peer-keepalive` `vrf` keyword. It refers to the default rib.
   - This is confusing in several ways because `peer-keepalive`'s *default* vrf is the `management` vrf.

 - Second issue: When changing only one optional value (`pkl_vrf`) while other optional values are idempotent (`pkl_src`), the result is that the idempotent values are ignored; unfortunately the device cli *replaces* the entire command, in which case the idempotent values are removed.
   - e.g. playbook specifies this:
     ```
     { pkl_dest: 10.1.1.1, pkl_src: 10.2.2.2, pkl_vrf: my_vrf }
     ```

     ```
     peer-keepalive dest 10.1.1.1 src 10.2.2.2             # original

     peer-keepalive dest 10.1.1.1 src 10.2.2.2 vrf my_vrf  # intended result

     peer-keepalive dest 10.1.1.1 vrf my_vrf               # actual result
     ```

 - Third issue: the `pkl` getter was relying on positional data. This broke when the `udp` keyword nvgen'd where `vrf` used to appear (shifting all keywords to the right).

- Tested on regression platforms: `N3K,N6k,N7K,N9K,N3K-F,N9K-F`

* PEP fixes

* PEP fix 2

* pkl should merge by default, not override

* rmv debugs

* add mike's tests

* fix comments
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.