Minor fixes and additions to f5 modules (#39987)

* Fixes to docs
* Fix escaping quotes in bigip_command
* F5 coding conventions cleanup
* “warn” and “chdir” parameters added to bigip_command

lib/ansible/modules/network/f5/bigip_asm_policy.py

@@ -18,7 +18,7 @@ module: bigip_asm_policy
 short_description: Manage BIG-IP ASM policies
 description:
    - Manage BIG-IP ASM policies.
-version_added: "2.5"
+version_added: 2.5
 options:
   active:
     description:
@@ -215,14 +215,11 @@ name:
 import os
 import time
 
-from distutils.version import LooseVersion
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.basic import env_fallback
-
+from distutils.version import LooseVersion
-HAS_DEVEL_IMPORTS = False
 
 try:
-    # Sideband repository used for dev
     from library.module_utils.network.f5.bigip import HAS_F5SDK
     from library.module_utils.network.f5.bigip import F5Client
     from library.module_utils.network.f5.common import F5ModuleError
@@ -234,9 +231,7 @@ try:
         from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
-    HAS_DEVEL_IMPORTS = True
 except ImportError:
-    # Upstream Ansible
     from ansible.module_utils.network.f5.bigip import HAS_F5SDK
     from ansible.module_utils.network.f5.bigip import F5Client
     from ansible.module_utils.network.f5.common import F5ModuleError

lib/ansible/modules/network/f5/bigip_command.py

@@ -21,7 +21,12 @@ description:
     read from the device. This module includes an argument that will cause
     the module to wait for a specific condition before returning or timing
     out if the condition is not met.
-version_added: "2.4"
+  - This module is B(not) idempotent, nor will it ever be. It is intended as
+    a stop-gap measure to satisfy automation requirements until such a time as
+    a real module has been developed to configure in the way you need.
+  - If you are using this module, you should probably also be filing an issue
+    to have a B(real) module created for your needs.
+version_added: 2.4
 options:
   commands:
     description:
@@ -30,12 +35,9 @@ options:
         is returned. If the I(wait_for) argument is provided, the
         module is not returned until the condition is satisfied or
         the number of retries as expired.
-      - The I(commands) argument also accepts an alternative form
+      - Only C(tmsh) commands are supported. If you are piping or adding additional
-        that allows for complex values that specify the command
+        logic that is outside of C(tmsh) (such as grep'ing, awk'ing or other shell
-        to run and the output format to return. This can be done
+        related things that are not C(tmsh), this behavior is not supported.
-        on a command by command basis. The complex argument supports
-        the keywords C(command) and C(output) where C(command) is the
-        command to run and C(output) is 'text' or 'one-line'.
     required: True
   wait_for:
     description:
@@ -78,7 +80,21 @@ options:
         - rest
         - cli
     default: rest
-    version_added: "2.5"
+    version_added: 2.5
+  warn:
+    description:
+      - Whether the module should raise warnings related to command idempotency
+        or not.
+      - Note that the F5 Ansible developers specifically leave this on to make you
+        aware that your usage of this module may be better served by official F5
+        Ansible modules. This module should always be used as a last resort.
+    default: True
+    type: bool
+    version_added: 2.6
+  chdir:
+    description:
+      - Change into this directory before running the command.
+    version_added: 2.6
 extends_documentation_fragment: f5
 author:
   - Tim Rupp (@caphrim007)
@@ -102,6 +118,7 @@ EXAMPLES = r'''
     password: secret
     user: admin
     validate_certs: no
+  register: result
   delegate_to: localhost
 
 - name: run multiple commands on remote nodes
@@ -127,6 +144,7 @@ EXAMPLES = r'''
     password: secret
     user: admin
     validate_certs: no
+  register: result
   delegate_to: localhost
 
 - name: tmsh prefixes will automatically be handled
@@ -139,73 +157,78 @@ EXAMPLES = r'''
     user: admin
     validate_certs: no
   delegate_to: localhost
+
+- name: Delete all LTM nodes in Partition1, assuming no dependencies exist
+  bigip_command:
+    commands:
+      - delete ltm node all
+    chdir: Partition1
+    server: lb.mydomain.com
+    password: secret
+    user: admin
+    validate_certs: no
+  delegate_to: localhost
 '''
 
 RETURN = r'''
 stdout:
-  description: The set of responses from the commands
+  description: The set of responses from the commands.
   returned: always
   type: list
   sample: ['...', '...']
 stdout_lines:
-  description: The value of stdout split into a list
+  description: The value of stdout split into a list.
   returned: always
   type: list
   sample: [['...', '...'], ['...'], ['...']]
 failed_conditions:
-  description: The list of conditionals that have failed
+  description: The list of conditionals that have failed.
   returned: failed
   type: list
   sample: ['...', '...']
+warn:
+  description: Whether or not to raise warnings about modification commands.
+  returned: changed
+  type: bool
+  sample: True
 '''
 
 import re
 import time
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six import string_types
 from ansible.module_utils.network.common.parsing import FailedConditionsError
 from ansible.module_utils.network.common.parsing import Conditional
 from ansible.module_utils.network.common.utils import ComplexList
 from ansible.module_utils.network.common.utils import to_list
+from ansible.module_utils.six import string_types
 from collections import deque
 
-HAS_DEVEL_IMPORTS = False
-
 try:
-    # Sideband repository used for dev
     from library.module_utils.network.f5.bigip import HAS_F5SDK
     from library.module_utils.network.f5.bigip import F5Client
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import AnsibleF5Parameters
     from library.module_utils.network.f5.common import cleanup_tokens
-    from library.module_utils.network.f5.common import fqdn_name
     from library.module_utils.network.f5.common import is_cli
     from library.module_utils.network.f5.common import f5_argument_spec
     try:
         from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
-    HAS_DEVEL_IMPORTS = True
-    try:
-        from library.module_utils.network.f5.common import run_commands
-        HAS_CLI_TRANSPORT = True
-    except ImportError:
-        HAS_CLI_TRANSPORT = False
 except ImportError:
-    # Upstream Ansible
     from ansible.module_utils.network.f5.bigip import HAS_F5SDK
     from ansible.module_utils.network.f5.bigip import F5Client
     from ansible.module_utils.network.f5.common import F5ModuleError
     from ansible.module_utils.network.f5.common import AnsibleF5Parameters
     from ansible.module_utils.network.f5.common import cleanup_tokens
-    from ansible.module_utils.network.f5.common import fqdn_name
     from ansible.module_utils.network.f5.common import is_cli
     from ansible.module_utils.network.f5.common import f5_argument_spec
     try:
         from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
+
 try:
     from ansible.module_utils.network.f5.common import run_commands
     HAS_CLI_TRANSPORT = True
@@ -214,52 +237,159 @@ except ImportError:
 
 
 class Parameters(AnsibleF5Parameters):
-    returnables = ['stdout', 'stdout_lines', 'warnings']
+    returnables = ['stdout', 'stdout_lines', 'warnings', 'executed_commands']
 
     def to_return(self):
         result = {}
+        try:
             for returnable in self.returnables:
                 result[returnable] = getattr(self, returnable)
             result = self._filter_params(result)
             return result
+        except Exception:
+            return result
 
-    def _listify(self, item):
+    @property
-        if isinstance(item, string_types):
+    def raw_commands(self):
-            result = [item]
+        if self._values['commands'] is None:
+            return []
+        if isinstance(self._values['commands'], string_types):
+            result = [self._values['commands']]
         else:
-            result = item
+            result = self._values['commands']
         return result
 
-    @property
+    def convert_commands(self, commands):
-    def commands(self):
+        result = []
-        commands = self._listify(self._values['commands'])
+        for command in commands:
-        commands = deque(commands)
+            tmp = dict(
-        if not is_cli(self.module):
+                command='',
-            commands.appendleft(
+                pipeline=''
-                'tmsh modify cli preference pager disabled'
+            )
+
+            command = command.replace("'", "\\'")
+            pipeline = command.split('|', 1)
+            tmp['command'] = pipeline[0]
+            try:
+                tmp['pipeline'] = pipeline[1]
+            except IndexError:
+                pass
+            result.append(tmp)
+        return result
+
+    def convert_commands_cli(self, commands):
+        result = []
+        for command in commands:
+            tmp = dict(
+                command='',
+                pipeline=''
             )
-        commands = map(self._ensure_tmsh_prefix, list(commands))
+
-        return list(commands)
+            pipeline = command.split('|', 1)
+            tmp['command'] = pipeline[0]
+            try:
+                tmp['pipeline'] = pipeline[1]
+            except IndexError:
+                pass
+            result.append(tmp)
+        return result
+
+    def merge_command_dict(self, command):
+        if command['pipeline'] != '':
+            escape_patterns = r'([$"])'
+            command['pipeline'] = re.sub(escape_patterns, r'\\\1', command['pipeline'])
+            command['command'] = '{0} | {1}'.format(command['command'], command['pipeline']).strip()
+
+    def merge_command_dict_cli(self, command):
+        if command['pipeline'] != '':
+            command['command'] = '{0} | {1}'.format(command['command'], command['pipeline']).strip()
+
+    @property
+    def rest_commands(self):
+        # ['list ltm virtual']
+        commands = self.normalized_commands
+        commands = self.convert_commands(commands)
+        if self.chdir:
+            # ['cd /Common; list ltm virtual']
+            for command in commands:
+                self.addon_chdir(command)
+        # ['tmsh -c "cd /Common; list ltm virtual"']
+        for command in commands:
+            self.addon_tmsh(command)
+        for command in commands:
+            self.merge_command_dict(command)
+        result = [x['command'] for x in commands]
+        return result
+
+    @property
+    def cli_commands(self):
+        # ['list ltm virtual']
+        commands = self.normalized_commands
+        commands = self.convert_commands_cli(commands)
+        if self.chdir:
+            # ['cd /Common; list ltm virtual']
+            for command in commands:
+                self.addon_chdir(command)
+        if not self.is_tmsh:
+            # ['tmsh -c "cd /Common; list ltm virtual"']
+            for command in commands:
+                self.addon_tmsh_cli(command)
+        for command in commands:
+            self.merge_command_dict_cli(command)
+        result = [x['command'] for x in commands]
+        return result
+
+    @property
+    def normalized_commands(self):
+        if self._values['normalized_commands'] is None:
+            return None
+        return deque(self._values['normalized_commands'])
+
+    @property
+    def chdir(self):
+        if self._values['chdir'] is None:
+            return None
+        if self._values['chdir'].startswith('/'):
+            return self._values['chdir']
+        return '/{0}'.format(self._values['chdir'])
 
     @property
     def user_commands(self):
-        commands = self._listify(self._values['commands'])
+        commands = self.raw_commands
         return map(self._ensure_tmsh_prefix, commands)
 
-    def _ensure_tmsh_prefix(self, cmd):
+    @property
-        cmd = cmd.strip()
+    def wait_for(self):
-        if cmd[0:5] != 'tmsh ':
+        return self._values['wait_for'] or list()
-            cmd = 'tmsh ' + cmd.strip()
-        return cmd
 
+    def addon_tmsh(self, command):
+        escape_patterns = r'([$"])'
+        if command['command'].count('"') % 2 != 0:
+            raise Exception('Double quotes are unbalanced')
+        command['command'] = re.sub(escape_patterns, r'\\\\\\\1', command['command'])
+        command['command'] = 'tmsh -c \\\"{0}\\\"'.format(command['command'])
 
-class ModuleManager(object):
+    def addon_tmsh_cli(self, command):
+        if command['command'].count('"') % 2 != 0:
+            raise Exception('Double quotes are unbalanced')
+        command['command'] = 'tmsh -c "{0}"'.format(command['command'])
+
+    def addon_chdir(self, command):
+        command['command'] = "cd {0}; {1}".format(self.chdir, command['command'])
+
+
+class BaseManager(object):
     def __init__(self, *args, **kwargs):
         self.module = kwargs.get('module', None)
         self.client = kwargs.get('client', None)
         self.want = Parameters(params=self.module.params)
         self.want.update({'module': self.module})
         self.changes = Parameters(module=self.module)
+        self.valid_configs = [
+            'list', 'show', 'modify cli preference pager disabled'
+        ]
+        self.changed_command_prefixes = ('modify', 'create', 'delete')
+        self.warnings = list()
 
     def _to_lines(self, stdout):
         lines = list()
@@ -269,26 +399,6 @@ class ModuleManager(object):
             lines.append(item)
         return lines
 
-    def _is_valid_mode(self, cmd):
-        valid_configs = [
-            'list', 'show',
-            'modify cli preference pager disabled'
-        ]
-        if not is_cli(self.module):
-            valid_configs = list(map(self.want._ensure_tmsh_prefix, valid_configs))
-        if any(cmd.startswith(x) for x in valid_configs):
-            return True
-        return False
-
-    def is_tmsh(self):
-        try:
-            self._run_commands(self.module, 'tmsh help')
-        except F5ModuleError as ex:
-            if 'Syntax Error:' in str(ex):
-                return True
-            raise
-        return False
-
     def exec_module(self):
         result = dict()
 
@@ -299,38 +409,81 @@ class ModuleManager(object):
 
         result.update(**self.changes.to_return())
         result.update(dict(changed=changed))
+        self._announce_warnings(result)
+        return result
+
+    def _announce_warnings(self, result):
+        warnings = result.pop('warnings', [])
+        for warning in warnings:
+            self.module.warn(warning)
+
+    def notify_non_idempotent_commands(self, commands):
+        for index, item in enumerate(commands):
+            if any(item.startswith(x) for x in self.valid_configs):
+                return
+            else:
+                self.warnings.append(
+                    'Using "write" commands is not idempotent. You should use '
+                    'a module that is specifically made for that. If such a '
+                    'module does not exist, then please file a bug. The command '
+                    'in question is "{0}..."'.format(item[0:40])
+                )
+
+    @staticmethod
+    def normalize_commands(raw_commands):
+        if not raw_commands:
+            return None
+        result = []
+        for command in raw_commands:
+            command = command.strip()
+            if command[0:5] == 'tmsh ':
+                command = command[4:].strip()
+            result.append(command)
         return result
 
-    def _run_commands(self, module, commands):
+    def parse_commands(self):
-        return run_commands(module, commands)
+        results = []
+        commands = self._transform_to_complex_commands(self.commands)
+
+        for index, item in enumerate(commands):
+            # This needs to be removed so that the ComplexList used in to_commands
+            # will work correctly.
+            output = item.pop('output', None)
+
+            if output == 'one-line' and 'one-line' not in item['command']:
+                item['command'] += ' one-line'
+            elif output == 'text' and 'one-line' in item['command']:
+                item['command'] = item['command'].replace('one-line', '')
+
+            results.append(item)
+        return results
 
     def execute(self):
-        warnings = list()
+        if self.want.normalized_commands:
-        changed = ('tmsh modify', 'tmsh create', 'tmsh delete')
+            result = self.want.normalized_commands
-        commands = self.parse_commands(warnings)
+        else:
-        wait_for = self.want.wait_for or list()
+            result = self.normalize_commands(self.want.raw_commands)
+            self.want.update({'normalized_commands': result})
+        if not result:
+            return False
+        self.notify_non_idempotent_commands(self.want.normalized_commands)
+
+        commands = self.parse_commands()
         retries = self.want.retries
-        conditionals = [Conditional(c) for c in wait_for]
+        conditionals = [Conditional(c) for c in self.want.wait_for]
 
         if self.module.check_mode:
             return
 
         while retries > 0:
-            if is_cli(self.module) and HAS_CLI_TRANSPORT:
+            responses = self._execute(commands)
-                if self.is_tmsh():
+            self._check_known_errors(responses)
-                    for command in commands:
-                        command['command'] = command['command'][4:].strip()
-                responses = self._run_commands(self.module, commands)
-            else:
-                responses = self.execute_on_device(commands)
-
             for item in list(conditionals):
                 if item(responses):
                     if self.want.match == 'any':
                         conditionals = list()
                         break
                     conditionals.remove(item)
-
             if not conditionals:
                 break
 
@@ -338,22 +491,35 @@ class ModuleManager(object):
             retries -= 1
         else:
             failed_conditions = [item.raw for item in conditionals]
-            errmsg = 'One or more conditional statements have not been satisfied'
+            errmsg = 'One or more conditional statements have not been satisfied.'
             raise FailedConditionsError(errmsg, failed_conditions)
-
+        stdout_lines = self._to_lines(responses)
         changes = {
             'stdout': responses,
-            'stdout_lines': self._to_lines(responses),
+            'stdout_lines': stdout_lines,
-            'warnings': warnings
+            'executed_commands': self.commands
         }
+        if self.want.warn:
+            changes['warnings'] = self.warnings
         self.changes = Parameters(params=changes, module=self.module)
-        if any(x for x in self.want.user_commands if x.startswith(changed)):
+        if any(x for x in self.want.normalized_commands if x.startswith(self.changed_command_prefixes)):
             return True
         return False
 
-    def parse_commands(self, warnings):
+    def _check_known_errors(self, responses):
-        results = []
+        # A regex to match the error IDs used in the F5 v2 logging framework.
-        commands = list(deque(set(self.want.commands)))
+        pattern = r'^[0-9A-Fa-f]+:?\d+?:'
+
+        for resp in responses:
+            if 'usage: tmsh' in resp:
+                raise F5ModuleError(
+                    "tmsh command printed its 'help' message instead of running your command. "
+                    "This usually indicates unbalanced quotes."
+                )
+            if re.match(pattern, resp):
+                raise F5ModuleError(str(resp))
+
+    def _transform_to_complex_commands(self, commands):
         spec = dict(
             command=dict(key=True),
             output=dict(
@@ -361,44 +527,100 @@ class ModuleManager(object):
                 choices=['text', 'one-line']
             ),
         )
-
         transform = ComplexList(spec, self.module)
-        commands = transform(commands)
+        result = transform(commands)
+        return result
 
-        for index, item in enumerate(commands):
+
-            if not self._is_valid_mode(item['command']) and is_cli(self.module):
+class V1Manager(BaseManager):
-                warnings.append(
+    """Supports CLI (SSH) communication with the remote device
-                    'Using "write" commands is not idempotent. You should use '
+
-                    'a module that is specifically made for that. If such a '
+    """
-                    'module does not exist, then please file a bug. The command '
+    def _execute(self, commands):
-                    'in question is "%s..."' % item['command'][0:40]
+        if self.want.is_tmsh:
+            command = dict(
+                command="modify cli preference pager disabled"
             )
-            # This needs to be removed so that the ComplexList used in to_commands
+        else:
-            # will work correctly.
+            command = dict(
-            output = item.pop('output', None)
+                command="tmsh modify cli preference pager disabled"
+            )
+        self.execute_on_device(command)
+        return self.execute_on_device(commands)
 
-            if output == 'one-line' and 'one-line' not in item['command']:
+    @property
-                item['command'] += ' one-line'
+    def commands(self):
-            elif output == 'text' and 'one-line' in item['command']:
+        return self.want.cli_commands
-                item['command'] = item['command'].replace('one-line', '')
 
-            results.append(item)
+    def is_tmsh(self):
-        return results
+        try:
+            self.execute_on_device('tmsh -v')
+        except Exception as ex:
+            if 'Syntax Error:' in str(ex):
+                return True
+            raise
+        return False
+
+    def execute(self):
+        self.want.update({'is_tmsh': self.is_tmsh()})
+        return super(V1Manager, self).execute()
+
+    def execute_on_device(self, commands):
+        result = run_commands(self.module, commands)
+        return result
+
+
+class V2Manager(BaseManager):
+    """Supports REST communication with the remote device
+
+    """
+    def _execute(self, commands):
+        command = dict(
+            command="tmsh modify cli preference pager disabled"
+        )
+        self.execute_on_device(command)
+        return self.execute_on_device(commands)
+
+    @property
+    def commands(self):
+        return self.want.rest_commands
 
     def execute_on_device(self, commands):
         responses = []
-        escape_patterns = r'([$' + "'])"
         for item in to_list(commands):
-            command = re.sub(escape_patterns, r'\\\1', item['command'])
+            try:
+                command = '-c "{0}"'.format(item['command'])
                 output = self.client.api.tm.util.bash.exec_cmd(
                     'run',
-                utilCmdArgs='-c "{0}"'.format(command)
+                    utilCmdArgs=command
                 )
                 if hasattr(output, 'commandResult'):
-                responses.append(str(output.commandResult))
+                    responses.append(str(output.commandResult).strip())
+            except Exception as ex:
+                pass
         return responses
 
 
+class ModuleManager(object):
+    def __init__(self, *args, **kwargs):
+        self.kwargs = kwargs
+        self.module = kwargs.get('module', None)
+
+    def exec_module(self):
+        if is_cli(self.module) and HAS_CLI_TRANSPORT:
+            manager = self.get_manager('v1')
+        else:
+            manager = self.get_manager('v2')
+        result = manager.exec_module()
+        return result
+
+    def get_manager(self, type):
+        if type == 'v1':
+            return V1Manager(**self.kwargs)
+        elif type == 'v2':
+            return V2Manager(**self.kwargs)
+
+
 class ArgumentSpec(object):
     def __init__(self):
         self.supports_check_mode = True
@@ -427,7 +649,12 @@ class ArgumentSpec(object):
                 type='str',
                 default='rest',
                 choices=['cli', 'rest']
-            )
+            ),
+            warn=dict(
+                type='bool',
+                default='yes'
+            ),
+            chdir=dict()
         )
         self.argument_spec = {}
         self.argument_spec.update(f5_argument_spec)
@@ -442,15 +669,17 @@ def main():
         supports_check_mode=spec.supports_check_mode
     )
     if is_cli(module) and not HAS_F5SDK:
