Force delete share type extra specs

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):
 

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':

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:

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,

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.