From: Joel Coffman <joel.coffman@jhuapl.edu>
Date: Thu, 26 Mar 2015 22:14:01 +0000 (-0400)
Subject: Add test case for volume_encryption_metadata_get
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a92170c3ea25367fa7ca27f048722ba32d823697;p=openstack-build%2Fcinder-build.git

Add test case for volume_encryption_metadata_get

This change adds unit tests for the volume_encryption_metadata_get
function. The unit tests provide protection against regressions when
refactoring this code as part of follow-up patches.

Writing the unit tests also exposed a minor issue with the existing
implementation of the volume_encryption_metadata_get function. If the
volume type is not encrypted, then the existing implementation would
raise an exception due to volume_type_encryption_get returning None.
In practice, this issue would not be encountered due to separate
checks to ensure that the volume type is encrypted, but a small
refactoring obviates the need for these checks and allows the
volume_encryption_metadata_get function to be invoked for both
encrypted and "normal" volumes.

A separate patch will clean up the unnecessary checks to ensure that
the volume type was encrypted prior to calling this function.

Related to blueprint volume-encryption-metadata

Change-Id: I6ba62a58ac62a3b7bfb2b9d4d18b37ef458b616c
---

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 90ce73bfe..a728dc501 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2968,14 +2968,16 @@ def volume_encryption_metadata_get(context, volume_id, session=None):
     encryption_ref = volume_type_encryption_get(context,
                                                 volume_ref['volume_type_id'])
 
-    return {
+    values = {
         'encryption_key_id': volume_ref['encryption_key_id'],
-        'control_location': encryption_ref['control_location'],
-        'cipher': encryption_ref['cipher'],
-        'key_size': encryption_ref['key_size'],
-        'provider': encryption_ref['provider'],
     }
 
+    if encryption_ref:
+        for key in ['control_location', 'cipher', 'key_size', 'provider']:
+            values[key] = encryption_ref[key]
+
+    return values
+
 
 ####################
 
diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py
index cb36a28ee..c7056dc27 100644
--- a/cinder/tests/unit/test_db_api.py
+++ b/cinder/tests/unit/test_db_api.py
@@ -1058,7 +1058,7 @@ class DBAPIVolumeTypeTestCase(BaseTest):
 
 class DBAPIEncryptionTestCase(BaseTest):
 
-    """Tests for the db.api.volume_type_encryption_* methods."""
+    """Tests for the db.api.volume_(type_)?encryption_* methods."""
 
     _ignored_keys = [
         'deleted',
@@ -1157,6 +1157,29 @@ class DBAPIEncryptionTestCase(BaseTest):
                                           encryption['volume_type_id'])
         self.assertIsNone(encryption_get)
 
+    def test_volume_encryption_get(self):
+        # normal volume -- metadata should be None
+        volume = db.volume_create(self.ctxt, {})
+        values = db.volume_encryption_metadata_get(self.ctxt, volume.id)
+
+        self.assertEqual({'encryption_key_id': None}, values)
+
+        # encrypted volume -- metadata should match volume type
+        volume_type = self.created[0]
+
+        volume = db.volume_create(self.ctxt, {'volume_type_id':
+                                              volume_type['volume_type_id']})
+        values = db.volume_encryption_metadata_get(self.ctxt, volume.id)
+
+        expected = {
+            'encryption_key_id': volume.encryption_key_id,
+            'control_location': volume_type['control_location'],
+            'cipher': volume_type['cipher'],
+            'key_size': volume_type['key_size'],
+            'provider': volume_type['provider'],
+        }
+        self.assertEqual(expected, values)
+
 
 class DBAPIReservationTestCase(BaseTest):