-        module.fail_json(msg="The python f5-sdk module is required to use the rest api")
+        module.fail_json(msg="The python f5-sdk module is required to use the REST api")
 
     try:
         client = F5Client(**module.params)
         mm = ModuleManager(module=module, client=client)
         results = mm.exec_module()
+        if not is_cli(module):
             cleanup_tokens(client)
         module.exit_json(**results)
     except F5ModuleError as e:
+        if not is_cli(module):
             cleanup_tokens(client)
         module.fail_json(msg=str(e))
 

lib/ansible/modules/network/f5/bigip_config.py

@@ -24,7 +24,7 @@ description:
     configuration to disk. Since the F5 module only manipulate running
     configuration, it is important that you utilize this module to save
     that running config.
-version_added: "2.4"
+version_added: 2.4
 options:
   save:
     description:
@@ -38,23 +38,25 @@ options:
     default: no
   reset:
     description:
-      - Loads the default configuration on the device. If this option
+      - Loads the default configuration on the device.
-        is specified, the default configuration will be loaded before
+      - If this option is specified, the default configuration will be
-        any commands or other provided configuration is run.
+        loaded before any commands or other provided configuration is run.
     type: bool
     default: no
   merge_content:
     description:
       - Loads the specified configuration that you want to merge into
         the running configuration. This is equivalent to using the
