Make volume Glance metadata bulk update idempotent

The cinder.db.sqlalchemy.api.volume_glance_metadata_bulk_create function
has the following in its docstring:

    This API does not support changing the value of a key once it has
    been created.

However, it actually fails if a specified key exists and has the same
value. This can cause problems if volume creation is retried following a
failure, since the metadata may have already been created.

This change modifies the function to ignore keys that exist and have the
expected value, as per the docstring.

Related-Bug: #1823445

Change-Id: Ibc3f7b995c43bc281b6978bfcba4d1a50db41fa3

cinder/db/sqlalchemy/api.py

@@ -6191,6 +6191,9 @@ def volume_glance_metadata_create(context, volume_id, key, value):
     )
 
     if len(rows) > 0:
+        vol_glance_metadata = rows[0]
+        if vol_glance_metadata.value == str(value):
+            return
         raise exception.GlanceMetadataExists(key=key, volume_id=volume_id)
 
     vol_glance_metadata = models.VolumeGlanceMetadata()
@@ -6221,6 +6224,9 @@ def volume_glance_metadata_bulk_create(context, volume_id, metadata):
         )
 
         if len(rows) > 0:
+            vol_glance_metadata = rows[0]
+            if vol_glance_metadata.value == str(value):
+                continue
             raise exception.GlanceMetadataExists(key=key, volume_id=volume_id)
 
         vol_glance_metadata = models.VolumeGlanceMetadata()

cinder/tests/unit/test_db_api.py

@@ -1660,6 +1660,67 @@ class DBAPIVolumeTestCase(BaseTest):
                 image_name = meta_entry.value
         self.assertEqual(u'\xe4\xbd\xa0\xe5\xa5\xbd', image_name)
 
+    def test_volume_glance_metadata_create_idempotency(self):
+        volume = db.volume_create(self.ctxt,
+                                  {'host': 'h1',
+                                   'volume_type_id': fake.VOLUME_TYPE_ID})
+        db.volume_glance_metadata_create(self.ctxt, volume['id'],
+                                         'image_name',
+                                         u'\xe4\xbd\xa0\xe5\xa5\xbd')
+        db.volume_glance_metadata_create(self.ctxt, volume['id'],
+                                         'image_name',
+                                         u'\xe4\xbd\xa0\xe5\xa5\xbd')
+        glance_meta = db.volume_glance_metadata_get(self.ctxt, volume['id'])
+        self.assertEqual(1, len(glance_meta))
+
+    def test_volume_glance_metadata_create_immutability(self):
+        volume = db.volume_create(self.ctxt,
+                                  {'host': 'h1',
+                                   'volume_type_id': fake.VOLUME_TYPE_ID})
+        db.volume_glance_metadata_create(self.ctxt, volume['id'],
+                                         'image_name',
+                                         u'\xe4\xbd\xa0\xe5\xa5\xbd')
+        self.assertRaises(exception.GlanceMetadataExists,
+                          db.volume_glance_metadata_create,
+                          self.ctxt, volume['id'], 'image_name', 'new_meta')
+
+    def test_volume_glance_metadata_bulk_create(self):
+        volume = db.volume_create(self.ctxt,
+                                  {'host': 'h1',
+                                   'volume_type_id': fake.VOLUME_TYPE_ID})
+        metadata = {'foo': 'bar', 'baz': 'qux'}
+        db.volume_glance_metadata_bulk_create(self.ctxt, volume['id'],
+                                              metadata)
+        glance_meta = db.volume_glance_metadata_get(self.ctxt, volume['id'])
+        glance_meta = {m.key: m.value for m in glance_meta}
+        self.assertEqual(metadata, glance_meta)
+
+    def test_volume_glance_metadata_bulk_create_idempotency(self):
+        volume = db.volume_create(self.ctxt,
+                                  {'host': 'h1',
+                                   'volume_type_id': fake.VOLUME_TYPE_ID})
+        metadata = {'foo': 'bar', 'baz': 'qux'}
+        db.volume_glance_metadata_bulk_create(self.ctxt, volume['id'],
+                                              metadata)
+        db.volume_glance_metadata_bulk_create(self.ctxt, volume['id'],
+                                              metadata)
+        glance_meta = db.volume_glance_metadata_get(self.ctxt, volume['id'])
+        glance_meta = {m.key: m.value for m in glance_meta}
+        self.assertEqual(metadata, glance_meta)
+        self.assertEqual(2, len(glance_meta))
+
+    def test_volume_glance_metadata_bulk_create_immutability(self):
+        volume = db.volume_create(self.ctxt,
+                                  {'host': 'h1',
+                                   'volume_type_id': fake.VOLUME_TYPE_ID})
+        metadata = {'foo': 'bar', 'baz': 'qux'}
+        db.volume_glance_metadata_bulk_create(self.ctxt, volume['id'],
+                                              metadata)
+        metadata['foo'] = 'new_meta'
+        self.assertRaises(exception.GlanceMetadataExists,
+                          db.volume_glance_metadata_bulk_create,
+                          self.ctxt, volume['id'], metadata)
+
     def test_volume_glance_metadata_list_get(self):
         """Test volume_glance_metadata_list_get in DB API."""
         db.volume_create(self.ctxt, {'id': 'fake1', 'status': 'available',

releasenotes/notes/idempotent-glance-metadata-aa78e09736cf57d9.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    `Bug 1823445 <https://bugs.launchpad.net/cinder/+bug/1823445>`_: Fix
+    an issue with bulk updates of volume Glance metadata when keys exist
+    but are unchanged.