Repository: ansible/ansible
Pull request: Rel240/fix nxos interface ospf
##### SUMMARY
Fixes https://github.com/ansible/ansible/issues/28897
-->

##### ISSUE TYPE
 - Bugfix Pull Request

##### COMPONENT NAME
nxos_interface_ospf

##### ANSIBLE VERSION
```
ansible 2.4.0 (rel240/fix_nxos_interface_ospf d5930c22b3) last updated 2017/08/31 15:04:42 (GMT -400)
  config file = /Users/mwiebe/Projects/nxos_ansible/fix_ansible/test/integration/ansible.cfg
  configured module search path = [u'/Users/mwiebe/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/mwiebe/Projects/nxos_ansible/fix_ansible/lib/ansible
  executable location = /Users/mwiebe/Projects/nxos_ansible/fix_ansible/bin/ansible
  python version = 2.7.13 (default, Apr  4 2017, 08:47:57) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)]
```


##### ADDITIONAL INFORMATION
This PR addresses a problem where the interface name can be specified in the playbook where the leading character is either upper or lower case.

This PR resolves that issue and adds additional integration tests to avoid regressions.

Problem statement:
nxos_interface_ospf port-channel and loopback ints not idempotent
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
 - nxos_interface_ospf

##### NXOS VERSION
All

##### ANSIBLE VERSION
```
ansible 2.4.0 (rel240/fix_nxos_interface_ospf d5930c22b3) last updated 2017/08/31 15:04:42 (GMT -400)
  config file = /Users/mwiebe/Projects/nxos_ansible/fix_ansible/test/integration/ansible.cfg
  configured module search path = [u'/Users/mwiebe/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/mwiebe/Projects/nxos_ansible/fix_ansible/lib/ansible
  executable location = /Users/mwiebe/Projects/nxos_ansible/fix_ansible/bin/ansible
  python version = 2.7.13 (default, Apr  4 2017, 08:47:57) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)]
```

##### OS / ENVIRONMENT
MacOS

##### SUMMARY
If a loopback or port-channel interface is specified with the leading character capitalized the playbook is not idempotent.

```yaml
  - name: create port-channel and loopback interfaces
    nxos_config:
      commands:
        - interface port-channel10
        - interface port-channel11
        - interface loopback55 
        - interface loopback77
      match: none
      provider: "{{ connection }}"

  - name: "Ensure port-channels are layer3"
    nxos_config:
      commands:
        - no switchport
      parents:
        - "interface {{ item }}" 
      provider: "{{ connection }}"
    with_items:
      - port-channel10
      - port-channel11

  - name: configure ospf interface port-channel10
    nxos_interface_ospf: &configurepc
      interface: Port-channel10 
      ospf: 1
      area: 1
      cost: 55
      passive_interface: true
      hello_interval: 15
      dead_interval: 75
      state: present
      provider: "{{ connection }}"
    register: result

  - assert: *true

  - name: "Check Idempotence for port-channel10"
    nxos_interface_ospf: *configurepc
    register: result

  - assert: *false

  - name: configure ospf interface port-channel11 using lower case name
    nxos_interface_ospf: &configurepclower
      interface: port-channel11
      ospf: 1
      area: 1
      cost: 55
      passive_interface: true
      hello_interval: 15
      dead_interval: 75
      state: present
      provider: "{{ connection }}"
    register: result

  - assert: *true

  - name: "Check Idempotence for port-channel11"
    nxos_interface_ospf: *configurepclower
    register: result

  - assert: *false
```

