Fixes and additions to bigip_pool (#34205)

Removes deprecated host/port params. Adds metadata param. Adds new
classes to better support refactoring and maintenance.

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

@@ -94,21 +94,20 @@ options:
       - reset
       - drop
       - reselect
-  host:
-    description:
-      - Pool member IP.
-      - Deprecated in 2.4. Use the C(bigip_pool_member) module instead.
-    aliases:
-      - address
-  port:
-    description:
-      - Pool member port.
-      - Deprecated in 2.4. Use the C(bigip_pool_member) module instead.
   partition:
     description:
       - Device partition to manage resources on.
     default: Common
     version_added: 2.5
+  metdata:
+    description:
+      - Arbitrary key/value pairs that you can attach to a pool. This is useful in
+        situations where you might want to annotate a pool to me managed by Ansible.
+      - Key names will be stored as strings; this includes names that are numbers.
+      - Values for all of the keys will be stored as strings; this includes values
+        that are numbers.
+      - Data will be persisted, not ephemeral.
+    version_added: 2.5
 notes:
   - Requires BIG-IP software version >= 12.
   - F5 developed module 'F5-SDK' required (https://github.com/F5Networks/f5-common-python).
@@ -131,7 +130,7 @@ EXAMPLES = r'''
     state: present
     name: my-pool
     partition: Common
-    lb_method: least_connection_member
+    lb_method: least-connection-member
     slow_ramp_time: 120
   delegate_to: localhost
 
@@ -143,16 +142,16 @@ EXAMPLES = r'''
     state: present
     name: my-pool
     partition: Common
-    lb_method: round_robin
+    lb_method: round-robin
   delegate_to: localhost
 
 - name: Add pool member
-  bigip_pool:
+  bigip_pool_member:
     server: lb.mydomain.com
     user: admin
     password: secret
     state: present
-    name: my-pool
+    pool: my-pool
     partition: Common
     host: "{{ ansible_default_ipv4['address'] }}"
     port: 80
@@ -213,12 +212,12 @@ EXAMPLES = r'''
   delegate_to: localhost
 
 - name: Remove pool member from pool
-  bigip_pool:
+  bigip_pool_member:
     server: lb.mydomain.com
     user: admin
     password: secret
     state: absent
-    name: my-pool
+    pool: my-pool
     partition: Common
     host: "{{ ansible_default_ipv4['address'] }}"
     port: 80
@@ -233,6 +232,19 @@ EXAMPLES = r'''
     name: my-pool
     partition: Common
   delegate_to: localhost
+
+- name: Add metadata to pool
+  bigip_pool:
+    server: lb.mydomain.com
+    user: admin
+    password: secret
+    state: absent
+    name: my-pool
+    partition: Common
+    metadata:
+      ansible: 2.4
+      updated_at: 2017-12-20T17:50:46Z
+  delegate_to: localhost
 '''
 
 RETURN = r'''
@@ -266,16 +278,6 @@ lb_method:
   returned: changed
   type: string
   sample: round-robin
-host:
-  description: IP of pool member included in pool.
-  returned: changed
-  type: string
-  sample: 10.10.10.10
-port:
-  description: Port of pool member included in pool.
-  returned: changed
-  type: int
-  sample: 80
 slow_ramp_time:
   description: The new value that is set for the slow ramp-up time.
   returned: changed
@@ -286,6 +288,11 @@ reselect_tries:
   returned: changed
   type: int
   sample: 10
+metadata:
+  description: The new value of the pool.
+  returned: changed
+  type: dict
+  sample: {'key1': 'foo', 'key2': 'bar'}
 '''
 
 import re
@@ -320,19 +327,19 @@ class Parameters(AnsibleF5Parameters):
 
     api_attributes = [
         'description', 'name', 'loadBalancingMode', 'monitor', 'slowRampTime',
-        'reselectTries', 'serviceDownAction'
+        'reselectTries', 'serviceDownAction', 'metadata'
     ]
 
     returnables = [
         'monitor_type', 'quorum', 'monitors', 'service_down_action',
-        'description', 'lb_method', 'host', 'port', 'slow_ramp_time',
+        'description', 'lb_method', 'slow_ramp_time',
-        'reselect_tries', 'monitor', 'member_name', 'name', 'partition'
+        'reselect_tries', 'monitor', 'name', 'partition', 'metadata'
     ]
 
     updatables = [
         'monitor_type', 'quorum', 'monitors', 'service_down_action',
         'description', 'lb_method', 'slow_ramp_time', 'reselect_tries',
-        'host', 'port'
+        'metadata'
     ]
 
     def __init__(self, params=None):
@@ -366,36 +373,11 @@ class Parameters(AnsibleF5Parameters):
 
     @property
     def lb_method(self):
-        lb_map = {
-            'ratio_node_address': 'ratio-node',
-            'dynamic_ratio': 'dynamic-ratio-node',
-            'least_connection_member': 'least-connections-member',
-            'least_connection_node_address': 'least-connections-node',
-            'fastest_node_address': 'fastest-node',
-            'observed_node_address': 'observed-node',
-            'predictive_node_address': 'predictive-node',
-            'weighted_least_connection_member': 'weighted-least-connections-member',
-            'weighted_least_connection_node_address': 'weighted-least-connections-node',
-            'ratio_least_connection_member': 'ratio-least-connections-member',
-            'ratio_least_connection_node_address': 'ratio-least-connections-node'
-        }
         lb_method = self._values['lb_method']
         if lb_method is None:
             return None
 
         spec = ArgumentSpec()
-        if lb_method in spec.lb_choice_removed:
-            raise F5ModuleError(
-                "The provided lb_method is not supported"
-            )
-        if lb_method in spec.lb_choice_deprecated:
-            self._values['__warnings'].append(
-                dict(
-                    msg="The provided lb_method '{0}' is deprecated".format(lb_method),
-                    version='2.4'
-                )
-            )
-            lb_method = lb_map.get(lb_method, lb_method.replace('_', '-'))
         if lb_method not in spec.lb_choice:
             raise F5ModuleError('Provided lb_method is unknown')
         return lb_method
@@ -405,16 +387,6 @@ class Parameters(AnsibleF5Parameters):
             return '/{0}/{1}'.format(self.partition, value)
         return value
 
-    @property
-    def monitors_list(self):
-        if self._values['monitors'] is None:
-            return []
-        try:
-            result = re.findall(r'/\w+/[^\s}]+', self._values['monitors'])
-            return result
-        except Exception:
-            return self._values['monitors']
-
     @property
     def monitors(self):
         if self._values['monitors'] is None:
@@ -425,89 +397,6 @@ class Parameters(AnsibleF5Parameters):
             result = 'min %s of { %s }' % (self.quorum, monitors)
         else:
             result = ' and '.join(monitors).strip()
-
-        return result
-
-    @property
-    def quorum(self):
-        if self.kind == 'tm:ltm:pool:poolstate':
-            if self._values['monitors'] is None:
-                return None
-            pattern = r'min\s+(?P<quorum>\d+)\s+of'
-            matches = re.search(pattern, self._values['monitors'])
-            if matches:
-                quorum = matches.group('quorum')
-            else:
-                quorum = None
-        else:
-            quorum = self._values['quorum']
-        try:
-            if quorum is None:
-                return None
-            return int(quorum)
-        except ValueError:
-            raise F5ModuleError(
-                "The specified 'quorum' must be an integer."
-            )
-
-    @property
-    def monitor_type(self):
-        if self.kind == 'tm:ltm:pool:poolstate':
-            if self._values['monitors'] is None:
-                return None
-            pattern = r'min\s+\d+\s+of'
-            matches = re.search(pattern, self._values['monitors'])
-            if matches:
-                return 'm_of_n'
-            else:
-                return 'and_list'
-        else:
-            if self._values['monitor_type'] is None:
-                return None
-            return self._values['monitor_type']
-
-    @property
-    def host(self):
-        value = self._values['host']
-        if value is None:
-            return None
-        msg = "'%s' is not a valid IP address" % value
-        try:
-            IPAddress(value)
-        except AddrFormatError:
-            raise F5ModuleError(msg)
-        return value
-
-    @host.setter
-    def host(self, value):
-        self._values['host'] = value
-
-    @property
-    def port(self):
-        value = self._values['port']
-        if value is None:
-            return None
-        msg = "The provided port '%s' must be between 0 and 65535" % value
-        if value < 0 or value > 65535:
-            raise F5ModuleError(msg)
-        return value
-
-    @port.setter
-    def port(self, value):
-        self._values['port'] = value
-
-    @property
-    def member_name(self):
-        if self.host is None or self.port is None:
-            return None
-        mname = str(self.host) + ':' + str(self.port)
-        return mname
-
-    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):
@@ -522,11 +411,145 @@ class Parameters(AnsibleF5Parameters):
         result = self._filter_params(result)
         return result
 
+    def _verify_quorum_type(self, quorum):
+        try:
+            if quorum is None:
+                return None
+            return int(quorum)
+        except ValueError:
+            raise F5ModuleError(
+                "The specified 'quorum' must be an integer."
+            )
+
+
+class ApiParameters(Parameters):
+    @property
+    def quorum(self):
+        if self._values['monitors'] is None:
+            return None
+        pattern = r'min\s+(?P<quorum>\d+)\s+of'
+        matches = re.search(pattern, self._values['monitors'])
+        if matches:
+            quorum = matches.group('quorum')
+        else:
+            quorum = None
+        result = self._verify_quorum_type(quorum)
+        return result
+
+    @property
+    def monitor_type(self):
+        if self._values['monitors'] is None:
+            return None
+        pattern = r'min\s+\d+\s+of'
+        matches = re.search(pattern, self._values['monitors'])
+        if matches:
+            return 'm_of_n'
+        else:
+            return 'and_list'
+
+    @property
+    def monitors_list(self):
+        if self._values['monitors'] is None:
+            return []
+        try:
+            result = re.findall(r'/\w+/[^\s}]+', self._values['monitors'])
+            return result
+        except Exception:
+            return self._values['monitors']
+
+    @property
+    def metadata(self):
+        if self._values['metadata'] is None:
+            return None
+        result = []
+        for md in self._values['metadata']:
+            tmp = dict(name=str(md['name']))
+            if 'value' in md:
+                tmp['value'] = str(md['value'])
+            else:
+                tmp['value'] = ''
+            result.append(tmp)
+        return result
+
+
+class ModuleParameters(Parameters):
+    @property
+    def monitors_list(self):
+        if self._values['monitors'] is None:
+            return []
+        return self._values['monitors']
+
+    @property
+    def quorum(self):
+        if self._values['quorum'] is None:
+            return None
+        result = self._verify_quorum_type(self._values['quorum'])
+        return result
+
+    @property
+    def monitor_type(self):
+        if self._values['monitor_type'] is None:
+            return None
+        return self._values['monitor_type']
+
+    @property
+    def metadata(self):
+        if self._values['metadata'] is None:
+            return None
+        if self._values['metadata'] == '':
+            return []
+        result = []
+        try:
+            for k, v in iteritems(self._values['metadata']):
+                tmp = dict(name=str(k))
+                if v:
+                    tmp['value'] = str(v)
+                else:
+                    tmp['value'] = ''
+                result.append(tmp)
+        except AttributeError:
+            raise F5ModuleError(
+                "The 'metadata' parameter must be a dictionary of key/value pairs."
+            )
+        return result
+
 
 class Changes(Parameters):
+    def to_return(self):
+        result = {}
+        for returnable in self.returnables:
+            try:
+                result[returnable] = getattr(self, returnable)
+            except Exception:
+                pass
+            result = self._filter_params(result)
+        return result
+
+    @property
+    def monitors(self):
+        if self._values['monitors'] is None:
+            return None
+        return self._values['monitors']
+
+
+class UsableChanges(Changes):
     pass
 
 
+class ReportableChanges(Changes):
+    @property
+    def monitors(self):
+        result = sorted(re.findall(r'/\w+/[^\s}]+', self._values['monitors']))
+        return result
+
+    @property
+    def metadata(self):
+        result = dict()
+        for x in self._values['metadata']:
+            result[x['name']] = x['value']
+        return result
+
+
 class Difference(object):
     def __init__(self, want, have=None):
         self.want = want
@@ -548,6 +571,25 @@ class Difference(object):
         except AttributeError:
             return attr1
 
+    def to_tuple(self, items):
+        result = []
+        for x in items:
+            tmp = [(str(k), str(v)) for k, v in iteritems(x)]
+            result += tmp
+        return result
+
+    def _diff_complex_items(self, want, have):
+        if want == [] and have is None:
+            return None
+        if want is None:
+            return None
+        w = self.to_tuple(want)
+        h = self.to_tuple(have)
+        if set(w).issubset(set(h)):
+            return None
+        else:
+            return want
+
     @property
     def monitor_type(self):
         if self.want.monitor_type is None:
@@ -593,13 +635,26 @@ class Difference(object):
         if self.want.monitors != self.have.monitors:
             return self.want.monitors
 
+    @property
+    def metadata(self):
+        if self.want.metadata is None:
+            return None
+        elif len(self.want.metadata) == 0 and self.have.metadata is None:
+            return None
+        elif len(self.want.metadata) == 0:
+            return []
+        elif self.have.metadata is None:
+            return self.want.metadata
+        result = self._diff_complex_items(self.want.metadata, self.have.metadata)
+        return result
+
 
 class ModuleManager(object):
     def __init__(self, client):
         self.client = client
-        self.have = None
+        self.want = ModuleParameters(params=self.client.module.params)
-        self.want = Parameters(self.client.module.params)
+        self.have = ApiParameters()
-        self.changes = Changes()
+        self.changes = UsableChanges()
 
     def exec_module(self):
         changed = False
@@ -614,18 +669,15 @@ class ModuleManager(object):
         except iControlUnexpectedHTTPError as e:
             raise F5ModuleError(str(e))
 
-        changes = self.changes.to_return()
+        reportable = ReportableChanges(self.changes.to_return())
+        changes = reportable.to_return()
         result.update(**changes)
         result.update(dict(changed=changed))
-        self._announce_deprecations()
+        self._announce_deprecations(result)
         return result
 
-    def _announce_deprecations(self):
+    def _announce_deprecations(self, result):
-        warnings = []
+        warnings = result.pop('__warnings', [])
-        if self.want:
-            warnings += self.want._values.get('__warnings', [])
-        if self.have:
-            warnings += self.have._values.get('__warnings', [])
         for warning in warnings:
             self.client.module.deprecate(
                 msg=warning['msg'],
@@ -638,7 +690,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 = UsableChanges(changed)
 
     def _update_changed_options(self):
         diff = Difference(self.want, self.have)
@@ -649,25 +701,15 @@ 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 = UsableChanges(changed)
             return True
         return False
 
-    def _member_does_not_exist(self, members):
-        name = self.want.member_name
-        # Return False if name is None, so that we don't attempt to create it
-        if name is None:
-            return False
-        for member in members:
-            if member.name == name:
-                host, port = name.split(':')
-                self.have.host = host
-                self.have.port = int(port)
-                return False
-        return True
-
     def present(self):
         if self.exists():
             return self.update()
@@ -686,10 +728,7 @@ class ModuleManager(object):
         return False
 
     def update(self):
-        self.have, members, poolres = self.read_current_from_device()
+        self.have = self.read_current_from_device()
-        if not self.client.check_mode:
-            if self._member_does_not_exist(members):
-                self.create_member_on_device(poolres)
         if not self.should_update():
             return False
         if self.client.check_mode:
@@ -728,10 +767,6 @@ class ModuleManager(object):
         if self.client.check_mode:
             return True
         self.create_on_device()
-        if self.want.member_name:
-            self.have, members, poolres = self.read_current_from_device()
-            if self._member_does_not_exist(members):
-                self.create_member_on_device(poolres)
         return True
 
     def create_on_device(self):
@@ -740,12 +775,6 @@ class ModuleManager(object):
             partition=self.want.partition, **params
         )
 
-    def create_member_on_device(self, poolres):
-        poolres.members_s.members.create(
-            name=self.want.member_name,
-            partition=self.want.partition
-        )
-
     def update_on_device(self):
         params = self.want.api_params()
         result = self.client.api.tm.ltm.pools.pool.load(
@@ -765,69 +794,21 @@ class ModuleManager(object):
             name=self.want.name,
             partition=self.want.partition
         )
-        if self.want.member_name and self.want.port and self.want.pool:
+        result.delete()
-            member = result.members_s.members.load(
-                name=self.want.member_name,
-                partition=self.want.partition
-            )
-            if member:
-                member.delete()
-                self.delete_node_on_device()
-        else:
-            result.delete()
 
     def read_current_from_device(self):
-        tmp_res = self.client.api.tm.ltm.pools.pool.load(
+        resource = self.client.api.tm.ltm.pools.pool.load(
             name=self.want.name,
-            partition=self.want.partition
+            partition=self.want.partition,
+            requests_params=dict(
+                params='expandSubcollections=true'
+            )
         )
-        members = tmp_res.members_s.get_collection()
+        return ApiParameters(resource.attrs)
-
-        result = tmp_res.attrs
-        return Parameters(result), members, tmp_res
-
-    def delete_node_on_device(self):
-        resource = self.client.api.tm.ltm.nodes.node.load(
-            name=self.want.host,
-            partition=self.want.partition
-        )
-        try:
-            resource.delete()
-        except iControlUnexpectedHTTPError as e:
-            # If we cannot remove it, it is in use, it is up to user to delete
-            # it later.
-            if "is referenced by a member of pool" in str(e):
-                return
-            else:
-                raise
 
 
 class ArgumentSpec(object):
     def __init__(self):
-        self.lb_choice_deprecated = [
-            'round_robin',
-            'ratio_member',
-            'least_connection_member',
-            'observed_member',
-            'predictive_member',
-            'ratio_node_address',
-            'least_connection_node_address',
-            'fastest_node_address',
-            'observed_node_address',
-            'predictive_node_address',
-            'dynamic_ratio',
-            'fastest_app_response',
-            'least_sessions',
-            'dynamic_ratio_member',
-            'ratio_session',
-            'weighted_least_connection_member',
-            'ratio_least_connection_member',
-            'weighted_least_connection_node_address',
-            'ratio_least_connection_node_address'
-        ]
-        self.lb_choice_removed = [
-            'l3_addr'
-        ]
         self.lb_choice = [
             'dynamic-ratio-member',
             'dynamic-ratio-node',
@@ -849,7 +830,6 @@ class ArgumentSpec(object):
             'weighted-least-connections-member',
             'weighted-least-connections-node'
         ]
-        lb_choices = self.lb_choice_removed + self.lb_choice + self.lb_choice_deprecated
         self.supports_check_mode = True
         self.argument_spec = dict(
             name=dict(
@@ -857,7 +837,7 @@ class ArgumentSpec(object):
                 aliases=['pool']
             ),
             lb_method=dict(
-                choices=lb_choices
+                choices=self.lb_choice
             ),
             monitor_type=dict(
                 choices=[
@@ -883,14 +863,7 @@ class ArgumentSpec(object):
                 ]
             ),
             description=dict(),
-            host=dict(
+            metadata=dict(type='raw')
-                aliases=['address'],
-                removed_in_version='2.4'
-            ),
-            port=dict(
-                type='int',
-                removed_in_version='2.4'
-            )
         )
         self.f5_product_name = 'bigip'
 

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

@@ -22,14 +22,16 @@ from ansible.module_utils.f5_utils import AnsibleF5Client
 from ansible.module_utils.f5_utils import F5ModuleError
 
 try:
-    from library.bigip_pool import Parameters
+    from library.bigip_pool import ApiParameters
+    from library.bigip_pool import ModuleParameters
     from library.bigip_pool import ModuleManager
     from library.bigip_pool 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_pool import Parameters
+        from ansible.modules.network.f5.bigip_pool import ApiParameters
+        from ansible.modules.network.f5.bigip_pool import ModuleParameters
         from ansible.modules.network.f5.bigip_pool import ModuleManager
         from ansible.modules.network.f5.bigip_pool import ArgumentSpec
         from ansible.module_utils.f5_utils import iControlUnexpectedHTTPError
@@ -59,11 +61,6 @@ def load_fixture(name):
     return data
 
 
-class BigIpObj(object):
-    def __init__(self, **kwargs):
-        self.__dict__.update(kwargs)
-
-
 class TestParameters(unittest.TestCase):
     def test_module_parameters(self):
         args = dict(
@@ -72,33 +69,26 @@ class TestParameters(unittest.TestCase):
             quorum=1,
             slow_ramp_time=200,
             reselect_tries=5,
-            service_down_action='drop',
+            service_down_action='drop'
-            host='192.168.1.1',
-            port=8080
         )
 
-        p = Parameters(args)
+        p = ModuleParameters(args)
         assert p.monitor_type == 'm_of_n'
         assert p.quorum == 1
         assert p.monitors == 'min 1 of { /Common/Fake /Common/Fake2 }'
-        assert p.host == '192.168.1.1'
-        assert p.port == 8080
-        assert p.member_name == '192.168.1.1:8080'
         assert p.slow_ramp_time == 200
         assert p.reselect_tries == 5
         assert p.service_down_action == 'drop'
 
     def test_api_parameters(self):
-        m = ['/Common/Fake', '/Common/Fake2']
         args = dict(
-            monitor_type='and_list',
+            monitor="/Common/Fake and /Common/Fake2 ",
-            monitors=m,
             slowRampTime=200,
             reselectTries=5,
             serviceDownAction='drop'
         )
 
-        p = Parameters(args)
+        p = ApiParameters(args)
         assert p.monitors == '/Common/Fake and /Common/Fake2'
         assert p.slow_ramp_time == 200
         assert p.reselect_tries == 5
@@ -109,7 +99,7 @@ class TestParameters(unittest.TestCase):
             lb_method='obscure_hyphenated_fake_method',
         )
         with pytest.raises(F5ModuleError):
-            p = Parameters(args)
+            p = ModuleParameters(args)
             assert p.lb_method == 'foo'
 
     def test_unknown_api_lb_method(self):
@@ -117,7 +107,7 @@ class TestParameters(unittest.TestCase):
             loadBalancingMode='obscure_hypenated_fake_method'
         )
         with pytest.raises(F5ModuleError):
-            p = Parameters(args)
+            p = ApiParameters(args)
             assert p.lb_method == 'foo'
 
 
@@ -127,17 +117,13 @@ class TestManager(unittest.TestCase):
 
     def setUp(self):
         self.spec = ArgumentSpec()
-        self.loaded_members = []
-        members = load_fixture('pool_members_subcollection.json')
-        for item in members:
-            self.loaded_members.append(BigIpObj(**item))
 
     def test_create_pool(self, *args):
         set_module_args(dict(
             pool='fake_pool',
             description='fakepool',
             service_down_action='drop',
-            lb_method='round_robin',
+            lb_method='round-robin',
             partition='Common',
             slow_ramp_time=10,
             reselect_tries=1,
@@ -166,102 +152,10 @@ class TestManager(unittest.TestCase):
         assert results['slow_ramp_time'] == 10
         assert results['reselect_tries'] == 1
 
-    def test_create_pool_with_pool_member(self, *args):
-        set_module_args(dict(
-            pool='fake_pool',
-            partition='Common',
-            host='192.168.1.1',
-            port=8080,
-            server='localhost',
-            password='password',
-            user='admin'
-        ))
-
-        client = AnsibleF5Client(
-            argument_spec=self.spec.argument_spec,
-            supports_check_mode=self.spec.supports_check_mode,
-            f5_product_name=self.spec.f5_product_name
-        )
-
-        current = (
-            Parameters(
-                load_fixture('load_ltm_pool.json')
-            ),
-            self.loaded_members,
-            {},
-        )
-
-        mm = ModuleManager(client)
-        mm.create_on_device = Mock(return_value=True)
-        mm.exists = Mock(return_value=False)
-        mm.read_current_from_device = Mock(return_value=current)
-        mm.create_member_on_device = Mock(return_value=True)
-
-        results = mm.exec_module()
-
-        assert results['changed'] is True
-        assert results['host'] == '192.168.1.1'
-        assert results['port'] == 8080
-        assert results['member_name'] == '192.168.1.1:8080'
-
-    def test_create_pool_invalid_host(self, *args):
-        set_module_args(dict(
-            pool='fake_pool',
-            partition='Common',
-            host='this.will.fail',
-            port=8080,
-            server='localhost',
-            password='password',
-            user='admin'
-        ))
-
-        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)
-        mm.create_on_device = Mock(return_value=True)
-        mm.exists = Mock(return_value=False)
-
-        msg = "'this.will.fail' is not a valid IP address"
-
-        with pytest.raises(F5ModuleError) as err:
-            mm.exec_module()
-        assert str(err.value) == msg
-
-    def test_create_pool_invalid_port(self, *args):
-        set_module_args(dict(
-            pool='fake_pool',
-            partition='Common',
-            host='192.168.1.1',
-            port=98741,
-            server='localhost',
-            password='password',
-            user='admin'
-        ))
-
-        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)
-        mm.create_on_device = Mock(return_value=True)
-        mm.exists = Mock(return_value=False)
-
-        msg = "The provided port '98741' must be between 0 and 65535"
-
-        with pytest.raises(F5ModuleError) as err:
-            mm.exec_module()
-        assert str(err.value) == msg
-
     def test_create_pool_monitor_type_missing(self, *args):
         set_module_args(dict(
             pool='fake_pool',
-            lb_method='round_robin',
+            lb_method='round-robin',
             partition='Common',
             monitors=['/Common/tcp', '/Common/http'],
             server='localhost',
@@ -283,13 +177,13 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == '/Common/tcp and /Common/http'
+        assert results['monitors'] == ['/Common/http', '/Common/tcp']
         assert results['monitor_type'] == 'and_list'
 
     def test_create_pool_monitors_missing(self, *args):
         set_module_args(dict(
             pool='fake_pool',
-            lb_method='round_robin',
+            lb_method='round-robin',
             partition='Common',
             monitor_type='and_list',
             server='localhost',
@@ -317,7 +211,7 @@ class TestManager(unittest.TestCase):
     def test_create_pool_quorum_missing(self, *args):
         set_module_args(dict(
             pool='fake_pool',
-            lb_method='round_robin',
+            lb_method='round-robin',
             partition='Common',
             monitor_type='m_of_n',
             monitors=['/Common/tcp', '/Common/http'],
@@ -367,7 +261,7 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == '/Common/tcp and /Common/http'
+        assert results['monitors'] == ['/Common/http', '/Common/tcp']
         assert results['monitor_type'] == 'and_list'
 
     def test_create_pool_monitor_m_of_n(self, *args):
@@ -396,7 +290,7 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == 'min 1 of { /Common/tcp /Common/http }'
+        assert results['monitors'] == ['/Common/http', '/Common/tcp']
         assert results['monitor_type'] == 'm_of_n'
 
     def test_update_monitors(self, *args):
@@ -417,13 +311,7 @@ class TestManager(unittest.TestCase):
         )
         mm = ModuleManager(client)
 
-        current = (
+        current = ApiParameters(load_fixture('load_ltm_pool.json'))
-            Parameters(
-                load_fixture('load_ltm_pool.json')
-            ),
-            [],
-            {},
-        )
 
         mm.update_on_device = Mock(return_value=True)
         mm.exists = Mock(return_value=True)
@@ -433,79 +321,6 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['monitor_type'] == 'and_list'
-        assert results['monitors'] == '/Common/http and /Common/tcp'
-
-    def test_update_pool_new_member(self, *args):
-        set_module_args(dict(
-            name='test_pool',
-            partition='Common',
-            host='192.168.1.1',
-            port=8080,
-            server='localhost',
-            password='password',
-            user='admin'
-        ))
-
-        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)
-
-        current = (
-            Parameters(
-                load_fixture('load_ltm_pool.json')
-            ),
-            self.loaded_members,
-            {},
-        )
-
-        mm.update_on_device = Mock(return_value=True)
-        mm.exists = Mock(return_value=True)
-        mm.read_current_from_device = Mock(return_value=current)
-        mm.create_member_on_device = Mock(return_value=True)
-
-        results = mm.exec_module()
-
-        assert results['changed'] is True
-        assert results['host'] == '192.168.1.1'
-        assert results['port'] == 8080
-
-    def test_update_pool_member_exists(self, *args):
-        set_module_args(dict(
-            name='test_pool',
-            partition='Common',
-            host='1.1.1.1',
-            port=80,
-            server='localhost',
-            password='password',
-            user='admin'
-        ))
-
-        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)
-
-        current = (
-            Parameters(
-                load_fixture('load_ltm_pool.json')
-            ),
-            self.loaded_members,
-            {},
-        )
-
-        mm.update_on_device = Mock(return_value=True)
-        mm.exists = Mock(return_value=True)
-        mm.read_current_from_device = Mock(return_value=current)
-        mm.create_member_on_device = Mock(return_value=True)
-
-        results = mm.exec_module()
-
-        assert results['changed'] is False
 
     def test_create_pool_monitor_and_list_no_partition(self, *args):
         set_module_args(dict(
@@ -531,7 +346,7 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == '/Common/tcp and /Common/http'
+        assert results['monitors'] == ['/Common/http', '/Common/tcp']
         assert results['monitor_type'] == 'and_list'
 
     def test_create_pool_monitor_m_of_n_no_partition(self, *args):
@@ -559,7 +374,7 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == 'min 1 of { /Common/tcp /Common/http }'
+        assert results['monitors'] == ['/Common/http', '/Common/tcp']
         assert results['monitor_type'] == 'm_of_n'
 
     def test_create_pool_monitor_and_list_custom_partition(self, *args):
@@ -587,7 +402,7 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == '/Testing/tcp and /Testing/http'
+        assert results['monitors'] == ['/Testing/http', '/Testing/tcp']
         assert results['monitor_type'] == 'and_list'
 
     def test_create_pool_monitor_m_of_n_custom_partition(self, *args):
@@ -616,5 +431,32 @@ class TestManager(unittest.TestCase):
 
         assert results['changed'] is True
         assert results['name'] == 'fake_pool'
-        assert results['monitors'] == 'min 1 of { /Testing/tcp /Testing/http }'
+        assert results['monitors'] == ['/Testing/http', '/Testing/tcp']
         assert results['monitor_type'] == 'm_of_n'
+
+    def test_create_pool_with_metadata(self, *args):
+        set_module_args(dict(
+            pool='fake_pool',
+            metadata=dict(ansible='2.4'),
+            server='localhost',
+            password='password',
+            user='admin'
+        ))
+
+        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)
+        mm.create_on_device = Mock(return_value=True)
+        mm.exists = Mock(return_value=False)
+
+        results = mm.exec_module()
+
+        assert results['changed'] is True
+        assert results['name'] == 'fake_pool'
+        assert 'metadata' in results
+        assert 'ansible' in results['metadata']
+        assert results['metadata']['ansible'] == '2.4'