Remove multiatttach request parameter20.2.0

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] https://github.com/openstack/cinder/commit/f1bfd9790d2a7cac9a3e66417b11dc8e3edd8109
[5] https://docs.openstack.org/cinder/latest/admin/volume-multiattach.html#how-to-create-a-multiattach-volume
[6] https://github.com/openstack/cinder/commit/94dbf5cce2caff484460a1330feb6cbf7f3dd56a
[7] https://github.com/openstack/python-cinderclient/commit/adb141a2626192e8f45a911291895716d7c1c8a4
[8] https://github.com/openstack/python-cinderclient/commit/3c1b417959689c85a2f54505057ca995fedca075

Depends-On: https://review.opendev.org/c/openstack/tempest/+/875372
Closes-Bug: 2008259

Change-Id: I0ece6e279048abcc04b3674108290a80eca6bd62
(cherry picked from commit 32f1145b7ddf9a9a359e2359e7db63dbdd00b899)
(cherry picked from commit e2c3bcc6e380921bbe283b4a1b173216193c753d)
Conflicts: api-ref/source/v3/parameters.yaml
(cherry picked from commit a8a4cdcb2e099456d435028c924e51dcbdee33e9)
Conflicts: cinder/volume/flows/api/create_volume.py

14 files changed, 59 insertions, 121 deletions

diff --git a/api-ref/source/v2/parameters.yaml b/api-ref/source/v2/parameters.yaml
index 1b7fde4db..be357736f 100644
--- a/api-ref/source/v2/parameters.yaml
+++ b/api-ref/source/v2/parameters.yaml
@@ -1081,15 +1081,6 @@ mountpoint:
   in: body
   required: true
   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:
   description: |
     If true, this volume can attach to more than one
diff --git a/api-ref/source/v2/volumes-v2-volumes.inc b/api-ref/source/v2/volumes-v2-volumes.inc
index acd507943..07be3422e 100644
--- a/api-ref/source/v2/volumes-v2-volumes.inc
+++ b/api-ref/source/v2/volumes-v2-volumes.inc
@@ -183,7 +183,6 @@ Request
    - size: size
    - description: description_9
    - imageRef: imageRef
-   - multiattach: multiattach_req
    - availability_zone: availability_zone
    - source_volid: source_volid
    - name: volume_name_optional
diff --git a/api-ref/source/v3/parameters.yaml b/api-ref/source/v3/parameters.yaml
index 50bd3a171..aa30c2a12 100644
--- a/api-ref/source/v3/parameters.yaml
+++ b/api-ref/source/v3/parameters.yaml
@@ -1905,15 +1905,6 @@ mountpoint:
   in: body
   required: true
   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:
   description: |
     If true, this volume can attach to more than one
diff --git a/api-ref/source/v3/volumes-v3-volumes.inc b/api-ref/source/v3/volumes-v3-volumes.inc
index 86777887e..c1ffcf198 100644
--- a/api-ref/source/v3/volumes-v3-volumes.inc
+++ b/api-ref/source/v3/volumes-v3-volumes.inc
@@ -217,7 +217,6 @@ Request
    - availability_zone: availability_zone
    - source_volid: source_volid
    - description: description_vol
-   - multiattach: multiattach_req
    - snapshot_id: snapshot_id
    - backup_id: backup_id
    - name: volume_name_optional
diff --git a/cinder/api/schemas/volumes.py b/cinder/api/schemas/volumes.py
index f3f689770..08678cd56 100644
--- a/cinder/api/schemas/volumes.py
+++ b/cinder/api/schemas/volumes.py
@@ -49,6 +49,12 @@ create = {
                 'consistencygroup_id': parameter_types.optional_uuid,
                 'size': parameter_types.volume_size_allows_null,
                 '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,
                 'image_id': {'type': ['string', 'null'], 'minLength': 0,
                              'maxLength': 255},
diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index ddff0471e..67d2d29b5 100644
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -19,7 +19,6 @@ from http import HTTPStatus
 
 from oslo_config import cfg
 from oslo_log import log as logging
-from oslo_log import versionutils
 from oslo_utils import uuidutils
 import webob
 from webob import exc
@@ -261,14 +260,6 @@ class VolumeController(wsgi.Controller):
 
         kwargs['availability_zone'] = volume.get('availability_zone', 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:
             new_volume = self.volume_api.create(
diff --git a/cinder/api/v3/volumes.py b/cinder/api/v3/volumes.py
index 2d4a72f38..da97cfe82 100644
--- a/cinder/api/v3/volumes.py
+++ b/cinder/api/v3/volumes.py
@@ -15,7 +15,6 @@
 from http import HTTPStatus
 
 from oslo_log import log as logging
-from oslo_log import versionutils
 from oslo_utils import timeutils
 import webob
 from webob import exc
@@ -387,15 +386,14 @@ class VolumeController(volumes_v2.VolumeController):
 
         kwargs['availability_zone'] = volume.get('availability_zone', None)
         kwargs['scheduler_hints'] = volume.get('scheduler_hints', None)
-        multiattach = volume.get('multiattach', False)
-        kwargs['multiattach'] = multiattach
-
+        multiattach = utils.get_bool_param('multiattach', volume)
         if multiattach:
-            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)
+            msg = _("multiattach parameter has been removed. The default "
+                    "behavior is to use multiattach enabled volume types. "
+                    "Contact your administrator to create a multiattach "
+                    "enabled volume type and use it to create multiattach "
+                    "volumes.")
+            raise exc.HTTPBadRequest(explanation=msg)
         try:
             new_volume = self.volume_api.create(
                 context, size, volume.get('display_name'),
diff --git a/cinder/scheduler/filter_scheduler.py b/cinder/scheduler/filter_scheduler.py
index 70d99cfa8..be3974f38 100644
--- a/cinder/scheduler/filter_scheduler.py
+++ b/cinder/scheduler/filter_scheduler.py
@@ -333,19 +333,6 @@ class FilterScheduler(driver.Scheduler):
         self.populate_filter_properties(request_spec,
                                         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
         retry = filter_properties.get('retry', {})
         if retry.get('backends', []):
diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py
index 39e3e0768..7139a15c4 100644
--- a/cinder/tests/unit/api/v2/test_volumes.py
+++ b/cinder/tests/unit/api/v2/test_volumes.py
@@ -158,8 +158,7 @@ class VolumeApiTest(test.TestCase):
                              consistencygroup_id=None,
                              volume_type=None,
                              image_ref=None,
-                             image_id=None,
-                             multiattach=False):
+                             image_id=None):
         vol = {"size": size,
                "name": name,
                "description": description,
@@ -168,7 +167,6 @@ class VolumeApiTest(test.TestCase):
                "source_volid": source_volid,
                "consistencygroup_id": consistencygroup_id,
                "volume_type": volume_type,
-               "multiattach": multiattach,
                }
 
         if image_id is not None:
@@ -240,7 +238,6 @@ class VolumeApiTest(test.TestCase):
                 'consistencygroup': None,
                 'availability_zone': availability_zone,
                 'scheduler_hints': None,
-                'multiattach': False,
                 }
 
     @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full',
@@ -570,37 +567,6 @@ class VolumeApiTest(test.TestCase):
                           req,
                           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'},
               {'a': 'a' * 256},
               {'': 'a'},
diff --git a/cinder/tests/unit/api/v3/test_volumes.py b/cinder/tests/unit/api/v3/test_volumes.py
index f20e61d65..52d5f3fc9 100644
--- a/cinder/tests/unit/api/v3/test_volumes.py
+++ b/cinder/tests/unit/api/v3/test_volumes.py
@@ -619,7 +619,6 @@ class VolumeApiTest(test.TestCase):
             'consistencygroup': None,
             'availability_zone': availability_zone,
             'scheduler_hints': None,
-            'multiattach': False,
             'group': test_group,
         }
 
