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

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 = {
+ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['preview'],
-    'supported_by': 'community',
-    'metadata_version': '1.1'
-}
+                    '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):
@@ -291,10 +318,13 @@ class ModuleManager(object):
             change = diff.compare(k)
             if change is None:
                 continue
+            else:
+                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
 
+
+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))
 
 

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 <http://www.gnu.org/licenses/>.
+# 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")