Corrects parameters to flatten correctly. (#48531)

Also fixes unit tests and adds other code patterns that other f5
modules use

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

@@ -201,48 +201,28 @@ class Parameters(AnsibleF5Parameters):
             )
         return timeout
 
+    @property
+    def match_across_pools(self):
+        return flatten_boolean(self._values['match_across_pools'])
+
+    @property
+    def match_across_services(self):
+        return flatten_boolean(self._values['match_across_services'])
+
+    @property
+    def match_across_virtuals(self):
+        return flatten_boolean(self._values['match_across_virtuals'])
+
+    @property
+    def override_connection_limit(self):
+        return flatten_boolean(self._values['override_connection_limit'])
+
 
 class ApiParameters(Parameters):
     pass
 
 
 class ModuleParameters(Parameters):
-    @property
-    def match_across_pools(self):
-        result = flatten_boolean(self._values['match_across_pools'])
-        if result is None:
-            return None
-        if result == 'yes':
-            return 'enabled'
-        return 'disabled'
-
-    @property
-    def match_across_services(self):
-        result = flatten_boolean(self._values['match_across_services'])
-        if result is None:
-            return None
-        if result == 'yes':
-            return 'enabled'
-        return 'disabled'
-
-    @property
-    def match_across_virtuals(self):
-        result = flatten_boolean(self._values['match_across_virtuals'])
-        if result is None:
-            return None
-        if result == 'yes':
-            return 'enabled'
-        return 'disabled'
-
-    @property
-    def override_connection_limit(self):
-        result = flatten_boolean(self._values['override_connection_limit'])
-        if result is None:
-            return None
-        if result == 'yes':
-            return 'enabled'
-        return 'disabled'
-
     @property
     def parent(self):
         if self._values['parent'] is None:
@@ -264,41 +244,55 @@ class Changes(Parameters):
 
 
 class UsableChanges(Changes):
-    pass
+    @property
+    def match_across_pools(self):
+        if self._values['match_across_pools'] is None:
+            return None
+        elif self._values['match_across_pools'] == 'yes':
+            return 'enabled'
+        return 'disabled'
 
-
-class ReportableChanges(Changes):
     @property
     def match_across_services(self):
         if self._values['match_across_services'] is None:
             return None
-        elif self._values['match_across_services'] == 'enabled':
+        elif self._values['match_across_services'] == 'yes':
-            return 'yes'
+            return 'enabled'
-        return 'no'
+        return 'disabled'
 
     @property
     def match_across_virtuals(self):
         if self._values['match_across_virtuals'] is None:
             return None
-        elif self._values['match_across_virtuals'] == 'enabled':
+        elif self._values['match_across_virtuals'] == 'yes':
-            return 'yes'
+            return 'enabled'
-        return 'no'
+        return 'disabled'
-
-    @property
-    def match_across_pools(self):
-        if self._values['match_across_pools'] is None:
-            return None
-        elif self._values['match_across_pools'] == 'enabled':
-            return 'yes'
-        return 'no'
 
     @property
     def override_connection_limit(self):
         if self._values['override_connection_limit'] is None:
             return None
-        elif self._values['override_connection_limit'] == 'enabled':
+        elif self._values['override_connection_limit'] == 'yes':
-            return 'yes'
+            return 'enabled'
-        return 'no'
+        return 'disabled'
+
+
+class ReportableChanges(Changes):
+    @property
+    def match_across_pools(self):
+        return flatten_boolean(self._values['match_across_pools'])
+
+    @property
+    def match_across_services(self):
+        return flatten_boolean(self._values['match_across_services'])
+
+    @property
+    def match_across_virtuals(self):
+        return flatten_boolean(self._values['match_across_virtuals'])
+
+    @property
+    def override_connection_limit(self):
+        return flatten_boolean(self._values['override_connection_limit'])
 
 
 class Difference(object):
@@ -326,7 +320,7 @@ class Difference(object):
     def parent(self):
         if self.want.parent != self.have.parent:
             raise F5ModuleError(
-                "The parent monitor cannot be changed"
+                "The parent profile cannot be changed"
             )
 
 
@@ -459,7 +453,6 @@ class ModuleManager(object):
                 raise F5ModuleError(response['message'])
             else:
                 raise F5ModuleError(resp.content)
