Merge "Separate create and update rules for volume metadata"

cinder/api/v2/volume_metadata.py

@@ -50,11 +50,9 @@ class Controller(wsgi.Controller):
         context = req.environ['cinder.context']
         metadata = body['metadata']
 
-        new_metadata = self._update_volume_metadata(context,
+        new_metadata = self._update_volume_metadata(context, volume_id,
-                                                    volume_id,
+                                                    metadata, delete=False,
-                                                    metadata,
+                                                    use_create=True)
-                                                    delete=False)
-
         return {'metadata': new_metadata}
 
     def update(self, req, volume_id, id, body):
@@ -89,17 +87,17 @@ class Controller(wsgi.Controller):
 
         return {'metadata': new_metadata}
 
-    def _update_volume_metadata(self, context,
+    def _update_volume_metadata(self, context, volume_id, metadata,
-                                volume_id, metadata,
+                                delete=False, use_create=False):
-                                delete=False):
         try:
             volume = self.volume_api.get(context, volume_id)
-            return self.volume_api.update_volume_metadata(
+            if use_create:
-                context,
+                return self.volume_api.create_volume_metadata(context, volume,
-                volume,
+                                                              metadata)
-                metadata,
+            else:
-                delete,
+                return self.volume_api.update_volume_metadata(
-                meta_type=common.METADATA_TYPES.user)
+                    context, volume, metadata, delete,
+                    meta_type=common.METADATA_TYPES.user)
         # Not found exception will be handled at the wsgi level
         except (ValueError, AttributeError):
             msg = _("Malformed request body")

cinder/tests/unit/policy.json

@@ -8,6 +8,7 @@
     "volume:get_all": "",
     "volume:get_volume_metadata": "",
     "volume:get_volume_image_metadata": "",
+    "volume:create_volume_metadata": "",
     "volume:delete_volume_metadata": "",
     "volume:update_volume_metadata": "",
     "volume:get_volume_admin_metadata": "rule:admin_api",

cinder/tests/unit/test_volume.py

@@ -318,6 +318,7 @@ class AvailabilityZoneTestCase(BaseVolumeTestCase):
         self.assertEqual(expected, azs)
 
 
+@ddt.ddt
 class VolumeTestCase(BaseVolumeTestCase):
 
     def setUp(self):
@@ -754,12 +755,34 @@ class VolumeTestCase(BaseVolumeTestCase):
                           self.context,
                           volume_id)
 
+    @mock.patch('cinder.db.volume_metadata_update')
+    def test_create_volume_metadata(self, metadata_update):
+        metadata = {'fake_key': 'fake_value'}
+        metadata_update.return_value = metadata
+        volume = tests_utils.create_volume(self.context, **self.volume_params)
+        res = self.volume_api.create_volume_metadata(self.context,
+                                                     volume, metadata)
+        metadata_update.assert_called_once_with(self.context, volume.id,
+                                                metadata, False,
+                                                common.METADATA_TYPES.user)
+        self.assertEqual(metadata, res)
+
+    @ddt.data('maintenance', 'uploading')
+    def test_create_volume_metadata_maintenance(self, status):
+        metadata = {'fake_key': 'fake_value'}
+        volume = tests_utils.create_volume(self.context, **self.volume_params)
+        volume['status'] = status
+        self.assertRaises(exception.InvalidVolume,
+                          self.volume_api.create_volume_metadata,
+                          self.context,
+                          volume,
+                          metadata)
+
     def test_create_volume_with_invalid_metadata(self):
         """Test volume create with too much metadata fails."""
