OpenStack 2024.1

cinder/api/v3/volumes.py

@@ -36,6 +36,7 @@
 from cinder import objects
 from cinder.policies import volumes as policy
 from cinder import utils
+from cinder.volume import rpcapi
 
 LOG = logging.getLogger(__name__)
 
@@ -48,6 +49,7 @@ class VolumeController(volumes_v2.VolumeController):
     def __init__(self, ext_mgr=None):
         self.group_api = group_api.API()
         self.backup_api = backup_api.API()
+        self.rpcapi = rpcapi.VolumeAPI()
         super(VolumeController, self).__init__(ext_mgr)
 
     def delete(self, req, id):
@@ -237,7 +239,10 @@ def revert(self, req, id, body):
                     "the latest one of volume %(v_id)s.")
             raise exc.HTTPBadRequest(explanation=msg % {'s_id': snapshot_id,
                                                         'v_id': volume.id})
-        if volume.size != l_snap.volume_size:
+
+        can_revert_different_size = self.rpcapi.can_revert_different_size(
+            context, volume)
+        if volume.size != l_snap.volume_size and not can_revert_different_size:
             msg = _("Can't revert volume %(v_id)s to its latest snapshot "
                     "%(s_id)s. The volume size must be equal to the snapshot "
                     "size.")

cinder/tests/fake_driver.py

@@ -71,6 +71,10 @@ def create_snapshot(self, snapshot):
     def delete_snapshot(self, snapshot):
         pass
 
+    @volume_utils.trace_method
+    def can_revert_different_size(self):
+        return True
+
     @volume_utils.trace_method
     def ensure_export(self, context, volume):
         pass

cinder/tests/functional/api/client.py

@@ -231,6 +231,9 @@ def delete_volume(self, volume_id):
     def put_volume(self, volume_id, volume):
         return self.api_put('/volumes/%s' % volume_id, volume)['volume']
 
+    def post_volume_action(self, volume_id, action):
+        return self.api_post('/volumes/%s/action' % volume_id, action)
+
     def post_manage_volume(self, host=None, ref=None):
         if not host:
             host = "fake-host"

cinder/tests/functional/test_volumes.py

@@ -326,3 +326,44 @@ def test_create_and_update_volume(self):
         found_volume = self.api.get_volume(created_volume_id)
         self.assertEqual(created_volume_id, found_volume['id'])
         self.assertEqual('vol-one', found_volume['name'])
+
+    def test_snapshot_extend_and_revert_volume(self):
+        self.osapi_version = '3.40'
+
+        volume = self.api.post_volume({'volume': {
+            'size': 1, 'name': 'vol1'}})
+
+        # Wait (briefly) for creation. Delay is due to the 'message queue'
+        volume = self._poll_volume_while(volume["id"], ['creating'])
+        self.assertEqual('available', volume['status'])
+
+        # snapshot volume
+        snapshot = self.api.post_snapshot(
+            {'snapshot': {'volume_id': volume["id"],
+                          'name': 'test_snapshot'}})
+
+        # Wait (briefly) for creation. Delay is due to the 'message queue'
+        snapshot = self._poll_snapshot_while(snapshot["id"], ['creating'])
+        self.assertEqual('available', snapshot['status'])
+
+        # extend volume
+        self.api.post_volume_action(
+            volume["id"], {"os-extend": {"new_size": 2}})
+
+        # Wait (briefly) for extension. Delay is due to the 'message queue'
+        volume = self._poll_volume_while(volume["id"], ['extending'])
+        self.assertEqual('available', volume['status'])
+
+        # Check volume is extended
+        self.assertEqual(volume["size"], 2)
+
+        # revert volume
+        self.api.post_volume_action(
+            volume["id"], {'revert': {'snapshot_id': snapshot["id"]}})
+
+        # Wait (briefly) for revertion. Delay is due to the 'message queue'
+        volume = self._poll_volume_while(volume["id"], ['reverting'])
+        self.assertEqual('available', volume['status'])
+
+        # Check volume size changed back to snapshot's size
+        self.assertEqual(volume["size"], 1)

cinder/tests/unit/api/v3/test_volumes.py

@@ -48,6 +48,7 @@
 from cinder.tests.unit import utils as test_utils
 from cinder.volume import api as volume_api
 from cinder.volume import api as vol_get
