From 1b2411c6d2e5ff203d69d4f357bc78b72ed87f14 Mon Sep 17 00:00:00 2001 From: John R Barker Date: Fri, 31 Mar 2017 10:39:59 +0100 Subject: [PATCH] Fixes generators and other misc fixes (#22887) (#23172) fixed itertools.imap busting several things that used to be lists, profiles not being set correctly, upon create, when it was a separate method, allowed port having the wrong lowest port (zero is allowed), empty port value should just be interpreted as None. (cherry picked from commit 2a576a19993832d97abf4692b3bb35b1a846bcb1) --- .../network/f5/bigip_virtual_server.py | 81 +++++++++++++------ 1 file changed, 57 insertions(+), 24 deletions(-) diff --git a/lib/ansible/modules/network/f5/bigip_virtual_server.py b/lib/ansible/modules/network/f5/bigip_virtual_server.py index b28131dce29..f5394d59762 100644 --- a/lib/ansible/modules/network/f5/bigip_virtual_server.py +++ b/lib/ansible/modules/network/f5/bigip_virtual_server.py @@ -18,10 +18,11 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . -ANSIBLE_METADATA = {'metadata_version': '1.0', - 'status': ['preview'], - 'supported_by': 'community'} - +ANSIBLE_METADATA = { + 'status': ['preview'], + 'supported_by': 'community', + 'metadata_version': '1.0' +} DOCUMENTATION = ''' --- @@ -76,7 +77,9 @@ options: - ip port: description: - - Port of the virtual server . Required when state=present and vs does not exist + - Port of the virtual server. Required when state=present and vs does + not exist. If you specify a value for this field, it must be a number + between 0 and 65535. required: false default: None all_profiles: @@ -196,7 +199,6 @@ deleted: sample: "my-virtual-server" ''' - # map of state values STATES = { 'enabled': 'STATE_ENABLED', @@ -225,9 +227,19 @@ def vs_exists(api, vs): return result -def vs_create(api, name, destination, port, pool): - _profiles = [[{'profile_context': 'PROFILE_CONTEXT_TYPE_ALL', 'profile_name': 'tcp'}]] - created = False +def vs_create(api, name, destination, port, pool, profiles): + if profiles: + _profiles = [] + for profile in profiles: + _profiles.append( + dict( + profile_context='PROFILE_CONTEXT_TYPE_ALL', + profile_name=profile + ) + ) + else: + _profiles = [{'profile_context': 'PROFILE_CONTEXT_TYPE_ALL', 'profile_name': 'tcp'}] + # a bit of a hack to handle concurrent runs of this module. # even though we've checked the vs doesn't exist, # it may exist by the time we run create_vs(). @@ -238,12 +250,11 @@ def vs_create(api, name, destination, port, pool): definitions=[{'name': [name], 'address': [destination], 'port': port, 'protocol': 'PROTOCOL_TCP'}], wildmasks=['255.255.255.255'], resources=[{'type': 'RESOURCE_TYPE_POOL', 'default_pool_name': pool}], - profiles=_profiles) + profiles=[_profiles]) created = True return created except bigsuds.OperationFailed as e: - if "already exists" not in str(e): - raise Exception('Error on creating Virtual Server : %s' % e) + raise Exception('Error on creating Virtual Server : %s' % e) def vs_remove(api, name): @@ -298,9 +309,11 @@ def get_profiles(api, name): def set_profiles(api, name, profiles_list): updated = False + try: if profiles_list is None: return False + profiles_list = list(profiles_list) current_profiles = list(map(lambda x: x['profile_name'], get_profiles(api, name))) to_add_profiles = [] for x in profiles_list: @@ -322,6 +335,11 @@ def set_profiles(api, name, profiles_list): profiles=[to_add_profiles] ) updated = True + current_profiles = list(map(lambda x: x['profile_name'], get_profiles(api, name))) + if len(current_profiles) == 0: + raise F5ModuleError( + "Virtual servers must has at least one profile" + ) return updated except bigsuds.OperationFailed as e: raise Exception('Error on setting profiles : %s' % e) @@ -338,6 +356,7 @@ def set_policies(api, name, policies_list): try: if policies_list is None: return False + policies_list = list(policies_list) current_policies = get_policies(api, name) to_add_policies = [] for x in policies_list: @@ -376,6 +395,7 @@ def set_enabled_vlans(api, name, vlans_enabled_list): try: if vlans_enabled_list is None: return updated + vlans_enabled_list = list(vlans_enabled_list) current_vlans = get_vlan(api, name) # Set allowed list back to default ("all") @@ -632,18 +652,24 @@ def set_fallback_persistence_profile(api, partition, name, persistence_profile): def get_route_advertisement_status(api, address): - result = api.LocalLB.VirtualAddressV2.get_route_advertisement_state(virtual_addresses=[address]).pop(0) - result = result.split("STATE_")[-1].lower() + result = None + results = api.LocalLB.VirtualAddressV2.get_route_advertisement_state(virtual_addresses=[address]) + if results: + result = results.pop(0) + result = result.split("STATE_")[-1].lower() return result def set_route_advertisement_state(api, destination, partition, route_advertisement_state): updated = False + if route_advertisement_state is None: + return False + try: state = "STATE_%s" % route_advertisement_state.strip().upper() address = fq_name(partition, destination,) - current_route_advertisement_state=get_route_advertisement_status(api,address) + current_route_advertisement_state = get_route_advertisement_status(api,address) if current_route_advertisement_state != route_advertisement_state: api.LocalLB.VirtualAddressV2.set_route_advertisement_state(virtual_addresses=[address], states=[state]) updated = True @@ -659,15 +685,19 @@ def main(): choices=['present', 'absent', 'disabled', 'enabled']), name=dict(type='str', required=True, aliases=['vs']), destination=dict(type='str', aliases=['address', 'ip']), - port=dict(type='int'), + port=dict(type='str', default=None), all_policies=dict(type='list'), - all_profiles=dict(type='list'), + all_profiles=dict(type='list', default=None), all_rules=dict(type='list'), enabled_vlans=dict(type='list'), pool=dict(type='str'), description=dict(type='str'), snat=dict(type='str'), - route_advertisement_state=dict(type='str', default='disabled', choices=['enabled', 'disabled']), + route_advertisement_state=dict( + type='str', + default=None, + choices=['enabled', 'disabled'] + ), default_persistence_profile=dict(type='str'), fallback_persistence_profile=dict(type='str') )) @@ -696,6 +726,10 @@ def main(): name = fq_name(partition, module.params['name']) destination = module.params['destination'] port = module.params['port'] + if port == '' or port is None: + port = None + else: + port = int(port) all_profiles = fq_list_names(partition, module.params['all_profiles']) all_policies = fq_list_names(partition, module.params['all_policies']) all_rules = fq_list_names(partition, module.params['all_rules']) @@ -713,8 +747,8 @@ def main(): default_persistence_profile = fq_name(partition, module.params['default_persistence_profile']) fallback_persistence_profile = module.params['fallback_persistence_profile'] - if 1 > port > 65535: - module.fail_json(msg="valid ports must be in range 1 - 65535") + if 0 > port > 65535: + module.fail_json(msg="valid ports must be in range 0 - 65535") try: api = bigip_api(server, user, password, validate_certs, port=server_port) @@ -740,7 +774,7 @@ def main(): else: update = False if not vs_exists(api, name): - if (not destination) or (not port): + if (not destination) or (port is None): module.fail_json(msg="both destination and port must be supplied to create a VS") if not module.check_mode: # a bit of a hack to handle concurrent runs of this module. @@ -749,8 +783,7 @@ def main(): # this catches the exception and does something smart # about it! try: - vs_create(api, name, destination, port, pool) - set_profiles(api, name, all_profiles) + vs_create(api, name, destination, port, pool, all_profiles) set_policies(api, name, all_policies) set_enabled_vlans(api, name, all_enabled_vlans) set_rules(api, name, all_rules) @@ -789,7 +822,7 @@ def main(): result['changed'] |= set_route_advertisement_state(api, destination, partition, route_advertisement_state) api.System.Session.submit_transaction() except Exception as e: - raise Exception("Error on updating Virtual Server : %s" % e) + raise Exception("Error on updating Virtual Server : %s" % str(e)) else: # check-mode return value result = {'changed': True}