Drop pre-cinder 3.44 version compatibility

The new style volume attach flow semantics, which
were needed to support multiattach volumes, relies
on cinder v3.44 and was added in Queens:

  Ifc01dbf98545104c998ab96f65ff8623a6db0f28

Now that we're in Train, let's require Cinder API
to be at least running Queens level code and drop
the compat checks for queens level nova-compute code
and Cinder API older than Queens.

Lots of old style attachment test code is dropped
as a result since we don't need the version boundary
handling in the API code. The way a lot of the "new
flow" tests were written were as copies of existing
tests modified for the new flow so the old flow tests
could just be dropped later, and that's what happens
in this change.

Change-Id: I6a777b4b7a5729488f939df8c40e49bd40aec3dd

Author: mriedem

nova/compute/api.py

@@ -103,7 +103,6 @@
 AGGREGATE_ACTION_UPDATE_META = 'UpdateMeta'
 AGGREGATE_ACTION_DELETE = 'Delete'
 AGGREGATE_ACTION_ADD = 'Add'
-CINDER_V3_ATTACH_MIN_COMPUTE_VERSION = 24
 MIN_COMPUTE_TRUSTED_CERTS = 31
 MIN_COMPUTE_ABORT_QUEUED_LIVE_MIGRATION = 34
 MIN_COMPUTE_VOLUME_TYPE = 36
@@ -4026,46 +4025,17 @@ def _check_attach_and_reserve_volume(self, context, volume, instance,
         if volume['multiattach'] and not supports_multiattach:
             raise exception.MultiattachNotSupportedOldMicroversion()
 
-        if 'id' in instance:
-            # This is a volume attach to an existing instance, so
-            # we only care about the cell the instance is in.
-            min_compute_version = objects.Service.get_minimum_version(
-                context, 'nova-compute')
-        else:
-            # The instance is being created and we don't know which
-            # cell it's going to land in, so check all cells.
-            # NOTE(danms): We don't require all cells to report here since
-            # we're really concerned about the new-ness of cells that the
-            # instance may be scheduled into. If a cell doesn't respond here,
-            # then it won't be a candidate for the instance and thus doesn't
-            # matter.
-            min_compute_version = \
-                objects.service.get_minimum_version_all_cells(
-                    context, ['nova-compute'])
-
-        if min_compute_version >= CINDER_V3_ATTACH_MIN_COMPUTE_VERSION:
-            # Attempt a new style volume attachment, but fallback to old-style
-            # in case Cinder API 3.44 isn't available.
-            try:
-                attachment_id = self.volume_api.attachment_create(
-                    context, volume_id, instance.uuid)['id']
-                bdm.attachment_id = attachment_id
-                # NOTE(ildikov): In case of boot from volume the BDM at this
-                # point is not yet created in a cell database, so we can't
-                # call save().  When attaching a volume to an existing
-                # instance, the instance is already in a cell and the BDM has
-                # been created in that same cell so updating here in that case
-                # is "ok".
-                if bdm.obj_attr_is_set('id'):
-                    bdm.save()
-            except exception.CinderAPIVersionNotAvailable:
-                LOG.debug('The available Cinder microversion is not high '
-                          'enough to create new style volume attachment.')
-                self.volume_api.reserve_volume(context, volume_id)
-        else:
-            LOG.debug('The compute service version is not high enough to '
-                      'create a new style volume attachment.')
-            self.volume_api.reserve_volume(context, volume_id)
+        attachment_id = self.volume_api.attachment_create(
+            context, volume_id, instance.uuid)['id']
+        bdm.attachment_id = attachment_id
+        # NOTE(ildikov): In case of boot from volume the BDM at this
+        # point is not yet created in a cell database, so we can't
+        # call save().  When attaching a volume to an existing
+        # instance, the instance is already in a cell and the BDM has
+        # been created in that same cell so updating here in that case
+        # is "ok".
+        if bdm.obj_attr_is_set('id'):
+            bdm.save()
 
     # TODO(stephenfin): Fold this back in now that cells v1 no longer needs to
     # override it.
@@ -4155,27 +4125,14 @@ def attach_volume(self, context, instance, volume_id, device=None,
         if device and not block_device.match_device(device):
             raise exception.InvalidDevicePath(path=device)
 
-        # Check to see if the computes in this cell can support new-style
-        # volume attachments.
-        min_compute_version = objects.Service.get_minimum_version(
-            context, 'nova-compute')
-        if min_compute_version >= CINDER_V3_ATTACH_MIN_COMPUTE_VERSION:
-            try:
-                # Check to see if Cinder is new enough to create new-style
-                # attachments.
-                cinder.is_microversion_supported(context, '3.44')
-            except exception.CinderAPIVersionNotAvailable:
-                pass
-            else:
-                # Make sure the volume isn't already attached to this instance
-                # because based on the above checks, we'll use the new style
-                # attachment flow in _check_attach_and_reserve_volume and
-                # Cinder will allow multiple attachments between the same
-                # volume and instance but the old flow API semantics don't
-                # allow that so we enforce it here.
-                self._check_volume_already_attached_to_instance(context,
-                                                                instance,
-                                                                volume_id)
+        # Make sure the volume isn't already attached to this instance
+        # because we'll use the v3.44 attachment flow in
+        # _check_attach_and_reserve_volume and Cinder will allow multiple
+        # attachments between the same volume and instance but the old flow
+        # API semantics don't allow that so we enforce it here.
+        self._check_volume_already_attached_to_instance(context,
+                                                        instance,
+                                                        volume_id)
 
         volume = self.volume_api.get(context, volume_id)
         is_shelved_offloaded = instance.vm_state == vm_states.SHELVED_OFFLOADED

nova/tests/fixtures.py

@@ -1636,191 +1636,7 @@ def setUp(self):
             lambda *args, **kwargs: mock.MagicMock()))
 
 