+from cinder.volume import rpcapi
 
 DEFAULT_AZ = "zone1:host1"
 
@@ -999,10 +1000,13 @@ def _fake_create_snapshot(self, volume_id, volume_size=1):
         snapshot.create()
         return snapshot
 
+    @mock.patch.object(rpcapi.VolumeAPI, 'can_revert_different_size',
+                       return_value=False)
     @mock.patch.object(objects.Volume, 'get_latest_snapshot')
     @mock.patch.object(volume_api.API, 'get_volume')
     def test_volume_revert_with_snapshot_not_found(self, mock_volume,
-                                                   mock_latest):
+                                                   mock_latest,
+                                                   mock_different_size):
         fake_volume = self._fake_create_volume()
         mock_volume.return_value = fake_volume
         mock_latest.side_effect = exception.VolumeSnapshotNotFound(volume_id=
@@ -1016,10 +1020,13 @@ def test_volume_revert_with_snapshot_not_found(self, mock_volume,
                           req, 'fake_id', {'revert': {'snapshot_id':
                                                       'fake_snapshot_id'}})
 
+    @mock.patch.object(rpcapi.VolumeAPI, 'can_revert_different_size',
+                       return_value=False)
     @mock.patch.object(objects.Volume, 'get_latest_snapshot')
     @mock.patch.object(volume_api.API, 'get_volume')
     def test_volume_revert_with_snapshot_not_match(self, mock_volume,
-                                                   mock_latest):
+                                                   mock_latest,
+                                                   mock_different_size):
         fake_volume = self._fake_create_volume()
         mock_volume.return_value = fake_volume
         fake_snapshot = self._fake_create_snapshot(fake.UUID1)
@@ -1033,14 +1040,17 @@ def test_volume_revert_with_snapshot_not_match(self, mock_volume,
                           req, 'fake_id', {'revert': {'snapshot_id':
                                                       'fake_snapshot_id'}})
 
+    @mock.patch.object(rpcapi.VolumeAPI, 'can_revert_different_size',
+                       return_value=False)
     @mock.patch.object(objects.Volume, 'get_latest_snapshot')
     @mock.patch('cinder.objects.base.'
                 'CinderPersistentObject.update_single_status_where')
     @mock.patch.object(volume_api.API, 'get_volume')
     def test_volume_revert_update_status_failed(self,
                                                 mock_volume,
                                                 mock_update,
-                                                mock_latest):
+                                                mock_latest,
+                                                mock_different_size):
         fake_volume = self._fake_create_volume()
         fake_snapshot = self._fake_create_snapshot(fake_volume['id'])
         mock_volume.return_value = fake_volume
@@ -1067,10 +1077,13 @@ def test_volume_revert_update_status_failed(self,
                           req, fake_volume['id'], {'revert': {'snapshot_id':
                                                    fake_snapshot['id']}})
 
+    @mock.patch.object(rpcapi.VolumeAPI, 'can_revert_different_size',
+                       return_value=False)
     @mock.patch.object(objects.Volume, 'get_latest_snapshot')
     @mock.patch.object(volume_api.API, 'get_volume')
     def test_volume_revert_with_not_equal_size(self, mock_volume,
-                                               mock_latest):
+                                               mock_latest,
+                                               mock_different_size):
         fake_volume = self._fake_create_volume(size=2)
         fake_snapshot = self._fake_create_snapshot(fake_volume['id'],
                                                    volume_size=1)
@@ -1085,6 +1098,35 @@ def test_volume_revert_with_not_equal_size(self, mock_volume,
                           req, fake_volume['id'],
                           {'revert': {'snapshot_id': fake_snapshot['id']}})
 
