From bff862b05df6249709eb5f01b2a440cbd17ea3af Mon Sep 17 00:00:00 2001 From: Tim Rupp Date: Sun, 21 Jan 2018 10:11:27 -0800 Subject: [PATCH] Fixes bigip_asm_policy (#35154) This module had been unable to successfully create policies on different partitions. This appears to be fixed now --- lib/ansible/module_utils/network/f5/bigip.py | 30 ++++++---- lib/ansible/module_utils/network/f5/bigiq.py | 30 ++++++---- .../module_utils/network/f5/iworkflow.py | 30 ++++++---- .../modules/network/f5/bigip_asm_policy.py | 57 ++++++++++++++++--- .../network/f5/test_bigip_asm_policy.py | 12 ++-- 5 files changed, 113 insertions(+), 46 deletions(-) diff --git a/lib/ansible/module_utils/network/f5/bigip.py b/lib/ansible/module_utils/network/f5/bigip.py index 5174b699c0e..7df358b7e1b 100644 --- a/lib/ansible/module_utils/network/f5/bigip.py +++ b/lib/ansible/module_utils/network/f5/bigip.py @@ -7,6 +7,8 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import time + try: from f5.bigip import ManagementRoot from icontrol.exceptions import iControlUnexpectedHTTPError @@ -25,18 +27,24 @@ except ImportError: class F5Client(F5BaseClient): @property def api(self): - try: - result = ManagementRoot( - self.params['server'], - self.params['user'], - self.params['password'], - port=self.params['server_port'], - verify=self.params['validate_certs'], - token='tmos' - ) - except Exception: + result = None + for x in range(0, 10): + try: + result = ManagementRoot( + self.params['server'], + self.params['user'], + self.params['password'], + port=self.params['server_port'], + verify=self.params['validate_certs'], + token='tmos' + ) + break + except Exception: + time.sleep(3) + if result: + return result + else: raise F5ModuleError( 'Unable to connect to {0} on port {1}. ' 'Is "validate_certs" preventing this?'.format(self.params['server'], self.params['server_port']) ) - return result diff --git a/lib/ansible/module_utils/network/f5/bigiq.py b/lib/ansible/module_utils/network/f5/bigiq.py index 80fd8668354..dff9913f072 100644 --- a/lib/ansible/module_utils/network/f5/bigiq.py +++ b/lib/ansible/module_utils/network/f5/bigiq.py @@ -7,6 +7,8 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import time + try: from f5.bigiq import ManagementRoot from icontrol.exceptions import iControlUnexpectedHTTPError @@ -25,18 +27,24 @@ except ImportError: class F5Client(F5BaseClient): @property def api(self): - try: - result = ManagementRoot( - self.params['server'], - self.params['user'], - self.params['password'], - port=self.params['server_port'], - verify=self.params['validate_certs'], - token='local' - ) - except Exception: + result = None + for x in range(0, 10): + try: + result = ManagementRoot( + self.params['server'], + self.params['user'], + self.params['password'], + port=self.params['server_port'], + verify=self.params['validate_certs'], + token='local' + ) + break + except Exception: + time.sleep(3) + if result: + return result + else: raise F5ModuleError( 'Unable to connect to {0} on port {1}. ' 'Is "validate_certs" preventing this?'.format(self.params['server'], self.params['server_port']) ) - return result diff --git a/lib/ansible/module_utils/network/f5/iworkflow.py b/lib/ansible/module_utils/network/f5/iworkflow.py index 903b57478e5..c4f42a685c9 100644 --- a/lib/ansible/module_utils/network/f5/iworkflow.py +++ b/lib/ansible/module_utils/network/f5/iworkflow.py @@ -7,6 +7,8 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import time + try: from f5.iworkflow import ManagementRoot from icontrol.exceptions import iControlUnexpectedHTTPError @@ -25,18 +27,24 @@ except ImportError: class F5Client(F5BaseClient): @property def api(self): - try: - result = ManagementRoot( - self.params['server'], - self.params['user'], - self.params['password'], - port=self.params['server_port'], - verify=self.params['validate_certs'], - token='local' - ) - except Exception: + result = None + for x in range(0, 10): + try: + result = ManagementRoot( + self.params['server'], + self.params['user'], + self.params['password'], + port=self.params['server_port'], + verify=self.params['validate_certs'], + token='local' + ) + break + except Exception: + time.sleep(3) + if result: + return result + else: raise F5ModuleError( 'Unable to connect to {0} on port {1}. ' 'Is "validate_certs" preventing this?'.format(self.params['server'], self.params['server_port']) ) - return result diff --git a/lib/ansible/modules/network/f5/bigip_asm_policy.py b/lib/ansible/modules/network/f5/bigip_asm_policy.py index 09b37d08bf8..17be58feb17 100644 --- a/lib/ansible/modules/network/f5/bigip_asm_policy.py +++ b/lib/ansible/modules/network/f5/bigip_asm_policy.py @@ -530,7 +530,6 @@ class BaseManager(object): policies = self.client.api.tm.asm.policies_s.get_collection() if any(p.name == self.want.name and p.partition == self.want.partition for p in policies): return True - return False def _file_is_missing(self): @@ -541,7 +540,6 @@ class BaseManager(object): return False def create(self): - task = None if self.want.active is None: self.want.update(dict(active=False)) if self._file_is_missing(): @@ -556,13 +554,9 @@ class BaseManager(object): self.create_blank() else: if self.want.template is not None: - task = self.create_from_template_on_device() + self.create_from_template() elif self.want.file is not None: - task = self.import_to_device() - if not task: - return False - if not self.wait_for_task(task): - raise F5ModuleError('Import policy task failed.') + self.create_from_file() if self.want.active: self.activate() @@ -670,6 +664,7 @@ class BaseManager(object): partition=self.want.partition, policyTemplateReference=self.want.template_link ) + time.sleep(2) return result def create_on_device(self): @@ -721,6 +716,38 @@ class V1Manager(BaseManager): super(V1Manager, self).__init__(client=client, module=module) self.want = V1Parameters(params=module.params, client=client) + def create_from_file(self): + self.import_to_device() + self.remove_temp_policy_from_device() + + def create_from_template(self): + self.create_from_template_on_device() + + def create_from_template_on_device(self): + full_name = fqdn_name(self.want.partition, self.want.name) + cmd = 'tmsh create asm policy {0} policy-template {1}'.format(full_name, self.want.template) + self.client.api.tm.util.bash.exec_cmd( + 'run', + utilCmdArgs='-c "{0}"'.format(cmd) + ) + + def remove_temp_policy_from_device(self): + name = os.path.split(self.want.file)[1] + tpath_name = '/var/config/rest/downloads/{0}'.format(name) + self.client.api.tm.util.unix_rm.exec_cmd('run', utilCmdArgs=tpath_name) + + def import_to_device(self): + self.client.api.shared.file_transfer.uploads.upload_file(self.want.file) + time.sleep(2) + name = os.path.split(self.want.file)[1] + full_name = fqdn_name(self.want.partition, self.want.name) + cmd = 'tmsh load asm policy {0} file /var/config/rest/downloads/{1}'.format(full_name, name) + self.client.api.tm.util.bash.exec_cmd( + 'run', + utilCmdArgs='-c "{0}"'.format(cmd) + ) + return True + class V2Manager(BaseManager): def __init__(self, *args, **kwargs): @@ -729,6 +756,20 @@ class V2Manager(BaseManager): super(V2Manager, self).__init__(client=client, module=module) self.want = V2Parameters(params=module.params, client=client) + def create_from_template(self): + task = self.create_from_template_on_device() + if not task: + return False + if not self.wait_for_task(task): + raise F5ModuleError('Import policy task failed.') + + def create_from_file(self): + task = self.import_to_device() + if not task: + return False + if not self.wait_for_task(task): + raise F5ModuleError('Import policy task failed.') + class ArgumentSpec(object): def __init__(self): diff --git a/test/units/modules/network/f5/test_bigip_asm_policy.py b/test/units/modules/network/f5/test_bigip_asm_policy.py index 880c6ae8718..6c51642eca7 100644 --- a/test/units/modules/network/f5/test_bigip_asm_policy.py +++ b/test/units/modules/network/f5/test_bigip_asm_policy.py @@ -113,6 +113,7 @@ class TestManager(unittest.TestCase): v1.wait_for_task = Mock(side_effect=[True, True]) v1.read_current_from_device = Mock(return_value=current) v1.apply_on_device = Mock(return_value=True) + v1.remove_temp_policy_from_device = Mock(return_value=True) # Override methods to force specific logic in the module to happen mm = ModuleManager(module=module) @@ -348,6 +349,7 @@ class TestManager(unittest.TestCase): v1.import_to_device = Mock(return_value=True) v1.wait_for_task = Mock(side_effect=[True, True]) v1.read_current_from_device = Mock(return_value=current) + v1.remove_temp_policy_from_device = Mock(return_value=True) # Override methods to force specific logic in the module to happen mm = ModuleManager(module=module) @@ -478,15 +480,15 @@ class TestManager(unittest.TestCase): msg = 'Import policy task failed.' # Override methods to force specific logic in the module to happen - v1 = V1Manager(module=module) - v1.exists = Mock(return_value=False) - v1.import_to_device = Mock(return_value=True) - v1.wait_for_task = Mock(return_value=False) + v2 = V2Manager(module=module) + v2.exists = Mock(return_value=False) + v2.import_to_device = Mock(return_value=True) + v2.wait_for_task = Mock(return_value=False) # Override methods to force specific logic in the module to happen mm = ModuleManager(module=module) mm.version_is_less_than_13 = Mock(return_value=False) - mm.get_manager = Mock(return_value=v1) + mm.get_manager = Mock(return_value=v2) with pytest.raises(F5ModuleError) as err: mm.exec_module()