openstack/cinder
Code Issues Proposed changes

Merge "Snapshot in-use volumes without force flag"

Browse Source
This commit is contained in:
Zuul 2021-09-01 17:36:36 +00:00 committed by Gerrit Code Review
commit 6258cc99f9
11 changed files with 212 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
api-ref/source/v3/samples/versions/version-show-response.json
View File

@ -21,8 +21,8 @@
],
"min_version": "3.0",
"status": "CURRENT",
"updated": "2021-08-25T00:00:00Z",
"version": "3.65"
"updated": "2021-09-16T00:00:00Z",
"version": "3.66"
}
]
}

4
api-ref/source/v3/samples/versions/versions-response.json
View File

@ -21,8 +21,8 @@
],
"min_version": "3.0",
"status": "CURRENT",
"updated": "2021-08-25T00:00:00Z",
"version": "3.65"
"updated": "2021-09-16T00:00:00Z",
"version": "3.66"
}
]
}

6
api-ref/source/v3/volumes-v3-snapshots.inc
View File

@ -107,6 +107,12 @@ Create a snapshot
Creates a volume snapshot, which is a point-in-time, complete copy of a volume.
You can create a volume from a snapshot.
Prior to API version 3.66, a 'force' flag was required to create a snapshot of
an in-use volume, but this is no longer the case. From API version 3.66, the
'force' flag is invalid when passed in a volume snapshot request. (For
backward compatibility, however, a 'force' flag with a value evaluating to
True is silently ignored.)
Response codes
--------------

2
cinder/api/microversions.py
View File

