Fix collection of metadata for a snapshot of a volume-backed instance

Currently the snapshot derives its properties from the instance source:
an other snapshot or a bootable volume. But that sources could be
changed since the instance was booted.

To make instance snapshots independent of source changes this patch
collects metadata from instance system metadata rather than the sources.

Since it brings the only way to initialize image metadata, it fixes:
a) min_ram attribute is not inherited from a bootable volume
(LP #1369854).
b) empty container_format and disk_format attribute are inherited from a
source instance snapshot (LP #1439819).

Closes-Bug: #1369854
Closes-Bug: #1439819
Related-Bug: #1469179
Change-Id: I067f66356a5ebd738add1591a0069d8049f35c24
This commit is contained in:
Feodor Tersin 2015-07-06 21:42:14 +03:00
parent 17b1aeaeae
commit 8cf2d41344
4 changed files with 73 additions and 75 deletions

View File

@ -2292,20 +2292,20 @@ class API(base.Base):
:returns: the new image metadata
"""
# TODO(ft): Get metadata from the instance to avoid waste DB request
img = instance.image_ref
if not img:
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
context, instance.uuid)
properties = bdms.root_metadata(
context, self.image_api,
self.volume_api)
image_meta = {'properties': properties}
else:
image_meta = self.image_api.get(context, img)
image_meta['name'] = name
image_meta['is_public'] = False
image_meta = utils.get_image_from_system_metadata(
instance.system_metadata)
# the new image is simply a bucket of properties (particularly the
# block device mapping, kernel and ramdisk IDs) with no image data,
# hence the zero size
image_meta.update({'size': 0,
'name': name,
'is_public': False})
for attr in ('container_format', 'disk_format'):
image_meta.pop(attr, None)
properties = image_meta['properties']
# clean properties before filling
for key in ('block_device_mapping', 'bdm_v2', 'root_device_name'):
properties.pop(key, None)
if instance.root_device_name:
properties['root_device_name'] = instance.root_device_name
properties.update(extra_properties or {})
@ -2354,25 +2354,10 @@ class API(base.Base):
if quiesced:
self.compute_rpcapi.unquiesce_instance(context, instance, mapping)
# NOTE (ndipanov): Remove swap/ephemerals from mappings as they will be
# in the block_device_mapping for the new image.
image_mappings = properties.get('mappings')
if image_mappings:
properties['mappings'] = [m for m in image_mappings
if not block_device.is_swap_or_ephemeral(
m['virtual'])]
if mapping:
properties['block_device_mapping'] = mapping
properties['bdm_v2'] = True
for attr in ('status', 'location', 'id', 'owner'):
image_meta.pop(attr, None)
# the new image is simply a bucket of properties (particularly the
# block device mapping, kernel and ramdisk IDs) with no image data,
# hence the zero size
image_meta['size'] = 0
return self.image_api.create(context, image_meta)
@wrap_check_policy

View File

@ -940,16 +940,6 @@ class ServerActionsControllerTestV21(test.TestCase):
volume_size=1,
device_name='vda',
delete_on_termination=False)]
props = dict(kernel_id=_fake_id('b'),
ramdisk_id=_fake_id('c'),
root_device_name='/dev/vda',
block_device_mapping=bdm)
original_image = dict(properties=props,
container_format='ami',
status='active',
is_public=True)
image_service.create(None, original_image)
def fake_block_device_mapping_get_all_by_instance(context, inst_id,
use_slave=False):
@ -967,9 +957,15 @@ class ServerActionsControllerTestV21(test.TestCase):
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_block_device_mapping_get_all_by_instance)
instance = fakes.fake_instance_get(image_ref=original_image['id'],
system_metadata = dict(image_kernel_id=_fake_id('b'),
image_ramdisk_id=_fake_id('c'),
image_root_device_name='/dev/vda',
image_block_device_mapping=str(bdm),
image_container_format='ami')
instance = fakes.fake_instance_get(image_ref=str(uuid.uuid4()),
vm_state=vm_states.ACTIVE,
root_device_name='/dev/vda')
root_device_name='/dev/vda',
system_metadata=system_metadata)
self.stubs.Set(db, 'instance_get_by_uuid', instance)
self.mox.StubOutWithMock(self.controller.compute_api.compute_rpcapi,
@ -1053,9 +1049,12 @@ class ServerActionsControllerTestV21(test.TestCase):
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_block_device_mapping_get_all_by_instance)
instance = fakes.fake_instance_get(image_ref='',
vm_state=vm_states.ACTIVE,
root_device_name='/dev/vda')
instance = fakes.fake_instance_get(
image_ref='',
vm_state=vm_states.ACTIVE,
root_device_name='/dev/vda',
system_metadata={'image_test_key1': 'test_value1',
'image_test_key2': 'test_value2'})
self.stubs.Set(db, 'instance_get_by_uuid', instance)
self.mox.StubOutWithMock(self.controller.compute_api.compute_rpcapi,
@ -1065,18 +1064,14 @@ class ServerActionsControllerTestV21(test.TestCase):
exception.InstanceQuiesceNotSupported(instance_id='fake',
reason='test'))
fake_metadata = {'test_key1': 'test_value1',
'test_key2': 'test_value2'}
volume = dict(id=_fake_id('a'),
size=1,
host='fake',
display_description='fake',
volume_image_metadata=fake_metadata)
display_description='fake')
snapshot = dict(id=_fake_id('d'))
self.mox.StubOutWithMock(self.controller.compute_api, 'volume_api')
volume_api = self.controller.compute_api.volume_api
volume_api.get(mox.IgnoreArg(), volume['id']).AndReturn(volume)
volume_api.get(mox.IgnoreArg(), volume['id']).AndReturn(volume)
volume_api.create_snapshot_force(mox.IgnoreArg(), volume['id'],
mox.IgnoreArg(), mox.IgnoreArg()).AndReturn(snapshot)

View File

@ -469,7 +469,7 @@ def stub_instance(id=1, user_id=None, project_id=None, host=None,
availability_zone='', locked_by=None, cleaned=False,
memory_mb=0, vcpus=0, root_gb=0, ephemeral_gb=0,
instance_type=None, launch_index=0, kernel_id="",
ramdisk_id="", user_data=None):
ramdisk_id="", user_data=None, system_metadata=None):
if user_id is None:
user_id = 'fake_user'
if project_id is None:
@ -484,6 +484,7 @@ def stub_instance(id=1, user_id=None, project_id=None, host=None,
inst_type = flavors.get_flavor_by_flavor_id(int(flavor_id))
sys_meta = flavors.save_flavor_info({}, inst_type)
sys_meta.update(system_metadata or {})
if host is not None:
host = str(host)

View File

@ -20,6 +20,7 @@ import datetime
import iso8601
import mock
from mox3 import mox
from oslo_serialization import jsonutils
from oslo_utils import timeutils
from oslo_utils import uuidutils
@ -1978,41 +1979,41 @@ class _ComputeAPIUnitTestMixIn(object):
def _test_snapshot_volume_backed(self, quiesce_required, quiesce_fails,
vm_state=vm_states.ACTIVE):
params = dict(locked=True, vm_state=vm_state)
params = dict(locked=True, vm_state=vm_state,
system_metadata={'image_min_ram': '11',
'image_min_disk': '22',
'image_container_format': 'ami',
'image_disk_format': 'ami',
'image_ram_disk': 'fake_ram_disk_id',
'image_bdm_v2': 'True',
'image_block_device_mapping': '[]',
'image_mappings': '[]',
})
instance = self._create_instance_obj(params=params)
instance['root_device_name'] = 'vda'
instance_bdms = []
image_meta = {
'id': 'fake-image-id',
'properties': {'mappings': []},
'status': 'fake-status',
'location': 'far-away',
'owner': 'fake-tenant',
}
expect_meta = {
'name': 'test-snapshot',
'properties': {'root_device_name': 'vda',
'mappings': 'DONTCARE'},
'ram_disk': 'fake_ram_disk_id'},
'size': 0,
'is_public': False
'min_disk': '22',
'is_public': False,
'min_ram': '11',
}
quiesced = [False, False]
quiesce_expected = not quiesce_fails and vm_state == vm_states.ACTIVE
if quiesce_required:
image_meta['properties']['os_require_quiesce'] = 'yes'
instance.system_metadata['image_os_require_quiesce'] = 'yes'
expect_meta['properties']['os_require_quiesce'] = 'yes'
def fake_get_all_by_instance(context, instance, use_slave=False):
return copy.deepcopy(instance_bdms)
def fake_image_get(context, image_id):
return copy.deepcopy(image_meta)
def fake_image_create(context, image_meta, data=None):
self.assertThat(image_meta, matchers.DictMatches(expect_meta))
@ -2033,8 +2034,6 @@ class _ComputeAPIUnitTestMixIn(object):
self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
fake_get_all_by_instance)
self.stubs.Set(self.compute_api.image_api, 'get',
fake_image_get)
self.stubs.Set(self.compute_api.image_api, 'create',
fake_image_create)
self.stubs.Set(self.compute_api.volume_api, 'get',
@ -2073,19 +2072,37 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertEqual(quiesce_expected, quiesced[0])
self.assertEqual(quiesce_expected, quiesced[1])
image_mappings = [{'virtual': 'ami', 'device': 'vda'},
{'device': 'vda', 'virtual': 'ephemeral0'},
{'device': 'vdb', 'virtual': 'swap'},
{'device': 'vdc', 'virtual': 'ephemeral1'}]
instance.system_metadata['image_mappings'] = jsonutils.dumps(
[{'virtual': 'ami', 'device': 'vda'},
{'device': 'vda', 'virtual': 'ephemeral0'},
{'device': 'vdb', 'virtual': 'swap'},
{'device': 'vdc', 'virtual': 'ephemeral1'}])[:255]
instance.system_metadata['image_block_device_mapping'] = (
jsonutils.dumps(
[{'source_type': 'snapshot', 'destination_type': 'volume',
'guest_format': None, 'device_type': 'disk', 'boot_index': 1,
'disk_bus': 'ide', 'device_name': '/dev/vdf',
'delete_on_termination': True, 'snapshot_id': 'snapshot-2',
'volume_id': None, 'volume_size': 100, 'image_id': None,
'no_device': None}])[:255])
image_meta['properties']['mappings'] = image_mappings
expect_meta['properties']['mappings'] = [
{'virtual': 'ami', 'device': 'vda'}]
bdm = fake_block_device.FakeDbBlockDeviceDict(
{'no_device': False, 'volume_id': None, 'boot_index': -1,
'connection_info': 'inf', 'device_name': '/dev/vdh',
'source_type': 'blank', 'destination_type': 'local',
'guest_format': 'swap', 'delete_on_termination': True})
instance_bdms.append(bdm)
expect_meta['properties']['block_device_mapping'].append(
{'guest_format': 'swap', 'boot_index': -1, 'no_device': False,
'image_id': None, 'volume_id': None, 'disk_bus': None,
'volume_size': None, 'source_type': 'blank',
'device_type': None, 'snapshot_id': None,
'device_name': '/dev/vdh',
'destination_type': 'local', 'delete_on_termination': True})
quiesced = [False, False]
# Check that the mappgins from the image properties are included
# Check that the mappgins from the image properties are not included
self.compute_api.snapshot_volume_backed(
self.context, instance, 'test-snapshot')