Fixes for gtm wide ip (#33725)

Adds pools argument. Refactors code to support new conventions.
Adds more unit tests

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

@@ -11,7 +11,6 @@ __metaclass__ = type
 ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['preview'],
                     'supported_by': 'community'}
-
 DOCUMENTATION = r'''
 ---
 module: bigip_gtm_wide_ip
@@ -20,17 +19,19 @@ description:
   - Manages F5 BIG-IP GTM wide ip.
 version_added: "2.0"
 options:
-  lb_method:
+  pool_lb_method:
     description:
       - Specifies the load balancing method used to select a pool in this wide
         IP. This setting is relevant only when multiple pools are configured
         for a wide IP.
     required: True
+    aliases: ['lb_method']
     choices:
       - round-robin
       - ratio
       - topology
       - global-availability
+    version_added: 2.5
   name:
     description:
       - Wide IP name. This name must be formatted as a fully qualified
@@ -56,9 +57,9 @@ options:
   state:
     description:
       - When C(present) or C(enabled), ensures that the Wide IP exists and
-        is enabled. When C(absent), ensures that the Wide IP has been
+        is enabled.
-        removed. When C(disabled), ensures that the Wide IP exists and is
+      - When C(absent), ensures that the Wide IP has been removed.
-        disabled.
+      - When C(disabled), ensures that the Wide IP exists and is disabled.
     default: present
     choices:
       - present
@@ -71,6 +72,21 @@ options:
       - Device partition to manage resources on.
     default: Common
     version_added: 2.5
+  pools:
+    description:
+      - The pools that you want associated with the Wide IP.
+      - If C(ratio) is not provided when creating a new Wide IP, it will default
+        to 1.
+    suboptions:
+      name:
+        description:
+          - The name of the pool to include
+        required: true
+      ratio:
+        description:
+          - Ratio for the pool.
+          - The system uses this number with the Ratio load balancing method.
+    version_added: 2.5
 notes:
   - Requires the f5-sdk Python package on the host. This is as easy as pip
     install f5-sdk.
@@ -81,6 +97,7 @@ author:
   - Tim Rupp (@caphrim007)
 '''
 