-        C(tmsh) command C(load sys config from-terminal merge). If
+        C(tmsh) command C(load sys config from-terminal merge).
-        you need to read configuration from a file or template, use
+      - If you need to read configuration from a file or template, use
         Ansible's C(file) or C(template) lookup plugins respectively.
   verify:
     description:
       - Validates the specified configuration to see whether they are
-        valid to replace the running configuration. The running
+        valid to replace the running configuration.
-        configuration will not be changed.
+      - The running configuration will not be changed.
+      - When this parameter is set to C(yes), no change will be reported
+        by the module.
     type: bool
     default: no
 extends_documentation_fragment: f5
@@ -98,7 +100,6 @@ stdout:
   returned: always
   type: list
   sample: ['...', '...']
-
 stdout_lines:
   description: The value of stdout split into a list
   returned: always
@@ -109,43 +110,36 @@ stdout_lines:
 import os
 import tempfile
 
-try:
-    from StringIO import StringIO
-except ImportError:
-    from io import StringIO
-
 from ansible.module_utils.basic import AnsibleModule
 
-HAS_DEVEL_IMPORTS = False
-
 try:
-    # Sideband repository used for dev
     from library.module_utils.network.f5.bigip import HAS_F5SDK
     from library.module_utils.network.f5.bigip import F5Client
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import AnsibleF5Parameters
     from library.module_utils.network.f5.common import cleanup_tokens