-        return response['selfLink']
 
     def update_on_device(self):
         params = self.changes.api_params()
@@ -479,7 +472,6 @@ class ModuleManager(object):
                 raise F5ModuleError(response['message'])
             else:
                 raise F5ModuleError(resp.content)
-        return response['selfLink']
 
     def absent(self):
         if self.exists():
@@ -497,7 +489,7 @@ class ModuleManager(object):
             return True
         raise F5ModuleError(resp.content)
 
-    def read_current_from_device(self):
+    def read_current_from_device(self):  # lgtm [py/similar-function]
         uri = "https://{0}:{1}/mgmt/tm/ltm/persistence/source-addr/{2}".format(
             self.client.provider['server'],
             self.client.provider['server_port'],
@@ -551,8 +543,9 @@ def main():
         supports_check_mode=spec.supports_check_mode,
     )
 
+    client = F5RestClient(**module.params)
+
     try:
-        client = F5RestClient(**module.params)
         mm = ModuleManager(module=module, client=client)
         results = mm.exec_module()
         cleanup_tokens(client)

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

@@ -8,16 +8,12 @@ __metaclass__ = type
 
 import os
 import json
-import pytest
 import sys
 
 from nose.plugins.skip import SkipTest
 if sys.version_info < (2, 7):
     raise SkipTest("F5 Ansible modules require Python >= 2.7")
 
-from units.compat import unittest
-from units.compat.mock import Mock
-from units.compat.mock import patch
 from ansible.module_utils.basic import AnsibleModule
 
 try:
@@ -25,17 +21,25 @@ try:
     from library.modules.bigip_profile_persistence_src_addr import ModuleParameters
     from library.modules.bigip_profile_persistence_src_addr import ModuleManager
     from library.modules.bigip_profile_persistence_src_addr import ArgumentSpec
-    from library.module_utils.network.f5.common import F5ModuleError
+
-    from library.module_utils.network.f5.common import iControlUnexpectedHTTPError
+    # In Ansible 2.8, Ansible changed import paths.
-    from test.unit.modules.utils import set_module_args
+    from test.units.compat import unittest
+    from test.units.compat.mock import Mock
+    from test.units.compat.mock import patch
+
+    from test.units.modules.utils import set_module_args
 except ImportError:
     try:
         from ansible.modules.network.f5.bigip_profile_persistence_src_addr import ApiParameters
         from ansible.modules.network.f5.bigip_profile_persistence_src_addr import ModuleParameters
         from ansible.modules.network.f5.bigip_profile_persistence_src_addr import ModuleManager
         from ansible.modules.network.f5.bigip_profile_persistence_src_addr import ArgumentSpec
-        from ansible.module_utils.network.f5.common import F5ModuleError
+
-        from ansible.module_utils.network.f5.common import iControlUnexpectedHTTPError
+        # Ansible 2.8 imports
+        from units.compat import unittest
+        from units.compat.mock import Mock
+        from units.compat.mock import patch
+
         from units.modules.utils import set_module_args
     except ImportError:
         raise SkipTest("F5 Ansible modules require the f5-sdk Python library")
@@ -78,20 +82,20 @@ class TestParameters(unittest.TestCase):
         p = ModuleParameters(params=args)
         assert p.name == 'foo'
         assert p.parent == '/Common/bar'
-        assert p.match_across_services == 'disabled'
+        assert p.match_across_services == 'no'
-        assert p.match_across_virtuals == 'enabled'
+        assert p.match_across_virtuals == 'yes'
-        assert p.match_across_pools == 'disabled'
+        assert p.match_across_pools == 'no'
         assert p.hash_algorithm == 'carp'
         assert p.entry_timeout == 100
-        assert p.override_connection_limit == 'enabled'
+        assert p.override_connection_limit == 'yes'
 
     def test_api_parameters(self):
         args = load_fixture('load_ltm_profile_persistence_src_addr_1.json')
         p = ApiParameters(params=args)
         assert p.name == 'source_addr'
-        assert p.match_across_pools == 'disabled'
+        assert p.match_across_pools == 'no'
-        assert p.match_across_services == 'disabled'
+        assert p.match_across_services == 'no'
-        assert p.match_across_virtuals == 'disabled'
+        assert p.match_across_virtuals == 'no'
 
 
 class TestManager(unittest.TestCase):