From a3b3dbe220ed5701be0ef6b0ca1722579373b4e4 Mon Sep 17 00:00:00 2001 From: Tim Rupp Date: Fri, 1 Dec 2017 20:14:10 -0800 Subject: [PATCH] Refactors bigip_device_ntp (#33472) * Refactors bigip_device_ntp To be inline with current f5 conventions * Fixed incorrect key --- .../modules/network/f5/bigip_device_ntp.py | 332 ++++++++++-------- .../modules/network/f5/fixtures/load_ntp.json | 13 + .../network/f5/test_bigip_device_ntp.py | 252 +++++++++++++ 3 files changed, 444 insertions(+), 153 deletions(-) create mode 100644 test/units/modules/network/f5/fixtures/load_ntp.json create mode 100644 test/units/modules/network/f5/test_bigip_device_ntp.py diff --git a/lib/ansible/modules/network/f5/bigip_device_ntp.py b/lib/ansible/modules/network/f5/bigip_device_ntp.py index ae425bebe05..c9c6d97c82e 100644 --- a/lib/ansible/modules/network/f5/bigip_device_ntp.py +++ b/lib/ansible/modules/network/f5/bigip_device_ntp.py @@ -3,31 +3,31 @@ # # Copyright (c) 2017 F5 Networks Inc. # GNU General Public License v3.0 (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +from __future__ import absolute_import, division, print_function +__metaclass__ = type + ANSIBLE_METADATA = {'metadata_version': '1.1', 'status': ['preview'], 'supported_by': 'community'} -DOCUMENTATION = ''' +DOCUMENTATION = r''' --- module: bigip_device_ntp short_description: Manage NTP servers on a BIG-IP description: - - Manage NTP servers on a BIG-IP + - Manage NTP servers on a BIG-IP. version_added: "2.2" options: ntp_servers: description: - A list of NTP servers to set on the device. At least one of C(ntp_servers) or C(timezone) is required. - required: false - default: [] state: description: - The state of the NTP servers on the system. When C(present), guarantees that the NTP servers are set on the system. When C(absent), removes the specified NTP servers from the device configuration. - required: false default: present choices: - absent @@ -37,7 +37,6 @@ options: - The timezone to set for NTP lookups. At least one of C(ntp_servers) or C(timezone) is required. default: UTC - required: false notes: - Requires the f5-sdk Python package on the host. This is as easy as pip install f5-sdk. @@ -46,204 +45,231 @@ requirements: - f5-sdk author: - Tim Rupp (@caphrim007) + - Wojciech Wypior (@wojtek0806) ''' -EXAMPLES = ''' +EXAMPLES = r''' - name: Set NTP server bigip_device_ntp: - ntp_servers: - - "192.0.2.23" - password: "secret" - server: "lb.mydomain.com" - user: "admin" - validate_certs: "no" + ntp_servers: + - 192.0.2.23 + password: secret + server: lb.mydomain.com + user: admin + validate_certs: no delegate_to: localhost - name: Set timezone bigip_device_ntp: - password: "secret" - server: "lb.mydomain.com" - timezone: "America/Los_Angeles" - user: "admin" - validate_certs: "no" + password: secret + server: lb.mydomain.com + timezone: America/Los_Angeles + user: admin + validate_certs: no delegate_to: localhost ''' -RETURN = ''' +RETURN = r''' ntp_servers: - description: The NTP servers that were set on the device - returned: changed - type: list - sample: ["192.0.2.23", "192.0.2.42"] + description: The NTP servers that were set on the device + returned: changed + type: list + sample: ["192.0.2.23", "192.0.2.42"] timezone: - description: The timezone that was set on the device - returned: changed - type: string - sample: "true" + description: The timezone that was set on the device + returned: changed + type: string + sample: true ''' + +from ansible.module_utils.f5_utils import AnsibleF5Client +from ansible.module_utils.f5_utils import AnsibleF5Parameters +from ansible.module_utils.f5_utils import HAS_F5SDK +from ansible.module_utils.f5_utils import F5ModuleError + try: - from f5.bigip import ManagementRoot - from icontrol.session import iControlUnexpectedHTTPError - HAS_F5SDK = True + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError except ImportError: HAS_F5SDK = False -class BigIpDeviceNtp(object): - def __init__(self, *args, **kwargs): - if not HAS_F5SDK: - raise F5ModuleError("The python f5-sdk module is required") +class Parameters(AnsibleF5Parameters): + api_map = { + 'servers': 'ntp_servers' + } - # The params that change in the module - self.cparams = dict() + api_attributes = [ + 'servers', 'timezone', + ] - # Stores the params that are sent to the module - self.params = kwargs - self.api = ManagementRoot(kwargs['server'], - kwargs['user'], - kwargs['password'], - port=kwargs['server_port']) + updatables = [ + 'ntp_servers', 'timezone' + ] - def flush(self): - result = dict() + returnables = [ + 'ntp_servers', 'timezone' + ] + + absentables = [ + 'ntp_servers' + ] + + def to_return(self): + result = {} + for returnable in self.returnables: + result[returnable] = getattr(self, returnable) + result = self._filter_params(result) + return result + + 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 + + +class ModuleManager(object): + def __init__(self, client): + self.client = client + self.have = None + self.want = Parameters(self.client.module.params) + self.changes = Parameters() + + 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 + if changed: + self.changes = Parameters(changed) + return True + return False + + def _absent_changed_options(self): + changed = {} + for key in Parameters.absentables: + if getattr(self.want, key) is not None: + set_want = set(getattr(self.want, key)) + set_have = set(getattr(self.have, key)) + if set_want != set_have: + changed[key] = list(set_want) + if changed: + self.changes = Parameters(changed) + return True + return False + + def exec_module(self): changed = False - state = self.params['state'] + result = dict() + state = self.want.state try: if state == "present": - changed = self.present() + changed = self.update() elif state == "absent": changed = self.absent() except iControlUnexpectedHTTPError as e: raise F5ModuleError(str(e)) - if 'servers' in self.cparams: - self.cparams['ntp_servers'] = self.cparams.pop('servers') - - result.update(**self.cparams) + changes = self.changes.to_return() + result.update(**changes) result.update(dict(changed=changed)) return result - def read(self): - """Read information and transform it + 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 - The values that are returned by BIG-IP in the f5-sdk can have encoding - attached to them as well as be completely missing in some cases. + def should_update(self): + result = self._update_changed_options() + if result: + return True + return False - Therefore, this method will transform the data from the BIG-IP into a - format that is more easily consumable by the rest of the class and the - parameters that are supported by the module. - """ - p = dict() - r = self.api.tm.sys.ntp.load() - - if hasattr(r, 'servers'): - # Deliberately using sets to suppress duplicates - p['servers'] = set([str(x) for x in r.servers]) - if hasattr(r, 'timezone'): - p['timezone'] = str(r.timezone) - return p - - def present(self): - changed = False - params = dict() - current = self.read() - - check_mode = self.params['check_mode'] - ntp_servers = self.params['ntp_servers'] - timezone = self.params['timezone'] - - # NTP servers can be set independently - if ntp_servers is not None: - if 'servers' in current: - items = set(ntp_servers) - if items != current['servers']: - params['servers'] = list(ntp_servers) - else: - params['servers'] = ntp_servers - - # Timezone can be set independently - if timezone is not None: - if 'timezone' in current and current['timezone'] != timezone: - params['timezone'] = timezone - - if params: - changed = True - self.cparams = camel_dict_to_snake_dict(params) - if check_mode: - return changed - else: - return changed - - r = self.api.tm.sys.ntp.load() - r.update(**params) - r.refresh() - - return changed + def should_absent(self): + result = self._absent_changed_options() + if result: + return True + return False def absent(self): - changed = False - params = dict() - current = self.read() + self.have = self.read_current_from_device() + if not self.should_absent(): + return False + if self.client.check_mode: + return True + self.absent_on_device() + return True - check_mode = self.params['check_mode'] - ntp_servers = self.params['ntp_servers'] + def update_on_device(self): + params = self.want.api_params() + resource = self.client.api.tm.sys.ntp.load() + resource.update(**params) - if not ntp_servers: - raise F5ModuleError( - "Absent can only be used when removing NTP servers" + def read_current_from_device(self): + resource = self.client.api.tm.sys.ntp.load() + result = resource.attrs + return Parameters(result) + + def absent_on_device(self): + params = self.changes.api_params() + resource = self.client.api.tm.sys.ntp.load() + resource.update(**params) + + +class ArgumentSpec(object): + def __init__(self): + self.supports_check_mode = True + self.argument_spec = dict( + ntp_servers=dict( + required=False, + default=None, + type='list', + ), + timezone=dict( + required=False, + default=None, ) - - if ntp_servers and 'servers' in current: - servers = current['servers'] - new_servers = [x for x in servers if x not in ntp_servers] - - if servers != new_servers: - params['servers'] = new_servers - - if params: - changed = True - self.cparams = camel_dict_to_snake_dict(params) - if check_mode: - return changed - else: - return changed - - r = self.api.tm.sys.ntp.load() - r.update(**params) - r.refresh() - return changed + ) + self.required_one_of = [ + ['ntp_servers', 'timezone'] + ] + self.f5_product_name = 'bigip' def main(): - argument_spec = f5_argument_spec() + if not HAS_F5SDK: + raise F5ModuleError("The python f5-sdk module is required") - meta_args = dict( - ntp_servers=dict(required=False, type='list', default=None), - timezone=dict(default=None, required=False) - ) - argument_spec.update(meta_args) + spec = ArgumentSpec() - module = AnsibleModule( - argument_spec=argument_spec, - required_one_of=[ - ['ntp_servers', 'timezone'] - ], - supports_check_mode=True + client = AnsibleF5Client( + argument_spec=spec.argument_spec, + supports_check_mode=spec.supports_check_mode, + f5_product_name=spec.f5_product_name, + required_one_of=spec.required_one_of ) try: - obj = BigIpDeviceNtp(check_mode=module.check_mode, **module.params) - result = obj.flush() - - module.exit_json(**result) + mm = ModuleManager(client) + results = mm.exec_module() + client.module.exit_json(**results) except F5ModuleError as e: - module.fail_json(msg=str(e)) - -from ansible.module_utils.basic import * -from ansible.module_utils.ec2 import camel_dict_to_snake_dict -from ansible.module_utils.f5_utils import * + client.module.fail_json(msg=str(e)) if __name__ == '__main__': main() diff --git a/test/units/modules/network/f5/fixtures/load_ntp.json b/test/units/modules/network/f5/fixtures/load_ntp.json new file mode 100644 index 00000000000..3b55b4870ed --- /dev/null +++ b/test/units/modules/network/f5/fixtures/load_ntp.json @@ -0,0 +1,13 @@ +{ + "kind": "tm:sys:ntp:ntpstate", + "selfLink": "https://localhost/mgmt/tm/sys/ntp?ver=12.1.0", + "servers": [ + "192.168.1.1", + "192.168.1.2" + ], + "timezone": "America/Los_Angeles", + "restrictReference": { + "link": "https://localhost/mgmt/tm/sys/ntp/restrict?ver=12.1.0", + "isSubcollection": true + } +} \ No newline at end of file diff --git a/test/units/modules/network/f5/test_bigip_device_ntp.py b/test/units/modules/network/f5/test_bigip_device_ntp.py new file mode 100644 index 00000000000..8d9d089a89d --- /dev/null +++ b/test/units/modules/network/f5/test_bigip_device_ntp.py @@ -0,0 +1,252 @@ +# -*- coding: utf-8 -*- +# +# Copyright (c) 2017 F5 Networks Inc. +# GNU General Public License v3.0 (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +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 Mock +from ansible.compat.tests.mock import patch +from ansible.module_utils.f5_utils import AnsibleF5Client + +try: + from library.bigip_device_ntp import Parameters + from library.bigip_device_ntp import ModuleManager + from library.bigip_device_ntp import ArgumentSpec + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError + from test.unit.modules.utils import set_module_args +except ImportError: + try: + from ansible.modules.network.f5.bigip_device_ntp import Parameters + from ansible.modules.network.f5.bigip_device_ntp import ModuleManager + from ansible.modules.network.f5.bigip_device_ntp import ArgumentSpec + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError + from units.modules.utils import set_module_args + 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 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): + ntp = ['192.168.1.1', '192.168.1.2'] + args = dict( + ntp_servers=ntp, + timezone='Arctic/Longyearbyen' + ) + + p = Parameters(args) + assert p.ntp_servers == ntp + assert p.timezone == 'Arctic/Longyearbyen' + + def test_api_parameters(self): + ntp = ['192.168.1.1', '192.168.1.2'] + args = dict( + servers=ntp, + timezone='Arctic/Longyearbyen' + ) + + p = Parameters(args) + assert p.ntp_servers == ntp + assert p.timezone == 'Arctic/Longyearbyen' + + +@patch('ansible.module_utils.f5_utils.AnsibleF5Client._get_mgmt_root', + return_value=True) +class TestModuleManager(unittest.TestCase): + + def setUp(self): + self.spec = ArgumentSpec() + + def test_update_ntp_servers(self, *args): + ntp = ['10.1.1.1', '10.1.1.2'] + set_module_args( + dict( + ntp_servers=ntp, + server='localhost', + user='admin', + password='password' + ) + ) + + # Configure the parameters that would be returned by querying the + # remote device + current = Parameters( + load_fixture('load_ntp.json') + ) + + client = AnsibleF5Client( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode, + f5_product_name=self.spec.f5_product_name + ) + mm = ModuleManager(client) + + # Override methods to force specific logic in the module to happen + 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['ntp_servers'] == ntp + + def test_update_timezone(self, *args): + set_module_args( + dict( + timezone='Arctic/Longyearbyen', + server='localhost', + user='admin', + password='password' + ) + ) + + # Configure the parameters that would be returned by querying the + # remote device + current = Parameters( + load_fixture('load_ntp.json') + ) + + client = AnsibleF5Client( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode, + f5_product_name=self.spec.f5_product_name + ) + mm = ModuleManager(client) + + # Override methods to force specific logic in the module to happen + 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['timezone'] == 'Arctic/Longyearbyen' + + def test_update_ntp_servers_and_timezone(self, *args): + ntp = ['10.1.1.1', '10.1.1.2'] + set_module_args( + dict( + ntp_servers=ntp, + timezone='Arctic/Longyearbyen', + server='localhost', + user='admin', + password='password' + ) + ) + + # Configure the parameters that would be returned by querying the + # remote device + current = Parameters( + load_fixture('load_ntp.json') + ) + + client = AnsibleF5Client( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode, + f5_product_name=self.spec.f5_product_name + ) + mm = ModuleManager(client) + + # Override methods to force specific logic in the module to happen + 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['ntp_servers'] == ntp + assert results['timezone'] == 'Arctic/Longyearbyen' + + def test_absent_ntp_servers(self, *args): + ntp = [] + set_module_args( + dict( + ntp_servers=ntp, + timezone='America/Los_Angeles', + server='localhost', + user='admin', + password='password', + state='absent' + ) + ) + + # Configure the parameters that would be returned by querying the + # remote device + current = Parameters( + load_fixture('load_ntp.json') + ) + + client = AnsibleF5Client( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode, + f5_product_name=self.spec.f5_product_name + ) + mm = ModuleManager(client) + + # Override methods to force specific logic in the module to happen + mm.absent_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['ntp_servers'] == ntp + assert not hasattr(results, 'timezone') + + def test_absent_timezone(self, *args): + set_module_args( + dict( + timezone='', + server='localhost', + user='admin', + password='password', + state='absent' + ) + ) + + # Configure the parameters that would be returned by querying the + # remote device + current = Parameters( + load_fixture('load_ntp.json') + ) + + client = AnsibleF5Client( + argument_spec=self.spec.argument_spec, + supports_check_mode=self.spec.supports_check_mode, + f5_product_name=self.spec.f5_product_name + ) + mm = ModuleManager(client) + + # Override methods to force specific logic in the module to happen + mm.absent_on_device = Mock(return_value=True) + mm.read_current_from_device = Mock(return_value=current) + + results = mm.exec_module() + assert results['changed'] is False