+    @mock.patch.object(rpcapi.VolumeAPI, 'can_revert_different_size',
+                       return_value=True)
+    @mock.patch.object(volume_api.API, 'revert_to_snapshot', autospec=True)
+    @mock.patch.object(objects.Volume, 'get_latest_snapshot')
+    @mock.patch.object(volume_api.API, 'get_volume')
+    def test_volume_revert_with_not_equal_size_supported(self, mock_volume,
+                                                         mock_latest,
+                                                         mock_revert,
+                                                         mock_different_size):
+        fake_volume = self._fake_create_volume(size=2)
+        fake_snapshot = self._fake_create_snapshot(fake_volume['id'],
+                                                   volume_size=1)
+        mock_volume.return_value = fake_volume
+        mock_latest.return_value = fake_snapshot
+        req = fakes.HTTPRequest.blank('/v3/volumes/%s/revert'
+                                      % fake_volume['id'])
+        req.headers = mv.get_mv_header(mv.VOLUME_REVERT)
+        req.api_version_request = mv.get_api_version(
+            mv.VOLUME_REVERT)
+        self.controller.revert(
+            req, fake_volume['id'],
+            {'revert': {'snapshot_id': fake_snapshot['id']}},
+        )
+
+        context = req.environ['cinder.context']
+        mock_revert.assert_called_once_with(self.controller.volume_api,
+                                            context, fake_volume,
+                                            fake_snapshot)
+
     def test_view_get_attachments(self):
         fake_volume = self._fake_create_volume()
         fake_volume['attach_status'] = fields.VolumeAttachStatus.ATTACHING

cinder/tests/unit/brick/fake_lvm.py

@@ -55,6 +55,9 @@ def deactivate_lv(self, name):
     def lv_has_snapshot(self, name):
         return False
 
+    def extend_volume(self, lv_name, new_size):
+        pass
+
     def activate_lv(self, lv, is_snapshot=False, permanent=False):
         pass
 

cinder/tests/unit/policies/test_volume_actions.py

@@ -32,6 +32,7 @@
 from cinder.tests.unit import utils as test_utils
 from cinder.volume import api as volume_api
 from cinder.volume import manager as volume_manager
+from cinder.volume import rpcapi
 
 
 @ddt.ddt
@@ -160,7 +161,9 @@ def test_extend_attached_policy(self, user_id):
                                  id=volume.id, body=body)
 
     @ddt.data(*base.all_users)