-class CinderFixture(fixtures.Fixture):
-    """A fixture to volume operations"""
-
-    # the default project_id in OSAPIFixtures
-    tenant_id = '6f70656e737461636b20342065766572'
-
-    SWAP_OLD_VOL = 'a07f71dc-8151-4e7d-a0cc-cd24a3f11113'
-    SWAP_NEW_VOL = '227cc671-f30b-4488-96fd-7d0bf13648d8'
-    SWAP_ERR_OLD_VOL = '828419fa-3efb-4533-b458-4267ca5fe9b1'
-    SWAP_ERR_NEW_VOL = '9c6d9c2d-7a8f-4c80-938d-3bf062b8d489'
-
-    # This represents a bootable image-backed volume to test
-    # boot-from-volume scenarios.
-    IMAGE_BACKED_VOL = '6ca404f3-d844-4169-bb96-bc792f37de98'
-
-    def __init__(self, test):
-        super(CinderFixture, self).__init__()
-        self.test = test
-        self.swap_volume_instance_uuid = None
-        self.swap_volume_instance_error_uuid = None
-        self.reserved_volumes = list()
-        # This is a map of instance UUIDs mapped to a list of volume IDs.
-        # This map gets updated on attach/detach operations.
-        self.attachments = collections.defaultdict(list)
-
-    def volume_ids_for_instance(self, instance_uuid):
-        return self.attachments.get(instance_uuid)
-
-    def setUp(self):
-        super(CinderFixture, self).setUp()
-
-        def fake_get(self_api, context, volume_id, microversion=None):
-            # Check for the special swap volumes.
-            if volume_id in (CinderFixture.SWAP_OLD_VOL,
-                             CinderFixture.SWAP_ERR_OLD_VOL):
-                volume = {
-                             'status': 'available',
-                             'display_name': 'TEST1',
-                             'attach_status': 'detached',
-                             'id': volume_id,
-                             'multiattach': False,
-                             'size': 1
-                         }
-                if ((self.swap_volume_instance_uuid and
-                     volume_id == CinderFixture.SWAP_OLD_VOL) or
-                    (self.swap_volume_instance_error_uuid and
-                     volume_id == CinderFixture.SWAP_ERR_OLD_VOL)):
-                    instance_uuid = (self.swap_volume_instance_uuid
-                        if volume_id == CinderFixture.SWAP_OLD_VOL
-                        else self.swap_volume_instance_error_uuid)
-
-                    volume.update({
-                        'status': 'in-use',
-                        'attachments': {
-                            instance_uuid: {
-                                'mountpoint': '/dev/vdb',
-                                'attachment_id': volume_id
-                            }
-                        },
-                        'attach_status': 'attached'
-                    })
-                    return volume
-
-            # Check to see if the volume is attached.
-            for instance_uuid, volumes in self.attachments.items():
-                if volume_id in volumes:
-                    # The volume is attached.
-                    volume = {
-                        'status': 'in-use',
-                        'display_name': volume_id,
-                        'attach_status': 'attached',
-                        'id': volume_id,
-                        'multiattach': False,
-                        'size': 1,
-                        'attachments': {
-                            instance_uuid: {
-                                'attachment_id': volume_id,
-                                'mountpoint': '/dev/vdb'
-                            }
-                        }
-                    }
-                    break
-            else:
-                # This is a test that does not care about the actual details.
-                reserved_volume = (volume_id in self.reserved_volumes)
-                volume = {
-                    'status': 'attaching' if reserved_volume else 'available',
-                    'display_name': 'TEST2',
-                    'attach_status': 'detached',
-                    'id': volume_id,
-                    'multiattach': False,
-                    'size': 1
-                }
-
-            # Check for our special image-backed volume.
-            if volume_id == self.IMAGE_BACKED_VOL:
-                # Make it a bootable volume.
-                volume['bootable'] = True
-                # Add the image_id metadata.
-                volume['volume_image_metadata'] = {
-                    # There would normally be more image metadata in here...
-                    'image_id': '155d900f-4e14-4e4c-a73d-069cbf4541e6'
-                }
-
-            return volume
-
-        def fake_initialize_connection(self, context, volume_id, connector):
-            if volume_id == CinderFixture.SWAP_ERR_NEW_VOL:
-                # Return a tuple in order to raise an exception.
-                return ()
-            return {}
-
-        def fake_migrate_volume_completion(self, context, old_volume_id,
-                                           new_volume_id, error):
-            return {'save_volume_id': new_volume_id}
-
-        def fake_reserve_volume(self_api, context, volume_id):
-            self.reserved_volumes.append(volume_id)
-
-        def fake_unreserve_volume(self_api, context, volume_id):
-            # NOTE(mnaser): It's possible that we unreserve a volume that was
-            #               never reserved (ex: instance.volume_attach.error
-            #               notification tests)
-            if volume_id in self.reserved_volumes:
-                self.reserved_volumes.remove(volume_id)
-
-        def fake_attach(_self, context, volume_id, instance_uuid,
-                        mountpoint, mode='rw'):
-            # Check to see if the volume is already attached to any server.
-            for instance, volumes in self.attachments.items():
-                if volume_id in volumes:
-                    raise exception.InvalidInput(
-                        reason='Volume %s is already attached to '
-                               'instance %s' % (volume_id, instance))
-            # It's not attached so let's "attach" it.
-            self.attachments[instance_uuid].append(volume_id)
-
-        self.test.stub_out('nova.volume.cinder.API.attach',
-                           fake_attach)
-
-        def fake_detach(_self, context, volume_id, instance_uuid=None,
-                        attachment_id=None):
-            # NOTE(mnaser): It's possible that we unreserve a volume that was
-            #               never reserved (ex: instance.volume_attach.error
-            #               notification tests)
-            if volume_id in self.reserved_volumes:
-                self.reserved_volumes.remove(volume_id)
-
-            if instance_uuid is not None:
-                # If the volume isn't attached to this instance it will
-                # result in a ValueError which indicates a broken test or
-                # code, so we just let that raise up.
-                self.attachments[instance_uuid].remove(volume_id)
-            else:
-                for instance, volumes in self.attachments.items():
-                    if volume_id in volumes:
-                        volumes.remove(volume_id)
-                        break
-
-        self.test.stub_out('nova.volume.cinder.API.detach', fake_detach)
-
-        self.test.stub_out('nova.volume.cinder.API.begin_detaching',
-                           lambda *args, **kwargs: None)
-        self.test.stub_out('nova.volume.cinder.API.get',
-                           fake_get)
-        self.test.stub_out('nova.volume.cinder.API.initialize_connection',
-                           fake_initialize_connection)
-        self.test.stub_out(
-            'nova.volume.cinder.API.migrate_volume_completion',
-            fake_migrate_volume_completion)
-        self.test.stub_out('nova.volume.cinder.API.reserve_volume',
-                           fake_reserve_volume)
-        self.test.stub_out('nova.volume.cinder.API.roll_detaching',
-                           lambda *args, **kwargs: None)
-        self.test.stub_out('nova.volume.cinder.API.terminate_connection',
-                           lambda *args, **kwargs: None)
-        self.test.stub_out('nova.volume.cinder.API.unreserve_volume',
-                           fake_unreserve_volume)
-        self.test.stub_out('nova.volume.cinder.API.check_attached',
-                           lambda *args, **kwargs: None)
-
-
-# TODO(mriedem): We can probably pull some of the common parts from the
-# CinderFixture into a common mixin class for things like the variables
-# and fake_get.
+# TODO(mriedem): Just rename this to be CinderFixture.
 class CinderFixtureNewAttachFlow(fixtures.Fixture):
     """A fixture to volume operations with the new Cinder attach/detach API"""
 