@@ -1189,3 +1188,25 @@ class VolumeApiTest(test.TestCase):
         volumes = res_dict['volumes']
         self.assertEqual(1, len(volumes))
         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)
diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py
index 976079216..87e16b9ff 100644
--- a/cinder/tests/unit/volume/test_volume.py
+++ b/cinder/tests/unit/volume/test_volume.py
@@ -772,19 +772,6 @@ class VolumeTestCase(base.BaseVolumeTestCase):
         self.assertEqual(foo['id'], vol['volume_type_id'])
         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):
         if policy == vol_policy.MULTIATTACH_POLICY:
             raise exception.PolicyNotAuthorized(action='Test')
@@ -809,16 +796,6 @@ class VolumeTestCase(base.BaseVolumeTestCase):
                               1, 'admin-vol', 'description',
                               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)
     def test_create_volume_with_encrypted_volume_type_multiattach(self):
         ctxt = context.get_admin_context()
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 394511d6e..85a6deb2b 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -229,7 +229,6 @@ class API(base.Base):
                source_replica=None,
                consistencygroup: Optional[objects.ConsistencyGroup] = None,
                cgsnapshot: Optional[objects.CGSnapshot] = None,
-               multiattach: Optional[bool] = False,
                source_cg=None,
                group: Optional[objects.Group] = None,
                group_snapshot=None,
@@ -338,7 +337,6 @@ class API(base.Base):
             'optional_args': {'is_quota_committed': False},
             'consistencygroup': consistencygroup,
             'cgsnapshot': cgsnapshot,
-            'raw_multiattach': multiattach,
             'group': group,
             'group_snapshot': group_snapshot,
             'source_group': source_group,
diff --git a/cinder/volume/flows/api/create_volume.py b/cinder/volume/flows/api/create_volume.py
index 1715b5048..b15bd7726 100644
--- a/cinder/volume/flows/api/create_volume.py
+++ b/cinder/volume/flows/api/create_volume.py
@@ -440,8 +440,7 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask):
                 cgsnapshot,
                 group,
                 group_snapshot,
-                backup: Optional[dict],
-                multiattach: bool = False) -> Dict[str, Any]:
+                backup: Optional[dict]) -> Dict[str, Any]:
 
         utils.check_exclusive_options(snapshot=snapshot,
                                       imageRef=image_id,
@@ -489,11 +488,7 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask):
             volume_type = objects.VolumeType.get_by_name_or_id(
                 context, volume_type_id)
             extra_specs = volume_type.get('extra_specs', {})
-            # NOTE(tommylikehu): Although the parameter `multiattach` from
-            # 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)
+            multiattach = (extra_specs.get('multiattach', '') == '<is> True')
             if multiattach and encryption_key_id:
                 msg = _('Multiattach cannot be used with encrypted volumes.')
                 raise exception.InvalidVolume(reason=msg)
@@ -910,8 +905,7 @@ def get_flow(db_api, image_service_api, availability_zones, create_what,
         availability_zones,
         rebind={'size': 'raw_size',
                 'availability_zone': 'raw_availability_zone',
-                'volume_type': 'raw_volume_type',
-                'multiattach': 'raw_multiattach'}))
+                'volume_type': 'raw_volume_type'}))
     api_flow.add(QuotaReserveTask(),
                  EntryCreateTask(),
                  QuotaCommitTask())
diff --git a/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml b/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml
new file mode 100644
index 000000000..0b11c4a48
--- /dev/null
+++ b/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml
@@ -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.