-    def test_revert_policy(self, user_id):
+    @mock.patch.object(rpcapi.VolumeAPI, 'can_revert_different_size',
+                       return_value=False)
+    def test_revert_policy(self, user_id, mock_different_size):
         volume = self._create_volume()
         snap = test_utils.create_snapshot(
             self.project_member_context,

cinder/tests/unit/volume/drivers/test_rbd.py

@@ -1785,12 +1785,13 @@ def test_bad_locations(self):
 
     @common_mocks
     def test_cloneable(self):
-        with mock.patch.object(self.driver, '_get_fsid') as mock_get_fsid:
-            mock_get_fsid.return_value = 'abc'
-            location = 'rbd://abc/pool/image/snap'
-            info = {'disk_format': 'raw'}
-            self.assertTrue(self.driver._is_cloneable(location, info))
-            self.assertTrue(mock_get_fsid.called)
+        for disk_format in ('raw', 'iso'):
+            with mock.patch.object(self.driver, '_get_fsid') as mock_get_fsid:
+                mock_get_fsid.return_value = 'abc'
+                location = 'rbd://abc/pool/image/snap'
+                info = {'disk_format': disk_format}
+                self.assertTrue(self.driver._is_cloneable(location, info))
+                self.assertTrue(mock_get_fsid.called)
 
     @common_mocks
     def test_uncloneable_different_fsid(self):

cinder/tests/unit/volume/test_volume.py

@@ -2296,20 +2296,22 @@ def test__revert_to_snapshot(self, driver_error):
                 generic_revert.assert_called_once_with(self.context, {}, {})
 
     @ddt.data({},
+              {'size': 1},
               {'has_snapshot': True},
               {'use_temp_snapshot': True},
               {'use_temp_snapshot': True, 'has_snapshot': True})
     @ddt.unpack
     def test_revert_to_snapshot(self, has_snapshot=False,
-                                use_temp_snapshot=False):
+                                use_temp_snapshot=False,
+                                size=2):
         fake_volume = tests_utils.create_volume(self.context,
                                                 status='reverting',
                                                 project_id='123',
                                                 size=2)
         fake_snapshot = tests_utils.create_snapshot(self.context,
                                                     fake_volume['id'],
                                                     status='restoring',
-                                                    volume_size=1)
+                                                    volume_size=size)
         with mock.patch.object(self.volume,
                                '_revert_to_snapshot') as _revert,\
             mock.patch.object(self.volume,
@@ -2347,7 +2349,7 @@ def test_revert_to_snapshot(self, has_snapshot=False,
             fake_snapshot.refresh()
             self.assertEqual('available', fake_volume['status'])
             self.assertEqual('available', fake_snapshot['status'])
-            self.assertEqual(2, fake_volume['size'])
+            self.assertEqual(size, fake_volume['size'])
 
     def test_revert_to_snapshot_failed(self):
         fake_volume = tests_utils.create_volume(self.context,

cinder/volume/driver.py

@@ -2278,6 +2278,14 @@ def manage_existing(self, volume, existing_ref):
         msg = _("Manage existing volume not implemented.")
         raise NotImplementedError(msg)
 
+    def can_revert_different_size(self):
+        """Determine if the driver supports reverting to a different size.
+
+        Cinder Volume drivers can optinally support reverting to a snapshot of
+        volume with a different (smaller) size.
+        """
+        return False
+
     def revert_to_snapshot(self, context, volume, snapshot):
         """Revert volume to snapshot.
 

cinder/volume/drivers/rbd.py

@@ -1587,6 +1587,10 @@ def snapshot_revert_use_temp_snapshot(self) -> bool:
         """Disable the use of a temporary snapshot on revert."""
         return False
 
+    def can_revert_different_size(self):
+        """Ceph supports reverting to a snapshot of different size."""
+        return True
+
     def revert_to_snapshot(self,
                            context: context.RequestContext,
                            volume: Volume,
@@ -1929,9 +1933,9 @@ def _is_cloneable(self, image_location: str, image_meta: dict) -> bool:
             LOG.debug('%s is in a different ceph cluster.', image_location)
             return False
 
-        if image_meta['disk_format'] != 'raw':
+        if image_meta['disk_format'] not in ('raw', 'iso'):
             LOG.debug("RBD image clone requires image format to be "
-                      "'raw' but image %(image)s is '%(format)s'",
+                      "'raw' or 'iso', but image %(image)s is '%(format)s'",
                       {"image": image_location,
                        "format": image_meta['disk_format']})
             return False

cinder/volume/manager.py

@@ -1081,6 +1081,9 @@ def _clear_db(self, volume_ref, status) -> None:
             volume_ref.status = status
             volume_ref.save()
 
+    def can_revert_different_size(self, context):
+        return self.driver.can_revert_different_size()
+
     def _revert_to_snapshot_generic(self,
                                     ctxt: context.RequestContext,
                                     volume,
@@ -1207,6 +1210,49 @@ def revert_to_snapshot(self, context, volume, snapshot) -> None:
                             "status to %(status)s." % msg_args)
                 LOG.exception(msg, msg_args)
 
+        # Adjust volume size if the snapshot has another size
+        if volume.size != snapshot.volume_size:
+            decrease = volume.size - snapshot.volume_size
+
+            # Get reservations
+            reservations = None
+            try:
+                reserve_opts = {
+                    'gigabytes': -decrease,
+                }
+                QUOTAS.add_volume_type_opts(context,
+                                            reserve_opts,
+                                            volume.volume_type_id)
+                reservations = QUOTAS.reserve(context,
+                                              project_id=volume.project_id,
+                                              **reserve_opts)
+            except Exception:
+                LOG.exception("Failed to update usages reverting volume.",
+                              resource=volume)
+
+            volume.update({'size': snapshot.volume_size})
+            volume.save()
+
+            # Commit the reservations
+            if reservations:
+                QUOTAS.commit(context, reservations,
+                              project_id=volume.project_id)
+
+            pool = volume_utils.extract_host(volume.host, 'pool')
+            if pool is None:
+                # Legacy volume, put them into default pool
+                pool = self.driver.configuration.safe_get(
+                    'volume_backend_name') or volume_utils.extract_host(
+                        volume.host, 'pool', True)
+
+            try:
+                self.stats['pools'][pool]['allocated_capacity_gb'] -= decrease
+            except KeyError:
+                self.stats['pools'][pool] = dict(
+                    allocated_capacity_gb=0)
+
+            self.publish_service_capabilities(context)
+
         v_res = volume.update_single_status_where(
             'available', 'reverting')
         if not v_res: