diff --git a/manila/common/constants.py b/manila/common/constants.py index b96deee79c..f119917fd8 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -307,6 +307,8 @@ SHARE_LOCKED_BY_ACCESS_LOCK_REASON = 'Locked by access lock: %(lock_id)s' +ONE_WEEK_IN_SECONDS = (7 * 24 * 60 * 60) + class ExtraSpecs(object): diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 85c84369ba..c55df7dc6f 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -965,7 +965,8 @@ def create_share_from_snapshot(self, context, share, snapshot, src_share_instance = { 'id': share['id'], 'host': parent_share.get('host'), - 'share_server': parent_share_server or None + 'share_server': parent_share_server or None, + 'share_type_id': share.get('share_type_id'), } # NOTE(dviroel): Data Motion functions access share's 'share_server' # attribute to get vserser information. @@ -2351,11 +2352,15 @@ def _delete_share(self, share, vserver, vserver_client, # Share doesn't need to exist to be assigned to a fpolicy scope self._delete_fpolicy_for_share(share, vserver, vserver_client) - duration_sec = share.get('duration_seconds', 24 * 60 * 60) # 24 Hrs - force_delete_sec = CONF.force_delete_time * 60 * 60 - force_delete = False - if force_delete_sec > 0 and duration_sec < force_delete_sec: - force_delete = True + extra_specs = {} + if share.get('share_type_id'): + extra_specs = share_types.get_extra_specs_from_share(share) + force_delete_minutes = int( + extra_specs.get('netapp:force_delete_time_minutes', 0)) + duration_sec = share.get( + 'duration_seconds', constants.ONE_WEEK_IN_SECONDS) + force_delete = (force_delete_minutes > 0 + and duration_sec < force_delete_minutes * 60) if self._share_exists(share_name, vserver_client): clone_status = vserver_client.volume_clone_split_status(share_name) if clone_status == 'ongoing': diff --git a/manila/share/manager.py b/manila/share/manager.py index 39552068a0..c41a8b551d 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -85,17 +85,6 @@ default=False, help='Whether share servers will ' 'be deleted on deletion of the last share.'), - cfg.IntOpt('force_delete_time', - default=2, - min=0, - max=12, - help='Maximum time diff in hours to decide whether share will ' - 'be deleted or force deleted. If time difference between ' - 'share deletion time and creation time is less than ' - 'force_delete_time, share will be force deleted. In case ' - 'force delete fails, volume will be deleted in regular ' - 'way and it will stay in recovery queue for some time. ' - 'Value of 0 indicates disable force-delete feature.'), cfg.BoolOpt('unmanage_remove_access_rules', default=False, help='If set to True, then manila will deny access and remove ' @@ -4083,6 +4072,14 @@ def _get_share_details_from_instance(self, context, share_instance_id): share_server.get('backend_details') return (share, share_instance, share_server) + def _get_duration_seconds_for_instances(self, share_instance): + scheduled_at = share_instance.get('scheduled_at') + terminated_at = share_instance.get('terminated_at') + if scheduled_at and terminated_at: + duration = terminated_at - scheduled_at + return duration.total_seconds() + return constants.ONE_WEEK_IN_SECONDS + @run_concurrently @add_hooks @utils.require_driver_initialized @@ -4166,12 +4163,9 @@ def delete_share_instance(self, context, share_instance_id, force=False, return try: - scheduled_at = share_instance.get('scheduled_at') - terminated_at = share_instance.get('terminated_at') - if scheduled_at and terminated_at: - duration = terminated_at - scheduled_at - share_instance.update( - {'duration_seconds': duration.total_seconds()}) + duration_seconds = self._get_duration_seconds_for_instances( + share_instance) + share_instance.update({'duration_seconds': duration_seconds}) self.driver.delete_share(context, share_instance, share_server=share_server) except exception.ShareResourceNotFound: @@ -4304,12 +4298,9 @@ def do_deferred_share_deletion(self, ctxt): continue try: - scheduled_at = share_instance.get('scheduled_at') - terminated_at = share_instance.get('terminated_at') - if scheduled_at and terminated_at: - duration = terminated_at - scheduled_at - share_instance.update( - {'duration_seconds': duration.total_seconds()}) + duration_seconds = self._get_duration_seconds_for_instances( + share_instance) + share_instance.update({'duration_seconds': duration_seconds}) self.driver.delete_share(ctxt, share_instance, share_server=share_server) except exception.ShareResourceNotFound: diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 8fd5109353..cfd25e897b 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -1001,7 +1001,8 @@ def to_dict(self): self.temp_src_share = { 'id': self.fake_share['id'], 'host': self.parent_share['host'], - 'share_server': self.parent_share_server or None + 'share_server': self.parent_share_server or None, + 'share_type_id': self.fake_share.get('share_type_id'), } @ddt.data({'dest_cluster': fake.CLUSTER_NAME, 'is_flexgroup': False, @@ -1214,6 +1215,8 @@ def test__update_create_from_snapshot_status_driver_error(self): mock_pvt_storage_get = self.mock_object( self.library.private_storage, 'get', mock.Mock(return_value=json.dumps(copy_fake_src_share))) + self.mock_object(share_types, 'get_extra_specs_from_share', + mock.Mock(return_value={})) mock__create_continue = self.mock_object( self.library, '_create_from_snapshot_continue', mock.Mock(side_effect=exception.NetAppException)) @@ -2534,8 +2537,12 @@ def test_delete_share(self, force): vserver_client = mock.Mock() fake_share = copy.deepcopy(fake.SHARE) if force: - # assuming CONF.force_delete_time > 0 (which it is by default) - fake_share['duration_seconds'] = 0 + extra_specs = {'netapp:force_delete_time_minutes': '10'} + fake_share.update({'duration_seconds': 59}) + else: + extra_specs = {} + self.mock_object(share_types, 'get_extra_specs_from_share', + mock.Mock(return_value=extra_specs)) self.mock_object(self.library, '_get_vserver', mock.Mock(return_value=(fake.VSERVER1, @@ -2570,6 +2577,8 @@ def test_delete_share(self, force): def test__delete_share_no_remove_qos_and_export(self): vserver_client = mock.Mock() + self.mock_object(share_types, 'get_extra_specs_from_share', + mock.Mock(return_value={})) mock_share_exists = self.mock_object(self.library, '_share_exists', mock.Mock(return_value=True)) @@ -2632,6 +2641,8 @@ def test_delete_share_no_share_server(self, get_vserver_exception): def test_delete_share_not_found(self): vserver_client = mock.Mock() + self.mock_object(share_types, 'get_extra_specs_from_share', + mock.Mock(return_value={})) self.mock_object(self.library, '_get_vserver', mock.Mock(return_value=(fake.VSERVER1, diff --git a/releasenotes/notes/netapp-fix-invalidsharetype-force-delete-bfe7751e644cf5c6.yaml b/releasenotes/notes/netapp-fix-invalidsharetype-force-delete-bfe7751e644cf5c6.yaml new file mode 100644 index 0000000000..9254cc5360 --- /dev/null +++ b/releasenotes/notes/netapp-fix-invalidsharetype-force-delete-bfe7751e644cf5c6.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + NetApp driver: fixed the share type extra specs lookup in the share + delete workflow (added for ``netapp:force_delete_time_minutes`` + support), which could raise an ``InvalidShareType`` exception.