Repository: ansible/ansible
Pull request: fixes Traceback generated by nxos_igmp module
fixes #27488 

Problem statement:
(P1)nxos_igmp: module fails with traceback
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and master branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Bug Report

##### COMPONENT NAME
<!--- Name of the module/plugin/task/feature -->
- nxos_igmp

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
```
ansible 2.4.0 (devel b93b9e68d7) last updated 2017/07/26 14:19:36 (GMT -400)
  config file = None
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /root/agents-ci/ansible/lib/ansible
  executable location = /root/agents-ci/ansible/bin/ansible
  python version = 2.7.6 (default, Oct 26 2016, 20:30:19) [GCC 4.8.4]
```

##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say “N/A” for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.
-->
Ubuntu 14.04

##### SUMMARY
<!--- Explain the problem briefly -->
Traceback seen while applying playbook

##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->

<!--- Paste example playbooks or commands between quotes below -->
```yaml
---
- hosts: nxos
  connection: local
  gather_facts: no

  tasks:
  - name: "Configure igmp with default values"
    nxos_igmp:
      state: default
      provider: "{{ cli }}"
```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
No tracebacks.

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->

<!--- Paste verbatim command output between quotes below -->
```
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_D0RHMz/ansible_module_nxos_igmp.py", line 257, in <module>
    main()
  File "/tmp/ansible_D0RHMz/ansible_module_nxos_igmp.py", line 244, in main
    result['connected'] = module.connected   
AttributeError: 'AnsibleModule' object has no attribute 'connected'
FAILED! => {
    "changed": false, 
    "failed": true, 
    "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_D0RHMz/ansible_module_nxos_igmp.py\", line 257, in <module>\n    main()\n  File \"/tmp/ansible_D0RHMz/ansible_module_nxos_igmp.py\", line 244, in main\n    result['connected'] = module.connected\nAttributeError: 'AnsibleModule' object has no attribute 'connected'\n",  
    "module_stdout": "", 
    "msg": "MODULE FAILURE", 
    "rc": 0
}
```

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

Commit message: update module code to reflect review comments

Patch to review:
diff --git a/lib/ansible/modules/network/nxos/nxos_igmp.py b/lib/ansible/modules/network/nxos/nxos_igmp.py
index debc3388d4320b..ccb6416b1e70e8 100644
--- a/lib/ansible/modules/network/nxos/nxos_igmp.py
+++ b/lib/ansible/modules/network/nxos/nxos_igmp.py
@@ -84,106 +84,29 @@
 '''
 
 RETURN = '''
-proposed:
-    description: k/v pairs of parameters passed into module
-    returned: verbose mode
-    type: dict
-    sample: {"enforce_rtr_alert": true, "flush_routes": true}
-existing:
-    description: k/v pairs of existing IGMP configuration
-    returned: verbose mode
-    type: dict
-    sample: {"enforce_rtr_alert": true, "flush_routes": false}
-end_state:
-    description: k/v pairs of IGMP configuration after module execution
-    returned: verbose mode
-    type: dict
-    sample: {"enforce_rtr_alert": true, "flush_routes": true}
 updates:
     description: commands sent to the device
     returned: always
     type: list
     sample: ["ip igmp flush-routes"]
