Adds refactored bigip_hostname (#25841)

This adds a refactored bigip_hostname with unit tests and code
conventions that are common across other modules.

Unit tests are provided. Integration tests can be found here

https://github.com/F5Networks/f5-ansible/blob/devel/test/integration/bigip_hostname.yaml#L23
https://github.com/F5Networks/f5-ansible/tree/devel/test/integration/targets/bigip_hostname/tasks

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

@@ -18,10 +18,11 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
-ANSIBLE_METADATA = {'metadata_version': '1.0',
+ANSIBLE_METADATA = {
     'status': ['preview'],
-                    'supported_by': 'community'}
+    'supported_by': 'community',
-
+    'metadata_version': '1.0'
+}
 
 DOCUMENTATION = '''
 ---
@@ -34,7 +35,7 @@ options:
   hostname:
     description:
       - Hostname of the BIG-IP host.
-    required: true
+    required: True
 notes:
   - Requires the f5-sdk Python package on the host. This is as easy as pip
     install f5-sdk.
@@ -43,6 +44,7 @@ requirements:
   - f5-sdk
 author:
   - Tim Rupp (@caphrim007)
+  - Matthew Lam (@mryanlam)
 '''
 
 EXAMPLES = '''
@@ -63,127 +65,146 @@ hostname:
     sample: "big-ip01.internal"
 '''
 
-try:
+from ansible.module_utils.f5_utils import (
-    from f5.bigip.contexts import TransactionContextManager
+    AnsibleF5Client,
-    from f5.bigip import ManagementRoot
+    AnsibleF5Parameters,
-    from icontrol.session import iControlUnexpectedHTTPError
+    HAS_F5SDK,
-    HAS_F5SDK = True
+    F5ModuleError,
-except ImportError:
+    iControlUnexpectedHTTPError
-    HAS_F5SDK = False
+)
 
 
-class BigIpHostnameManager(object):
+class Parameters(AnsibleF5Parameters):
-    def __init__(self, *args, **kwargs):
+    api_attributes = ['hostname']
-        self.changed_params = dict()
+    updatables = ['hostname']
-        self.params = kwargs
+    returnables = ['hostname']
-        self.api = None
 
-    def connect_to_bigip(self, **kwargs):
+    def to_return(self):
-        return ManagementRoot(kwargs['server'],
+        result = {}
-                              kwargs['user'],
+        for returnable in self.returnables:
-                              kwargs['password'],
+            result[returnable] = getattr(self, returnable)
-                              port=kwargs['server_port'])
+        result = self._filter_params(result)
-
-    def ensure_hostname_is_present(self):
-        self.changed_params['hostname'] = self.params['hostname']
-
-        if self.params['check_mode']:
-            return True
-
-        tx = self.api.tm.transactions.transaction
-        with TransactionContextManager(tx) as api:
-            r = api.tm.sys.global_settings.load()
-            r.update(hostname=self.params['hostname'])
-
-        if self.hostname_exists():
-            return True
-        else:
-            raise F5ModuleError("Failed to set the hostname")
-
-    def hostname_exists(self):
-        if self.params['hostname'] == self.current_hostname():
-            return True
-        else:
-            return False
-
-    def present(self):
-        if self.hostname_exists():
-            return False
-        else:
-
-            return self.ensure_hostname_is_present()
-
-    def current_hostname(self):
-        r = self.api.tm.sys.global_settings.load()
-        return r.hostname
-
-    def apply_changes(self):
-        result = dict()
-
-        changed = self.apply_to_running_config()
-        if changed:
-            self.save_running_config()
-
-        result.update(**self.changed_params)
-        result.update(dict(changed=changed))
         return result
 
-    def apply_to_running_config(self):
+    def api_params(self):
+        result = {}
+        for api_attribute in self.api_attributes:
+            if self.api_map is not None and api_attribute in self.api_map:
+                result[api_attribute] = getattr(self, self.api_map[api_attribute])
+            else:
+                result[api_attribute] = getattr(self, api_attribute)
+        result = self._filter_params(result)
+        return result
+
+    @property
+    def hostname(self):
+        if self._values['hostname'] is None:
+            return None
+        return str(self._values['hostname'])
+
+
+class ModuleManager(object):
+    def __init__(self, client):
+        self.client = client
+        self.have = None
+        self.want = Parameters(self.client.module.params)
+        self.changes = Parameters()
+
+    def _set_changed_options(self):
+        changed = {}
+        for key in Parameters.returnables:
+            if getattr(self.want, key) is not None:
+                changed[key] = getattr(self.want, key)
+        if changed:
+            self.changes = Parameters(changed)
+
+    def _update_changed_options(self):
+        changed = {}
+        for key in Parameters.updatables:
+            if getattr(self.want, key) is not None:
+                attr1 = getattr(self.want, key)
+                attr2 = getattr(self.have, key)
+                if attr1 != attr2:
+                    changed[key] = attr1
+        self.changes = Parameters(changed)
+        if changed:
+            return True
+        return False
+
+    def exec_module(self):
+        result = dict()
+
         try:
-            self.api = self.connect_to_bigip(**self.params)
+            changed = self.update()
-            return self.present()
         except iControlUnexpectedHTTPError as e:
             raise F5ModuleError(str(e))
 
-    def save_running_config(self):
+        changes = self.changes.to_return()
-        self.api.tm.sys.config.exec_cmd('save')
+        result.update(**changes)
+        result.update(dict(changed=changed))
+        return result
+
+    def read_current_from_device(self):
+        resource = self.client.api.tm.sys.global_settings.load()
+        result = resource.attrs
+        return Parameters(result)
+
+    def update(self):
+        self.have = self.read_current_from_device()
+        if not self.should_update():
+            return False
+        if self.client.check_mode:
+            return True
+        self.update_on_device()
+        return True
+
+    def should_update(self):
+        result = self._update_changed_options()
+        if result:
+            return True
+        return False
+
+    def update_on_device(self):
+        params = self.want.api_params()
+        resource = self.client.api.tm.sys.global_settings.load()
+        resource.modify(**params)
+        self.client.api.tm.cm.devices.exec_cmd(
+            'mv', name=self.have.hostname, target=self.want.hostname
+        )
 
 
-class BigIpHostnameModuleConfig(object):
+class ArgumentSpec(object):
     def __init__(self):
-        self.argument_spec = dict()
-        self.meta_args = dict()
         self.supports_check_mode = True
-
+        self.argument_spec = dict(
-        self.initialize_meta_args()
+            hostname=dict(
-        self.initialize_argument_spec()
+                required=True,
-
+                default=None,
-    def initialize_meta_args(self):
+                type='str'
-        args = dict(
-            hostname=dict(required=True)
             )
-        self.meta_args = args
-
-    def initialize_argument_spec(self):
-        self.argument_spec = f5_argument_spec()
-        self.argument_spec.update(self.meta_args)
-
-    def create(self):
-        return AnsibleModule(
-            argument_spec=self.argument_spec,
-            supports_check_mode=self.supports_check_mode
         )
+        self.f5_product_name = 'bigip'
 
 
 def main():
     if not HAS_F5SDK:
         raise F5ModuleError("The python f5-sdk module is required")
 
-    config = BigIpHostnameModuleConfig()
+    spec = ArgumentSpec()
-    module = config.create()
+
+    client = AnsibleF5Client(
+        argument_spec=spec.argument_spec,
+        supports_check_mode=spec.supports_check_mode,
+        f5_product_name=spec.f5_product_name
+    )
 
     try:
-        obj = BigIpHostnameManager(
+        mm = ModuleManager(client)
-            check_mode=module.check_mode, **module.params
+        results = mm.exec_module()
-        )
+        client.module.exit_json(**results)
-        result = obj.apply_changes()
-
-        module.exit_json(**result)
     except F5ModuleError as e:
-        module.fail_json(msg=str(e))
+        client.module.fail_json(msg=str(e))
 
-from ansible.module_utils.basic import *
-from ansible.module_utils.f5_utils import *
 
 if __name__ == '__main__':
     main()

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

@@ -0,0 +1,121 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright 2017 F5 Networks Inc.
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+import os
+import json
+import sys
+
+from nose.plugins.skip import SkipTest
+if sys.version_info < (2, 7):
+    raise SkipTest("F5 Ansible modules require Python >= 2.7")
+
+from ansible.compat.tests import unittest
+from ansible.compat.tests.mock import patch, Mock
+from ansible.module_utils import basic
+from ansible.module_utils._text import to_bytes
+from ansible.module_utils.f5_utils import AnsibleF5Client
+
+try:
+    from library.bigip_hostname import Parameters
+    from library.bigip_hostname import ModuleManager
+    from library.bigip_hostname import ArgumentSpec
+except ImportError:
+    try:
+        from ansible.modules.network.f5.bigip_hostname import Parameters
+        from ansible.modules.network.f5.bigip_hostname import ModuleManager
+        from ansible.modules.network.f5.bigip_hostname import ArgumentSpec
+    except ImportError:
+        raise SkipTest("F5 Ansible modules require the f5-sdk Python library")
+
+fixture_path = os.path.join(os.path.dirname(__file__), 'fixtures')
+fixture_data = {}
+
+
+def set_module_args(args):
+    args = json.dumps({'ANSIBLE_MODULE_ARGS': args})
+    basic._ANSIBLE_ARGS = to_bytes(args)
+
+
+def load_fixture(name):
+    path = os.path.join(fixture_path, name)
+
+    if path in fixture_data:
+        return fixture_data[path]
+
+    with open(path) as f:
+        data = f.read()
+
+    try:
+        data = json.loads(data)
+    except Exception:
+        pass
+
+    fixture_data[path] = data
+    return data
+
+
+class TestParameters(unittest.TestCase):
+    def test_module_parameters(self):
+        args = dict(
+            hostname='foo.internal.com'
+        )
+        p = Parameters(args)
+        assert p.hostname == 'foo.internal.com'
+
+
+@patch('ansible.module_utils.f5_utils.AnsibleF5Client._get_mgmt_root',
+       return_value=True)
+class TestManager(unittest.TestCase):
+
+    def setUp(self):
+        self.spec = ArgumentSpec()
+
+    def test_update_hostname(self, *args):
+        set_module_args(dict(
+            hostname='foo2.internal.com',
+            password='passsword',
+            server='localhost',
+            user='admin'
+        ))
+
+        # Configure the parameters that would be returned by querying the
+        # remote device
+        current = Parameters(
+            dict(
+                hostname='foo.internal.com'
+            )
+        )
+        client = AnsibleF5Client(
+            argument_spec=self.spec.argument_spec,
+            supports_check_mode=self.spec.supports_check_mode,
+            f5_product_name=self.spec.f5_product_name
+        )
+
+        # Override methods to force specific logic in the module to happen
+        mm = ModuleManager(client)
+        mm.update_on_device = Mock(return_value=True)
+        mm.read_current_from_device = Mock(return_value=current)
+
+        results = mm.exec_module()
+
+        assert results['changed'] is True
+        assert results['hostname'] == 'foo2.internal.com'