From 8037eb7474ad52a3d01c202206584fff424c4f1f Mon Sep 17 00:00:00 2001 From: Tim Rupp Date: Tue, 31 Oct 2017 12:32:50 -0700 Subject: [PATCH] Various fixes for bigip_remote_syslog (#32404) This patch addresses a number of issues, large and small, that were identified by users in the downstream repo. * formatting of some code * specific option combinations leading to errors * missing includes for unit tests --- .../modules/network/f5/bigip_remote_syslog.py | 96 +++++++++++++------ .../network/f5/test_bigip_remote_syslog.py | 21 +--- 2 files changed, 74 insertions(+), 43 deletions(-) diff --git a/lib/ansible/modules/network/f5/bigip_remote_syslog.py b/lib/ansible/modules/network/f5/bigip_remote_syslog.py index 0c466bf0a5..f789ffafed 100644 --- a/lib/ansible/modules/network/f5/bigip_remote_syslog.py +++ b/lib/ansible/modules/network/f5/bigip_remote_syslog.py @@ -8,11 +8,9 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type -ANSIBLE_METADATA = { - 'status': ['preview'], - 'supported_by': 'community', - 'metadata_version': '1.1' -} +ANSIBLE_METADATA = {'metadata_version': '1.1', + 'status': ['preview'], + 'supported_by': 'community'} DOCUMENTATION = r''' module: bigip_remote_syslog @@ -103,10 +101,6 @@ except ImportError: class Parameters(AnsibleF5Parameters): - api_map = { - 'remotePort': 'remote_port' - } - updatables = [ 'remote_port', 'local_ip', 'remoteServers' ] @@ -199,10 +193,22 @@ class Parameters(AnsibleF5Parameters): ) +class Changes(Parameters): + @property + def remote_port(self): + return self._values['remote_port'] + + @property + def local_ip(self): + return self._values['local_ip'] + + class Difference(object): def __init__(self, want, have=None): self.want = want self.have = have + self._local_ip = None + self._remote_port = None def compare(self, param): try: @@ -231,9 +237,13 @@ class Difference(object): """ changed = False - if self.have.remoteServers is None: + if self.want.remote_host is None: return None - current_hosts = dict((d['host'], d) for (i, d) in enumerate(self.have.remoteServers)) + if self.have.remoteServers is None: + remote = dict() + else: + remote = self.have.remoteServers + current_hosts = dict((d['host'], d) for (i, d) in enumerate(remote)) if self.want.state == 'absent': del current_hosts[self.want.remote_host] @@ -243,12 +253,14 @@ class Difference(object): if self.want.remote_host in current_hosts: item = current_hosts[self.want.remote_host] if self.want.remote_port is not None: - if item['remotePort'] != self.want.remote_port: + if int(item['remotePort']) != self.want.remote_port: item['remotePort'] = self.want.remote_port + self._remote_port = self.want.remote_port changed = True if self.want.local_ip is not None: if item['localIp'] != self.want.local_ip: item['localIp'] = self.want.local_ip + self._local_ip = self.want.local_ip changed = True else: changed = True @@ -260,11 +272,26 @@ class Difference(object): ) if self.want.remote_port is not None: current_hosts[host]['remotePort'] = self.want.remote_port + self._remote_port = self.want.remote_port if self.want.local_ip is not None: current_hosts[host]['localIp'] = self.want.local_ip + self._local_ip = self.want.local_ip if changed: result = [v for (k, v) in iteritems(current_hosts)] return result + return None + + @property + def remote_port(self): + _ = self.remoteServers + if self._remote_port: + return self._remote_port + + @property + def local_ip(self): + _ = self.remoteServers + if self._local_ip: + return self._local_ip class ModuleManager(object): @@ -272,7 +299,7 @@ class ModuleManager(object): self.client = client self.have = None self.want = Parameters(self.client.module.params) - self.changes = Parameters() + self.changes = Changes() def _set_changed_options(self): changed = {} @@ -280,7 +307,7 @@ class ModuleManager(object): if getattr(self.want, key) is not None: changed[key] = getattr(self.want, key) if changed: - self.changes = Parameters(changed) + self.changes = Changes(changed) self.changes.update({'remote_host': self.want.remote_host}) def _update_changed_options(self): @@ -292,9 +319,12 @@ class ModuleManager(object): if change is None: continue else: - changed[k] = change + if isinstance(change, dict): + changed.update(change) + else: + changed[k] = change if changed: - self.changes = Parameters(changed) + self.changes = Changes(changed) self.changes.update({'remote_host': self.want.remote_host}) return True return False @@ -420,26 +450,38 @@ class ArgumentSpec(object): self.f5_product_name = 'bigip' -def main(): +def cleanup_tokens(client): try: - spec = ArgumentSpec() - - client = AnsibleF5Client( - argument_spec=spec.argument_spec, - supports_check_mode=spec.supports_check_mode, - f5_product_name=spec.f5_product_name + resource = client.api.shared.authz.tokens_s.token.load( + name=client.api.icrs.token ) + resource.delete() + except Exception: + pass - if not HAS_F5SDK: - raise F5ModuleError("The python f5-sdk module is required") - if not HAS_NETADDR: - raise F5ModuleError("The python netaddr module is required") +def main(): + if not HAS_F5SDK: + raise F5ModuleError("The python f5-sdk module is required") + if not HAS_NETADDR: + raise F5ModuleError("The python netaddr module is required") + + spec = ArgumentSpec() + + client = AnsibleF5Client( + argument_spec=spec.argument_spec, + supports_check_mode=spec.supports_check_mode, + f5_product_name=spec.f5_product_name + ) + + try: mm = ModuleManager(client) results = mm.exec_module() + cleanup_tokens(client) client.module.exit_json(**results) except F5ModuleError as e: + cleanup_tokens(client) client.module.fail_json(msg=str(e)) diff --git a/test/units/modules/network/f5/test_bigip_remote_syslog.py b/test/units/modules/network/f5/test_bigip_remote_syslog.py index 0d9ffc5d29..f78c099fc0 100644 --- a/test/units/modules/network/f5/test_bigip_remote_syslog.py +++ b/test/units/modules/network/f5/test_bigip_remote_syslog.py @@ -1,21 +1,7 @@ # -*- 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 Liccense for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . +# 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 @@ -40,12 +26,15 @@ try: from library.bigip_remote_syslog import ArgumentSpec from library.bigip_remote_syslog import HAS_F5SDK from library.bigip_remote_syslog import HAS_NETADDR + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError except ImportError: try: from ansible.modules.network.f5.bigip_remote_syslog import Parameters from ansible.modules.network.f5.bigip_remote_syslog import ModuleManager from ansible.modules.network.f5.bigip_remote_syslog import ArgumentSpec from ansible.modules.network.f5.bigip_remote_syslog import HAS_F5SDK + from ansible.modules.network.f5.bigip_remote_syslog import HAS_NETADDR + from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError except ImportError: raise SkipTest("F5 Ansible modules require the f5-sdk Python library")