removes deprecated parameters (#61066)

adds diff functionality fixes issue with serverscope parameter not setting on device
2019-08-22 04:50:40 +01:00 · 2019-08-22 04:50:40 +01:00 · 3c19c75560
commit 3c19c75560
parent 3fa7423e8e
4 changed files with 29 additions and 156 deletions
--- a/lib/ansible/modules/network/f5/bigip_gtm_wide_ip.py
+++ b/lib/ansible/modules/network/f5/bigip_gtm_wide_ip.py
@ -25,8 +25,6 @@ options:
      - 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.
-      - The C(round_robin) value is deprecated and will be removed in Ansible 2.9.
-      - The C(global_availability) value is deprecated and will be removed in Ansible 2.9.
    type: str
    required: True
    aliases: ['lb_method']
@ -35,8 +33,6 @@ options:
      - ratio
      - topology
      - global-availability
-      - global_availability
-      - round_robin
    version_added: 2.5
  name:
    description:
@ -333,26 +329,6 @@ class ModuleParameters(Parameters):
        if self._values['pool_lb_method'] is None:
            return None
        lb_method = str(self._values['pool_lb_method'])
-        if lb_method == 'global_availability':
-            if self._values['__warnings'] is None:
-                self._values['__warnings'] = []
-            self._values['__warnings'].append(
-                dict(
-                    msg='The provided pool_lb_method is deprecated',
-                    version='2.4'
-                )
-            )
-            lb_method = 'global-availability'
-        elif lb_method == 'round_robin':
-            if self._values['__warnings'] is None:
-                self._values['__warnings'] = []
-            self._values['__warnings'].append(
-                dict(
-                    msg='The provided pool_lb_method is deprecated',
-                    version='2.4'
-                )
-            )
-            lb_method = 'round-robin'
        return lb_method

    @property
@ -908,9 +884,6 @@ class ArgumentSpec(object):
    def __init__(self):
        lb_method_choices = [
            'round-robin', 'topology', 'ratio', 'global-availability',
-
-            # TODO(Remove in Ansible 2.9)
-            'round_robin', 'global_availability'
        ]
        self.supports_check_mode = True
        argument_spec = dict(
--- a/lib/ansible/modules/network/f5/bigip_virtual_address.py
+++ b/lib/ansible/modules/network/f5/bigip_virtual_address.py
@ -128,13 +128,6 @@ options:
      - when_any_available
    aliases: ['advertise_route']
    version_added: 2.6
-  use_route_advertisement:
-    description:
-      - Specifies whether the system uses route advertisement for this
-        virtual address.
-      - When disabled, the system does not advertise routes for this virtual address.
-      - Deprecated. Use the C(route_advertisement) parameter instead.
-    type: bool
  route_advertisement:
    description:
      - Specifies whether the system uses route advertisement for this
@ -223,7 +216,7 @@ EXAMPLES = r'''
  bigip_virtual_address:
    state: present
    address: 10.10.10.10
-    use_route_advertisement: yes
+    route_advertisement: any
    provider:
      server: lb.mydomain.net
      user: admin
@ -232,11 +225,11 @@ EXAMPLES = r'''
 '''

 RETURN = r'''
-use_route_advertisement:
-  description: The new setting for whether to use route advertising or not.
+availability_calculation:
+  description: Specifies what routes of the virtual address the system advertises.
  returned: changed
-  type: bool
-  sample: true
+  type: str
+  sample: always
 auto_delete:
  description: New setting for auto deleting virtual address.
  returned: changed
@ -342,6 +335,7 @@ class Parameters(AnsibleF5Parameters):
        'traffic_group',
        'route_domain',
        'spanning',
+        'availability_calculation',
    ]

    api_attributes = [
@ -356,6 +350,7 @@ class Parameters(AnsibleF5Parameters):
        'serverScope',
        'trafficGroup',
        'spanning',
+        'serverScope',
    ]

    @property
@ -434,24 +429,11 @@ class Parameters(AnsibleF5Parameters):

    @property
    def route_advertisement_type(self):
-        if self.use_route_advertisement:
-            return self.use_route_advertisement
-        elif self.route_advertisement:
+        if self.route_advertisement:
            return self.route_advertisement
        else:
            return self._values['route_advertisement_type']

-    @property
-    def use_route_advertisement(self):
-        if self._values['use_route_advertisement'] is None:
-            return None
-        if self._values['use_route_advertisement'] in BOOLEANS_TRUE:
-            return 'enabled'
-        elif self._values['use_route_advertisement'] == 'enabled':
-            return 'enabled'
-        else:
-            return 'disabled'
-
    @property
    def route_advertisement(self):
        if self._values['route_advertisement'] is None:
@ -694,8 +676,25 @@ class ModuleManager(object):
        reportable = ReportableChanges(params=self.changes.to_return())
        changes = reportable.to_return()
        result.update(**changes)
+
+        if self.module._diff and self.have:
+            result['diff'] = self.make_diff()
+
        result.update(dict(changed=changed))
        self._announce_deprecations(result)
+
+        return result
+
+    def _grab_attr(self, item):
+        result = dict()
+        updatables = Parameters.updatables
+        for k in updatables:
+            if getattr(item, k) is not None:
+                result[k] = getattr(item, k)
+        return result
+
+    def make_diff(self):
+        result = dict(before=self._grab_attr(self.have), after=self._grab_attr(self.want))
        return result

    def should_update(self):
@ -779,15 +778,10 @@ class ModuleManager(object):
        return True

    def exists(self):
-        # This addresses cases where the name includes a % sign. The URL in the REST
-        # API escapes a % sign as %25. If you don't do this, you will get errors in
-        # the exists() method.
-        name = self.want.name
-        name = name.replace('%', '%25')
        uri = "https://{0}:{1}/mgmt/tm/ltm/virtual-address/{2}".format(
            self.client.provider['server'],
            self.client.provider['server_port'],
-            transform_name(self.want.partition, name)
+            transform_name(self.want.partition, self.want.name)
        )
        resp = self.client.api.get(uri)
        try:
@ -799,12 +793,10 @@ class ModuleManager(object):
        return True

    def read_current_from_device(self):
-        name = self.want.name
-        name = name.replace('%', '%25')
        uri = "https://{0}:{1}/mgmt/tm/ltm/virtual-address/{2}".format(
            self.client.provider['server'],
            self.client.provider['server_port'],
-            transform_name(self.want.partition, name)
+            transform_name(self.want.partition, self.want.name)
        )
        resp = self.client.api.get(uri)
        try:
@ -821,12 +813,10 @@ class ModuleManager(object):

    def update_on_device(self):
        params = self.changes.api_params()
-        name = self.want.name
-        name = name.replace('%', '%25')
        uri = "https://{0}:{1}/mgmt/tm/ltm/virtual-address/{2}".format(
            self.client.provider['server'],
            self.client.provider['server_port'],
-            transform_name(self.want.partition, name)
+            transform_name(self.want.partition, self.want.name)
        )
        resp = self.client.api.patch(uri, json=params)
        try:
@ -864,12 +854,10 @@ class ModuleManager(object):
        return response['selfLink']

    def remove_from_device(self):
-        name = self.want.name
-        name = name.replace('%', '%25')
        uri = "https://{0}:{1}/mgmt/tm/ltm/virtual-address/{2}".format(
            self.client.provider['server'],
            self.client.provider['server_port'],
-            transform_name(self.want.partition, name)
+            transform_name(self.want.partition, self.want.name)
        )
        resp = self.client.api.delete(uri)
        if resp.status == 200:
@ -906,12 +894,6 @@ class ArgumentSpec(object):
            ),
            route_domain=dict(),
            spanning=dict(type='bool'),
