diff --git a/README.md b/README.md index ecf4ad6..69463cc 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,14 @@ This collection follows the [Ansible project's Code of Conduct](https://docs.ans # Release Notes +## 21.9.0 + +### New Options + - azure_rm_netapp_volume - `feature_flags` to selectively enable/disable a feature. + +### Bug Fixes + - azure_rm_netapp_volume - 'Change Ownership' is not permitted when creating NFSv4.1 volume with latest azure-mgmt-netapp package (4.0.0). + ## 21.8.1 ### Bug Fixes diff --git a/changelogs/fragments/DEVOPS-4246.yaml b/changelogs/fragments/DEVOPS-4246.yaml new file mode 100644 index 0000000..781042d --- /dev/null +++ b/changelogs/fragments/DEVOPS-4246.yaml @@ -0,0 +1,4 @@ +minor_changes: + - azure_rm_netapp_volume - new option ``feature_flags`` to selectively enable/disable a feature. +bugfixes: + - azure_rm_netapp_volume - 'Change Ownership' is not permitted when creating NFSv4.1 volume with latest azure-mgmt-netapp package (4.0.0). diff --git a/galaxy.yml b/galaxy.yml index bbe2e24..9f3e3e0 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -1,6 +1,6 @@ namespace: "netapp" name: "azure" -version: "21.8.1" +version: "21.9.0" authors: - "NetApp Ansible Team " license_file: COPYING diff --git a/plugins/module_utils/azure_rm_netapp_common.py b/plugins/module_utils/azure_rm_netapp_common.py index 521e4c6..e2debe4 100644 --- a/plugins/module_utils/azure_rm_netapp_common.py +++ b/plugins/module_utils/azure_rm_netapp_common.py @@ -14,8 +14,9 @@ HAS_AZURE_COLLECTION = True NEW_STYLE = None -COLLECTION_VERSION = "21.8.1" -IMPORT_ERRORS = list() +COLLECTION_VERSION = "21.9.0" +IMPORT_ERRORS = [] +SDK_VERSION = "0.0.0" if 'pytest' in sys.modules: from ansible_collections.netapp.azure.plugins.module_utils.netapp_module import AzureRMModuleBaseMock as AzureRMModuleBase @@ -45,12 +46,18 @@ except ImportError as exc: IMPORT_ERRORS.append(str(exc)) +try: + from azure.mgmt.netapp import VERSION as SDK_VERSION +except ImportError as exc: + IMPORT_ERRORS.append(str(exc)) + class AzureRMNetAppModuleBase(AzureRMModuleBase): ''' Wrapper around AzureRMModuleBase base class ''' def __init__(self, derived_arg_spec, required_if=None, supports_check_mode=False, supports_tags=True): self._netapp_client = None self._new_style = NEW_STYLE + self._sdk_version = SDK_VERSION super(AzureRMNetAppModuleBase, self).__init__(derived_arg_spec=derived_arg_spec, required_if=required_if, supports_check_mode=supports_check_mode, @@ -91,6 +98,10 @@ def netapp_client(self): def new_style(self): return self._new_style + @property + def sdk_version(self): + return self._sdk_version + def get_method(self, category, name): try: methods = getattr(self.netapp_client, category) @@ -116,3 +127,30 @@ def fail_when_import_errors(self, import_errors, has_azure_mgmt_netapp=True): self.module.fail_json(msg=msg) msg += str(import_errors) raise ImportError(msg) + + def has_feature(self, feature_name): + feature = self.get_feature(feature_name) + if isinstance(feature, bool): + return feature + self.module.fail_json(msg="Error: expected bool type for feature flag: %s" % feature_name) + + def get_feature(self, feature_name): + ''' if the user has configured the feature, use it + otherwise, use our default + ''' + default_flags = dict( + # TODO: review need for these + # trace_apis=False, # if true, append REST requests/responses to /tmp/azure_apis.log + # check_required_params_for_none=True, + # deprecation_warning=True, + # show_modified=True, + # + # preview features in ANF + ignore_change_ownership_mode=True + ) + + if self.parameters.get('feature_flags') is not None and feature_name in self.parameters['feature_flags']: + return self.parameters['feature_flags'][feature_name] + if feature_name in default_flags: + return default_flags[feature_name] + self.module.fail_json(msg="Internal error: unexpected feature flag: %s" % feature_name) diff --git a/plugins/module_utils/netapp_module.py b/plugins/module_utils/netapp_module.py index ff53858..9ee758c 100644 --- a/plugins/module_utils/netapp_module.py +++ b/plugins/module_utils/netapp_module.py @@ -50,6 +50,10 @@ def __init__(self, derived_arg_spec, required_if=None, supports_check_mode=False # remove values with a default of None (not required) self.module_arg_spec = dict([item for item in self.module_arg_spec.items() if item[0] in self.parameters]) + def update_tags(self, tags): + self.module.log('update_tags called with:', tags) + return None, None + def cmp(obj1, obj2): """ @@ -83,17 +87,17 @@ class NetAppModule(): ''' def __init__(self): - self.log = list() + self.log = [] self.changed = False self.parameters = {'name': 'not intialized'} self.zapi_string_keys = dict() self.zapi_bool_keys = dict() - self.zapi_list_keys = dict() - self.zapi_int_keys = dict() - self.zapi_required = dict() + self.zapi_list_keys = {} + self.zapi_int_keys = {} + self.zapi_required = {} def set_parameters(self, ansible_params): - self.parameters = dict() + self.parameters = {} for param in ansible_params: if ansible_params[param] is not None: self.parameters[param] = ansible_params[param] @@ -109,11 +113,7 @@ def get_cd_action(self, current, desired): is_present = 'present' action = cd_action(current=is_present, desired = self.desired.state()) ''' - if 'state' in desired: - desired_state = desired['state'] - else: - desired_state = 'present' - + desired_state = desired['state'] if 'state' in desired else 'present' if current is None and desired_state == 'absent': return None if current is not None and desired_state == 'present': @@ -125,7 +125,7 @@ def get_cd_action(self, current, desired): return 'create' def compare_and_update_values(self, current, desired, keys_to_compare): - updated_values = dict() + updated_values = {} is_changed = False for key in keys_to_compare: if key in current: @@ -185,7 +185,7 @@ def get_modified_attributes(self, current, desired, get_list_diff=False): aggregate name) ''' # if the object does not exist, we can't modify it - modified = dict() + modified = {} if current is None: return modified @@ -224,7 +224,7 @@ def is_rename_action(self, source, target): # idempotency (or) new_name_is_already_in_use # alternatively we could delete B and rename A to B return False - if source is None and target is not None: + if source is None: # do nothing, maybe the rename was already done return False # source is not None and target is None: @@ -238,7 +238,7 @@ def filter_out_none_entries(self, list_or_dict): """ if isinstance(list_or_dict, dict): - result = dict() + result = {} for key, value in list_or_dict.items(): if isinstance(value, (list, dict)): sub = self.filter_out_none_entries(value) @@ -251,7 +251,7 @@ def filter_out_none_entries(self, list_or_dict): return result if isinstance(list_or_dict, list): - alist = list() + alist = [] for item in list_or_dict: if isinstance(item, (list, dict)): sub = self.filter_out_none_entries(item) @@ -267,4 +267,5 @@ def filter_out_none_entries(self, list_or_dict): @staticmethod def get_not_none_values_from_dict(parameters, keys): + # python 2.6 does not support dict comprehension using k: v return dict((key, value) for key, value in parameters.items() if key in keys and value is not None) diff --git a/plugins/modules/azure_rm_netapp_volume.py b/plugins/modules/azure_rm_netapp_volume.py index 33aa54c..487787e 100644 --- a/plugins/modules/azure_rm_netapp_volume.py +++ b/plugins/modules/azure_rm_netapp_volume.py @@ -28,75 +28,83 @@ options: name: - description: - - The name of the volume. - required: true - type: str + description: + - The name of the volume. + required: true + type: str file_path: - description: - - A unique file path for the volume. Used when creating mount targets. - type: str + description: + - A unique file path for the volume. Used when creating mount targets. + type: str pool_name: - description: - - The name of the capacity pool. - required: true - type: str + description: + - The name of the capacity pool. + required: true + type: str account_name: - description: - - The name of the NetApp account. - required: true - type: str + description: + - The name of the NetApp account. + required: true + type: str location: - description: - - Resource location. - - Required for create. - type: str + description: + - Resource location. + - Required for create. + type: str subnet_name: - description: - - Azure resource name for a delegated subnet. Must have the delegation Microsoft.NetApp/volumes. - - Provide name of the subnet ID. - - Required for create. - type: str - aliases: ['subnet_id'] - version_added: 21.1.0 + description: + - Azure resource name for a delegated subnet. Must have the delegation Microsoft.NetApp/volumes. + - Provide name of the subnet ID. + - Required for create. + type: str + aliases: ['subnet_id'] + version_added: 21.1.0 virtual_network: - description: - - The name of the virtual network required for the subnet to create a volume. - - Required for create. - type: str + description: + - The name of the virtual network required for the subnet to create a volume. + - Required for create. + type: str service_level: - description: - - The service level of the file system. - - default is Premium. - type: str - choices: ['Premium', 'Standard', 'Ultra'] + description: + - The service level of the file system. + - default is Premium. + type: str + choices: ['Premium', 'Standard', 'Ultra'] vnet_resource_group_for_subnet: - description: - - Only required if virtual_network to be used is of different resource_group. - - Name of the resource group for virtual_network and subnet_name to be used. - type: str - version_added: "20.5.0" + description: + - Only required if virtual_network to be used is of different resource_group. + - Name of the resource group for virtual_network and subnet_name to be used. + type: str + version_added: "20.5.0" size: - description: - - Provisioned size of the volume (in GiB). - - Minimum size is 100 GiB. Upper limit is 100TiB - - default is 100GiB. - version_added: "20.5.0" - type: int + description: + - Provisioned size of the volume (in GiB). + - Minimum size is 100 GiB. Upper limit is 100TiB + - default is 100GiB. + version_added: "20.5.0" + type: int protocol_types: - description: - - Protocol types - NFSv3, NFSv4.1, CIFS (for SMB). - type: list - elements: str - version_added: 21.2.0 + description: + - Protocol types - NFSv3, NFSv4.1, CIFS (for SMB). + type: list + elements: str + version_added: 21.2.0 state: - description: - - State C(present) will check that the volume exists with the requested configuration. - - State C(absent) will delete the volume. - default: present - choices: ['present', 'absent'] - type: str - + description: + - State C(present) will check that the volume exists with the requested configuration. + - State C(absent) will delete the volume. + default: present + choices: ['present', 'absent'] + type: str + feature_flags: + description: + - Enable or disable a new feature. + - This can be used to enable an experimental feature or disable a new feature that breaks backward compatibility. + - Supported keys and values are subject to change without notice. Unknown keys are ignored. + type: dict + version_added: 21.9.0 +notes: + - feature_flags is setting ignore_change_ownership_mode to true by default to bypass a 'change ownership mode' issue with azure-mgmt-netapp 4.0.0. ''' EXAMPLES = ''' @@ -136,7 +144,7 @@ AZURE_OBJECT_CLASS = 'NetAppAccount' HAS_AZURE_MGMT_NETAPP = False -IMPORT_ERRORS = list() +IMPORT_ERRORS = [] ONE_GIB = 1073741824 try: @@ -175,10 +183,11 @@ def __init__(self): size=dict(type='int', required=False), vnet_resource_group_for_subnet=dict(type='str', required=False), service_level=dict(type='str', required=False, choices=['Premium', 'Standard', 'Ultra']), - protocol_types=dict(type='list', elements='str') + protocol_types=dict(type='list', elements='str'), + feature_flags=dict(type='dict') ) self.na_helper = NetAppModule() - self.parameters = dict() + self.parameters = {} # import errors are handled in AzureRMModuleBase super(AzureRMNetAppVolume, self).__init__(derived_arg_spec=self.module_arg_spec, @@ -188,13 +197,22 @@ def __init__(self): @staticmethod def dict_from_volume_object(volume_object): + + def replace_list_of_objects_with_list_of_dicts(adict, key): + if adict.get(key): + adict[key] = [vars(x) for x in adict[key]] + current_dict = vars(volume_object) attr = 'subnet_id' if attr in current_dict: current_dict['subnet_name'] = current_dict.pop(attr).split('/')[-1] attr = 'mount_targets' + replace_list_of_objects_with_list_of_dicts(current_dict, attr) + attr = 'export_policy' if current_dict.get(attr): - current_dict[attr] = [vars(mount) for mount in current_dict[attr]] + attr_dict = vars(current_dict[attr]) + replace_list_of_objects_with_list_of_dicts(attr_dict, 'rules') + current_dict[attr] = attr_dict return current_dict def get_azure_netapp_volume(self): @@ -227,6 +245,9 @@ def get_export_policy_rules(self): rule_index=1, allowed_clients='0.0.0.0/0', unix_read_write=True) + if self.has_feature('ignore_change_ownership_mode') and self.sdk_version >= '4.0.0': + # https://github.com/Azure/azure-sdk-for-python/issues/20356 + options['chown_mode'] = None for protocol in ('cifs', 'nfsv3', 'nfsv41'): options[protocol] = protocol in ptypes return VolumePropertiesExportPolicy(rules=[ExportPolicyRule(**options)]) @@ -239,6 +260,12 @@ def create_azure_netapp_volume(self): options = self.na_helper.get_not_none_values_from_dict(self.parameters, ['protocol_types', 'service_level', 'tags', 'usage_threshold']) rules = self.get_export_policy_rules() if rules is not None: + # TODO: other options to expose ? + # options['throughput_mibps'] = 1.6 + # options['encryption_key_source'] = 'Microsoft.NetApp' + # options['security_style'] = 'Unix' + # options['unix_permissions'] = '0770' + # required for NFSv4 options['export_policy'] = rules subnet_id = '/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/virtualNetworks/%s/subnets/%s'\ % (self.azure_auth.subscription_id, diff --git a/tests/unit/plugins/modules/test_azure_rm_netapp_volume.py b/tests/unit/plugins/modules/test_azure_rm_netapp_volume.py index fb2608f..0fb235d 100644 --- a/tests/unit/plugins/modules/test_azure_rm_netapp_volume.py +++ b/tests/unit/plugins/modules/test_azure_rm_netapp_volume.py @@ -67,16 +67,16 @@ def __init__(self): self.valid_volumes = ['test1', 'test2'] def get(self, resource_group, account_name, pool_name, volume_name): # pylint: disable=unused-argument - if volume_name not in self.valid_volumes: - invalid = Response() - invalid.status_code = 404 - raise CloudError(response=invalid) - else: + if volume_name in self.valid_volumes: return Mock(name=volume_name, subnet_id='/resid/whatever/subnet_name', mount_targets=[Mock(ip_address='1.2.3.4')] ) + invalid = Response() + invalid.status_code = 404 + raise CloudError(response=invalid) + def create_or_update(self, body, resource_group, account_name, pool_name, volume_name): # pylint: disable=unused-argument return None @@ -133,7 +133,8 @@ def set_default_args(): 'subnet_name': subnet_id, 'virtual_network': virtual_network, 'size': size, - 'protocol_types': 'nfs' + 'protocol_types': 'nfs', + 'tags': {'owner': 'laurentn'} }) @@ -196,7 +197,7 @@ def test_ensure_create_called(mock_create, mock_get, client_f, patch_ansible): def test_create(mock_get, client_f, patch_ansible): # pylint: disable=unused-argument data = dict(set_default_args()) data['name'] = 'create' - data['protocol_types'] = 'nfsv4.1' + data['protocol_types'] = ['nfsv4.1'] set_module_args(data) mock_get.side_effect = [ None, # first get @@ -364,10 +365,12 @@ def test_modify(mock_get, client_f, patch_ansible): # pylint: disable=unused-ar data = dict(set_default_args()) data['name'] = 'modify' data['size'] = 200 + data['tags'] = {'added_tag': 'new_tag'} set_module_args(data) mock_get.side_effect = [ dict(mount_targets=[dict(ip_address='11.22.33.44')], # first get creation_token='abcd', + tags={}, usage_threshold=0), dict(mount_targets=[dict(ip_address='11.22.33.44')], # get after modify creation_token='abcd', @@ -384,6 +387,7 @@ def test_modify(mock_get, client_f, patch_ansible): # pylint: disable=unused-ar data['debug'] = False my_obj.exec_module(**data) assert exc.value.args[0]['changed'] + print('modify', exc.value.args[0]) expected_mount_path = '11.22.33.44:/abcd' assert exc.value.args[0]['mount_path'] == expected_mount_path @@ -469,3 +473,25 @@ def test_get_export_policy_rules(client_f, patch_ansible): rule = vars(rules[0]) assert rule['nfsv41'] assert not rule['cifs'] + + +def test_dict_from_object(): + set_module_args(set_default_args()) + my_obj = volume_module() + # just for fun + module_dict = my_obj.dict_from_volume_object(my_obj) + print('Module dict', module_dict) + + rule_object = Mock() + rule_object.ip_address = '10.10.10.10' + export_policy_object = Mock() + export_policy_object.rules = [rule_object] + volume_object = Mock() + volume_object.export_policy = export_policy_object + volume_dict = my_obj.dict_from_volume_object(volume_object) + print('Volume dict', volume_dict) + assert 'export_policy' in volume_dict + assert 'rules' in volume_dict['export_policy'] + assert isinstance(volume_dict['export_policy']['rules'], list) + assert len(volume_dict['export_policy']['rules']) == 1 + assert 'ip_address' in volume_dict['export_policy']['rules'][0]