```shell
TASK [nxos_nxapi : Configure NXAPI] **************************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tests/cli/configure.yaml:12
<n9k.example.com> connection transport is cli
<n9k.example.com> using connection plugin network_cli
<n9k.example.com> socket_path: /root/.ansible/pc/45bea83692
Using module file /root/agents-ci/ansible/lib/ansible/modules/network/nxos/nxos_nxapi.py
<n9k.example.com> ESTABLISH LOCAL CONNECTION FOR USER: root
<n9k.example.com> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo $HOME/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875 `" && echo ansible-tmp-1503671260.47-120714788718875="` echo $HOME/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875 `" ) && sleep 0'
<n9k.example.com> PUT /tmp/tmp3oNHSO TO /root/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875/nxos_nxapi.py
<n9k.example.com> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875/ /root/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875/nxos_nxapi.py && sleep 0'
<n9k.example.com> EXEC /bin/sh -c '/usr/bin/python /root/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875/nxos_nxapi.py; rm -rf "/root/.ansible/tmp/ansible-tmp-1503671260.47-120714788718875/" > /dev/null 2>&1 && sleep 0'
changed: [n9k.example.com] => {
    "changed": true, 
    "commands": [
        "feature nxapi", 
        "no nxapi http", 
        "nxapi https port 9443"
    ], 
    "failed": false, 
    "invocation": {
        "module_args": {
            "config": null, 
            "enable_http": false, 
            "enable_https": true, 
            "host": null, 
            "http": false, 
            "http_port": null, 
            "https": true, 
            "https_port": 9443, 
            "password": null, 
            "port": null, 
            "provider": null, 
            "sandbox": null, 
            "ssh_keyfile": null, 
            "state": "present", 
            "timeout": null, 
            "transport": "cli", 
            "use_ssl": null, 
            "username": null, 
            "validate_certs": null
        }
    }
}

TASK [nxos_nxapi : nxos_command] *****************************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tests/cli/configure.yaml:21
<n9k.example.com> connection transport is cli
<n9k.example.com> using connection plugin network_cli
<n9k.example.com> socket_path: /root/.ansible/pc/45bea83692
Using module file /root/agents-ci/ansible/lib/ansible/modules/network/nxos/nxos_command.py
<n9k.example.com> ESTABLISH LOCAL CONNECTION FOR USER: root
<n9k.example.com> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo $HOME/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916 `" && echo ansible-tmp-1503671267.5-100246849607916="` echo $HOME/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916 `" ) && sleep 0'
<n9k.example.com> PUT /tmp/tmpBHbQM3 TO /root/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916/nxos_command.py
<n9k.example.com> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916/ /root/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916/nxos_command.py && sleep 0'
<n9k.example.com> EXEC /bin/sh -c '/usr/bin/python /root/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916/nxos_command.py; rm -rf "/root/.ansible/tmp/ansible-tmp-1503671267.5-100246849607916/" > /dev/null 2>&1 && sleep 0'
ok: [n9k.example.com] => {
    "changed": false, 
    "failed": false, 
    "invocation": {
        "module_args": {
            "commands": [
                "show nxapi | json"
            ], 
            "host": null, 
            "interval": 1, 
            "match": "all", 
            "password": null, 
            "port": null, 
            "provider": null, 
            "retries": 10, 
            "ssh_keyfile": null, 
            "timeout": null, 
            "transport": "cli", 
            "use_ssl": null, 
            "username": null, 
            "validate_certs": null, 
            "wait_for": null
        }
    }, 
    "stdout": [
        {
            "TABLE_listen_on_port": {
                "ROW_listen_on_port": {
                    "l_port": "9443"
                }
            }, 
            "operation_status": {
                "o_status": "nxapi enabled"
            }
        }
    ], 
    "stdout_lines": [
        {
            "TABLE_listen_on_port": {
                "ROW_listen_on_port": {
                    "l_port": "9443"
                }
            }, 
            "operation_status": {
                "o_status": "nxapi enabled"
            }
        }
    ]
}

TASK [nxos_nxapi : include] **********************************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tests/cli/configure.yaml:27
skipping: [n9k.example.com] => {
    "changed": false, 
    "skip_reason": "Conditional result was False", 
    "skipped": true
}

TASK [nxos_nxapi : include] **********************************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tests/cli/configure.yaml:30
included: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes.yaml for n9k.example.com

TASK [nxos_nxapi : Assert configuration changes] *************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tasks/platform/default/assert_changes.yaml:2
ok: [n9k.example.com] => {
    "changed": false, 
    "failed": false, 
    "msg": "All assertions passed"
}

TASK [nxos_nxapi : Configure NXAPI again] ********************************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tests/cli/configure.yaml:33
<n9k.example.com> connection transport is cli
<n9k.example.com> using connection plugin network_cli
<n9k.example.com> socket_path: /root/.ansible/pc/45bea83692
Using module file /root/agents-ci/ansible/lib/ansible/modules/network/nxos/nxos_nxapi.py
<n9k.example.com> ESTABLISH LOCAL CONNECTION FOR USER: root
<n9k.example.com> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo $HOME/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841 `" && echo ansible-tmp-1503671269.57-152293043310841="` echo $HOME/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841 `" ) && sleep 0'
<n9k.example.com> PUT /tmp/tmppH8Yno TO /root/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841/nxos_nxapi.py
<n9k.example.com> EXEC /bin/sh -c 'chmod u+x /root/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841/ /root/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841/nxos_nxapi.py && sleep 0'
<n9k.example.com> EXEC /bin/sh -c '/usr/bin/python /root/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841/nxos_nxapi.py; rm -rf "/root/.ansible/tmp/ansible-tmp-1503671269.57-152293043310841/" > /dev/null 2>&1 && sleep 0'
changed: [n9k.example.com] => {
    "changed": true, 
    "commands": [
        "no nxapi http", 
        "nxapi https port 9443"
    ], 
    "failed": false, 
    "invocation": {
        "module_args": {
            "config": null, 
            "enable_http": false, 
            "enable_https": true, 
            "host": null, 
            "http": false, 
            "http_port": null, 
            "https": true, 
            "https_port": 9443, 
            "password": null, 
            "port": null, 
            "provider": null, 
            "sandbox": null, 
            "ssh_keyfile": null, 
            "state": "present", 
            "timeout": null, 
            "transport": "cli", 
            "use_ssl": null, 
            "username": null, 
            "validate_certs": null
        }
    }
}

