small fixes and additions to bigip_virtual_address (#40362)

* Updated parameter names to match updated features in BIGIP
* Added support for route domains

2 files changed, 186 insertions, 60 deletions

diff --git a/lib/ansible/modules/network/f5/bigip_virtual_address.py b/lib/ansible/modules/network/f5/bigip_virtual_address.py
index 56d7b3f707..db757a3ea4 100644
--- a/lib/ansible/modules/network/f5/bigip_virtual_address.py
+++ b/lib/ansible/modules/network/f5/bigip_virtual_address.py
@@ -90,7 +90,7 @@ options:
       - absent
       - enabled
       - disabled
-  advertise_route:
+  availability_calculation:
     description:
       - Specifies what routes of the virtual address the system advertises.
         When C(when_any_available), advertises the route when any virtual
@@ -101,12 +101,46 @@ options:
       - always
       - when_all_available
       - 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.
+        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
+        virtual address.
+      - When disabled, the system does not advertise routes for this virtual address.
+      - The majority of these options are only supported on versions 13.0.0-HF1 or
+        higher. On versions less than this, all choices expect C(disabled) will
+        translate to C(enabled).
+      - When C(always), the BIG-IP system will always advertise the route for the
+        virtual address, regardless of availability status. This requires an C(enabled)
+        virtual address.
+      - When C(enabled), the BIG-IP system will advertise the route for the available
+        virtual address, based on the calculation method in the availability calculation.
+      - When C(disabled), the BIG-IP system will not advertise the route for the virtual
+        address, regardless of the availability status.
+      - When C(selective), you can also selectively enable ICMP echo responses, which
+        causes the BIG-IP system to internally enable or disable responses based on
+        virtual server state. Either C(any) virtual server, C(all) virtual servers, or
+        C(always), regardless of the state of any virtual server.
+      - When C(any), the BIG-IP system will advertise the route for the virtual address
+        when any virtual server is available.
+      - When C(all), the BIG-IP system will advertise the route for the virtual address
+        when all virtual servers are available.
+    choices:
+      - disabled
+      - enabled
+      - always
+      - selective
+      - any
+      - all
+    version_added: 2.6
   partition:
     description:
       - Device partition to manage resources on.
@@ -118,6 +152,11 @@ options:
         if this value is not specified, the default of C(/Common/traffic-group-1)
         will be used.
     version_added: 2.5
+  route_domain:
+    description:
+      - The route domain of the C(address) that you want to use.
+      - This value cannot be modified after it is set.
+    version_added: 2.6
 notes:
   - Requires the netaddr Python package on the host. This is as easy as pip
     install netaddr.
@@ -197,6 +236,7 @@ from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.basic import env_fallback
 from ansible.module_utils.parsing.convert_bool import BOOLEANS_TRUE
 from ansible.module_utils.parsing.convert_bool import BOOLEANS_FALSE
+from distutils.version import LooseVersion
 
 try:
     from library.module_utils.network.f5.bigip import HAS_F5SDK
@@ -232,24 +272,24 @@ except ImportError:
 
 class Parameters(AnsibleF5Parameters):
     api_map = {
-        'routeAdvertisement': 'use_route_advertisement',
+        'routeAdvertisement': 'route_advertisement_type',
         'autoDelete': 'auto_delete',
         'icmpEcho': 'icmp_echo',
         'connectionLimit': 'connection_limit',
-        'serverScope': 'advertise_route',
+        'serverScope': 'availability_calculation',
         'mask': 'netmask',
         'arp': 'arp_state',
         'trafficGroup': 'traffic_group',
     }
 
     updatables = [
-        'use_route_advertisement', 'auto_delete', 'icmp_echo', 'connection_limit',
-        'arp_state', 'enabled', 'advertise_route', 'traffic_group', 'address'
+        'route_advertisement_type', 'auto_delete', 'icmp_echo', 'connection_limit',
+        'arp_state', 'enabled', 'availability_calculation', 'traffic_group'
     ]
 
     returnables = [
-        'use_route_advertisement', 'auto_delete', 'icmp_echo', 'connection_limit',
-        'netmask', 'arp_state', 'address', 'state', 'traffic_group'
+        'route_advertisement_type', 'auto_delete', 'icmp_echo', 'connection_limit',
+        'netmask', 'arp_state', 'address', 'state', 'traffic_group', 'route_domain'
     ]
 
     api_attributes = [
@@ -258,14 +298,14 @@ class Parameters(AnsibleF5Parameters):
     ]
 
     @property
-    def advertise_route(self):
-        if self._values['advertise_route'] is None:
+    def availability_calculation(self):
+        if self._values['availability_calculation'] is None:
             return None
-        elif self._values['advertise_route'] in ['any', 'when_any_available']:
+        elif self._values['availability_calculation'] in ['any', 'when_any_available']:
             return 'any'
-        elif self._values['advertise_route'] in ['all', 'when_all_available']:
+        elif self._values['availability_calculation'] in ['all', 'when_all_available']:
             return 'all'
-        elif self._values['advertise_route'] in ['none', 'always']:
+        elif self._values['availability_calculation'] in ['none', 'always']:
             return 'none'
 
     @property
@@ -275,17 +315,6 @@ class Parameters(AnsibleF5Parameters):
         return int(self._values['connection_limit'])
 
     @property
-    def use_route_advertisement(self):
-        if self._values['use_route_advertisement'] is None:
-            return None
-        elif self._values['use_route_advertisement'] in BOOLEANS_TRUE:
-            return 'enabled'
-        elif self._values['use_route_advertisement'] == 'enabled':
-            return 'enabled'
-        else:
-            return 'disabled'
-
-    @property
     def enabled(self):
         if self._values['state'] in ['enabled', 'present']:
             return 'yes'
@@ -299,18 +328,6 @@ class Parameters(AnsibleF5Parameters):
             return None
 
     @property
-    def address(self):
-        if self._values['address'] is None:
-            return None
-        try:
-            ip = netaddr.IPAddress(self._values['address'])
-            return str(ip)
-        except netaddr.core.AddrFormatError:
-            raise F5ModuleError(
-                "The provided 'address' is not a valid IP address"
-            )
-
-    @property
     def netmask(self):
         if self._values['netmask'] is None:
             return None
@@ -355,6 +372,39 @@ class Parameters(AnsibleF5Parameters):
                 "Traffic groups can only exist in /Common"
             )
 