@@ -1874,8 +1690,8 @@ def fake_get(self_api, context, volume_id, microversion=None):
             # Check for the special swap volumes.
             attachments = self.volume_to_attachment[volume_id]
 
-            if volume_id in (CinderFixture.SWAP_OLD_VOL,
-                             CinderFixture.SWAP_ERR_OLD_VOL):
+            if volume_id in (self.SWAP_OLD_VOL,
+                             self.SWAP_ERR_OLD_VOL):
                 volume = {
                              'status': 'available',
                              'display_name': 'TEST1',
@@ -1885,11 +1701,11 @@ def fake_get(self_api, context, volume_id, microversion=None):
                              'size': 1
                          }
                 if ((self.swap_volume_instance_uuid and
-                     volume_id == CinderFixture.SWAP_OLD_VOL) or
+                     volume_id == self.SWAP_OLD_VOL) or
                     (self.swap_volume_instance_error_uuid and
-                     volume_id == CinderFixture.SWAP_ERR_OLD_VOL)):
+                     volume_id == self.SWAP_ERR_OLD_VOL)):
                     instance_uuid = (self.swap_volume_instance_uuid
-                        if volume_id == CinderFixture.SWAP_OLD_VOL
+                        if volume_id == self.SWAP_OLD_VOL
                         else self.swap_volume_instance_error_uuid)
 
                     if attachments:
