openstack/cinder
Code Issues Proposed changes

Fix unnecessary migration on retype

Browse Source 
Retyping a volume with different QoS entries will require a migration,
even if both QoS entries have the exact same data.

This is caused by an incorrect comparison of the QoS information where
we are comparing unnecessary fields: created_at, updated_at, and
deleted_at.

The reason why the unit tests were not picking this up is because the
test was using 'created_at' as one of the spec keys (something artificial
that would not happen in the real world) and it was replacing the value
from the QoS entity.

This patch removes those 3 fields from the QoS comparison, avoiding
unnecessary migrations on retyping.

Close-Bug: #1901188
Change-Id: I979d4db90ee40b4c0a37cd198d7dfcfd7f8d75f7
This commit is contained in:
Gorka Eguileor 2020-10-29 12:28:46 +01:00
parent 0f2eff2ec3
commit 7feafe808d
3 changed files with 15 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
cinder/tests/unit/test_volume_types.py
View File

@ -469,7 +469,7 @@ class VolumeTypeTestCase(test.TestCase):
self.assertEqual(('val1', 'val0'), diff['extra_specs']['key1'])
# qos_ref 1 and 2 have the same specs, while 3 has different
qos_keyvals1 = {'k1': 'v1', 'k2': 'v2', 'k3': 'v3', 'created_at': 'v4'}
qos_keyvals1 = {'k1': 'v1', 'k2': 'v2', 'k3': 'v3', 'k4': 'v4'}
qos_keyvals2 = {'k1': 'v0', 'k2': 'v2', 'k3': 'v3'}
qos_ref1 = qos_specs.create(self.ctxt, 'qos-specs-1', qos_keyvals1)
qos_ref2 = qos_specs.create(self.ctxt, 'qos-specs-2', qos_keyvals1)
@ -483,6 +483,8 @@ class VolumeTypeTestCase(test.TestCase):
diff, same = volume_types.volume_types_diff(self.ctxt, type_ref1['id'],
type_ref2['id'])
self.assertTrue(same)
for k in ('id', 'name', 'created_at', 'updated_at', 'deleted_at'):
self.assertNotIn(k, diff['qos_specs'])
self.assertEqual(('val1', 'val1'), diff['extra_specs']['key1'])
self.assertEqual(('v1', 'v1'), diff['qos_specs']['k1'])
qos_specs.disassociate_qos_specs(self.ctxt, qos_ref2['id'],
@ -525,7 +527,7 @@ class VolumeTypeTestCase(test.TestCase):
'k1': (None, 'v1'),
'k2': (None, 'v2'),
'k3': (None, 'v3'),
'created_at': (None, 'v4')}, diff['qos_specs'])
'k4': (None, 'v4')}, diff['qos_specs'])
self.assertEqual({'cipher': (None, 'c1'),
'control_location': (None, 'front-end'),
'deleted': (None, False),

9
cinder/volume/volume_types.py
View File

@ -36,8 +36,9 @@ from cinder import utils
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
QUOTAS = quota.QUOTAS
ENCRYPTION_IGNORED_FIELDS = ['volume_type_id', 'created_at', 'updated_at',
'deleted_at', 'encryption_id']
ENCRYPTION_IGNORED_FIELDS = ('volume_type_id', 'created_at', 'updated_at',
'deleted_at', 'encryption_id')
QOS_IGNORED_FIELDS = ('id', 'name', 'created_at', 'updated_at', 'deleted_at')
DEFAULT_VOLUME_TYPE = "__DEFAULT__"
MIN_SIZE_KEY = "provisioning:min_vol_size"
@ -360,8 +361,8 @@ def volume_types_diff(context, vol_type_id1, vol_type_id2):
"""
def _fix_qos_specs(qos_specs):
if qos_specs:
qos_specs.pop('id', None)
qos_specs.pop('name', None)
for key in QOS_IGNORED_FIELDS:
qos_specs.pop(key, None)
qos_specs.update(qos_specs.pop('specs', {}))
def _fix_encryption_specs(encryption):

6
releasenotes/notes/fix-unnecessary-migration-on-retype-67cedb1bd8e4c4b2.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
`Bug #1901188 <https://bugs.launchpad.net/cinder/+bug/1901188>`_: Fix
unnecessary migration on retype when QoS has the same elements in both
types.