+    @property
+    def route_advertisement_type(self):
+        if self.use_route_advertisement:
+            return self.use_route_advertisement
+        elif 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:
+            return None
+        version = self.client.api.tmos_version
+        if LooseVersion(version) <= LooseVersion('13.0.0'):
+            if self._values['route_advertisement'] == 'disabled':
+                return 'disabled'
+            else:
+                return 'enabled'
+        else:
+            return self._values['route_advertisement']
+
     def to_return(self):
         result = {}
         for returnable in self.returnables:
@@ -369,10 +419,50 @@ class ApiParameters(Parameters):
 
 class ModuleParameters(Parameters):
     @property
+    def address(self):
+        if self._values['address'] is None:
+            return None
+        try:
+            ip = netaddr.IPAddress(self._values['address'])
+            return str(ip)
+        except netaddr.core.AddrFormatError:
+            raise F5ModuleError(
+                "The provided 'address' is not a valid IP address"
+            )
+
+    @property
+    def route_domain(self):
+        if self._values['route_domain'] is None:
+            return None
+        try:
+            return int(self._values['route_domain'])
+        except ValueError:
+            try:
+                rd = self.client.api.tm.net.route_domains.route_domain.load(
+                    name=self._values['route_domain'],
+                    partition=self.partition
+                )
+                return int(rd.id)
+            except iControlUnexpectedHTTPError:
+                raise F5ModuleError(
+                    "The specified 'route_domain' was not found."
+                )
+
+    @property
+    def full_address(self):
+        if self.route_domain is not None:
+            return '{0}%{1}'.format(self.address, self.route_domain)
+        return self.address
+
+    @property
     def name(self):
         if self._values['name'] is None:
-            return str(self.address)
-        return self._values['name']
+            result = str(self.address)
+            if self.route_domain:
+                result = "{0}%{1}".format(result, self.route_domain)
+        else:
+            result = self._values['name']
+        return result
 
 
 class Changes(Parameters):
@@ -380,7 +470,14 @@ class Changes(Parameters):
 
 
 class UsableChanges(Changes):
-    pass
+    @property
+    def address(self):
+        if self._values['address'] is None:
+            return None
+        if self._values['route_domain'] is None:
+            return self._values['address']
+        result = "{0}%{1}".format(self._values['address'], self._values['route_domain'])
+        return result
 
 
 class ReportableChanges(Changes):
@@ -485,16 +582,23 @@ class ModuleManager(object):
         return changed
 
     def read_current_from_device(self):
+        name = self.want.name
+        name = name.replace('%', '%25')
         resource = self.client.api.tm.ltm.virtual_address_s.virtual_address.load(
-            name=self.want.name,
+            name=name,
             partition=self.want.partition
         )
         result = resource.attrs
         return ApiParameters(params=result)
 
     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')
         result = self.client.api.tm.ltm.virtual_address_s.virtual_address.exists(
-            name=self.want.name,
+            name=name,
             partition=self.want.partition
         )
         return result
@@ -508,7 +612,7 @@ class ModuleManager(object):
                     "the virtual address if you need to do this."
                 )
         if self.want.address is not None:
-            if self.have.address != self.want.address:
+            if self.have.address != self.want.full_address:
                 raise F5ModuleError(
                     "The address cannot be changed. Delete and recreate "
                     "the virtual address if you need to do this."
@@ -522,8 +626,10 @@ class ModuleManager(object):
 
     def update_on_device(self):
         params = self.changes.api_params()
+        name = self.want.name
+        name = name.replace('%', '%25')
         resource = self.client.api.tm.ltm.virtual_address_s.virtual_address.load(
-            name=self.want.name,
+            name=name,
             partition=self.want.partition
         )
         resource.modify(**params)
@@ -545,7 +651,7 @@ class ModuleManager(object):
         self.client.api.tm.ltm.virtual_address_s.virtual_address.create(
             name=self.want.name,
             partition=self.want.partition,
-            address=self.want.address,
+            address=self.changes.address,
             **params
         )
 
@@ -558,8 +664,10 @@ class ModuleManager(object):
         return True
 
     def remove_from_device(self):
+        name = self.want.name
+        name = name.replace('%', '%25')
         resource = self.client.api.tm.ltm.virtual_address_s.virtual_address.load(
-            name=self.want.name,
+            name=name,
             partition=self.want.partition
         )
         resource.delete()
@@ -591,17 +699,30 @@ class ArgumentSpec(object):
             icmp_echo=dict(
                 choices=['enabled', 'disabled', 'selective'],
             ),
-            advertise_route=dict(
+            availability_calculation=dict(
                 choices=['always', 'when_all_available', 'when_any_available'],
+                aliases=['advertise_route']
             ),
             use_route_advertisement=dict(
-                type='bool'
+                type='bool',
+                removed_in_version=2.9,
+            ),
+            route_advertisement=dict(
+                choices=[
+                    'disabled',
+                    'enabled',
+                    'always',
+                    'selective',
+                    'any',
+                    'all',
+                ]
             ),
             traffic_group=dict(),
             partition=dict(
                 default='Common',
                 fallback=(env_fallback, ['F5_PARTITION'])
-            )
+            ),
+            route_domain=dict()
         )
         self.argument_spec = {}
         self.argument_spec.update(f5_argument_spec)
@@ -609,6 +730,9 @@ class ArgumentSpec(object):
         self.required_one_of = [
             ['name', 'address']
         ]
+        self.mutually_exclusive = [
+            ['use_route_advertisement', 'route_advertisement']
+        ]
 
 
 def main():
diff --git a/test/units/modules/network/f5/test_bigip_virtual_address.py b/test/units/modules/network/f5/test_bigip_virtual_address.py
index be3536ac54..7fdd990433 100644
--- a/test/units/modules/network/f5/test_bigip_virtual_address.py
+++ b/test/units/modules/network/f5/test_bigip_virtual_address.py
@@ -21,6 +21,7 @@ from ansible.module_utils.basic import AnsibleModule
 
 try:
     from library.modules.bigip_virtual_address import Parameters
+    from library.modules.bigip_virtual_address import ModuleParameters
     from library.modules.bigip_virtual_address import ModuleManager
     from library.modules.bigip_virtual_address import ArgumentSpec
     from library.module_utils.network.f5.common import F5ModuleError
@@ -29,6 +30,7 @@ try:
 except ImportError:
     try:
         from ansible.modules.network.f5.bigip_virtual_address import Parameters
+        from ansible.modules.network.f5.bigip_virtual_address import ModuleParameters
         from ansible.modules.network.f5.bigip_virtual_address import ModuleManager
         from ansible.modules.network.f5.bigip_virtual_address import ArgumentSpec
         from ansible.module_utils.network.f5.common import F5ModuleError
@@ -69,10 +71,10 @@ class TestParameters(unittest.TestCase):
             arp_state='enabled',
             auto_delete='enabled',
             icmp_echo='enabled',
-            advertise_route='always',
+            availability_calculation='always',
             use_route_advertisement='yes'
         )
-        p = Parameters(params=args)
+        p = ModuleParameters(params=args)
         assert p.state == 'present'
         assert p.address == '1.1.1.1'
         assert p.netmask == '2.2.2.2'
@@ -80,8 +82,8 @@ class TestParameters(unittest.TestCase):
         assert p.arp_state == 'enabled'
         assert p.auto_delete is True
         assert p.icmp_echo == 'enabled'
-        assert p.advertise_route == 'none'
-        assert p.use_route_advertisement == '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')
@@ -94,22 +96,22 @@ class TestParameters(unittest.TestCase):
         assert p.state == 'enabled'
         assert p.icmp_echo == 'enabled'
         assert p.netmask == '255.255.255.255'
-        assert p.use_route_advertisement == 'disabled'
-        assert p.advertise_route == 'any'
+        assert p.route_advertisement_type == 'disabled'
+        assert p.availability_calculation == 'any'
 
     def test_module_parameters_advertise_route_all(self):
         args = dict(
-            advertise_route='when_all_available'
+            availability_calculation='when_all_available'
         )
         p = Parameters(params=args)
-        assert p.advertise_route == 'all'
+        assert p.availability_calculation == 'all'
 
     def test_module_parameters_advertise_route_any(self):
         args = dict(
-            advertise_route='when_any_available'
+            availability_calculation='when_any_available'
         )
         p = Parameters(params=args)
-        assert p.advertise_route == 'any'
+        assert p.availability_calculation == 'any'
 
     def test_module_parameters_icmp_echo_disabled(self):
         args = dict(
@@ -143,7 +145,7 @@ class TestParameters(unittest.TestCase):
         args = dict(
             use_route_advertisement='no'
         )
-        p = Parameters(params=args)
+        p = ModuleParameters(params=args)
         assert p.use_route_advertisement == 'disabled'
 
     def test_module_parameters_state_present(self):