TASK [nxos_nxapi : Assert configuration is idempotent] *******************************************************************************************************************************************************************
task path: /root/agents-ci/ansible/test/integration/targets/nxos_nxapi/tests/cli/configure.yaml:42
fatal: [n9k.example.com]: FAILED! => {
    "assertion": "result.changed == false", 
    "changed": false, 
    "evaluated_to": false, 
    "failed": true
}
	to retry, use: --limit @/root/agents-ci/ansible/test/integration/nxos.retry

PLAY RECAP ***************************************************************************************************************************************************************************************************************
n9k.example.com      : ok=17   changed=3    unreachable=0    failed=1   
```

Hints from issue discussion:
cc @GGabriele @jedelman8 @privateip @rahushen @rcarrillocruz @trishnaguha
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: Handle all characters upper or lower with tests

Patch to review:
diff --git a/lib/ansible/modules/network/nxos/nxos_interface_ospf.py b/lib/ansible/modules/network/nxos/nxos_interface_ospf.py
index f135d7fb13dcbf..ae936d79cb948c 100644
--- a/lib/ansible/modules/network/nxos/nxos_interface_ospf.py
+++ b/lib/ansible/modules/network/nxos/nxos_interface_ospf.py
@@ -394,7 +394,16 @@ def main():
                                                'message_digest_password']],
                            supports_check_mode=True)
 
-    if not module.params['interface'].startswith('loopback') and not module.params['interface'].startswith('port-channel'):
+    # Normalize interface input data.
+    #
+    # * For port-channel and loopback interfaces expection is all lower case names.
+    # * All other interfaces the expectation is an uppercase leading character
+    #   followed by lower case characters.
+    #
+    if re.match(r'(port-channel|loopback)', module.params['interface'], re.I):
+        module.params['interface'] = module.params['interface'].lower()
+    else:
+        module.params['interface'] = module.params['interface'].lower()
         module.params['interface'] = module.params['interface'].capitalize()
 
     warnings = list()
diff --git a/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml b/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml
index 9315fc102a8df1..43dea5eaea3984 100644
--- a/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml
+++ b/test/integration/targets/nxos_interface_ospf/tests/common/sanity.yaml
@@ -33,10 +33,20 @@
     provider: "{{ connection }}"
   ignore_errors: yes
 
+- name: "Remove possibly existing port-channel and loopback ints"
+  nxos_config: &removepcandlb
+    commands:
+      - no interface port-channel10
+      - no interface port-channel11
+      - no interface loopback55
+      - no interface loopback77
+    provider: "{{ connection }}"
+  ignore_errors: yes
+
 - block:
   - name: configure ospf interface
     nxos_interface_ospf: &configure
-      interface: "{{ testint }}"
+      interface: "{{ nxos_int1|upper }}"
       ospf: 1
       area: 1
       cost: 55
@@ -80,6 +90,109 @@
 
   - assert: *false
 
+  - name: create port-channel and loopback interfaces
+    nxos_config:
+      commands:
+        - interface port-channel10
+        - interface port-channel11
+        - interface loopback55
+        - interface loopback77
+      match: none
+      provider: "{{ connection }}"
+
+  - name: "Ensure port-channels are layer3"
+    nxos_config:
+      commands:
+        - no switchport
+      parents:
+        - "interface {{ item }}"
+      provider: "{{ connection }}"
+    with_items:
+      - port-channel10
+      - port-channel11
+
+  - name: configure ospf interface port-channel10
+    nxos_interface_ospf: &configurepc
+      interface: Port-channel10
+      ospf: 1
+      area: 1
+      cost: 55
+      passive_interface: true
+      hello_interval: 15
+      dead_interval: 75
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence for port-channel10"
+    nxos_interface_ospf: *configurepc
+    register: result
+
+  - assert: *false
+
+  - name: configure ospf interface port-channel11 using lower case name
+    nxos_interface_ospf: &configurepclower
+      interface: port-channel11
+      ospf: 1
+      area: 1
+      cost: 55
+      passive_interface: true
+      hello_interval: 15
+      dead_interval: 75
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence for port-channel11"
+    nxos_interface_ospf: *configurepclower
+    register: result
+
+  - assert: *false
+
+  - name: configure ospf interface loopback55
+    nxos_interface_ospf: &configurelb
+      interface: LOOPBACK55
+      ospf: 1
+      area: 1
+      cost: 55
+      hello_interval: 15
+      dead_interval: 75
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence for loopback55"
+    nxos_interface_ospf: *configurelb
+    register: result
+
+  - assert: *false
+
+  - name: configure ospf interface loopback77 using lower case name
+    nxos_interface_ospf: &configurelblower
+      interface: loopback77
+      ospf: 1
+      area: 1
+      cost: 77
+      hello_interval: 45
+      dead_interval: 75
+      state: present
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence for loopback77"
+    nxos_interface_ospf: *configurelblower
+    register: result
+
+  - assert: *false
+
   - name: remove ospf interface config
     nxos_interface_ospf: &removeconfig
       interface: "{{ testint }}"
@@ -115,5 +228,8 @@
   - name: "Interface cleanup"
     nxos_config: *intdefault
 
+  - name: "Remove port-channel and loopback ints"
+    nxos_config: *removepcandlb
+
   always:
   - debug: msg="END TRANSPORT:{{ connection.transport }} nxos_interface_ospf sanity test"


Code review comments:
diff_hunk: @@ -394,7 +394,16 @@ def main():
                                                'message_digest_password']],
                            supports_check_mode=True)
 
-    if not module.params['interface'].startswith('loopback') and not module.params['interface'].startswith('port-channel'):
+    # Normalize interface input data.
+    #
+    # * For port-channel and loopback interfaces expection is all lower case names.
+    # * All other interfaces the expectation is an uppercase leading character
+    #   followed by lower case characters.
+    #
+    if re.match(r'(port-channel|loopback)', module.params['interface'], re.I):
+        module.params['interface'] = module.params['interface'].lower()
+    else:
+        module.params['interface'] = module.params['interface'].lower()
path: lib/ansible/modules/network/nxos/nxos_interface_ospf.py
text: @user1:
`str.capitalize()` ensures the first letter is uppercase and the rest are lowercase, I can't see any reason to keep this line here.

@author:
Your right. I will update and remove it.