-    from library.module_utils.network.f5.common import fqdn_name
     from library.module_utils.network.f5.common import f5_argument_spec
     try:
         from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
-    HAS_DEVEL_IMPORTS = True
 except ImportError:
-    # Upstream Ansible
     from ansible.module_utils.network.f5.bigip import HAS_F5SDK
     from ansible.module_utils.network.f5.bigip import F5Client
     from ansible.module_utils.network.f5.common import F5ModuleError
     from ansible.module_utils.network.f5.common import AnsibleF5Parameters
     from ansible.module_utils.network.f5.common import cleanup_tokens
-    from ansible.module_utils.network.f5.common import fqdn_name
     from ansible.module_utils.network.f5.common import f5_argument_spec
     try:
         from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
 
+try:
+    from StringIO import StringIO
+except ImportError:
+    from io import StringIO
+
 
 class Parameters(AnsibleF5Parameters):
     returnables = ['stdout', 'stdout_lines']
@@ -182,15 +176,14 @@ class ModuleManager(object):
         return lines
 
     def exec_module(self):
-        result = dict()
+        result = {}
-
         try:
-            self.execute()
+            changed = self.execute()
         except iControlUnexpectedHTTPError as e:
             raise F5ModuleError(str(e))
 
         result.update(**self.changes.to_return())