-
-            # Deprecated pair - route advertisement
-            use_route_advertisement=dict(
-                type='bool',
-                removed_in_version=2.9,
-            ),
            route_advertisement=dict(
                choices=[
                    'disabled',
@ -937,7 +919,6 @@ class ArgumentSpec(object):
            ['name', 'address']
        ]
        self.mutually_exclusive = [
-            ['use_route_advertisement', 'route_advertisement'],
            ['arp_state', 'arp']
        ]

--- a/test/units/modules/network/f5/test_bigip_gtm_wide_ip.py
+++ b/test/units/modules/network/f5/test_bigip_gtm_wide_ip.py
@ -225,76 +225,6 @@ class TestTypedManager(unittest.TestCase):
        assert results['state'] == 'present'
        assert results['lb_method'] == 'round-robin'

-    def test_create_wideip_deprecated_lb_method1(self, *args):
-        set_module_args(dict(
-            name='foo.baz.bar',
-            lb_method='round_robin',
-            type='a',
-            provider=dict(
-                server='localhost',
-                password='password',
-                user='admin'
-            )
-        ))
-
-        module = AnsibleModule(
-            argument_spec=self.spec.argument_spec,
-            supports_check_mode=self.spec.supports_check_mode
-        )
-
-        # Override methods in the specific type of manager
-        tm = TypedManager(module=module, params=module.params)
-        tm.exists = Mock(return_value=False)
-        tm.create_on_device = Mock(return_value=True)
-        tm.version_is_less_than_12 = Mock(return_value=False)
-
-        # Override methods to force specific logic in the module to happen
-        mm = ModuleManager(module=module)
-        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_deprecated_lb_method2(self, *args):
-        set_module_args(dict(
-            name='foo.baz.bar',
-            lb_method='global_availability',
-            type='a',
-            provider=dict(
-                server='localhost',
-                password='password',
-                user='admin'
-            )
-        ))
-
-        module = AnsibleModule(
-            argument_spec=self.spec.argument_spec,
-            supports_check_mode=self.spec.supports_check_mode
-        )
-
-        # Override methods in the specific type of manager
-        tm = TypedManager(module=module, params=module.params)
-        tm.exists = Mock(return_value=False)
-        tm.create_on_device = Mock(return_value=True)
-        tm.version_is_less_than_12 = Mock(return_value=False)
-
-        # Override methods to force specific logic in the module to happen
-        mm = ModuleManager(module=module)
-        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'] == 'global-availability'
-
    def test_create_wideip_with_pool(self, *args):
        set_module_args(dict(
            name='foo.baz.bar',
--- a/test/units/modules/network/f5/test_bigip_virtual_address.py
+++ b/test/units/modules/network/f5/test_bigip_virtual_address.py
@ -36,7 +36,6 @@ except ImportError:
    # Ansible 2.8 imports
    from units.compat import unittest
    from units.compat.mock import Mock
-
    from units.modules.utils import set_module_args


@ -73,7 +72,6 @@ class TestParameters(unittest.TestCase):
            auto_delete='enabled',
            icmp_echo='enabled',
            availability_calculation='always',
-            use_route_advertisement='yes'
        )
        p = ModuleParameters(params=args)
        assert p.state == 'present'
@ -84,7 +82,6 @@ class TestParameters(unittest.TestCase):
        assert p.auto_delete is True
        assert p.icmp_echo == 'enabled'
        assert p.availability_calculation == 'none'
-        assert p.route_advertisement_type == 'enabled'

    def test_api_parameters(self):
        args = load_fixture('load_ltm_virtual_address_default.json')
@ -142,13 +139,6 @@ class TestParameters(unittest.TestCase):
        p = ModuleParameters(params=args)
        assert p.arp_state == 'disabled'

-    def test_module_parameters_use_route_advert_disabled(self):
-        args = dict(
-            use_route_advertisement='no'
-        )
-        p = ModuleParameters(params=args)
-        assert p.use_route_advertisement == 'disabled'
-
    def test_module_parameters_state_present(self):
        args = dict(
            state='present'
@ -196,7 +186,6 @@ class TestManager(unittest.TestCase):
            auto_delete='enabled',
            icmp_echo='enabled',
            advertise_route='always',
-            use_route_advertisement='yes',
            provider=dict(
                server='localhost',
                password='password',