-        volume_api = cinder.volume.api.API()
         test_meta = {'fake_key': 'fake_value' * 256}
         self.assertRaises(exception.InvalidVolumeMetadataSize,
-                          volume_api.create,
+                          self.volume_api.create,
                           self.context,
                           1,
                           'name',
@@ -777,11 +800,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = tests_utils.create_volume(self.context, metadata=test_meta1,
                                            **self.volume_params)
         self.volume.create_volume(self.context, volume.id, volume=volume)
-
-        volume_api = cinder.volume.api.API()
-
         # update user metadata associated with the volume.
-        result_meta = volume_api.update_volume_metadata(
+        result_meta = self.volume_api.update_volume_metadata(
             self.context,
             volume,
             test_meta2,
@@ -790,7 +810,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertEqual(test_meta2, result_meta)
 
         # create image metadata associated with the volume.
-        result_meta = volume_api.update_volume_metadata(
+        result_meta = self.volume_api.update_volume_metadata(
             self.context,
             volume,
             test_meta1,
@@ -799,7 +819,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertEqual(test_meta1, result_meta)
 
         # update image metadata associated with the volume.
-        result_meta = volume_api.update_volume_metadata(
+        result_meta = self.volume_api.update_volume_metadata(
             self.context,
             volume,
             test_meta2,
@@ -809,7 +829,7 @@ class VolumeTestCase(BaseVolumeTestCase):
 
         # update volume metadata with invalid metadta type.
         self.assertRaises(exception.InvalidMetadataType,
-                          volume_api.update_volume_metadata,
+                          self.volume_api.update_volume_metadata,
                           self.context,
                           volume,
                           test_meta1,
@@ -823,9 +843,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = tests_utils.create_volume(self.context, metadata=test_meta1,
                                            **self.volume_params)
         volume['status'] = 'maintenance'
-        volume_api = cinder.volume.api.API()
         self.assertRaises(exception.InvalidVolume,
-                          volume_api.update_volume_metadata,
+                          self.volume_api.update_volume_metadata,
                           self.context,
                           volume,
                           test_meta1,
@@ -836,9 +855,8 @@ class VolumeTestCase(BaseVolumeTestCase):
     def test_update_with_ovo(self, volume_update):
         """Test update volume using oslo_versionedobject."""
         volume = tests_utils.create_volume(self.context, **self.volume_params)
-        volume_api = cinder.volume.api.API()
         updates = {'display_name': 'foobbar'}
-        volume_api.update(self.context, volume, updates)
+        self.volume_api.update(self.context, volume, updates)
         volume_update.assert_called_once_with(self.context, volume.id,
                                               updates)
         self.assertEqual('foobbar', volume.display_name)
@@ -852,11 +870,8 @@ class VolumeTestCase(BaseVolumeTestCase):
                                            **self.volume_params)
         volume_id = volume['id']
         self.volume.create_volume(self.context, volume_id, volume=volume)
-
-        volume_api = cinder.volume.api.API()
-
         # delete user metadata associated with the volume.
-        volume_api.delete_volume_metadata(
+        self.volume_api.delete_volume_metadata(
             self.context,
             volume,
             'fake_key2',
@@ -866,7 +881,7 @@ class VolumeTestCase(BaseVolumeTestCase):
                          db.volume_metadata_get(self.context, volume_id))
 
         # create image metadata associated with the volume.
-        result_meta = volume_api.update_volume_metadata(
+        result_meta = self.volume_api.update_volume_metadata(
             self.context,
             volume,
             test_meta1,
@@ -876,7 +891,7 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertEqual(test_meta1, result_meta)
 
         # delete image metadata associated with the volume.
-        volume_api.delete_volume_metadata(
+        self.volume_api.delete_volume_metadata(
             self.context,
             volume,
             'fake_key2',
@@ -891,7 +906,7 @@ class VolumeTestCase(BaseVolumeTestCase):
 
         # delete volume metadata with invalid metadta type.
         self.assertRaises(exception.InvalidMetadataType,
-                          volume_api.delete_volume_metadata,
+                          self.volume_api.delete_volume_metadata,
                           self.context,
                           volume,
                           'fake_key1',
@@ -904,9 +919,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = tests_utils.create_volume(self.context, metadata=test_meta1,
                                            **self.volume_params)
         volume['status'] = 'maintenance'
-        volume_api = cinder.volume.api.API()
         self.assertRaises(exception.InvalidVolume,
-                          volume_api.delete_volume_metadata,
+                          self.volume_api.delete_volume_metadata,
                           self.context,
                           volume,
                           'fake_key1',
@@ -918,9 +932,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         volume = tests_utils.create_volume(self.context, metadata=test_meta1,
                                            **self.volume_params)
         volume['status'] = 'maintenance'
-        volume_api = cinder.volume.api.API()
         self.assertRaises(exception.InvalidVolume,
-                          volume_api.attach,
+                          self.volume_api.attach,
                           self.context,
                           volume, None, None, None, None)
 

cinder/volume/api.py

@@ -987,6 +987,15 @@ class API(base.Base):
                  resource=volume)
         return dict(rv)
 
+    @wrap_check_policy
+    def create_volume_metadata(self, context, volume, metadata):
+        """Creates volume metadata."""
+        db_meta = self._update_volume_metadata(context, volume, metadata)
+
+        LOG.info(_LI("Create volume metadata completed successfully."),
+                 resource=volume)
+        return db_meta
+
     @wrap_check_policy
     def delete_volume_metadata(self, context, volume,
                                key, meta_type=common.METADATA_TYPES.user):
@@ -1000,26 +1009,28 @@ class API(base.Base):
         LOG.info(_LI("Delete volume metadata completed successfully."),
                  resource=volume)
 
-    @wrap_check_policy
+    def _update_volume_metadata(self, context, volume, metadata, delete=False,
-    def update_volume_metadata(self, context, volume,
+                                meta_type=common.METADATA_TYPES.user):
-                               metadata, delete=False,
-                               meta_type=common.METADATA_TYPES.user):
-        """Updates or creates volume metadata.
-
-        If delete is True, metadata items that are not specified in the
-        `metadata` argument will be deleted.
-
-        """
         if volume['status'] in ('maintenance', 'uploading'):
             msg = _('Updating volume metadata is not allowed for volumes in '
                     '%s status.') % volume['status']
             LOG.info(msg, resource=volume)
             raise exception.InvalidVolume(reason=msg)
         utils.check_metadata_properties(metadata)
-        db_meta = self.db.volume_metadata_update(context, volume['id'],
+        return self.db.volume_metadata_update(context, volume['id'],
-                                                 metadata,
+                                              metadata, delete, meta_type)
-                                                 delete,
+
-                                                 meta_type)
+    @wrap_check_policy
+    def update_volume_metadata(self, context, volume, metadata, delete=False,
+                               meta_type=common.METADATA_TYPES.user):
+        """Updates volume metadata.
+
+        If delete is True, metadata items that are not specified in the
+        `metadata` argument will be deleted.
+
+        """
+        db_meta = self._update_volume_metadata(context, volume, metadata,
+                                               delete, meta_type)
 
         # TODO(jdg): Implement an RPC call for drivers that may use this info
 

etc/cinder/policy.json

@@ -10,6 +10,7 @@
     "volume:get": "rule:admin_or_owner",
     "volume:get_all": "rule:admin_or_owner",
     "volume:get_volume_metadata": "rule:admin_or_owner",
+    "volume:create_volume_metadata": "rule:admin_or_owner",
     "volume:delete_volume_metadata": "rule:admin_or_owner",
     "volume:update_volume_metadata": "rule:admin_or_owner",
     "volume:get_volume_admin_metadata": "rule:admin_api",

releasenotes/notes/create-update-rules-b46cf9c07c5a3966.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - Separate create and update rules for volume metadata.
+upgrade:
+  - If policy for update volume metadata is modified in a desired way
+    it's needed to add a desired rule for create volume metadata.