From 0e906b086509ff9ac2aeb3a616b9e2472e59659c Mon Sep 17 00:00:00 2001 From: Chris Archibald Date: Tue, 27 Aug 2019 06:05:20 -0700 Subject: [PATCH] bug fixes for group (#60887) * bug fixes for group * trigger rebuild * fixes --- .../netapp/na_elementsw_access_group.py | 28 ++- .../netapp/test_na_elementsw_access_group.py | 175 ++++++++++++++++++ 2 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 test/units/modules/storage/netapp/test_na_elementsw_access_group.py diff --git a/lib/ansible/modules/storage/netapp/na_elementsw_access_group.py b/lib/ansible/modules/storage/netapp/na_elementsw_access_group.py index e5d12f6e443..87788184f1d 100644 --- a/lib/ansible/modules/storage/netapp/na_elementsw_access_group.py +++ b/lib/ansible/modules/storage/netapp/na_elementsw_access_group.py @@ -136,6 +136,10 @@ import ansible.module_utils.netapp as netapp_utils from ansible.module_utils.netapp_elementsw_module import NaElementSWModule HAS_SF_SDK = netapp_utils.has_sf_sdk() +try: + import solidfire.common +except ImportError: + HAS_SF_SDK = False class ElementSWAccessGroup(object): @@ -218,7 +222,7 @@ class ElementSWAccessGroup(object): try: account_id = self.elementsw_helper.account_exists(self.account_id) return account_id - except Exception: + except solidfire.common.ApiServerError: return None def get_volume_id(self): @@ -298,14 +302,14 @@ class ElementSWAccessGroup(object): changed = False update_group = False + input_account_id = self.account_id if self.account_id is not None: - self.account_id_valid = self.get_account_id() - + self.account_id = self.get_account_id() if self.state == 'present' and self.volumes is not None: - if self.account_id_valid: + if self.account_id: self.volumes = self.get_volume_id() else: - self.module.fail_json(msg='Error: Specified account id %s does not exist ' % self.account_id) + self.module.fail_json(msg='Error: Specified account id "%s" does not exist.' % str(input_account_id)) group_detail = self.get_access_group(self.access_group_name) @@ -320,10 +324,16 @@ class ElementSWAccessGroup(object): # If state - present, check for any parameter of exising group needs modification. if self.volumes is not None and len(self.volumes) > 0: # Compare the volume list - for volumeID in group_detail.volumes: - if volumeID not in self.volumes: - update_group = True - changed = True + if not group_detail.volumes: + # If access group does not have any volume attached + update_group = True + changed = True + else: + for volumeID in group_detail.volumes: + if volumeID not in self.volumes: + update_group = True + changed = True + break elif self.initiators is not None and group_detail.initiators != self.initiators: update_group = True diff --git a/test/units/modules/storage/netapp/test_na_elementsw_access_group.py b/test/units/modules/storage/netapp/test_na_elementsw_access_group.py new file mode 100644 index 00000000000..b14f7e6df92 --- /dev/null +++ b/test/units/modules/storage/netapp/test_na_elementsw_access_group.py @@ -0,0 +1,175 @@ +''' unit test for Ansible module: na_elementsw_account.py ''' + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +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 + +if not netapp_utils.has_sf_sdk(): + pytestmark = pytest.mark.skip('skipping as missing required SolidFire Python SDK') + +from ansible.modules.storage.netapp.na_elementsw_access_group \ + import ElementSWAccessGroup as my_module # module under test + + +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) + + +ADD_ERROR = 'some_error_in_add_access_group' + + +class MockSFConnection(object): + ''' mock connection to ElementSW host ''' + + class Bunch(object): # pylint: disable=too-few-public-methods + ''' create object with arbitrary attributes ''' + def __init__(self, **kw): + ''' called with (k1=v1, k2=v2), creates obj.k1, obj.k2 with values v1, v2 ''' + setattr(self, '__dict__', kw) + + def __init__(self, force_error=False, where=None): + ''' save arguments ''' + self.force_error = force_error + self.where = where + + def list_volume_access_groups(self, *args, **kwargs): # pylint: disable=unused-argument + ''' build access_group list: access_groups.name, access_groups.account_id ''' + access_groups = list() + access_group_list = self.Bunch(volume_access_groups=access_groups) + return access_group_list + + def create_volume_access_group(self, *args, **kwargs): # pylint: disable=unused-argument + ''' We don't check the return code, but could force an exception ''' + if self.force_error and 'add' in self.where: + # The module does not check for a specific exception :( + raise OSError(ADD_ERROR) + + def get_account_by_name(self, *args, **kwargs): # pylint: disable=unused-argument + ''' returns account_id ''' + if self.force_error and 'account_id' in self.where: + account_id = None + else: + account_id = 1 + print('account_id', account_id) + account = self.Bunch(account_id=account_id) + result = self.Bunch(account=account) + return result + + +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) + + 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']) + + @patch('ansible.module_utils.netapp.create_sf_connection') + def test_ensure_command_called(self, mock_create_sf_connection): + ''' a more interesting test ''' + set_module_args({ + 'state': 'present', + 'name': 'element_groupname', + 'account_id': 'element_account_id', + 'hostname': 'hostname', + 'username': 'username', + 'password': 'password', + }) + # my_obj.sfe will be assigned a MockSFConnection object: + mock_create_sf_connection.return_value = MockSFConnection() + my_obj = my_module() + with pytest.raises(AnsibleExitJson) as exc: + # It may not be a good idea to start with apply + # More atomic methods can be easier to mock + my_obj.apply() + print(exc.value.args[0]) + assert exc.value.args[0]['changed'] + + @patch('ansible.module_utils.netapp.create_sf_connection') + def test_check_error_reporting_on_add_exception(self, mock_create_sf_connection): + ''' a more interesting test ''' + set_module_args({ + 'state': 'present', + 'name': 'element_groupname', + 'account_id': 'element_account_id', + 'hostname': 'hostname', + 'username': 'username', + 'password': 'password', + }) + # my_obj.sfe will be assigned a MockSFConnection object: + mock_create_sf_connection.return_value = MockSFConnection(force_error=True, where=['add']) + my_obj = my_module() + with pytest.raises(AnsibleFailJson) as exc: + # It may not be a good idea to start with apply + # More atomic methods can be easier to mock + # apply() is calling list_accounts() and add_account() + my_obj.apply() + print(exc.value.args[0]) + message = 'Error creating volume access group element_groupname: %s' % ADD_ERROR + assert exc.value.args[0]['msg'] == message + + @patch('ansible.module_utils.netapp.create_sf_connection') + def test_check_error_reporting_on_invalid_account_id(self, mock_create_sf_connection): + ''' a more interesting test ''' + set_module_args({ + 'state': 'present', + 'name': 'element_groupname', + 'account_id': 'element_account_id', + 'volumes': ['volume1'], + 'hostname': 'hostname', + 'username': 'username', + 'password': 'password', + }) + # my_obj.sfe will be assigned a MockSFConnection object: + mock_create_sf_connection.return_value = MockSFConnection(force_error=True, where=['account_id']) + my_obj = my_module() + with pytest.raises(AnsibleFailJson) as exc: + # It may not be a good idea to start with apply + # More atomic methods can be easier to mock + # apply() is calling list_accounts() and add_account() + my_obj.apply() + print(exc.value.args[0]) + message = 'Error: Specified account id "%s" does not exist.' % 'element_account_id' + assert exc.value.args[0]['msg'] == message