-        result.update(dict(changed=True))
+        result.update(dict(changed=changed))
         return result
 
     def execute(self):
@@ -217,6 +210,9 @@ class ModuleManager(object):
             'stdout_lines': self._to_lines(responses)
         }
         self.changes = Parameters(params=changes)
+        if self.want.verify:
+            return False
+        return True
 
     def _detect_errors(self, stdout):
         errors = [

lib/ansible/modules/network/f5/bigip_configsync_action.py

@@ -20,7 +20,7 @@ description:
   - Allows one to run different config-sync actions. These actions allow
     you to manually sync your configuration across multiple BIG-IPs when
     those devices are in an HA pair.
-version_added: "2.4"
+version_added: 2.4
 options:
   device_group:
     description:
@@ -56,7 +56,7 @@ author:
 
 EXAMPLES = r'''
 - name: Sync configuration from device to group
-  bigip_configsync_actions:
+  bigip_configsync_action:
     device_group: foo-group
     sync_device_to_group: yes
     server: lb.mydomain.com
@@ -66,7 +66,7 @@ EXAMPLES = r'''
   delegate_to: localhost
 
 - name: Sync configuration from most recent device to the current host
-  bigip_configsync_actions:
+  bigip_configsync_action:
     device_group: foo-group
     sync_most_recent_to_device: yes
     server: lb.mydomain.com
@@ -76,7 +76,7 @@ EXAMPLES = r'''
   delegate_to: localhost
 
 - name: Perform an initial sync of a device to a new device group
-  bigip_configsync_actions:
+  bigip_configsync_action:
     device_group: new-device-group
     sync_device_to_group: yes
     server: lb.mydomain.com
@@ -93,45 +93,38 @@ RETURN = r'''
 import re
 import time
 
-try:
-    from objectpath import Tree
-    HAS_OBJPATH = True
-except ImportError:
-    HAS_OBJPATH = False
-
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.basic import BOOLEANS_TRUE
+from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE
-
-HAS_DEVEL_IMPORTS = False
 
 try:
-    # Sideband repository used for dev
     from library.module_utils.network.f5.bigip import HAS_F5SDK
     from library.module_utils.network.f5.bigip import F5Client
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import AnsibleF5Parameters
     from library.module_utils.network.f5.common import cleanup_tokens
-    from library.module_utils.network.f5.common import fqdn_name
     from library.module_utils.network.f5.common import f5_argument_spec
     try:
         from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
-    HAS_DEVEL_IMPORTS = True
 except ImportError:
-    # Upstream Ansible
     from ansible.module_utils.network.f5.bigip import HAS_F5SDK
     from ansible.module_utils.network.f5.bigip import F5Client
     from ansible.module_utils.network.f5.common import F5ModuleError
     from ansible.module_utils.network.f5.common import AnsibleF5Parameters
     from ansible.module_utils.network.f5.common import cleanup_tokens
-    from ansible.module_utils.network.f5.common import fqdn_name
     from ansible.module_utils.network.f5.common import f5_argument_spec
     try:
         from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
 
+try:
+    from objectpath import Tree
+    HAS_OBJPATH = True
+except ImportError:
+    HAS_OBJPATH = False
+
 
 class Parameters(AnsibleF5Parameters):
     api_attributes = []

lib/ansible/modules/network/f5/bigip_device_connectivity.py

@@ -21,7 +21,7 @@ description:
     has synchronization and failover connectivity information (IP addresses) that
     you define as part of HA pairing or clustering. This module allows you to configure
     that information.
-version_added: "2.5"
+version_added: 2.5
 options:
   config_sync_ip:
     description:
@@ -139,30 +139,23 @@ multicast_port:
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.six import iteritems
 
-HAS_DEVEL_IMPORTS = False
-
 try:
-    # Sideband repository used for dev
     from library.module_utils.network.f5.bigip import HAS_F5SDK
     from library.module_utils.network.f5.bigip import F5Client
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import AnsibleF5Parameters
     from library.module_utils.network.f5.common import cleanup_tokens
-    from library.module_utils.network.f5.common import fqdn_name
     from library.module_utils.network.f5.common import f5_argument_spec
     try:
         from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     except ImportError:
         HAS_F5SDK = False
-    HAS_DEVEL_IMPORTS = True
 except ImportError:
-    # Upstream Ansible
     from ansible.module_utils.network.f5.bigip import HAS_F5SDK
     from ansible.module_utils.network.f5.bigip import F5Client
     from ansible.module_utils.network.f5.common import F5ModuleError
     from ansible.module_utils.network.f5.common import AnsibleF5Parameters
     from ansible.module_utils.network.f5.common import cleanup_tokens
-    from ansible.module_utils.network.f5.common import fqdn_name
     from ansible.module_utils.network.f5.common import f5_argument_spec
     try:
         from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError

test/units/modules/network/f5/test_bigip_asm_policy.py

@@ -21,12 +21,12 @@ from ansible.compat.tests.mock import patch
 from ansible.module_utils.basic import AnsibleModule
 
 try:
-    from library.bigip_asm_policy import V1Parameters
+    from library.modules.bigip_asm_policy import V1Parameters
-    from library.bigip_asm_policy import V2Parameters
+    from library.modules.bigip_asm_policy import V2Parameters
-    from library.bigip_asm_policy import ModuleManager
+    from library.modules.bigip_asm_policy import ModuleManager
-    from library.bigip_asm_policy import V1Manager
+    from library.modules.bigip_asm_policy import V1Manager
-    from library.bigip_asm_policy import V2Manager
+    from library.modules.bigip_asm_policy import V2Manager
-    from library.bigip_asm_policy import ArgumentSpec
+    from library.modules.bigip_asm_policy import ArgumentSpec
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     from test.unit.modules.utils import set_module_args

test/units/modules/network/f5/test_bigip_command.py

@@ -20,9 +20,11 @@ from ansible.compat.tests.mock import Mock
 from ansible.module_utils.basic import AnsibleModule
 
 try:
-    from library.bigip_command import Parameters
+    from library.modules.bigip_command import Parameters
-    from library.bigip_command import ModuleManager
+    from library.modules.bigip_command import ModuleManager
-    from library.bigip_command import ArgumentSpec
+    from library.modules.bigip_command import V1Manager
+    from library.modules.bigip_command import V2Manager
+    from library.modules.bigip_command import ArgumentSpec
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     from test.unit.modules.utils import set_module_args
@@ -30,6 +32,8 @@ except ImportError:
     try:
         from ansible.modules.network.f5.bigip_command import Parameters
         from ansible.modules.network.f5.bigip_command import ModuleManager
+        from ansible.modules.network.f5.bigip_command import V1Manager
+        from ansible.modules.network.f5.bigip_command import V2Manager
         from ansible.modules.network.f5.bigip_command import ArgumentSpec
         from ansible.module_utils.network.f5.common import F5ModuleError
         from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError
@@ -92,15 +96,18 @@ class TestManager(unittest.TestCase):
             supports_check_mode=self.spec.supports_check_mode
         )
 
