From 5fcd835a3cf7c46b3fc09e5acaa55a628b4f8f45 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Mon, 3 Jun 2019 10:28:17 -0700 Subject: [PATCH 1/5] and new features --- .../storage/netapp/na_ontap_volume_clone.py | 140 +++++++++++------- 1 file changed, 85 insertions(+), 55 deletions(-) diff --git a/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py b/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py index 61988ff5cfda93..8796b12a696e17 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py @@ -30,23 +30,31 @@ description: - The parent volume of the volume clone being created. required: true - volume: + type: str + name: description: - The name of the volume clone being created. required: true + type: str + aliases: + - volume vserver: description: - Vserver in which the volume clone should be created. required: true + type: str parent_snapshot: description: - Parent snapshot in which volume clone is created off. + type: str parent_vserver: description: - Vserver of parent volume in which clone is created off. + type: str qos_policy_group_name: description: - The qos-policy-group-name which should be set for volume clone. + type: str space_reserve: description: - The space_reserve setting which should be used for the volume clone. @@ -59,6 +67,17 @@ version_added: '2.8' description: - Junction path of the volume. + type: str + uid: + version_added: '2.9' + description: + - The UNIX group ID for the clone volume. + type: int + gid: + version_added: '2.9' + description: + - The UNIX user ID for the clone volume. + type: int ''' EXAMPLES = """ @@ -70,22 +89,27 @@ hostname: "{{ netapp hostname }}" vserver: vs_hack parent_volume: normal_volume - volume: clone_volume_7 + name: clone_volume_7 space_reserve: none parent_snapshot: backup1 junction_path: /clone_volume_7 + uid: 1 + gid: 1 """ RETURN = """ """ +import traceback from ansible.module_utils.basic import AnsibleModule import ansible.module_utils.netapp as netapp_utils +from ansible.module_utils.netapp_module import NetAppModule +from ansible.module_utils._text import to_native HAS_NETAPP_LIB = netapp_utils.has_netapp_lib() -class NetAppOntapVolumeClone(object): +class NetAppONTAPVolumeClone(object): """ Creates a volume clone """ @@ -98,39 +122,33 @@ def __init__(self): self.argument_spec.update(dict( state=dict(required=False, choices=['present'], default='present'), parent_volume=dict(required=True, type='str'), - volume=dict(required=True, type='str'), + name=dict(required=True, type='str', aliases=["volume"]), vserver=dict(required=True, type='str'), parent_snapshot=dict(required=False, type='str', default=None), parent_vserver=dict(required=False, type='str', default=None), qos_policy_group_name=dict(required=False, type='str', default=None), space_reserve=dict(required=False, choices=['volume', 'none'], default=None), volume_type=dict(required=False, choices=['rw', 'dp']), - junction_path=dict(required=False, type='str', default=None) + junction_path=dict(required=False, type='str', default=None), + uid=dict(required=False, type='int'), + gid=dict(required=False, type='int') )) self.module = AnsibleModule( argument_spec=self.argument_spec, - supports_check_mode=True + supports_check_mode=True, + required_together=[ + ['uid', 'gid'] + ] ) - parameters = self.module.params - - # set up state variables - self.state = parameters['state'] - self.parent_snapshot = parameters['parent_snapshot'] - self.parent_volume = parameters['parent_volume'] - self.parent_vserver = parameters['parent_vserver'] - self.qos_policy_group_name = parameters['qos_policy_group_name'] - self.space_reserve = parameters['space_reserve'] - self.volume = parameters['volume'] - self.volume_type = parameters['volume_type'] - self.vserver = parameters['vserver'] - self.junction_path = parameters['junction_path'] + self.na_helper = NetAppModule() + self.parameters = self.na_helper.set_parameters(self.module.params) if HAS_NETAPP_LIB is False: self.module.fail_json(msg="the python NetApp-Lib module is required") else: - self.server = netapp_utils.setup_na_ontap_zapi(module=self.module, vserver=self.vserver) + self.server = netapp_utils.setup_na_ontap_zapi(module=self.module, vserver=self.parameters['vserver']) return def create_volume_clone(self): @@ -138,59 +156,71 @@ def create_volume_clone(self): Creates a new volume clone """ clone_obj = netapp_utils.zapi.NaElement('volume-clone-create') - clone_obj.add_new_child("parent-volume", self.parent_volume) - clone_obj.add_new_child("volume", self.volume) - if self.qos_policy_group_name: - clone_obj.add_new_child("qos-policy-group-name", self.qos_policy_group_name) - if self.space_reserve: - clone_obj.add_new_child("space-reserve", self.space_reserve) - if self.parent_snapshot: - clone_obj.add_new_child("parent-snapshot", self.parent_snapshot) - if self.parent_vserver: - clone_obj.add_new_child("parent-vserver", self.parent_vserver) - if self.volume_type: - clone_obj.add_new_child("volume-type", self.volume_type) - if self.junction_path: - clone_obj.add_new_child("junction-path", self.junction_path) - self.server.invoke_successfully(clone_obj, True) - - def does_volume_clone_exists(self): + clone_obj.add_new_child("parent-volume", self.parameters['parent_volume']) + clone_obj.add_new_child("volume", self.parameters['volume']) + if self.parameters.get('qos_policy_group_name'): + clone_obj.add_new_child("qos-policy-group-name", self.parameters['qos_policy_group_name']) + if self.parameters.get('space_reserve'): + clone_obj.add_new_child("space-reserve", self.parameters['space_reserve']) + if self.parameters.get('parent_snapshot'): + clone_obj.add_new_child("parent-snapshot", self.parameters['parent_snapshot']) + if self.parameters.get('parent_vserver'): + clone_obj.add_new_child("parent-vserver", self.parameters['parent_vserver']) + if self.parameters.get('volume_type'): + clone_obj.add_new_child("volume-type", self.parameters['volume_type']) + if self.parameters.get('junction_path'): + clone_obj.add_new_child("junction-path", self.parameters['junction_path']) + if self.parameters.get('uid'): + clone_obj.add_new_child("uid", str(self.parameters['uid'])) + clone_obj.add_new_child("gid", str(self.parameters['gid'])) + try: + self.server.invoke_successfully(clone_obj, True) + except netapp_utils.zapi.NaApiError as exc: + self.module.fail_json(msg='Error creating volume clone: %s: %s' % + (self.parameters['volume'], to_native(exc)), exception=traceback.format_exc()) + + def get_volume_clone(self): clone_obj = netapp_utils.zapi.NaElement('volume-clone-get') - clone_obj.add_new_child("volume", self.volume) + clone_obj.add_new_child("volume", self.parameters['volume']) try: results = self.server.invoke_successfully(clone_obj, True) - except netapp_utils.zapi.NaApiError: - return False - attributes = results.get_child_by_name('attributes') - info = attributes.get_child_by_name('volume-clone-info') - parent_volume = info.get_child_content('parent-volume') - if parent_volume == self.parent_volume: - return True - self.module.fail_json(msg="Error clone %s already exists for parent %s" % (self.volume, parent_volume)) + if results.get_child_by_name('attributes'): + attributes = results.get_child_by_name('attributes') + info = attributes.get_child_by_name('volume-clone-info') + parent_volume = info.get_child_content('parent-volume') + # checking if clone volume name already used to create by same parent volume + if parent_volume == self.parameters['parent_volume']: + return results + except netapp_utils.zapi.NaApiError as error: + # Error 15661 denotes an volume clone not being found. + if to_native(error.code) == "15661": + pass + else: + self.module.fail_json(msg='Error fetching volume clone information %s: %s' % + (self.parameters['volume'], to_native(error)), exception=traceback.format_exc()) + return None def apply(self): """ Run Module based on play book """ - changed = False netapp_utils.ems_log_event("na_ontap_volume_clone", self.server) - existing_volume_clone = self.does_volume_clone_exists() - if existing_volume_clone is False: # create clone - changed = True - if changed: + current = self.get_volume_clone() + cd_action = self.na_helper.get_cd_action(current, self.parameters) + if self.na_helper.changed: if self.module.check_mode: pass else: - self.create_volume_clone() - - self.module.exit_json(changed=changed) + if cd_action == 'create': + self.create_volume_clone() + self.module.exit_json(changed=self.na_helper.changed) def main(): """ Creates the NetApp Ontap Volume Clone object and runs the correct play task """ - obj = NetAppOntapVolumeClone() + obj = NetAppONTAPVolumeClone() obj.apply() From 3aade1d97ab919ae5e3faa079db6dfc410746516 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Mon, 17 Jun 2019 08:30:58 -0700 Subject: [PATCH 2/5] fix issues --- lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py b/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py index 8796b12a696e17..5fbe4e86d58b4f 100644 --- a/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py +++ b/lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py @@ -71,12 +71,12 @@ uid: version_added: '2.9' description: - - The UNIX group ID for the clone volume. + - The UNIX user ID for the clone volume. type: int gid: version_added: '2.9' description: - - The UNIX user ID for the clone volume. + - The UNIX group ID for the clone volume. type: int ''' From 5021bfcec18743afefec95711e58bac73aa57038 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Mon, 17 Jun 2019 08:48:29 -0700 Subject: [PATCH 3/5] fix issues --- test/sanity/validate-modules/ignore.txt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/sanity/validate-modules/ignore.txt b/test/sanity/validate-modules/ignore.txt index 572eef719a3e26..e0aa7f33cab6ad 100644 --- a/test/sanity/validate-modules/ignore.txt +++ b/test/sanity/validate-modules/ignore.txt @@ -1,5 +1,5 @@ -lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E337 -lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E338 +3579lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E337 +3579lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E338 lib/ansible/modules/cloud/alicloud/ali_instance_info.py E337 lib/ansible/modules/cloud/alicloud/ali_instance_info.py E338 lib/ansible/modules/cloud/alicloud/ali_instance.py E337 @@ -3576,7 +3576,6 @@ lib/ansible/modules/storage/netapp/na_ontap_user.py E337 lib/ansible/modules/storage/netapp/na_ontap_user.py E338 lib/ansible/modules/storage/netapp/na_ontap_user_role.py E337 lib/ansible/modules/storage/netapp/na_ontap_user_role.py E338 -lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py E337 lib/ansible/modules/storage/netapp/na_ontap_volume_clone.py E338 lib/ansible/modules/storage/netapp/na_ontap_volume.py E337 lib/ansible/modules/storage/netapp/na_ontap_volume.py E338 From c9ae11001f89574d7bf638e8777cf68096367eba Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Mon, 17 Jun 2019 14:12:54 -0700 Subject: [PATCH 4/5] fix issues --- test/sanity/validate-modules/ignore.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sanity/validate-modules/ignore.txt b/test/sanity/validate-modules/ignore.txt index e0aa7f33cab6ad..66db4533d066f2 100644 --- a/test/sanity/validate-modules/ignore.txt +++ b/test/sanity/validate-modules/ignore.txt @@ -1,5 +1,5 @@ -3579lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E337 -3579lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E338 +lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E337 +lib/ansible/modules/cloud/alicloud/ali_instance_facts.py E338 lib/ansible/modules/cloud/alicloud/ali_instance_info.py E337 lib/ansible/modules/cloud/alicloud/ali_instance_info.py E338 lib/ansible/modules/cloud/alicloud/ali_instance.py E337 From ab9e2578b748038359767926f5f699a9175039e3 Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Thu, 11 Jul 2019 08:41:23 -0700 Subject: [PATCH 5/5] add unit tests --- .../netapp/test_na_ontap_volume_clone.py | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 test/units/modules/storage/netapp/test_na_ontap_volume_clone.py diff --git a/test/units/modules/storage/netapp/test_na_ontap_volume_clone.py b/test/units/modules/storage/netapp/test_na_ontap_volume_clone.py new file mode 100644 index 00000000000000..2aadb0660d18b5 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_ontap_volume_clone.py @@ -0,0 +1,179 @@ +# (c) 2018, NetApp, Inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +''' unit tests ONTAP Ansible module: na_ontap_volume_clone''' + +from __future__ import print_function +import json +import pytest + +from units.compat import unittest +from units.compat.mock import patch +from ansible.module_utils import basic +from ansible.module_utils._text import to_bytes +import ansible.module_utils.netapp as netapp_utils + +from ansible.modules.storage.netapp.na_ontap_volume_clone \ + import NetAppONTAPVolumeClone as my_module + +if not netapp_utils.has_netapp_lib(): + pytestmark = pytest.mark.skip('skipping as missing required netapp_lib') + + +def set_module_args(args): + """prepare arguments so that they will be picked up during module creation""" + args = json.dumps({'ANSIBLE_MODULE_ARGS': args}) + basic._ANSIBLE_ARGS = to_bytes(args) # pylint: disable=protected-access + + +class AnsibleExitJson(Exception): + """Exception class to be raised by module.exit_json and caught by the test case""" + pass + + +class AnsibleFailJson(Exception): + """Exception class to be raised by module.fail_json and caught by the test case""" + pass + + +def exit_json(*args, **kwargs): # pylint: disable=unused-argument + """function to patch over exit_json; package return data into an exception""" + if 'changed' not in kwargs: + kwargs['changed'] = False + raise AnsibleExitJson(kwargs) + + +def fail_json(*args, **kwargs): # pylint: disable=unused-argument + """function to patch over fail_json; package return data into an exception""" + kwargs['failed'] = True + raise AnsibleFailJson(kwargs) + + +class MockONTAPConnection(object): + ''' mock server connection to ONTAP host ''' + + def __init__(self, kind=None): + ''' save arguments ''' + self.type = kind + self.xml_in = None + self.xml_out = None + + def invoke_successfully(self, xml, enable_tunneling): # pylint: disable=unused-argument + ''' mock invoke_successfully returning xml data ''' + self.xml_in = xml + if self.type == 'volume_clone': + xml = self.build_volume_clone_info() + elif self.type == 'volume_clone_fail': + raise netapp_utils.zapi.NaApiError(code='TEST', message="This exception is from the unit test") + self.xml_out = xml + return xml + + @staticmethod + def build_volume_clone_info(): + ''' build xml data for volume-clone-info ''' + xml = netapp_utils.zapi.NaElement('xml') + data = {'attributes': {'volume-clone-info': {'volume': 'ansible', + 'parent-volume': 'ansible'}}} + xml.translate_struct(data) + return xml + + +class TestMyModule(unittest.TestCase): + ''' a group of related Unit Tests ''' + + def setUp(self): + self.mock_module_helper = patch.multiple(basic.AnsibleModule, + exit_json=exit_json, + fail_json=fail_json) + self.mock_module_helper.start() + self.addCleanup(self.mock_module_helper.stop) + self.server = MockONTAPConnection() + self.onbox = False + + def set_default_args(self): + if self.onbox: + hostname = '10.10.10.10' + username = 'username' + password = 'password' + vserver = 'ansible' + volume = 'ansible' + parent_volume = 'ansible' + else: + hostname = '10.10.10.10' + username = 'username' + password = 'password' + vserver = 'ansible' + volume = 'ansible' + parent_volume = 'ansible' + return dict({ + 'hostname': hostname, + 'username': username, + 'password': password, + 'vserver': vserver, + 'volume': volume, + 'parent_volume': parent_volume + }) + + def test_module_fail_when_required_args_missing(self): + ''' required arguments are reported as errors ''' + with pytest.raises(AnsibleFailJson) as exc: + set_module_args({}) + my_module() + print('Info: %s' % exc.value.args[0]['msg']) + + def test_ensure_get_called(self): + ''' test get_volume_clone() for non-existent volume clone''' + set_module_args(self.set_default_args()) + my_obj = my_module() + my_obj.server = self.server + assert my_obj.get_volume_clone() is None + + def test_ensure_get_called_existing(self): + ''' test get_volume_clone() for existing volume clone''' + set_module_args(self.set_default_args()) + my_obj = my_module() + my_obj.server = MockONTAPConnection(kind='volume_clone') + assert my_obj.get_volume_clone() + + @patch('ansible.modules.storage.netapp.na_ontap_volume_clone.NetAppONTAPVolumeClone.create_volume_clone') + def test_successful_create(self, create_volume_clone): + ''' creating volume_clone and testing idempotency ''' + module_args = { + 'parent_snapshot': 'abc', + 'parent_vserver': 'abc', + 'volume_type': 'dp', + 'qos_policy_group_name': 'abc', + 'junction_path': 'abc', + 'uid': '1', + 'gid': '1' + } + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = self.server + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + assert exc.value.args[0]['changed'] + create_volume_clone.assert_called_with() + # to reset na_helper from remembering the previous 'changed' value + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('volume_clone') + with pytest.raises(AnsibleExitJson) as exc: + my_obj.apply() + assert not exc.value.args[0]['changed'] + + def test_if_all_methods_catch_exception(self): + module_args = {} + module_args.update(self.set_default_args()) + set_module_args(module_args) + my_obj = my_module() + if not self.onbox: + my_obj.server = MockONTAPConnection('volume_clone_fail') + with pytest.raises(AnsibleFailJson) as exc: + my_obj.get_volume_clone() + assert 'Error fetching volume clone information ' in exc.value.args[0]['msg'] + with pytest.raises(AnsibleFailJson) as exc: + my_obj.create_volume_clone() + assert 'Error creating volume clone: ' in exc.value.args[0]['msg']