@@ -1956,7 +1772,7 @@ def fake_get(self_api, context, volume_id, microversion=None):
 
             return volume
 
-        def fake_migrate_volume_completion(self, context, old_volume_id,
+        def fake_migrate_volume_completion(_self, context, old_volume_id,
                                            new_volume_id, error):
             return {'save_volume_id': new_volume_id}
 
@@ -2021,7 +1837,7 @@ def fake_attachment_update(_self, context, attachment_id, connector,
                               'connection_info': {'data':
                                                   {'foo': 'bar',
                                                    'target_lun': '1'}}}
-            if attachment_id == CinderFixtureNewAttachFlow.SWAP_ERR_ATTACH_ID:
+            if attachment_id == self.SWAP_ERR_ATTACH_ID:
                 # This intentionally triggers a TypeError for the
                 # instance.volume_swap.error versioned notification tests.
                 attachment_ref = {'connection_info': ()}

nova/tests/functional/api_sample_tests/test_volumes.py

@@ -17,7 +17,6 @@
 
 from oslo_utils.fixture import uuidsentinel as uuids
 
-from nova.compute import api as compute_api
 from nova import context
 from nova import objects
 from nova.tests import fixtures
@@ -28,10 +27,6 @@
 from nova.tests.unit import fake_instance
 
 
-COMPUTE_VERSION_OLD_ATTACH_FLOW = \
-    compute_api.CINDER_V3_ATTACH_MIN_COMPUTE_VERSION - 1
-
-
 class SnapshotsSampleJsonTests(api_sample_base.ApiSampleTestBaseV21):
     sample_dir = "os-volumes"
 
@@ -263,11 +258,9 @@ def _get_vol_attachment_subs(self, subs):
         return subs
 
     def test_attach_volume_to_server(self):
-        self.stub_out('nova.objects.Service.get_minimum_version',
-                      lambda *a, **k: COMPUTE_VERSION_OLD_ATTACH_FLOW)
         self.stub_out('nova.volume.cinder.API.get', fakes.stub_volume_get)
-        self.stub_out('nova.volume.cinder.API.reserve_volume',
-                      lambda *a, **k: None)
+        self.stub_out('nova.volume.cinder.API.attachment_create',
+                      lambda *a, **k: {'id': uuids.attachment_id})
         device_name = '/dev/vdd'
         bdm = objects.BlockDeviceMapping()
         bdm['device_name'] = device_name
@@ -294,8 +287,6 @@ def test_attach_volume_to_server(self):
                               response, 200)
 
     def test_attach_volume_to_server_new_flow(self):
-        self.stub_out('nova.volume.cinder.is_microversion_supported',
-                      lambda *a, **k: None)
         self.stub_out('nova.volume.cinder.API.get', fakes.stub_volume_get)
         self.stub_out('nova.volume.cinder.API.attachment_create',
                       lambda *a, **k: {'id': uuids.volume})