-changed:
-    description: check to see if a change was made on the device
-    returned: always
-    type: boolean
-    sample: true
 '''
-
-import re
-from ansible.module_utils.nxos import get_config, load_config, run_commands
+from ansible.module_utils.nxos import load_config, run_commands
 from ansible.module_utils.nxos import nxos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.netcfg import CustomNetworkConfig
-
-PARAM_TO_COMMAND_KEYMAP = {
-    'flush_routes': 'ip igmp flush-routes',
-    'enforce_rtr_alert': 'ip igmp enforce-router-alert'
-}
-
-
-def get_value(arg, config):
-    REGEX = re.compile(r'{0}\s*$'.format(PARAM_TO_COMMAND_KEYMAP[arg]), re.M)
-    value = False
-    try:
-        if REGEX.search(config):
-            value = True
-    except TypeError:
-        value = False
-    return value
-
-
-def get_existing(module, args):
-    existing = {}
-    config = str(get_config(module))
 
-    for arg in args:
-        existing[arg] = get_value(arg, config)
-    return existing
+def get_current(module):
+    output = run_commands(module, {'command': 'show running-config', 'output': 'text'})
+    return {
+        'flush_routes': 'ip igmp flush-routes' in output[0],
+        'enforce_rtr_alert': 'ip igmp enforce-router-alert' in output[0]
+    }
 
 
-def invoke(name, *args, **kwargs):
-    func = globals().get(name)
-    if func:
-        return func(*args, **kwargs)
-
-
-def get_commands(module, existing, proposed, candidate):
-    commands = list()
-    proposed_commands = apply_key_map(PARAM_TO_COMMAND_KEYMAP, proposed)
-    existing_commands = apply_key_map(PARAM_TO_COMMAND_KEYMAP, existing)
-    if module.params['state'] == 'default':
-        for key, value in proposed_commands.items():
-            if existing_commands.get(key):
-                commands.append('no {0}'.format(key))
-    else:
-        for key, value in proposed_commands.items():
-            if value is True:
-                commands.append(key)
-            else:
-                if existing_commands.get(key):
-                    commands.append('no {0}'.format(key))
-
-    if module.params['restart']:
-        commands.append('restart igmp')
-
-    if commands:
-        parents = []
-        candidate.add(commands, parents=parents)
-
-
-def apply_key_map(key_map, table):
-    new_dict = {}
-    for key, value in table.items():
-        new_key = key_map.get(key)
-        if new_key:
-            value = table.get(key)
-            if value:
-                new_dict[new_key] = value
-            else:
-                new_dict[new_key] = value
-    return new_dict
+def get_desired(module):
+    return {
+        'flush_routes': module.params['flush_routes'],
+        'enforce_rtr_alert': module.params['enforce_rtr_alert']
+    }
 
 
 def main():
@@ -191,7 +114,9 @@ def main():
         flush_routes=dict(type='bool'),
         enforce_rtr_alert=dict(type='bool'),
         restart=dict(type='bool', default=False),
+
         state=dict(choices=['present', 'default'], default='present'),
+
         include_defaults=dict(default=False),
         config=dict(),
         save=dict(type='bool', default=False)
@@ -206,53 +131,43 @@ def main():
     check_args(module, warnings)
 
 
-    state = module.params['state']
-    restart = module.params['restart']
-
-    if (state == 'default' and (module.params['flush_routes'] is not None or
-            module.params['enforce_rtr_alert'] is not None)):
-        module.fail_json(msg='When state=default other params have no effect.')
+    current = get_current(module)
+    desired = get_desired(module)
 
-    args =  [
-        "flush_routes",
-        "enforce_rtr_alert",
-    ]
+    import q
+    q(current)
+    q(desired)
 
-    existing = invoke('get_existing', module, args)
-    end_state = existing
+    state = module.params['state']
+    restart = module.params['restart']
 
-    proposed = dict((k, v) for k, v in module.params.items()
-                if v is not None and k in args)
+    commands = list()
 
-    proposed_args = proposed.copy()
     if state == 'default':
-        proposed_args = dict((k, False) for k in args)
+        if current['flush_routes']:
+            commands.append('no ip igmp flush-routes')
+        if current['enforce_rtr_alert']:
+            commands.append('no ip igmp enforce-router-alert')
 
-    result = {}
-    if (state == 'present' or (state == 'default' and
-            True in existing.values()) or restart):
-        candidate = CustomNetworkConfig(indent=3)
-        invoke('get_commands', module, existing, proposed_args, candidate)
-        response = load_config(module, candidate)
-        result.update(response)
+    elif state == 'present':
+        if desired['flush_routes'] and not current['flush_routes']:
+            commands.append('ip igmp flush-routes')
+        if desired['enforce_rtr_alert'] and not current['enforce_rtr_alert']:
+            commands.append('ip igmp enforce-router-alert')
 
-    else:
-        result['updates'] = []
+    result = {'changed': False, 'updates': commands, 'warnings': warnings}
+    q(commands)
 
-    if restart:
-        proposed['restart'] = restart
-    result['connected'] = module.connected
-    if module._verbosity > 0:
-        end_state = invoke('get_existing', module, args)
-        result['end_state'] = end_state
-        result['existing'] = existing
-        result['proposed'] = proposed
+    if commands:
+        if not module.check_mode:
+            load_config(module, commands)
+        result['changed'] = True
 
-    result['warnings'] = warnings
+    if module.params['restart']:
+        run_commands(module, 'restart igmp')
 
     module.exit_json(**result)
 
 
 if __name__ == '__main__':
     main()
-


Code review comments:
diff_hunk: @@ -206,53 +131,43 @@ def main():
     check_args(module, warnings)
 
 
-    state = module.params['state']
-    restart = module.params['restart']
-
-    if (state == 'default' and (module.params['flush_routes'] is not None or
-            module.params['enforce_rtr_alert'] is not None)):
-        module.fail_json(msg='When state=default other params have no effect.')
+    current = get_current(module)
+    desired = get_desired(module)
 
-    args =  [
-        "flush_routes",
-        "enforce_rtr_alert",
-    ]
+    import q
path: lib/ansible/modules/network/nxos/nxos_igmp.py
text: @user1:
Yay for `q`, but might want to take it out.

@author:
doh!.. updated