+
 EXAMPLES = r'''
 - name: Set lb method
   bigip_gtm_wide_ip:
@@ -111,6 +128,7 @@ 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
+from ansible.module_utils.six import iteritems
 from distutils.version import LooseVersion
 
 try:
@@ -120,16 +138,12 @@ except ImportError:
 
 
 class Parameters(AnsibleF5Parameters):
-    updatables = ['lb_method']
+    api_map = {
-    returnables = ['name', 'lb_method', 'state']
+        'poolLbMode': 'pool_lb_method'
-    api_attributes = ['poolLbMode', 'enabled', 'disabled']
+    }
-
+    updatables = ['pool_lb_method', 'state', 'pools']
-    def to_return(self):
+    returnables = ['name', 'pool_lb_method', 'state', 'pools']
-        result = {}
+    api_attributes = ['poolLbMode', 'enabled', 'disabled', 'pools']
-        for returnable in self.returnables:
-            result[returnable] = getattr(self, returnable)
-        result = self._filter_params(result)
-        return result
 
     def api_params(self):
         result = {}
@@ -141,8 +155,47 @@ class Parameters(AnsibleF5Parameters):
         result = self._filter_params(result)
         return result
 
+    def _fqdn_name(self, value):
+        if value is not None and not value.startswith('/'):
+            return '/{0}/{1}'.format(self.partition, value)
+        return value
+
+
+class ApiParameters(Parameters):
     @property
-    def lb_method(self):
+    def disabled(self):
+        if self._values['disabled'] is True:
+            return True
+        return False
+
+    @property
+    def enabled(self):
+        if self._values['enabled'] is True:
+            return True
+        return False
+
+    @property
+    def pools(self):
+        result = []
+        if self._values['pools'] is None:
+            return None
+        pools = sorted(self._values['pools'], key=lambda x: x['order'])
+        for item in pools:
+            pool = dict()
+            pool.update(item)
+            name = '/{0}/{1}'.format(item['partition'], item['name'])
+            del pool['nameReference']
+            del pool['order']
+            del pool['name']
+            del pool['partition']
+            pool['name'] = name
+            result.append(pool)
+        return result
+
+
+class ModuleParameters(Parameters):
+    @property
+    def pool_lb_method(self):
         deprecated = [
             'return_to_dns', 'null', 'static_persist', 'vs_capacity',
             'least_conn', 'lowest_rtt', 'lowest_hops', 'packet_rate', 'cpu',
@@ -178,25 +231,6 @@ class Parameters(AnsibleF5Parameters):
             lb_method = 'round-robin'
         return lb_method
 
-    @lb_method.setter
-    def lb_method(self, value):
-        self._values['lb_method'] = value
-
-    @property
-    def collection(self):
-        type_map = dict(
-            a='a_s',
-            aaaa='aaaas',
-            cname='cnames',
-            mx='mxs',
-            naptr='naptrs',
-            srv='srvs'
-        )
-        if self._values['type'] is None:
-            return None
-        wideip_type = self._values['type']
-        return type_map[wideip_type]
-
     @property
     def type(self):
         if self._values['type'] is None:
@@ -213,14 +247,6 @@ class Parameters(AnsibleF5Parameters):
             )
         return self._values['name']
 
-    @property
-    def poolLbMode(self):
-        return self.lb_method
-
-    @poolLbMode.setter
-    def poolLbMode(self, value):
-        self.lb_method = value
-
     @property
     def state(self):
         if self._values['state'] == 'enabled':
@@ -233,8 +259,6 @@ class Parameters(AnsibleF5Parameters):
             return False
         elif self._values['state'] in ['present', 'enabled']:
             return True
-        elif self._values['enabled'] is True:
-            return True
         else:
             return None
 
@@ -244,11 +268,105 @@ class Parameters(AnsibleF5Parameters):
             return True
         elif self._values['state'] in ['present', 'enabled']:
             return False
-        elif self._values['disabled'] is True:
-            return True
         else:
             return None
 
+    @property
+    def pools(self):
+        result = []
+        if self._values['pools'] is None:
+            return None
+        for item in self._values['pools']:
+            pool = dict()
+            if 'ratio' in item:
+                pool['ratio'] = item['ratio']
+            pool['name'] = self._fqdn_name(item['name'])
+            result.append(pool)
+        return result
+
+
+class Changes(Parameters):
+    def to_return(self):
+        result = {}
+        try:
+            for returnable in self.returnables:
+                change = getattr(self, returnable)
+                if isinstance(change, dict):
+                    result.update(change)
+                else:
+                    result[returnable] = change
+            result = self._filter_params(result)
+        except Exception:
+            pass
+        return result
+
+
+class UsableChanges(Changes):
+    pass
+
+
+class ReportableChanges(Changes):
+    @property
+    def pool_lb_method(self):
+        result = dict(
+            lb_method=self._values['pool_lb_method'],
+            pool_lb_method=self._values['pool_lb_method'],
+        )
+        return result
+
+
+class Difference(object):
+    def __init__(self, want, have=None):
+        self.want = want
+        self.have = have
+
+    def compare(self, param):
+        try:
+            result = getattr(self, param)
+            return result
+        except AttributeError:
+            return self.__default(param)
+
+    def __default(self, param):
+        attr1 = getattr(self.want, param)
+        try:
+            attr2 = getattr(self.have, param)
+            if attr1 != attr2:
+                return attr1
+        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 state(self):
+        if self.want.state == 'disabled' and self.have.enabled:
+            return self.want.state
+        elif self.want.state in ['present', 'enabled'] and self.have.disabled:
+            return self.want.state
+
+    @property
+    def pools(self):
+        result = self._diff_complex_items(self.want.pools, self.have.pools)
+        return result
+
 
 class ModuleManager(object):
     def __init__(self, client):
@@ -278,9 +396,9 @@ class ModuleManager(object):
 class BaseManager(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 = Parameters()
+        self.changes = UsableChanges()
 
     def _set_changed_options(self):
         changed = {}
@@ -288,24 +406,23 @@ class BaseManager(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):
-        changed = {}
+        diff = Difference(self.want, self.have)
-        for key in Parameters.updatables:
+        updatables = Parameters.updatables
-            if getattr(self.want, key) is not None:
+        changed = dict()
-                attr1 = getattr(self.want, key)
+        for k in updatables:
-                attr2 = getattr(self.have, key)
+            change = diff.compare(k)
-                if attr1 != attr2:
+            if change is None:
-                    changed[key] = attr1
+                continue
-
+            else:
-        if self.want.state == 'disabled' and self.have.enabled:
+                if isinstance(change, dict):
-            changed['state'] = self.want.state
+                    changed.update(change)
-        elif self.want.state in ['present', 'enabled'] and self.have.disabled:
+                else:
-            changed['state'] = self.want.state
+                    changed[k] = change
-
         if changed:
-            self.changes = Parameters(changed)
+            self.changes = UsableChanges(changed)
             return True
         return False
 
@@ -322,18 +439,15 @@ class BaseManager(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'],
@@ -407,7 +521,7 @@ class UntypedManager(BaseManager):
             partition=self.want.partition
         )
         result = resource.attrs
-        return Parameters(result)
+        return ApiParameters(result)
 
     def create_on_device(self):
         params = self.want.api_params()
@@ -434,10 +548,19 @@ class TypedManager(BaseManager):
                 "The 'type' option is required for BIG-IP instances "
                 "greater than or equal to 12.x"
             )
+        type_map = dict(
+            a='a_s',
+            aaaa='aaaas',
+            cname='cnames',
+            mx='mxs',
+            naptr='naptrs',
+            srv='srvs'
+        )
+        self.collection = type_map[self.want.type]
 
     def exists(self):
         wideips = self.client.api.tm.gtm.wideips
-        collection = getattr(wideips, self.want.collection)
+        collection = getattr(wideips, self.collection)
         resource = getattr(collection, self.want.type)
         result = resource.exists(
             name=self.want.name,
@@ -448,7 +571,7 @@ class TypedManager(BaseManager):
     def update_on_device(self):
         params = self.want.api_params()
         wideips = self.client.api.tm.gtm.wideips
-        collection = getattr(wideips, self.want.collection)
+        collection = getattr(wideips, self.collection)
         resource = getattr(collection, self.want.type)
         result = resource.load(
             name=self.want.name,
@@ -458,19 +581,19 @@ class TypedManager(BaseManager):
 
     def read_current_from_device(self):
         wideips = self.client.api.tm.gtm.wideips
-        collection = getattr(wideips, self.want.collection)
+        collection = getattr(wideips, self.collection)
         resource = getattr(collection, self.want.type)
         result = resource.load(
             name=self.want.name,
             partition=self.want.partition
         )
         result = result.attrs
-        return Parameters(result)
+        return ApiParameters(result)
 
     def create_on_device(self):
         params = self.want.api_params()
         wideips = self.client.api.tm.gtm.wideips
-        collection = getattr(wideips, self.want.collection)
+        collection = getattr(wideips, self.collection)
         resource = getattr(collection, self.want.type)
         resource.create(
             name=self.want.name,
@@ -480,7 +603,7 @@ class TypedManager(BaseManager):
 
     def remove_from_device(self):
         wideips = self.client.api.tm.gtm.wideips
-        collection = getattr(wideips, self.want.collection)
+        collection = getattr(wideips, self.collection)
         resource = getattr(collection, self.want.type)
         result = resource.load(
             name=self.want.name,
@@ -504,8 +627,9 @@ class ArgumentSpec(object):
         lb_method_choices = deprecated + supported
         self.supports_check_mode = True
         self.argument_spec = dict(
-            lb_method=dict(
+            pool_lb_method=dict(
-                choices=lb_method_choices
+                choices=lb_method_choices,
+                aliases=['lb_method']
             ),
             name=dict(
                 required=True,
@@ -519,6 +643,13 @@ class ArgumentSpec(object):
             state=dict(
                 default='present',
                 choices=['absent', 'present', 'enabled', 'disabled']
+            ),
+            pools=dict(
+                type='list',
+                options=dict(
+                    name=dict(required=True),
+                    ratio=dict(type='int')
+                )
             )
         )
         self.f5_product_name = 'bigip'

test/units/modules/network/f5/fixtures/load_gtm_wide_ip_with_pools.json

@@ -0,0 +1,30 @@
+{
+  "kind": "tm:gtm:wideip:a:astate",
+  "name": "foo.bar.com",
+  "partition": "Common",
+  "fullPath": "/Common/foo.bar.com",
+  "generation": 135,
+  "selfLink": "https://localhost/mgmt/tm/gtm/wideip/a/~Common~foo.bar.com?ver=13.0.0",
+  "enabled": true,
+  "failureRcode": "noerror",
+  "failureRcodeResponse": "disabled",
+  "failureRcodeTtl": 0,
+  "lastResortPool": "",
+  "minimalResponse": "enabled",
+  "persistCidrIpv4": 32,
+  "persistCidrIpv6": 128,
+  "persistence": "disabled",
+  "poolLbMode": "round-robin",
+  "ttlPersistence": 3600,
+  "pools": [
+      {
+          "name": "baz",
+          "partition": "Common",
+          "order": 0,
+          "ratio": 10,
+          "nameReference": {
+              "link": "https://localhost/mgmt/tm/gtm/pool/a/~Common~baz?ver=13.0.0"
+          }
+      }
+  ]
+}

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

@@ -22,7 +22,8 @@ from ansible.module_utils.f5_utils import AnsibleF5Client
 from ansible.module_utils.f5_utils import F5ModuleError
 
 try:
-    from library.bigip_gtm_wide_ip import Parameters
+    from library.bigip_gtm_wide_ip import ApiParameters
+    from library.bigip_gtm_wide_ip import ModuleParameters
     from library.bigip_gtm_wide_ip import ModuleManager
     from library.bigip_gtm_wide_ip import ArgumentSpec
     from library.bigip_gtm_wide_ip import UntypedManager
@@ -31,7 +32,8 @@ try:
     from test.unit.modules.utils import set_module_args
 except ImportError:
     try:
-        from ansible.modules.network.f5.bigip_gtm_wide_ip import Parameters
+        from ansible.modules.network.f5.bigip_gtm_wide_ip import ApiParameters
+        from ansible.modules.network.f5.bigip_gtm_wide_ip import ModuleParameters
         from ansible.modules.network.f5.bigip_gtm_wide_ip import ModuleManager
         from ansible.modules.network.f5.bigip_gtm_wide_ip import ArgumentSpec
         from ansible.modules.network.f5.bigip_gtm_wide_ip import UntypedManager
@@ -67,28 +69,49 @@ class TestParameters(unittest.TestCase):
     def test_module_parameters(self):
         args = dict(
             name='foo.baz.bar',
-            lb_method='round-robin'
+            lb_method='round-robin',
         )
-        p = Parameters(args)
+        p = ModuleParameters(args)
         assert p.name == 'foo.baz.bar'
-        assert p.lb_method == 'round-robin'
+        assert p.pool_lb_method == 'round-robin'
+
+    def test_module_pools(self):
+        args = dict(
+            pools=[
+                dict(
+                    name='foo',
+                    ratio='100'
+                )
+            ]
+        )
+        p = ModuleParameters(args)
+        assert len(p.pools) == 1
 
     def test_api_parameters(self):
         args = dict(
             name='foo.baz.bar',
             poolLbMode='round-robin'
         )
-        p = Parameters(args)
+        p = ApiParameters(args)
         assert p.name == 'foo.baz.bar'
-        assert p.lb_method == 'round-robin'
+        assert p.pool_lb_method == 'round-robin'
 
-    def test_api_not_fqdn_name(self):
+    def test_api_pools(self):
+        args = load_fixture('load_gtm_wide_ip_with_pools.json')
+        p = ApiParameters(args)
+        assert len(p.pools) == 1
+        assert 'name' in p.pools[0]
+        assert 'ratio' in p.pools[0]
+        assert p.pools[0]['name'] == '/Common/baz'
+        assert p.pools[0]['ratio'] == 10
+
+    def test_module_not_fqdn_name(self):
         args = dict(
             name='foo.baz',
-            poolLbMode='round-robin'
+            lb_method='round-robin'
         )
         with pytest.raises(F5ModuleError) as excinfo:
-            p = Parameters(args)
+            p = ModuleParameters(args)
             assert p.name == 'foo.baz'
         assert 'The provided name must be a valid FQDN' in str(excinfo)
 
@@ -238,3 +261,122 @@ class TestTypedManager(unittest.TestCase):
         assert results['name'] == 'foo.baz.bar'
         assert results['state'] == 'present'
         assert results['lb_method'] == 'global-availability'
+
+    def test_create_wideip_with_pool(self, *args):
+        set_module_args(dict(
+            name='foo.baz.bar',
+            lb_method='round-robin',
+            type='a',
+            pools=[
+                dict(
+                    name='foo',
+                    ratio=10
+                )
+            ],
+            password='passsword',
+            server='localhost',
+            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
+        )
+
+        # Override methods in the specific type of manager
+        tm = TypedManager(client)
+        tm.exists = Mock(return_value=False)
+        tm.create_on_device = Mock(return_value=True)
+
+        # Override methods to force specific logic in the module to happen
+        mm = ModuleManager(client)
+        mm.version_is_less_than_12 = Mock(return_value=False)
+        mm.get_manager = Mock(return_value=tm)
+
+        results = mm.exec_module()
+
+        assert results['changed'] is True
+        assert results['name'] == 'foo.baz.bar'
+        assert results['state'] == 'present'
+        assert results['lb_method'] == 'round-robin'
+
+    def test_create_wideip_with_pool_idempotent(self, *args):
+        set_module_args(dict(
+            name='foo.bar.com',
+            lb_method='round-robin',
+            type='a',
+            pools=[
+                dict(
+                    name='baz',
+                    ratio=10
+                )
+            ],
+            password='passsword',
+            server='localhost',
+            user='admin'
+        ))
+
+        current = ApiParameters(load_fixture('load_gtm_wide_ip_with_pools.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
+        )
+
+        # Override methods in the specific type of manager
+        tm = TypedManager(client)
+        tm.exists = Mock(return_value=True)
+        tm.read_current_from_device = Mock(return_value=current)
+
+        # Override methods to force specific logic in the module to happen
+        mm = ModuleManager(client)
+        mm.version_is_less_than_12 = Mock(return_value=False)
+        mm.get_manager = Mock(return_value=tm)
+
+        results = mm.exec_module()
+
+        assert results['changed'] is False
+
+    def test_update_wideip_with_pool(self, *args):
+        set_module_args(dict(
+            name='foo.bar.com',
+            lb_method='round-robin',
+            type='a',
+            pools=[
+                dict(
+                    name='baz',
+                    ratio=10
+                ),
+                dict(
+                    name='alec',
+                    ratio=100
+                )
+            ],
+            password='passsword',
+            server='localhost',
+            user='admin'
+        ))
+
+        current = ApiParameters(load_fixture('load_gtm_wide_ip_with_pools.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
+        )
+
+        # Override methods in the specific type of manager
+        tm = TypedManager(client)
+        tm.exists = Mock(return_value=True)
+        tm.read_current_from_device = Mock(return_value=current)
+        tm.update_on_device = Mock(return_value=True)
+
+        # Override methods to force specific logic in the module to happen
+        mm = ModuleManager(client)
+        mm.version_is_less_than_12 = Mock(return_value=False)
+        mm.get_manager = Mock(return_value=tm)
+
+        results = mm.exec_module()
+
+        assert results['changed'] is True
+        assert 'pools' in results