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:
parent
17b1aeaeae
commit
8cf2d41344
|
@ -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
|
||||
|
|
|
@ -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)
|
||||
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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')
|
||||
|
||||
|
|
Loading…
Reference in New Issue