@ -169,6 +169,8 @@ ENCRYPTION_KEY_ID_IN_DETAILS = '3.64'
USE_QUOTA = '3.65'
SNAPSHOT_IN_USE = '3.66'
def get_mv_header(version):
"""Gets a formatted HTTP microversion header.

5
cinder/api/openstack/api_version_request.py
View File

@ -151,14 +151,15 @@ REST_API_VERSION_HISTORY = """
* 3.65 - Include 'consumes_quota' in volume and snapshot details
- Accept 'consumes_quota' filter in volume and snapshot list
operation.
* 3.66 - Allow snapshotting in-use volumes without force flag.
"""
# The minimum and maximum versions of the API supported
# The default api version request is defined to be the
# minimum version of the API supported.
_MIN_API_VERSION = "3.0"
_MAX_API_VERSION = "3.65"
UPDATED = "2021-08-25T00:00:00Z"
_MAX_API_VERSION = "3.66"
UPDATED = "2021-09-16T00:00:00Z"
# NOTE(cyeoh): min and max versions declared as functions so we can

7
cinder/api/openstack/rest_api_version_history.rst
View File

@ -498,3 +498,10 @@ whether the resource is consuming quota or not. Also, accept a
``consumes_quota`` filter, which takes a boolean value, in the volume and
snapshot list requests. (The default listing behavior is not to use this
filter.)
3.66 (Maximum in Xena)
----------------------
Volume snapshots of in-use volumes can be created without the 'force' flag.
Although the 'force' flag is now considered invalid when passed in a volume
snapshot request, for backward compatibility, the 'force' flag with a value
evaluating to True is silently ignored.

63
cinder/api/v3/snapshots.py
View File

@ -16,19 +16,30 @@
"""The volumes snapshots V3 API."""
import ast
from http import HTTPStatus
from oslo_log import log as logging
from oslo_utils import strutils
from webob import exc
from cinder.api import api_utils
from cinder.api import common
from cinder.api import microversions as mv
from cinder.api.openstack import wsgi
from cinder.api.schemas import snapshots as snapshot
from cinder.api.v2 import snapshots as snapshots_v2
from cinder.api.v3.views import snapshots as snapshot_views
from cinder.api import validation
from cinder import utils
LOG = logging.getLogger(__name__)
SNAPSHOT_IN_USE_FLAG_MSG = (
f"Since microversion {mv.SNAPSHOT_IN_USE} the 'force' flag is "
"invalid for this request. For backward compatability, however, when "
"the 'force' flag is passed with a value evaluating to True, it is "
"silently ignored.")
class SnapshotsController(snapshots_v2.SnapshotsController):
"""The Snapshots API controller for the OpenStack API."""
@ -131,6 +142,58 @@ class SnapshotsController(snapshots_v2.SnapshotsController):
total_count)
return snapshots
@wsgi.response(HTTPStatus.ACCEPTED)
@validation.schema(snapshot.create)
def create(self, req, body):
"""Creates a new snapshot."""
kwargs = {}
context = req.environ['cinder.context']
snapshot = body['snapshot']
kwargs['metadata'] = snapshot.get('metadata', None)
volume_id = snapshot['volume_id']
volume = self.volume_api.get(context, volume_id)
req_version = req.api_version_request
force_flag = snapshot.get('force')
force = False
if force_flag is not None:
# note: this won't raise because it passed schema validation
force = strutils.bool_from_string(force_flag, strict=True)
if req_version.matches(mv.SNAPSHOT_IN_USE):
# strictly speaking, the 'force' flag is invalid for
# mv.SNAPSHOT_IN_USE, but we silently ignore a True
# value for backward compatibility
if force is False:
raise exc.HTTPBadRequest(
explanation=SNAPSHOT_IN_USE_FLAG_MSG)
LOG.info("Create snapshot from volume %s", volume_id)
self.validate_name_and_description(snapshot, check_length=False)
if 'name' in snapshot:
snapshot['display_name'] = snapshot.pop('name')
if force:
new_snapshot = self.volume_api.create_snapshot_force(
context,
volume,
snapshot.get('display_name'),
snapshot.get('description'),
**kwargs)
else:
if req_version.matches(mv.SNAPSHOT_IN_USE):
kwargs['allow_in_use'] = True
new_snapshot = self.volume_api.create_snapshot(
context,
volume,
snapshot.get('display_name'),
snapshot.get('description'),
**kwargs)
req.cache_db_snapshot(new_snapshot)
return self._view_builder.detail(req, new_snapshot)
def create_resource(ext_mgr):
return wsgi.Resource(SnapshotsController(ext_mgr))

54
cinder/tests/unit/api/v3/test_snapshots.py
View File

@ -17,6 +17,7 @@ from unittest import mock
import ddt
from oslo_utils import strutils
from webob import exc
from cinder.api import microversions as mv
from cinder.api.v3 import snapshots
@ -400,3 +401,56 @@ class SnapshotApiTest(test.TestCase):
# verify some snapshot is returned
self.assertNotEqual(0, len(res_dict['snapshots']))
@mock.patch('cinder.volume.api.API.create_snapshot')
def test_snapshot_create_allow_in_use(self, mock_create):
req = create_snapshot_query_with_metadata(
'{"key2": "val2"}',
mv.SNAPSHOT_IN_USE)
body = {'snapshot': {'volume_id': fake.VOLUME_ID}}
self.controller.create(req, body=body)
self.assertIn('allow_in_use', mock_create.call_args_list[0][1])
self.assertTrue(mock_create.call_args_list[0][1]['allow_in_use'])
@mock.patch('cinder.volume.api.API.create_snapshot')
def test_snapshot_create_allow_in_use_negative(self, mock_create):
req = create_snapshot_query_with_metadata(
'{"key2": "val2"}',
mv.get_prior_version(mv.SNAPSHOT_IN_USE))
body = {'snapshot': {'volume_id': fake.VOLUME_ID}}
self.controller.create(req, body=body)
self.assertNotIn('allow_in_use', mock_create.call_args_list[0][1])
@ddt.data(False, 'false', 'f', 'no', 'n', '0', 'off')
@mock.patch('cinder.volume.api.API.create_snapshot')
def test_snapshot_create_force_false(self, force_flag, mock_create):
snapshot_name = 'Snapshot Test Name'
snapshot_description = 'Snapshot Test Desc'
snapshot = {
"volume_id": fake.VOLUME_ID,
"force": force_flag,
"name": snapshot_name,
"description": snapshot_description
}
body = dict(snapshot=snapshot)
req = create_snapshot_query_with_metadata(
'{"key2": "val2"}',
mv.SNAPSHOT_IN_USE)
self.assertRaises(exc.HTTPBadRequest,
self.controller.create,
req,
body=body)
mock_create.assert_not_called()
# prevent regression -- shouldn't raise for pre-mv-3.66
req = create_snapshot_query_with_metadata(
'{"key2": "val2"}',
mv.get_prior_version(mv.SNAPSHOT_IN_USE))
self.controller.create(req, body=body)
# ... but also shouldn't allow an in-use snapshot
self.assertNotIn('allow_in_use', mock_create.call_args_list[0][1])

55
cinder/tests/unit/volume/test_snapshot.py
View File

@ -335,7 +335,7 @@ class SnapshotTestCase(base.BaseVolumeTestCase):
def test_create_snapshot_force(self):
"""Test snapshot in use can be created forcibly."""
instance_uuid = '12345678-1234-5678-1234-567812345678'
instance_uuid = '12345678-1234-4678-1234-567812345678'
# create volume and attach to the instance
volume = tests_utils.create_volume(self.context, **self.volume_params)
self.volume.create_volume(self.context, volume)
@ -359,6 +359,7 @@ class SnapshotTestCase(base.BaseVolumeTestCase):
snapshot_ref.destroy()
db.volume_destroy(self.context, volume['id'])
def test_create_snapshot_force_host(self):
# create volume and attach to the host
volume = tests_utils.create_volume(self.context, **self.volume_params)
self.volume.create_volume(self.context, volume)
@ -382,6 +383,58 @@ class SnapshotTestCase(base.BaseVolumeTestCase):
snapshot_ref.destroy()
db.volume_destroy(self.context, volume['id'])
def test_create_snapshot_in_use(self):
"""Test snapshot in use can be created forcibly."""
instance_uuid = 'a14dc210-d43b-4792-a608-09fe0824de54'
# create volume and attach to the instance
volume = tests_utils.create_volume(self.context, **self.volume_params)
self.volume.create_volume(self.context, volume)
values = {'volume_id': volume['id'],
'instance_uuid': instance_uuid,
'attach_status': fields.VolumeAttachStatus.ATTACHING, }
attachment = db.volume_attach(self.context, values)
db.volume_attached(self.context, attachment['id'], instance_uuid,
None, '/dev/sda1')
volume_api = cinder.volume.api.API()
volume = volume_api.get(self.context, volume['id'])
self.assertRaises(exception.InvalidVolume,
volume_api.create_snapshot,
self.context, volume,
'fake_name', 'fake_description')
snapshot_ref = volume_api.create_snapshot(self.context,
volume,
'fake_name',
'fake_description',
allow_in_use=True)
snapshot_ref.destroy()
db.volume_destroy(self.context, volume['id'])
# create volume and attach to the host
volume = tests_utils.create_volume(self.context, **self.volume_params)
self.volume.create_volume(self.context, volume)
values = {'volume_id': volume['id'],
'attached_host': 'fake_host',
'attach_status': fields.VolumeAttachStatus.ATTACHING, }
attachment = db.volume_attach(self.context, values)
db.volume_attached(self.context, attachment['id'], None,
'fake_host', '/dev/sda1')
volume_api = cinder.volume.api.API()
volume = volume_api.get(self.context, volume['id'])
self.assertRaises(exception.InvalidVolume,
volume_api.create_snapshot,
self.context, volume,
'fake_name', 'fake_description')
snapshot_ref = volume_api.create_snapshot(self.context,
volume,
'fake_name',
'fake_description',
allow_in_use=True)
snapshot_ref.destroy()
db.volume_destroy(self.context, volume['id'])
@mock.patch('cinder.image.image_utils.qemu_img_info')
def test_create_snapshot_from_bootable_volume(self, mock_qemu_info):
"""Test create snapshot from bootable volume."""

17
cinder/volume/api.py
View File

@ -850,12 +850,13 @@ class API(base.Base):
volume, name, description,
force=False, metadata=None,
cgsnapshot_id=None,
group_snapshot_id=None):
group_snapshot_id=None,
allow_in_use=False):
volume.assert_not_frozen()
snapshot = self.create_snapshot_in_db(
context, volume, name,
description, force, metadata, cgsnapshot_id,
True, group_snapshot_id)
True, group_snapshot_id, allow_in_use)
# NOTE(tommylikehu): We only wrap the 'size' attribute here
# because only the volume's host is passed and only capacity is
# validated in the scheduler now.
@ -872,12 +873,15 @@ class API(base.Base):
force, metadata,
cgsnapshot_id,
commit_quota=True,
group_snapshot_id=None):
group_snapshot_id=None,
allow_in_use=False):
self._create_snapshot_in_db_validate(context, volume)
utils.check_metadata_properties(metadata)
valid_status = ["available", "in-use"] if force else ["available"]
valid_status = ('available',)
if force or allow_in_use:
valid_status = ('available', 'in-use')
if volume['status'] not in valid_status:
msg = _("Volume %(vol_id)s status must be %(status)s, "
@ -1063,10 +1067,11 @@ class API(base.Base):
def create_snapshot(self, context,
volume, name, description,
metadata=None, cgsnapshot_id=None,
group_snapshot_id=None):
group_snapshot_id=None,
allow_in_use=False):
result = self._create_snapshot(context, volume, name, description,
False, metadata, cgsnapshot_id,
group_snapshot_id)
group_snapshot_id, allow_in_use)
LOG.info("Snapshot create request issued successfully.",
resource=result)
return result

8
releasenotes/notes/snapshot-in-use-without-force-86c6d74ebc9c0d60.yaml Normal file
View File

@ -0,0 +1,8 @@
---
features:
- |
As of API version 3.66, volume snapshots of in-use volumes can be created
without passing the 'force' flag, and the 'force' flag is considered
invalid for this request. For backward compatability, however, when the
'force' flag is passed with a value evaluating to True, it is silently
ignored.