+        m1 = V2Manager(module=module)
+        m1.execute_on_device = Mock(return_value=['resp1', 'resp2'])
+
         mm = ModuleManager(module=module)
         mm._run_commands = Mock(return_value=[])
-        mm.execute_on_device = Mock(return_value=[])
+        mm.get_manager = Mock(return_value=m1)
 
         results = mm.exec_module()
 
         assert results['changed'] is False
         assert mm._run_commands.call_count == 0
-        assert mm.execute_on_device.call_count == 1
+        assert m1.execute_on_device.call_count == 2
 
     def test_run_single_modification_command(self, *args):
         set_module_args(dict(
@@ -116,15 +123,19 @@ class TestManager(unittest.TestCase):
             argument_spec=self.spec.argument_spec,
             supports_check_mode=self.spec.supports_check_mode
         )
+
+        m1 = V2Manager(module=module)
+        m1.execute_on_device = Mock(return_value=['resp1', 'resp2'])
+
         mm = ModuleManager(module=module)
         mm._run_commands = Mock(return_value=[])
-        mm.execute_on_device = Mock(return_value=[])
+        mm.get_manager = Mock(return_value=m1)
 
         results = mm.exec_module()
 
         assert results['changed'] is True
         assert mm._run_commands.call_count == 0
-        assert mm.execute_on_device.call_count == 1
+        assert m1.execute_on_device.call_count == 2
 
     def test_cli_command(self, *args):
         set_module_args(dict(
@@ -141,9 +152,13 @@ class TestManager(unittest.TestCase):
             argument_spec=self.spec.argument_spec,
             supports_check_mode=self.spec.supports_check_mode
         )
+
+        m1 = V1Manager(module=module)
+        m1.execute_on_device = Mock(return_value=['resp1', 'resp2', 'resp3'])
+
         mm = ModuleManager(module=module)
         mm._run_commands = Mock(return_value=[])
-        mm.execute_on_device = Mock(return_value=[])
+        mm.get_manager = Mock(return_value=m1)
 
         results = mm.exec_module()
 
@@ -158,8 +173,7 @@ class TestManager(unittest.TestCase):
         #
         # Can we change this in the future by making the terminal plugin
         # find this out ahead of time?
-        assert mm._run_commands.call_count == 2
+        assert m1.execute_on_device.call_count == 3
-        assert mm.execute_on_device.call_count == 0
 
     def test_command_with_commas(self, *args):
         set_module_args(dict(
@@ -167,7 +181,7 @@ class TestManager(unittest.TestCase):
               tmsh create /auth ldap system-auth {bind-dn uid=binduser,
               cn=users,dc=domain,dc=com bind-pw $ENCRYPTEDPW check-roles-group
               enabled search-base-dn cn=users,dc=domain,dc=com servers add {
-              ldap.server.com } }"
+              ldap.server.com } }
             """,
             server='localhost',
             user='admin',
@@ -177,12 +191,101 @@ class TestManager(unittest.TestCase):
             argument_spec=self.spec.argument_spec,
             supports_check_mode=self.spec.supports_check_mode
         )
+        m1 = V2Manager(module=module)
+        m1.execute_on_device = Mock(return_value=['resp1', 'resp2'])
+
         mm = ModuleManager(module=module)
-        mm._run_commands = Mock(return_value=[])
+        mm.get_manager = Mock(return_value=m1)
-        mm.execute_on_device = Mock(return_value=[])
 
         results = mm.exec_module()
 
         assert results['changed'] is True
-        assert mm._run_commands.call_count == 0
+        assert m1.execute_on_device.call_count == 2
-        assert mm.execute_on_device.call_count == 1
+
+    def test_normalizing_command_show(self, *args):
+        args = dict(
+            commands=[
+                "show sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'show sys version'
+
+    def test_normalizing_command_delete(self, *args):
+        args = dict(
+            commands=[
+                "delete sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'delete sys version'
+
+    def test_normalizing_command_modify(self, *args):
+        args = dict(
+            commands=[
+                "modify sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'modify sys version'
+
+    def test_normalizing_command_list(self, *args):
+        args = dict(
+            commands=[
+                "list sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'list sys version'
+
+    def test_normalizing_command_tmsh_show(self, *args):
+        args = dict(
+            commands=[
+                "tmsh show sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'show sys version'
+
+    def test_normalizing_command_tmsh_delete(self, *args):
+        args = dict(
+            commands=[
+                "tmsh delete sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'delete sys version'
+
+    def test_normalizing_command_tmsh_modify(self, *args):
+        args = dict(
+            commands=[
+                "tmsh modify sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'modify sys version'
+
+    def test_normalizing_command_tmsh_list(self, *args):
+        args = dict(
+            commands=[
+                "tmsh list sys version"
+            ],
+        )
+
+        result = V2Manager.normalize_commands(args['commands'])
+
+        assert result[0] == 'list sys version'

test/units/modules/network/f5/test_bigip_config.py

@@ -20,9 +20,9 @@ from ansible.compat.tests.mock import patch
 from ansible.module_utils.basic import AnsibleModule
 
 try:
-    from library.bigip_config import Parameters
+    from library.modules.bigip_config import Parameters
-    from library.bigip_config import ModuleManager
+    from library.modules.bigip_config import ModuleManager
-    from library.bigip_config import ArgumentSpec
+    from library.modules.bigip_config import ArgumentSpec
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     from test.unit.modules.utils import set_module_args

test/units/modules/network/f5/test_bigip_configsync_action.py

@@ -20,17 +20,17 @@ from ansible.compat.tests.mock import patch
 from ansible.module_utils.basic import AnsibleModule
 
 try:
-    from library.bigip_configsync_actions import Parameters
+    from library.modules.bigip_configsync_action import Parameters
-    from library.bigip_configsync_actions import ModuleManager
+    from library.modules.bigip_configsync_action import ModuleManager
-    from library.bigip_configsync_actions import ArgumentSpec
+    from library.modules.bigip_configsync_action import ArgumentSpec
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     from test.unit.modules.utils import set_module_args
 except ImportError:
     try:
-        from ansible.modules.network.f5.bigip_configsync_actions import Parameters
+        from ansible.modules.network.f5.bigip_configsync_action import Parameters
-        from ansible.modules.network.f5.bigip_configsync_actions import ModuleManager
+        from ansible.modules.network.f5.bigip_configsync_action import ModuleManager
-        from ansible.modules.network.f5.bigip_configsync_actions import ArgumentSpec
+        from ansible.modules.network.f5.bigip_configsync_action import ArgumentSpec
         from ansible.module_utils.network.f5.common import F5ModuleError
         from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError
         from units.modules.utils import set_module_args

test/units/modules/network/f5/test_bigip_device_connectivity.py

@@ -21,10 +21,10 @@ from ansible.compat.tests.mock import patch
 from ansible.module_utils.basic import AnsibleModule
 
 try:
-    from library.bigip_device_connectivity import ApiParameters
+    from library.modules.bigip_device_connectivity import ApiParameters
-    from library.bigip_device_connectivity import ModuleParameters
+    from library.modules.bigip_device_connectivity import ModuleParameters
-    from library.bigip_device_connectivity import ModuleManager
+    from library.modules.bigip_device_connectivity import ModuleManager
-    from library.bigip_device_connectivity import ArgumentSpec
+    from library.modules.bigip_device_connectivity import ArgumentSpec
     from library.module_utils.network.f5.common import F5ModuleError
     from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
     from test.unit.modules.utils import set_module_args