Remove multiatttach request parameter
The initial cinder design[1][2][3] allowed users to create mutliattach volumes by spcifying the ``multiattach`` parameter in the request body of volume create operation (``--allow-multiattach`` option in cinderclient). This functionality changed in Queens with the introduction of microversion 3.50[4] where we used volume types to store the multiattach capabilities. Any volume created with a multiattach volume type will be a multiattach volume[5]. While implementing the new functionality, we had to keep backward compatibility with the *old way* of creating multiattach volumes. We deprecated the ``multiattach`` (``--allow-multiattach`` on cinderclient side) parameter in the queens release[6][7]. We also removed the support of the ``--allow-multiattach`` optional parameter from cinderclient in the train release[8] but the API side never removed the compatibility code to disallow functionality of creating multiattach volumes by using the ``multiattach`` parameter (instead of a multiattach volume type). This patch removes the support of providing the ``multiattach`` parameter in the request body of a volume create operation and will fail with a BadRequest exception stating the reason of failure and how it can be fixed. [1] https://blueprints.launchpad.net/cinder/+spec/multi-attach-volume [2] https://review.opendev.org/c/openstack/cinder/+/85847/ [3] https://review.opendev.org/c/openstack/python-cinderclient/+/85856 [4]f1bfd9790d
[5] https://docs.openstack.org/cinder/latest/admin/volume-multiattach.html#how-to-create-a-multiattach-volume [6]94dbf5cce2
[7]adb141a262
[8]3c1b417959
Depends-On: https://review.opendev.org/c/openstack/tempest/+/875372 Closes-Bug: 2008259 Change-Id: I0ece6e279048abcc04b3674108290a80eca6bd62 (cherry picked from commit32f1145b7d
)
This commit is contained in:
parent
7ee5824c64
commit
e2c3bcc6e3
@ -1081,15 +1081,6 @@ mountpoint:
|
|||||||
in: body
|
in: body
|
||||||
required: true
|
required: true
|
||||||
type: string
|
type: string
|
||||||
multiattach_req:
|
|
||||||
description: |
|
|
||||||
To enable this volume to attach to more than one
|
|
||||||
server, set this value to ``true``. Default is ``false``.
|
|
||||||
Note that support for multiattach volumes depends on the volume
|
|
||||||
type being used.
|
|
||||||
in: body
|
|
||||||
required: false
|
|
||||||
type: boolean
|
|
||||||
multiattach_resp:
|
multiattach_resp:
|
||||||
description: |
|
description: |
|
||||||
If true, this volume can attach to more than one
|
If true, this volume can attach to more than one
|
||||||
|
@ -183,7 +183,6 @@ Request
|
|||||||
- size: size
|
- size: size
|
||||||
- description: description_9
|
- description: description_9
|
||||||
- imageRef: imageRef
|
- imageRef: imageRef
|
||||||
- multiattach: multiattach_req
|
|
||||||
- availability_zone: availability_zone
|
- availability_zone: availability_zone
|
||||||
- source_volid: source_volid
|
- source_volid: source_volid
|
||||||
- name: volume_name_optional
|
- name: volume_name_optional
|
||||||
|
@ -2126,15 +2126,6 @@ multiattach:
|
|||||||
in: body
|
in: body
|
||||||
required: false
|
required: false
|
||||||
type: string
|
type: string
|
||||||
multiattach_req:
|
|
||||||
description: |
|
|
||||||
To enable this volume to attach to more than one
|
|
||||||
server, set this value to ``true``. Default is ``false``.
|
|
||||||
Note that support for multiattach volumes depends on the volume
|
|
||||||
type being used. See :ref:`valid boolean values <valid-boolean-values>`
|
|
||||||
in: body
|
|
||||||
required: false
|
|
||||||
type: boolean
|
|
||||||
multiattach_resp:
|
multiattach_resp:
|
||||||
description: |
|
description: |
|
||||||
If true, this volume can attach to more than one
|
If true, this volume can attach to more than one
|
||||||
|
@ -218,7 +218,6 @@ Request
|
|||||||
- availability_zone: availability_zone
|
- availability_zone: availability_zone
|
||||||
- source_volid: source_volid
|
- source_volid: source_volid
|
||||||
- description: description_vol
|
- description: description_vol
|
||||||
- multiattach: multiattach_req
|
|
||||||
- snapshot_id: snapshot_id
|
- snapshot_id: snapshot_id
|
||||||
- backup_id: backup_id
|
- backup_id: backup_id
|
||||||
- name: volume_name_optional
|
- name: volume_name_optional
|
||||||
|
@ -49,6 +49,12 @@ create = {
|
|||||||
'consistencygroup_id': parameter_types.optional_uuid,
|
'consistencygroup_id': parameter_types.optional_uuid,
|
||||||
'size': parameter_types.volume_size_allows_null,
|
'size': parameter_types.volume_size_allows_null,
|
||||||
'availability_zone': parameter_types.availability_zone,
|
'availability_zone': parameter_types.availability_zone,
|
||||||
|
# The functionality to create a multiattach volume by the
|
||||||
|
# multiattach parameter is removed.
|
||||||
|
# We accept the parameter but raise a BadRequest stating the
|
||||||
|
# "new way" of creating multiattach volumes i.e. with a
|
||||||
|
# multiattach volume type so users using the "old way"
|
||||||
|
# have ease of moving into the new functionality.
|
||||||
'multiattach': parameter_types.optional_boolean,
|
'multiattach': parameter_types.optional_boolean,
|
||||||
'image_id': {'type': ['string', 'null'], 'minLength': 0,
|
'image_id': {'type': ['string', 'null'], 'minLength': 0,
|
||||||
'maxLength': 255},
|
'maxLength': 255},
|
||||||
|
@ -19,7 +19,6 @@ from http import HTTPStatus
|
|||||||
|
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
from oslo_log import versionutils
|
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
import webob
|
import webob
|
||||||
from webob import exc
|
from webob import exc
|
||||||
@ -261,14 +260,6 @@ class VolumeController(wsgi.Controller):
|
|||||||
|
|
||||||
kwargs['availability_zone'] = volume.get('availability_zone', None)
|
kwargs['availability_zone'] = volume.get('availability_zone', None)
|
||||||
kwargs['scheduler_hints'] = volume.get('scheduler_hints', None)
|
kwargs['scheduler_hints'] = volume.get('scheduler_hints', None)
|
||||||
kwargs['multiattach'] = utils.get_bool_param('multiattach', volume)
|
|
||||||
|
|
||||||
if kwargs.get('multiattach', False):
|
|
||||||
msg = ("The option 'multiattach' "
|
|
||||||
"is deprecated and will be removed in a future "
|
|
||||||
"release. The default behavior going forward will "
|
|
||||||
"be to specify multiattach enabled volume types.")
|
|
||||||
versionutils.report_deprecated_feature(LOG, msg)
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
new_volume = self.volume_api.create(
|
new_volume = self.volume_api.create(
|
||||||
|
@ -15,7 +15,6 @@
|
|||||||
from http import HTTPStatus
|
from http import HTTPStatus
|
||||||
|
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
from oslo_log import versionutils
|
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
import webob
|
import webob
|
||||||
from webob import exc
|
from webob import exc
|
||||||
@ -387,15 +386,14 @@ class VolumeController(volumes_v2.VolumeController):
|
|||||||
|
|
||||||
kwargs['availability_zone'] = volume.get('availability_zone', None)
|
kwargs['availability_zone'] = volume.get('availability_zone', None)
|
||||||
kwargs['scheduler_hints'] = volume.get('scheduler_hints', None)
|
kwargs['scheduler_hints'] = volume.get('scheduler_hints', None)
|
||||||
multiattach = volume.get('multiattach', False)
|
multiattach = utils.get_bool_param('multiattach', volume)
|
||||||
kwargs['multiattach'] = multiattach
|
|
||||||
|
|
||||||
if multiattach:
|
if multiattach:
|
||||||
msg = ("The option 'multiattach' "
|
msg = _("multiattach parameter has been removed. The default "
|
||||||
"is deprecated and will be removed in a future "
|
"behavior is to use multiattach enabled volume types. "
|
||||||
"release. The default behavior going forward will "
|
"Contact your administrator to create a multiattach "
|
||||||
"be to specify multiattach enabled volume types.")
|
"enabled volume type and use it to create multiattach "
|
||||||
versionutils.report_deprecated_feature(LOG, msg)
|
"volumes.")
|
||||||
|
raise exc.HTTPBadRequest(explanation=msg)
|
||||||
try:
|
try:
|
||||||
new_volume = self.volume_api.create(
|
new_volume = self.volume_api.create(
|
||||||
context, size, volume.get('display_name'),
|
context, size, volume.get('display_name'),
|
||||||
|
@ -335,19 +335,6 @@ class FilterScheduler(driver.Scheduler):
|
|||||||
self.populate_filter_properties(request_spec,
|
self.populate_filter_properties(request_spec,
|
||||||
filter_properties)
|
filter_properties)
|
||||||
|
|
||||||
# If multiattach is enabled on a volume, we need to add
|
|
||||||
# multiattach to extra specs, so that the capability
|
|
||||||
# filtering is enabled.
|
|
||||||
multiattach = request_spec['volume_properties'].get('multiattach',
|
|
||||||
False)
|
|
||||||
if multiattach and 'multiattach' not in resource_type.get(
|
|
||||||
'extra_specs', {}):
|
|
||||||
if 'extra_specs' not in resource_type:
|
|
||||||
resource_type['extra_specs'] = {}
|
|
||||||
|
|
||||||
resource_type['extra_specs'].update(
|
|
||||||
multiattach='<is> True')
|
|
||||||
|
|
||||||
# Revert volume consumed capacity if it's a rescheduled request
|
# Revert volume consumed capacity if it's a rescheduled request
|
||||||
retry = filter_properties.get('retry', {})
|
retry = filter_properties.get('retry', {})
|
||||||
if retry.get('backends', []):
|
if retry.get('backends', []):
|
||||||
|
@ -158,8 +158,7 @@ class VolumeApiTest(test.TestCase):
|
|||||||
consistencygroup_id=None,
|
consistencygroup_id=None,
|
||||||
volume_type=None,
|
volume_type=None,
|
||||||
image_ref=None,
|
image_ref=None,
|
||||||
image_id=None,
|
image_id=None):
|
||||||
multiattach=False):
|
|
||||||
vol = {"size": size,
|
vol = {"size": size,
|
||||||
"name": name,
|
"name": name,
|
||||||
"description": description,
|
"description": description,
|
||||||
@ -168,7 +167,6 @@ class VolumeApiTest(test.TestCase):
|
|||||||
"source_volid": source_volid,
|
"source_volid": source_volid,
|
||||||
"consistencygroup_id": consistencygroup_id,
|
"consistencygroup_id": consistencygroup_id,
|
||||||
"volume_type": volume_type,
|
"volume_type": volume_type,
|
||||||
"multiattach": multiattach,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if image_id is not None:
|
if image_id is not None:
|
||||||
@ -240,7 +238,6 @@ class VolumeApiTest(test.TestCase):
|
|||||||
'consistencygroup': None,
|
'consistencygroup': None,
|
||||||
'availability_zone': availability_zone,
|
'availability_zone': availability_zone,
|
||||||
'scheduler_hints': None,
|
'scheduler_hints': None,
|
||||||
'multiattach': False,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
@mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full',
|
@mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full',
|
||||||
@ -570,37 +567,6 @@ class VolumeApiTest(test.TestCase):
|
|||||||
req,
|
req,
|
||||||
body=body)
|
body=body)
|
||||||
|
|
||||||
def test_volume_create_with_invalid_multiattach(self):
|
|
||||||
vol = self._vol_in_request_body(multiattach="InvalidBool")
|
|
||||||
body = {"volume": vol}
|
|
||||||
req = fakes.HTTPRequest.blank('/v3/volumes')
|
|
||||||
|
|
||||||
self.assertRaises(exception.ValidationError,
|
|
||||||
self.controller.create,
|
|
||||||
req,
|
|
||||||
body=body)
|
|
||||||
|
|
||||||
@mock.patch.object(volume_api.API, 'create', autospec=True)
|
|
||||||
@mock.patch.object(volume_api.API, 'get', autospec=True)
|
|
||||||
@mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full',
|
|
||||||
autospec=True)
|
|
||||||
def test_volume_create_with_valid_multiattach(self,
|
|
||||||
volume_type_get,
|
|
||||||
get, create):
|
|
||||||
create.side_effect = v2_fakes.fake_volume_api_create
|
|
||||||
get.side_effect = v2_fakes.fake_volume_get
|
|
||||||
volume_type_get.side_effect = v2_fakes.fake_volume_type_get
|
|
||||||
|
|
||||||
vol = self._vol_in_request_body(multiattach=True)
|
|
||||||
body = {"volume": vol}
|
|
||||||
|
|
||||||
ex = self._expected_vol_from_controller(multiattach=True)
|
|
||||||
|
|
||||||
req = fakes.HTTPRequest.blank('/v3/volumes')
|
|
||||||
res_dict = self.controller.create(req, body=body)
|
|
||||||
|
|
||||||
self.assertEqual(ex, res_dict)
|
|
||||||
|
|
||||||
@ddt.data({'a' * 256: 'a'},
|
@ddt.data({'a' * 256: 'a'},
|
||||||
{'a': 'a' * 256},
|
{'a': 'a' * 256},
|
||||||
{'': 'a'},
|
{'': 'a'},
|
||||||
|
@ -619,7 +619,6 @@ class VolumeApiTest(test.TestCase):
|
|||||||
'consistencygroup': None,
|
'consistencygroup': None,
|
||||||
'availability_zone': availability_zone,
|
'availability_zone': availability_zone,
|
||||||
'scheduler_hints': None,
|
'scheduler_hints': None,
|
||||||
'multiattach': False,
|
|
||||||
'group': test_group,
|
'group': test_group,
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -1189,3 +1188,25 @@ class VolumeApiTest(test.TestCase):
|
|||||||
volumes = res_dict['volumes']
|
volumes = res_dict['volumes']
|
||||||
self.assertEqual(1, len(volumes))
|
self.assertEqual(1, len(volumes))
|
||||||
self.assertEqual(vols[1].id, volumes[0]['id'])
|
self.assertEqual(vols[1].id, volumes[0]['id'])
|
||||||
|
|
||||||
|
def test_create_volume_with_multiattach_param(self):
|
||||||
|
"""Tests creating a volume with multiattach=True but no multiattach
|
||||||
|
|
||||||
|
volume type.
|
||||||
|
|
||||||
|
This test verifies that providing the multiattach parameter will error
|
||||||
|
out the request since it is removed and the recommended way is to
|
||||||
|
create a multiattach volume using a multiattach volume type.
|
||||||
|
"""
|
||||||
|
req = fakes.HTTPRequest.blank('/v3/volumes')
|
||||||
|
|
||||||
|
body = {'volume': {
|
||||||
|
'name': 'test name',
|
||||||
|
'description': 'test desc',
|
||||||
|
'size': 1,
|
||||||
|
'multiattach': True}}
|
||||||
|
exc = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||||
|
self.controller.create,
|
||||||
|
req, body=body)
|
||||||
|
self.assertIn("multiattach parameter has been removed",
|
||||||
|
exc.explanation)
|
||||||
|
@ -776,19 +776,6 @@ class VolumeTestCase(base.BaseVolumeTestCase):
|
|||||||
self.assertEqual(foo['id'], vol['volume_type_id'])
|
self.assertEqual(foo['id'], vol['volume_type_id'])
|
||||||
self.assertTrue(vol['multiattach'])
|
self.assertTrue(vol['multiattach'])
|
||||||
|
|
||||||
def test_create_volume_with_multiattach_flag(self):
|
|
||||||
"""Tests creating a volume with multiattach=True but no special type.
|
|
||||||
|
|
||||||
This tests the pre 3.50 microversion behavior of being able to create
|
|
||||||
a volume with the multiattach request parameter regardless of a
|
|
||||||
multiattach-capable volume type.
|
|
||||||
"""
|
|
||||||
volume_api = cinder.volume.api.API()
|
|
||||||
volume = volume_api.create(
|
|
||||||
self.context, 1, 'name', 'description', multiattach=True,
|
|
||||||
volume_type=self.vol_type)
|
|
||||||
self.assertTrue(volume.multiattach)
|
|
||||||
|
|
||||||
def _fail_multiattach_policy_authorize(self, policy):
|
def _fail_multiattach_policy_authorize(self, policy):
|
||||||
if policy == vol_policy.MULTIATTACH_POLICY:
|
if policy == vol_policy.MULTIATTACH_POLICY:
|
||||||
raise exception.PolicyNotAuthorized(action='Test')
|
raise exception.PolicyNotAuthorized(action='Test')
|
||||||
@ -813,16 +800,6 @@ class VolumeTestCase(base.BaseVolumeTestCase):
|
|||||||
1, 'admin-vol', 'description',
|
1, 'admin-vol', 'description',
|
||||||
volume_type=foo)
|
volume_type=foo)
|
||||||
|
|
||||||
def test_create_volume_with_multiattach_flag_not_authorized(self):
|
|
||||||
"""Test policy unauthorized create with multiattach flag."""
|
|
||||||
volume_api = cinder.volume.api.API()
|
|
||||||
|
|
||||||
with mock.patch.object(self.context, 'authorize') as mock_auth:
|
|
||||||
mock_auth.side_effect = self._fail_multiattach_policy_authorize
|
|
||||||
self.assertRaises(exception.PolicyNotAuthorized,
|
|
||||||
volume_api.create, self.context, 1, 'name',
|
|
||||||
'description', multiattach=True)
|
|
||||||
|
|
||||||
@mock.patch.object(key_manager, 'API', fake_keymgr.fake_api)
|
@mock.patch.object(key_manager, 'API', fake_keymgr.fake_api)
|
||||||
def test_create_volume_with_encrypted_volume_type_multiattach(self):
|
def test_create_volume_with_encrypted_volume_type_multiattach(self):
|
||||||
ctxt = context.get_admin_context()
|
ctxt = context.get_admin_context()
|
||||||
|
@ -230,7 +230,6 @@ class API(base.Base):
|
|||||||
source_replica=None,
|
source_replica=None,
|
||||||
consistencygroup: Optional[objects.ConsistencyGroup] = None,
|
consistencygroup: Optional[objects.ConsistencyGroup] = None,
|
||||||
cgsnapshot: Optional[objects.CGSnapshot] = None,
|
cgsnapshot: Optional[objects.CGSnapshot] = None,
|
||||||
multiattach: Optional[bool] = False,
|
|
||||||
source_cg=None,
|
source_cg=None,
|
||||||
group: Optional[objects.Group] = None,
|
group: Optional[objects.Group] = None,
|
||||||
group_snapshot=None,
|
group_snapshot=None,
|
||||||
@ -339,7 +338,6 @@ class API(base.Base):
|
|||||||
'optional_args': {'is_quota_committed': False},
|
'optional_args': {'is_quota_committed': False},
|
||||||
'consistencygroup': consistencygroup,
|
'consistencygroup': consistencygroup,
|
||||||
'cgsnapshot': cgsnapshot,
|
'cgsnapshot': cgsnapshot,
|
||||||
'raw_multiattach': multiattach,
|
|
||||||
'group': group,
|
'group': group,
|
||||||
'group_snapshot': group_snapshot,
|
'group_snapshot': group_snapshot,
|
||||||
'source_group': source_group,
|
'source_group': source_group,
|
||||||
|
@ -444,8 +444,7 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask):
|
|||||||
cgsnapshot,
|
cgsnapshot,
|
||||||
group,
|
group,
|
||||||
group_snapshot,
|
group_snapshot,
|
||||||
backup: Optional[dict],
|
backup: Optional[dict]) -> dict[str, Any]:
|
||||||
multiattach: bool = False) -> dict[str, Any]:
|
|
||||||
|
|
||||||
utils.check_exclusive_options(snapshot=snapshot,
|
utils.check_exclusive_options(snapshot=snapshot,
|
||||||
imageRef=image_id,
|
imageRef=image_id,
|
||||||
@ -493,11 +492,7 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask):
|
|||||||
volume_type = objects.VolumeType.get_by_name_or_id(
|
volume_type = objects.VolumeType.get_by_name_or_id(
|
||||||
context, volume_type_id)
|
context, volume_type_id)
|
||||||
extra_specs = volume_type.get('extra_specs', {})
|
extra_specs = volume_type.get('extra_specs', {})
|
||||||
# NOTE(tommylikehu): Although the parameter `multiattach` from
|
multiattach = (extra_specs.get('multiattach', '') == '<is> True')
|
||||||
# create volume API is deprecated now, we still need to consider
|
|
||||||
# it when multiattach is not enabled in volume type.
|
|
||||||
multiattach = (extra_specs.get(
|
|
||||||
'multiattach', '') == '<is> True' or multiattach)
|
|
||||||
if multiattach and encryption_key_id:
|
if multiattach and encryption_key_id:
|
||||||
msg = _('Multiattach cannot be used with encrypted volumes.')
|
msg = _('Multiattach cannot be used with encrypted volumes.')
|
||||||
raise exception.InvalidVolume(reason=msg)
|
raise exception.InvalidVolume(reason=msg)
|
||||||
@ -914,8 +909,7 @@ def get_flow(db_api, image_service_api, availability_zones, create_what,
|
|||||||
availability_zones,
|
availability_zones,
|
||||||
rebind={'size': 'raw_size',
|
rebind={'size': 'raw_size',
|
||||||
'availability_zone': 'raw_availability_zone',
|
'availability_zone': 'raw_availability_zone',
|
||||||
'volume_type': 'raw_volume_type',
|
'volume_type': 'raw_volume_type'}))
|
||||||
'multiattach': 'raw_multiattach'}))
|
|
||||||
api_flow.add(QuotaReserveTask(),
|
api_flow.add(QuotaReserveTask(),
|
||||||
EntryCreateTask(),
|
EntryCreateTask(),
|
||||||
QuotaCommitTask())
|
QuotaCommitTask())
|
||||||
|
@ -0,0 +1,20 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
`Bug #2008259 <https://bugs.launchpad.net/cinder/+bug/2008259>`_:
|
||||||
|
Fixed the volume create functionality where non-admin users were
|
||||||
|
able to create multiattach volumes by providing the `multiattach`
|
||||||
|
parameter in the request body. Now we can only create multiattach
|
||||||
|
volumes using a multiattach volume type, which is also the
|
||||||
|
recommended way.
|
||||||
|
other:
|
||||||
|
- |
|
||||||
|
Removed the ability to create multiattach volumes by specifying
|
||||||
|
`multiattach` parameter in the request body of a volume create
|
||||||
|
operation. This functionality is unsafe, can lead to data loss,
|
||||||
|
and has been deprecated since the Queens release.
|
||||||
|
The recommended method for creating a multiattach volume is to
|
||||||
|
use a volume type that supports multiattach. By default, volume
|
||||||
|
types can only be created by the operator. Users who have a need
|
||||||
|
for multiattach volumes should contact their operator if a suitable
|
||||||
|
volume type is not available.
|
Loading…
Reference in New Issue
Block a user