Cascade + force volume delete parameters

Allow volume delete to take parameters "cascade",
or "force", or both.

A new policy field, "volume:force_delete" is added
with the default of "rule:admin_api".

Implements: blueprint volume-delete-parameters

APIImpact: New parameters to volume delete

Change-Id: Ic47cfcf1cc7d172d7f9d5b093233035f797982f5

cinder/api/openstack/api_version_request.py

@@ -72,6 +72,7 @@ REST_API_VERSION_HISTORY = """
              volume group.
     * 3.21 - Show provider_id in detailed view of a volume for admin.
     * 3.22 - Add filtering based on metadata for snapshot listing.
+    * 3.23 - Allow passing force parameter to volume delete.
 """
 
 # The minimum and maximum versions of the API supported
@@ -79,7 +80,7 @@ REST_API_VERSION_HISTORY = """
 # minimum version of the API supported.
 # Explicitly using /v1 or /v2 enpoints will still work
 _MIN_API_VERSION = "3.0"
-_MAX_API_VERSION = "3.22"
+_MAX_API_VERSION = "3.23"
 _LEGACY_API_VERSION1 = "1.0"
 _LEGACY_API_VERSION2 = "2.0"
 

cinder/api/v3/volumes.py

@@ -15,6 +15,7 @@
 
 from oslo_log import log as logging
 from oslo_utils import uuidutils
+import webob
 from webob import exc
 
 from cinder.api import common
@@ -24,6 +25,7 @@ from cinder.api.v3.views import volumes as volume_views_v3
 from cinder import exception
 from cinder import group as group_api
 from cinder.i18n import _, _LI
+import cinder.policy
 from cinder import utils
 from cinder.volume import volume_types
 
@@ -32,6 +34,17 @@ LOG = logging.getLogger(__name__)
 SUMMARY_BASE_MICRO_VERSION = '3.12'
 
 
+def check_policy(context, action, target_obj=None):
+    target = {
+        'project_id': context.project_id,
+        'user_id': context.user_id
+    }
+    target.update(target_obj or {})
+
+    _action = 'volume:%s' % action
+    cinder.policy.enforce(context, _action, target)
+
+
 class VolumeController(volumes_v2.VolumeController):
     """The Volumes API controller for the OpenStack API V3."""
 
@@ -41,6 +54,35 @@ class VolumeController(volumes_v2.VolumeController):
         self.group_api = group_api.API()
         super(VolumeController, self).__init__(ext_mgr)
 
+    def delete(self, req, id):
+        """Delete a volume."""
+        context = req.environ['cinder.context']
+        req_version = req.api_version_request
+
+        cascade = utils.get_bool_param('cascade', req.params)
+        force = False
+
+        params = ""
+        if req_version.matches('3.23'):
+            force = utils.get_bool_param('force', req.params)
+            if cascade or force:
+                params = "(cascade: %(c)s, force: %(f)s)" % {'c': cascade,
+                                                             'f': force}
+
+        msg = _LI("Delete volume with id: %(id)s %(params)s")
+        LOG.info(msg, {'id': id, 'params': params}, context=context)
+
+        if force:
+            check_policy(context, 'force_delete')
+
+        volume = self.volume_api.get(context, id)
+
+        self.volume_api.delete(context, volume,
+                               cascade=cascade,
+                               force=force)
+
+        return webob.Response(status_int=202)
+
     def _get_volumes(self, req, is_detail):
         """Returns a list of volumes, transformed through view builder."""
 

cinder/db/api.py

@@ -387,6 +387,10 @@ def volume_has_undeletable_snapshots_filter():
     return IMPL.volume_has_undeletable_snapshots_filter()
 
 
+def volume_has_snapshots_in_a_cgsnapshot_filter():
+    return IMPL.volume_has_snapshots_in_a_cgsnapshot_filter()
+
+
 def volume_has_attachments_filter():
     return IMPL.volume_has_attachments_filter()
 

cinder/db/sqlalchemy/api.py

@@ -2384,6 +2384,12 @@ def volume_has_undeletable_snapshots_filter():
                  models.Snapshot.status.notin_(deletable_statuses))))
 
 
+def volume_has_snapshots_in_a_cgsnapshot_filter():
+    return sql.exists().where(
+        and_(models.Volume.id == models.Snapshot.volume_id,
+             models.Snapshot.cgsnapshot_id.isnot(None)))
+
+
 def volume_has_attachments_filter():
     return sql.exists().where(
         and_(models.Volume.id == models.VolumeAttachment.volume_id,

cinder/tests/unit/policy.json

@@ -14,6 +14,7 @@
     "volume:get_volume_admin_metadata": "rule:admin_api",
     "volume:update_volume_admin_metadata": "rule:admin_api",
     "volume:delete": "",
+    "volume:force_delete": "rule:admin_api",
     "volume:update": "",
     "volume:attach": "",
     "volume:detach": "",

cinder/tests/unit/test_volume.py

@@ -4507,6 +4507,26 @@ class VolumeTestCase(base.BaseVolumeTestCase):
                           volume,
                           cascade=True)
 
+    def test_cascade_force_delete_volume_with_snapshots_error(self):
+        """Test volume force deletion with errored dependent snapshots."""
+        volume = tests_utils.create_volume(self.context,
+                                           host='fakehost')
+
+        snapshot = create_snapshot(volume.id,
+                                   size=volume.size,
+                                   status=fields.SnapshotStatus.ERROR_DELETING)
+        self.volume.create_snapshot(self.context, snapshot)
+
+        volume_api = cinder.volume.api.API()
+
+        volume_api.delete(self.context, volume, cascade=True, force=True)
+
+        snapshot = objects.Snapshot.get_by_id(self.context, snapshot.id)
+        self.assertEqual('deleting', snapshot.status)
+
+        volume = objects.Volume.get_by_id(self.context, volume.id)
+        self.assertEqual('deleting', volume.status)
+
     @mock.patch.object(fake_driver.FakeLoggingVolumeDriver, 'get_volume_stats')
     @mock.patch.object(driver.BaseVD, '_init_vendor_properties')
     def test_get_capabilities(self, mock_init_vendor, mock_get_volume_stats):

cinder/volume/api.py

@@ -406,8 +406,13 @@ class API(base.Base):
                                   'error_extending', 'error_managing')
 
         if cascade:
-            # Allow deletion if all snapshots are in an expected state
+            if force:
-            filters = [~db.volume_has_undeletable_snapshots_filter()]
+                # Ignore status checks, but ensure snapshots are not part
+                # of a cgsnapshot.
+                filters = [~db.volume_has_snapshots_in_a_cgsnapshot_filter()]
+            else:
+                # Allow deletion if all snapshots are in an expected state
+                filters = [~db.volume_has_undeletable_snapshots_filter()]
         else:
             # Don't allow deletion of volume with snapshots
             filters = [~db.volume_has_snapshots_filter()]
@@ -429,9 +434,11 @@ class API(base.Base):
 
         if cascade:
             values = {'status': 'deleting'}
-            expected = {'status': ('available', 'error', 'deleting'),
+            expected = {'cgsnapshot_id': None,
-                        'cgsnapshot_id': None,
                         'group_snapshot_id': None}
+            if not force:
+                expected['status'] = ('available', 'error', 'deleting')
+
             snapshots = objects.snapshot.SnapshotList.get_all_for_volume(
                 context, volume.id)
             for s in snapshots:

etc/cinder/policy.json

@@ -6,6 +6,7 @@
 
     "volume:create": "",
     "volume:delete": "rule:admin_or_owner",
+    "volume:force_delete": "rule:admin_api",
     "volume:get": "rule:admin_or_owner",
     "volume:get_all": "rule:admin_or_owner",
     "volume:get_volume_metadata": "rule:admin_or_owner",

releasenotes/notes/delete_parameters-6f44fece22a7787d.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - The ``force`` boolean parameter has been added to the volume
+    delete API.  It may be used in combination with ``cascade``.
+    This also means that volume force delete is available
+    in the base volume API rather than only in the
+    ``volume_admin_actions`` extension.
+upgrade:
+  - There is a new policy option ``volume:force_delete`` which
+    controls access to the ability to specify force delete
+    via the volume delete API.  This is separate from the
+    pre-existing ``volume-admin-actions:force_delete`` policy
+    check.