openstack
/
nova
Code Issues Proposed changes

VMware: use instance UUID as instance name 

The patch set remove the instance_name parameter that was passed
to spawn and other methods. This parameter was used for the naming
of the rescue and resize VM's. The fact that those are now done on
the original instance enables us to drop this 'instance_name'
parameter.

This simplifies the code and unit tests.

Change-Id: I7262d0b69fc08144b5675cfeafb5a40668a472d0
This commit is contained in:
Gary Kotton 2015-03-03 05:23:43 -08:00
parent 0476a9e60d
commit 86f385b955
5 changed files with 40 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/tests/unit/virt/vmwareapi/test_driver_api.py
View File

@ -707,11 +707,10 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
def _fake_spawn(context, instance, image_meta, injected_files,
admin_password, network_info, block_device_info=None,
instance_name=None, power_on=True):
power_on=True):
return self._spawn(context, instance, image_meta,
injected_files, admin_password, network_info,
block_device_info=block_device_info,
instance_name=instance_name,
power_on=self._power_on)
with (

9
nova/tests/unit/virt/vmwareapi/test_vm_util.py
View File

@ -466,7 +466,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
fake_factory = fake.FakeFactory()
result = vm_util.get_vm_create_spec(fake_factory,
self._instance,
self._instance.uuid,
'fake-datastore', [],
extra_specs)
@ -508,7 +507,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
fake_factory = fake.FakeFactory()
result = vm_util.get_vm_create_spec(fake_factory,
self._instance,
self._instance.uuid,
'fake-datastore', [],
extra_specs)
expected = fake_factory.create('ns0:VirtualMachineConfigSpec')
@ -555,7 +553,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
fake_factory = fake.FakeFactory()
result = vm_util.get_vm_create_spec(fake_factory,
self._instance,
self._instance.uuid,
'fake-datastore', [],
extra_specs)
expected = fake_factory.create('ns0:VirtualMachineConfigSpec')
@ -604,7 +601,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
fake_factory = fake.FakeFactory()
result = vm_util.get_vm_create_spec(fake_factory,
self._instance,
self._instance.uuid,
'fake-datastore', [],
extra_specs)
expected = fake_factory.create('ns0:VirtualMachineConfigSpec')
@ -655,7 +651,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
fake_factory = fake.FakeFactory()
result = vm_util.get_vm_create_spec(fake_factory,
self._instance,
self._instance.uuid,
'fake-datastore', [],
extra_specs)
expected = fake_factory.create('ns0:VirtualMachineConfigSpec')
@ -756,7 +751,7 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
config_spec = vm_util.get_vm_create_spec(
session.vim.client.factory,
self._instance, self._instance.uuid, 'fake-datastore', [],
self._instance, 'fake-datastore', [],
vm_util.ExtraSpecs(),
os_type='invalid_os_type')
@ -1096,7 +1091,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
extra_specs = vm_util.ExtraSpecs(hw_version='vmx-08')
result = vm_util.get_vm_create_spec(fake.FakeFactory(),
self._instance,
self._instance.uuid,
'fake-datastore', [],
extra_specs=extra_specs)
self.assertEqual('vmx-08', result.version)
@ -1106,7 +1100,6 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
extra_specs = vm_util.ExtraSpecs()
create_spec = vm_util.get_vm_create_spec(fake.FakeFactory(),
self._instance,
self._instance.uuid,
datastore.name, [],
extra_specs,
profile_spec='fake_profile_spec')

47
nova/tests/unit/virt/vmwareapi/test_vmops.py
View File

@ -854,9 +854,9 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
from_image.assert_called_once_with(self._instance.image_ref, {})
get_vm_config_info.assert_called_once_with(self._instance,
image_info, None, extra_specs.storage_policy)
image_info, extra_specs.storage_policy)
build_virtual_machine.assert_called_once_with(self._instance,
vi.instance_name, image_info, vi.dc_info, vi.datastore, [],
image_info, vi.dc_info, vi.datastore, [],
extra_specs)
enlist_image.assert_called_once_with(image_info.image_id,
vi.datastore, vi.dc_info.ref)
@ -921,9 +921,9 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
from_image.assert_called_once_with(instance.image_ref, {})
get_vm_config_info.assert_called_once_with(instance, image_info,
None, extra_specs.storage_policy)
extra_specs.storage_policy)
build_virtual_machine.assert_called_once_with(instance,
vi.instance_name, image_info, vi.dc_info, vi.datastore, [],
image_info, vi.dc_info, vi.datastore, [],
extra_specs)
volumeops.attach_root_volume.assert_called_once_with(
connection_info1, instance, vi.datastore.ref,
@ -979,10 +979,10 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
block_device_info=bdi)
from_image.assert_called_once_with(instance.image_ref, {})
get_vm_config_info.assert_called_once_with(instance, image_info, None,
get_vm_config_info.assert_called_once_with(instance, image_info,
extra_specs.storage_policy)
build_virtual_machine.assert_called_once_with(instance,
vi.instance_name, image_info, vi.dc_info, vi.datastore, [],
image_info, vi.dc_info, vi.datastore, [],
extra_specs)
def test_get_ds_browser(self):
@ -1018,7 +1018,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
mock_imagecache = mock.Mock()
mock_imagecache.get_image_cache_folder.return_value = cache_root_folder
vi = vmops.VirtualMachineInstanceConfigInfo(
self._instance, "fake_uuid", image_info,
self._instance, image_info,
self._ds, self._dc_info, mock_imagecache)
sized_cached_image_ds_loc = cache_root_folder.join(
@ -1068,7 +1068,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
mock_imagecache = mock.Mock()
mock_imagecache.get_image_cache_folder.return_value = cache_root_folder
vi = vmops.VirtualMachineInstanceConfigInfo(
self._instance, "fake_uuid", image_info,
self._instance, image_info,
self._ds, self._dc_info, mock_imagecache)
self._vmops._volumeops = mock.Mock()
@ -1112,7 +1112,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
mock_imagecache = mock.Mock()
mock_imagecache.get_image_cache_folder.return_value = cache_root_folder
vi = vmops.VirtualMachineInstanceConfigInfo(
self._instance, "fake_uuid", image_info,
self._instance, image_info,
self._ds, self._dc_info, mock_imagecache)
self._vmops._volumeops = mock.Mock()
@ -1253,7 +1253,6 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
mock_get_create_spec.assert_called_once_with(
self._session.vim.client.factory,
self._instance,
'fake_uuid',
'fake_ds',
[],
extra_specs,
@ -1340,8 +1339,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
def _test_get_spawn_vm_config_info(self,
mock_get_datacenter_ref_and_name,
mock_get_datastore,
image_size_bytes=0,
instance_name=None):
image_size_bytes=0):
image_info = images.VMwareImage(
image_id=self._image_id,
file_size=image_size_bytes,
@ -1350,16 +1348,12 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
mock_get_datastore.return_value = self._ds
mock_get_datacenter_ref_and_name.return_value = self._dc_info
vi = self._vmops._get_vm_config_info(
self._instance, image_info, instance_name=instance_name)
vi = self._vmops._get_vm_config_info(self._instance, image_info)
self.assertEqual(image_info, vi.ii)
self.assertEqual(self._ds, vi.datastore)
self.assertEqual(self._instance.root_gb, vi.root_gb)
self.assertEqual(self._instance, vi.instance)
if instance_name is not None:
self.assertEqual(instance_name, vi.instance_name)
else:
self.assertEqual(self._instance.uuid, vi.instance_name)
self.assertEqual(self._instance.uuid, vi.instance.uuid)
cache_image_path = '[%s] vmware_base/%s/%s.vmdk' % (
self._ds.name, self._image_id, self._image_id)
@ -1379,12 +1373,6 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
self._test_get_spawn_vm_config_info,
image_size_bytes=image_size)
def test_get_spawn_vm_config_info_with_instance_name(self):
image_size = (self._instance.root_gb) * units.Gi / 2
self._test_get_spawn_vm_config_info(
image_size_bytes=image_size,
instance_name="foo_instance_name")
def test_spawn(self):
self._test_spawn()
@ -1427,7 +1415,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
file_size=7,
linked_clone=False)
vi = vmops.VirtualMachineInstanceConfigInfo(
self._instance, 'fake_uuid', image_info,
self._instance, image_info,
self._ds, self._dc_info, mock.Mock())
return vi
@ -1502,7 +1490,6 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
extra_specs = vm_util.ExtraSpecs()
vm_ref = self._vmops.build_virtual_machine(self._instance,
'fake-instance-name',
image, self._dc_info,
self._ds,
self.network_info,
@ -1511,10 +1498,8 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
vm = vmwareapi_fake._get_object(vm_ref)
# Test basic VM parameters
self.assertEqual('fake-instance-name', vm.name)
# NOTE(mdbooth): The instanceUuid behaviour below is apparently
# deliberate.
self.assertEqual('fake-instance-name',
self.assertEqual(self._instance.uuid, vm.name)
self.assertEqual(self._instance.uuid,
vm.get('summary.config.instanceUuid'))
self.assertEqual(self._instance_values['vcpus'],
vm.get('summary.config.numCpu'))
@ -1640,7 +1625,7 @@ class VMwareVMOpsTestCase(test.NoDBTestCase):
mock_imagecache = mock.Mock()
mock_imagecache.get_image_cache_folder.return_value = cache_root_folder
vi = vmops.VirtualMachineInstanceConfigInfo(
self._instance, "fake_uuid", image_info,
self._instance, image_info,
self._ds, self._dc_info, mock_imagecache)
return vi

10
nova/virt/vmwareapi/vm_util.py
View File

@ -161,18 +161,16 @@ def _get_allocation_info(client_factory, extra_specs):
return allocation
def get_vm_create_spec(client_factory, instance, name, data_store_name,
def get_vm_create_spec(client_factory, instance, data_store_name,
vif_infos, extra_specs,
os_type=constants.DEFAULT_OS_TYPE,
profile_spec=None):
"""Builds the VM Create spec."""
config_spec = client_factory.create('ns0:VirtualMachineConfigSpec')
config_spec.name = name
config_spec.name = instance.uuid
config_spec.guestId = os_type
# The name is the unique identifier for the VM. This will either be the
# instance UUID or the instance UUID with suffix '-rescue' for VM's that
# are in rescue mode
config_spec.instanceUuid = name
# The name is the unique identifier for the VM.
config_spec.instanceUuid = instance.uuid
# set the Hardware version
config_spec.version = extra_specs.hw_version

51
nova/virt/vmwareapi/vmops.py
View File

@ -89,18 +89,13 @@ DcInfo = collections.namedtuple('DcInfo',
class VirtualMachineInstanceConfigInfo(object):
"""Parameters needed to create and configure a new instance."""
def __init__(self, instance, instance_name, image_info,
datastore, dc_info, image_cache):
def __init__(self, instance, image_info, datastore, dc_info, image_cache):
# Some methods called during spawn take the instance parameter purely
# for logging purposes.
# TODO(vui) Clean them up, so we no longer need to keep this variable
self.instance = instance
# Get the instance name. In some cases this may differ from the 'uuid',
# for example when the spawn of a rescue instance takes place.
self.instance_name = instance_name or instance.uuid
self.ii = image_info
self.root_gb = instance.root_gb
self.datastore = datastore
@ -244,8 +239,8 @@ class VMwareVMOps(object):
datastore.ref,
str(uploaded_iso_path))
def build_virtual_machine(self, instance, instance_name, image_info,
dc_info, datastore, network_info, extra_specs):
def build_virtual_machine(self, instance, image_info, dc_info, datastore,
network_info, extra_specs):
vif_infos = vmwarevif.get_vif_info(self._session,
self._cluster,
utils.is_neutron(),
@ -261,7 +256,6 @@ class VMwareVMOps(object):
client_factory = self._session.vim.client.factory
config_spec = vm_util.get_vm_create_spec(client_factory,
instance,
instance_name,
datastore.name,
vif_infos,
extra_specs,
@ -442,7 +436,7 @@ class VMwareVMOps(object):
tmp_image_ds_loc.parent,
vi.cache_image_folder)
def _get_vm_config_info(self, instance, image_info, instance_name=None,
def _get_vm_config_info(self, instance, image_info,
storage_policy=None):
"""Captures all relevant information from the spawn parameters."""
@ -461,7 +455,6 @@ class VMwareVMOps(object):
dc_info = self.get_datacenter_ref_and_name(datastore.ref)
return VirtualMachineInstanceConfigInfo(instance,
instance_name,
image_info,
datastore,
dc_info,
@ -556,20 +549,19 @@ class VMwareVMOps(object):
def spawn(self, context, instance, image_meta, injected_files,
admin_password, network_info, block_device_info=None,
instance_name=None, power_on=True):
power_on=True):
client_factory = self._session.vim.client.factory
image_info = images.VMwareImage.from_image(instance.image_ref,
image_meta)
extra_specs = self._get_extra_specs(instance.flavor)
vi = self._get_vm_config_info(instance, image_info, instance_name,
vi = self._get_vm_config_info(instance, image_info,
extra_specs.storage_policy)
# Creates the virtual machine. The virtual machine reference returned
# is unique within Virtual Center.
vm_ref = self.build_virtual_machine(instance,
vi.instance_name,
image_info,
vi.dc_info,
vi.datastore,
@ -577,8 +569,8 @@ class VMwareVMOps(object):
extra_specs)
# Cache the vm_ref. This saves a remote call to the VC. This uses the
# instance_name. This covers all use cases including rescue and resize.
vm_util.vm_ref_cache_update(vi.instance_name, vm_ref)
# instance uuid.
vm_util.vm_ref_cache_update(instance.uuid, vm_ref)
# Set the machine.id parameter of the instance to inject
# the NIC configuration inside the VM
@ -876,19 +868,10 @@ class VMwareVMOps(object):
self._session._wait_for_task(reset_task)
LOG.debug("Did hard reboot of VM", instance=instance)
def _destroy_instance(self, instance, destroy_disks=True,
instance_name=None):
def _destroy_instance(self, instance, destroy_disks=True):
# Destroy a VM instance
# Get the instance name. In some cases this may differ from the 'uuid',
# for example when the spawn of a rescue instance takes place.
if instance_name is None:
instance_name = instance.uuid
try:
vm_ref = vm_util.get_vm_ref_from_name(self._session, instance_name)
if vm_ref is None:
LOG.warning(_LW('Instance does not exist on backend'),
instance=instance)
return
vm_ref = vm_util.get_vm_ref(self._session, instance)
lst_properties = ["config.files.vmPathName", "runtime.powerState",
"datastore"]
props = self._session._call_method(vim_util,
@ -941,11 +924,14 @@ class VMwareVMOps(object):
LOG.warning(_LW("In vmwareapi:vmops:_destroy_instance, "
"exception while deleting the VM contents "
"from the disk"), exc_info=True)
except exception.InstanceNotFound:
LOG.warning(_LW('Instance does not exist on backend'),
instance=instance)
except Exception:
LOG.exception(_LE('Destroy instance failed'),
instance=instance)
finally:
vm_util.vm_ref_cache_delete(instance_name)
vm_util.vm_ref_cache_delete(instance.uuid)
def destroy(self, instance, destroy_disks=True):
"""Destroy a VM instance.
@ -1034,7 +1020,6 @@ class VMwareVMOps(object):
image_info = images.VMwareImage.from_image(instance.image_ref,
image_meta)
vi = VirtualMachineInstanceConfigInfo(instance,
None,
image_info,
datastore,
dc_info,
@ -1636,8 +1621,8 @@ class VMwareVMOps(object):
def _use_disk_image_as_full_clone(self, vm_ref, vi):
"""Uses cached image disk by copying it into the VM directory."""
instance_folder = vi.instance_name
root_disk_name = "%s.vmdk" % vi.instance_name
instance_folder = vi.instance.uuid
root_disk_name = "%s.vmdk" % vi.instance.uuid
root_disk_ds_loc = vi.datastore.build_path(instance_folder,
root_disk_name)
@ -1737,8 +1722,8 @@ class VMwareVMOps(object):
# Optionally create and attach blank disk
if vi.root_gb > 0:
instance_folder = vi.instance_name
root_disk_name = "%s.vmdk" % vi.instance_name
instance_folder = vi.instance.uuid
root_disk_name = "%s.vmdk" % vi.instance.uuid
root_disk_ds_loc = vi.datastore.build_path(instance_folder,
root_disk_name)