VMware: Fixes the instance resize problem

The fix includes separating out methods for
associating/disassociating a vsphere vm from the
openstack instance. Modifying the resize workflow
to use the above mentioned methods.

Closes-Bug: #1295381

Change-Id: I92acdd5cd00f739d504738413d3b63a2e17f2866

nova/tests/virt/vmwareapi/test_driver_api.py

@@ -937,6 +937,22 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
         vm_ref = vm_util.get_vm_ref(self.conn._session, self.instance)
         self.assertIsNotNone(vm_ref, 'VM Reference cannot be none')
 
+    def test_search_vm_ref_by_identifier(self):
+        self._create_vm()
+        vm_ref = vm_util.search_vm_ref_by_identifier(self.conn._session,
+                                            self.instance['uuid'])
+        self.assertIsNotNone(vm_ref, 'VM Reference cannot be none')
+        fake_vm = vmwareapi_fake._get_objects("VirtualMachine").objects[0]
+        fake_vm.set("summary.config.instanceUuid", "foo")
+        fake_vm.set("name", "foo")
+        fake_vm.get('config.extraConfig["nvp.vm-uuid"]').value = "foo"
+        self.assertIsNone(vm_util.search_vm_ref_by_identifier(
+                                    self.conn._session, self.instance['uuid']),
+                          "VM Reference should be none")
+        self.assertIsNotNone(
+                vm_util.search_vm_ref_by_identifier(self.conn._session, "foo"),
+                "VM Reference should not be none")
+
     def test_get_object_for_optionvalue(self):
         self._create_vm()
         vms = self.conn._session._call_method(vim_util, "get_objects",
@@ -2069,6 +2085,7 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase):
             return None
 
         self._create_vm()
+        vm_ref_orig = vm_util.get_vm_ref(self.conn._session, self.instance)
         flavor = {'name': 'fake', 'flavorid': 'fake_id'}
         self.stubs.Set(self.conn._vmops, "_update_instance_progress",
                        fake_update_instance_progress)
@@ -2077,6 +2094,64 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase):
         self.conn.migrate_disk_and_power_off(self.context, self.instance,
                                              'fake_dest', flavor,
                                              None)
+        vm_ref = vm_util.get_vm_ref(self.conn._session, self.instance)
+        self.assertNotEqual(vm_ref_orig.value, vm_ref.value,
+                             "These should be different")
+
+    def test_disassociate_vmref_from_instance(self):
+        self._create_vm()
+        vm_ref = vm_util.get_vm_ref(self.conn._session, self.instance)
+        vm_util.disassociate_vmref_from_instance(self.conn._session,
+                                        self.instance, vm_ref, "-backup")
+        self.assertRaises(exception.InstanceNotFound,
+                    vm_util.get_vm_ref, self.conn._session, self.instance)
+
+    def test_clone_vmref_for_instance(self):
+        self._create_vm()
+        vm_ref = vm_util.get_vm_ref(self.conn._session, self.instance)
+        vm_util.disassociate_vmref_from_instance(self.conn._session,
+                                            self.instance, vm_ref, "-backup")
+        host_ref = vmwareapi_fake._get_object_refs("HostSystem")[0]
+        ds_ref = vmwareapi_fake._get_object_refs("Datastore")[0]
+        dc_obj = vmwareapi_fake._get_objects("Datacenter").objects[0]
+        vm_util.clone_vmref_for_instance(self.conn._session, self.instance,
+                                         vm_ref, host_ref, ds_ref,
+                                         dc_obj.get("vmFolder"))
+        self.assertIsNotNone(
+                        vm_util.get_vm_ref(self.conn._session, self.instance),
+                        "No VM found")
+        cloned_vm_ref = vm_util.get_vm_ref(self.conn._session, self.instance)
+        self.assertNotEqual(vm_ref.value, cloned_vm_ref.value,
+                            "Reference for the cloned VM should be different")
+        vm_obj = vmwareapi_fake._get_vm_mdo(vm_ref)
+        cloned_vm_obj = vmwareapi_fake._get_vm_mdo(cloned_vm_ref)
+        self.assertEqual(vm_obj.name, self.instance['uuid'] + "-backup",
+                       "Original VM name should be with suffix -backup")
+        self.assertEqual(cloned_vm_obj.name, self.instance['uuid'],
+                       "VM name does not match instance['uuid']")
+        self.assertRaises(error_util.MissingParameter,
+                          vm_util.clone_vmref_for_instance, self.conn._session,
+                          self.instance, None, host_ref, ds_ref,
+                          dc_obj.get("vmFolder"))
+
+    def test_associate_vmref_for_instance(self):
+        self._create_vm()
+        vm_ref = vm_util.get_vm_ref(self.conn._session, self.instance)
+        # First disassociate the VM from the instance so that we have a VM
+        # to later associate using the associate_vmref_for_instance method
+        vm_util.disassociate_vmref_from_instance(self.conn._session,
+                                            self.instance, vm_ref, "-backup")
+        # Ensure that the VM is indeed disassociated and that we cannot find
+        # the VM using the get_vm_ref method
+        self.assertRaises(exception.InstanceNotFound,
+                    vm_util.get_vm_ref, self.conn._session, self.instance)
+        # Associate the VM back to the instance
+        vm_util.associate_vmref_for_instance(self.conn._session, self.instance,
+                                             suffix="-backup")
+        # Verify if we can get the VM reference
+        self.assertIsNotNone(
+                        vm_util.get_vm_ref(self.conn._session, self.instance),
+                        "No VM found")
 
     def test_confirm_migration(self):
         self._create_vm()

nova/virt/vmwareapi/error_util.py

@@ -137,6 +137,10 @@ class UseLinkedCloneConfigurationFault(VMwareDriverConfigurationException):
     msg_fmt = _("No default value for use_linked_clone found.")
 
 
+class MissingParameter(VMwareDriverException):
+    msg_fmt = _("Missing parameter : %(param)s")
+
+
 class AlreadyExistsException(VMwareDriverException):
     msg_fmt = _("Resource already exists.")
     code = 409

nova/virt/vmwareapi/fake.py

@@ -441,6 +441,21 @@ class VirtualMachine(ManagedObject):
         """Called to reconfigure the VM. Actually customizes the property
         setting of the Virtual Machine object.
         """
+
+        if hasattr(val, 'name') and val.name:
+            self.set("name", val.name)
+
+        if hasattr(val, 'extraConfig'):
+            extraConfigs = _merge_extraconfig(
+                                    self.get("config.extraConfig").OptionValue,
+                                    val.extraConfig)
+            self.get("config.extraConfig").OptionValue = extraConfigs
+
+        if hasattr(val, 'instanceUuid') and val.instanceUuid is not None:
+            if val.instanceUuid == "":
+                val.instanceUuid = uuidutils.generate_uuid()
+            self.set("summary.config.instanceUuid", val.instanceUuid)
+
         try:
             if not hasattr(val, 'deviceChange'):
                 return
@@ -466,8 +481,7 @@ class VirtualMachine(ManagedObject):
             self.set("config.hardware.device", [disk, controller,
                                                   self.device[0]])
         except AttributeError:
-            # Case of Reconfig of VM to set extra params
+            pass
-            self.set("config.extraConfig", val.extraConfig)
 
 
 class Network(ManagedObject):
@@ -959,6 +973,19 @@ def _get_vm_mdo(vm_ref):
     return _db_content.get("VirtualMachine")[vm_ref]
 
 
+def _merge_extraconfig(existing, changes):
+    """Imposes the changes in extraConfig over the existing extraConfig."""
+    existing = existing or []
+    if (changes):
+        for c in changes:
+            if len([x for x in existing if x.key == c.key]) > 0:
+                extraConf = [x for x in existing if x.key == c.key][0]
+                extraConf.value = c.value
+            else:
+                existing.append(c)
+    return existing
+
+
 class FakeFactory(object):
     """Fake factory class for the suds client."""
 
@@ -1142,7 +1169,39 @@ class FakeVim(object):
 
     def _clone_vm(self, method, *args, **kwargs):
         """Fakes a VM clone."""
-        return self._just_return_task(method)
+        """Creates and registers a VM object with the Host System."""
+        source_vmref = args[0]
+        source_vm_mdo = _get_vm_mdo(source_vmref)
+        clone_spec = kwargs.get("spec")
+        ds = _db_content["Datastore"].keys()[0]
+        host = _db_content["HostSystem"].keys()[0]
+        vm_dict = {
+         "name": kwargs.get("name"),
+         "ds": source_vm_mdo.get("datastore"),
+         "runtime_host": source_vm_mdo.get("runtime.host"),
+         "powerstate": source_vm_mdo.get("runtime.powerState"),
+         "vmPathName": source_vm_mdo.get("config.files.vmPathName"),
+         "numCpu": source_vm_mdo.get("summary.config.numCpu"),
+         "mem": source_vm_mdo.get("summary.config.memorySizeMB"),
+         "extra_config": source_vm_mdo.get("config.extraConfig").OptionValue,
+         "virtual_device": source_vm_mdo.get("config.hardware.device"),
+         "instanceUuid": source_vm_mdo.get("summary.config.instanceUuid")}
+
+        if clone_spec.config is not None:
+            # Impose the config changes specified in the config property
+            if (hasattr(clone_spec.config, 'instanceUuid') and
+               clone_spec.config.instanceUuid is not None):
+                vm_dict["instanceUuid"] = clone_spec.config.instanceUuid
+
+            if hasattr(clone_spec.config, 'extraConfig'):
+                extraConfigs = _merge_extraconfig(vm_dict["extra_config"],
+                                                clone_spec.config.extraConfig)
+                vm_dict["extra_config"] = extraConfigs
+
+        virtual_machine = VirtualMachine(**vm_dict)
+        _create_object("VirtualMachine", virtual_machine)
+        task_mdo = create_task(method, "success")
+        return task_mdo.obj
 
     def _unregister_vm(self, method, *args, **kwargs):
         """Unregisters a VM from the Host System."""

nova/virt/vmwareapi/vm_util.py

@@ -29,6 +29,7 @@ from nova.openstack.common.gettextutils import _
 from nova.openstack.common import log as logging
 from nova.openstack.common import units
 from nova import utils
+from nova.virt.vmwareapi import error_util
 from nova.virt.vmwareapi import vim_util
 
 CONF = cfg.CONF
@@ -570,13 +571,15 @@ def detach_virtual_disk_spec(client_factory, device, destroy_disk=False):
 
 
 def clone_vm_spec(client_factory, location,
-                  power_on=False, snapshot=None, template=False):
+                  power_on=False, snapshot=None, template=False, config=None):
     """Builds the VM clone spec."""
     clone_spec = client_factory.create('ns0:VirtualMachineCloneSpec')
     clone_spec.location = location
     clone_spec.powerOn = power_on
     if snapshot:
         clone_spec.snapshot = snapshot
+    if config is not None:
+        clone_spec.config = config
     clone_spec.template = template
     return clone_spec
 
@@ -827,15 +830,27 @@ def _get_vm_ref_from_extraconfig(session, instance_uuid):
 def get_vm_ref(session, instance):
     """Get reference to the VM through uuid or vm name."""
     uuid = instance['uuid']
-    vm_ref = (_get_vm_ref_from_vm_uuid(session, uuid) or
+    vm_ref = (search_vm_ref_by_identifier(session, uuid) or
-                  _get_vm_ref_from_extraconfig(session, uuid) or
+              _get_vm_ref_from_name(session, instance['name']))
-                  _get_vm_ref_from_uuid(session, uuid) or
-                  _get_vm_ref_from_name(session, instance['name']))
     if vm_ref is None:
         raise exception.InstanceNotFound(instance_id=uuid)
     return vm_ref
 
 
+def search_vm_ref_by_identifier(session, identifier):
+    """Searches VM reference using the identifier.
+
+    This method is primarily meant to separate out part of the logic for
+    vm_ref search that could be use directly in the special case of
+    migrating the instance. For querying VM linked to an instance always
+    use get_vm_ref instead.
+    """
+    vm_ref = (_get_vm_ref_from_vm_uuid(session, identifier) or
+              _get_vm_ref_from_extraconfig(session, identifier) or
+              _get_vm_ref_from_uuid(session, identifier))
+    return vm_ref
+
+
 def get_host_ref_from_id(session, host_id, property_list=None):
     """Get a host reference object for a host_id string."""
 
@@ -1347,3 +1362,96 @@ def get_vmdk_adapter_type(adapter_type):
     else:
         vmdk_adapter_type = adapter_type
     return vmdk_adapter_type
+
+
+def clone_vmref_for_instance(session, instance, vm_ref, host_ref, ds_ref,
+                                vmfolder_ref):
+    """Clone VM and link the cloned VM to the instance.
+
+    Clones the passed vm_ref into a new VM and links the cloned vm to
+    the passed instance.
+    """
+    if vm_ref is None:
+        LOG.warn(_("vmwareapi:vm_util:clone_vmref_for_instance, called "
+                   "with vm_ref=None"))
+        raise error_util.MissingParameter(param="vm_ref")
+    # Get the clone vm spec
+    client_factory = session._get_vim().client.factory
+    rel_spec = relocate_vm_spec(client_factory, ds_ref, host_ref)
+    extra_opts = {'nvp.vm-uuid': instance['uuid']}
+    config_spec = get_vm_extra_config_spec(client_factory, extra_opts)
+    config_spec.instanceUuid = instance['uuid']
+    clone_spec = clone_vm_spec(client_factory, rel_spec, config=config_spec)
+
+    # Clone VM on ESX host
+    LOG.debug(_("Cloning VM for instance %s"), instance['uuid'],
+               instance=instance)
+    vm_clone_task = session._call_method(session._get_vim(), "CloneVM_Task",
+                                         vm_ref, folder=vmfolder_ref,
+                                         name=instance['uuid'],
+                                         spec=clone_spec)
+    session._wait_for_task(vm_clone_task)
+    LOG.debug(_("Cloned VM for instance %s"), instance['uuid'],
+               instance=instance)
+    # Invalidate the cache, so that it is refetched the next time
+    vm_ref_cache_delete(instance['uuid'])
+
+
+def disassociate_vmref_from_instance(session, instance, vm_ref=None,
+                                      suffix='-orig'):
+    """Disassociates the VM linked to the instance.
+
+    Disassociates the VM linked to the instance by performing the following
+    1. Update the extraConfig property for nvp.vm-uuid to be replaced with
+    instance[uuid]+suffix
+    2. Rename the VM to be instance[uuid]+suffix instead
+    3. Reset the instanceUUID of the VM to a new generated value
+    """
+    if vm_ref is None:
+        vm_ref = get_vm_ref(session, instance)
+    extra_opts = {'nvp.vm-uuid': instance['uuid'] + suffix}
+    client_factory = session._get_vim().client.factory
+    reconfig_spec = get_vm_extra_config_spec(client_factory, extra_opts)
+    reconfig_spec.name = instance['uuid'] + suffix
+    reconfig_spec.instanceUuid = ''
+    LOG.debug(_("Disassociating VM from instance %s"), instance['uuid'],
+               instance=instance)
+    reconfig_task = session._call_method(session._get_vim(), "ReconfigVM_Task",
+                                         vm_ref, spec=reconfig_spec)
+    session._wait_for_task(reconfig_task)
+    LOG.debug(_("Disassociated VM from instance %s"), instance['uuid'],
+               instance=instance)
+    # Invalidate the cache, so that it is refetched the next time
+    vm_ref_cache_delete(instance['uuid'])
+
+
+def associate_vmref_for_instance(session, instance, vm_ref=None,
+                                    suffix='-orig'):
+    """Associates the VM to the instance.
+
+    Associates the VM to the instance by performing the following
+    1. Update the extraConfig property for nvp.vm-uuid to be replaced with
+    instance[uuid]
+    2. Rename the VM to be instance[uuid]
+    3. Reset the instanceUUID of the VM to be instance[uuid]
+    """
+    if vm_ref is None:
+        vm_ref = search_vm_ref_by_identifier(session,
+                                             instance['uuid'] + suffix)
+        if vm_ref is None:
+            raise exception.InstanceNotFound(instance_id=instance['uuid']
+                                            + suffix)
+    extra_opts = {'nvp.vm-uuid': instance['uuid']}
+    client_factory = session._get_vim().client.factory
+    reconfig_spec = get_vm_extra_config_spec(client_factory, extra_opts)
+    reconfig_spec.name = instance['uuid']
+    reconfig_spec.instanceUuid = instance['uuid']
+    LOG.debug(_("Associating VM to instance %s"), instance['uuid'],
+               instance=instance)
+    reconfig_task = session._call_method(session._get_vim(), "ReconfigVM_Task",
+                                         vm_ref, spec=reconfig_spec)
+    session._wait_for_task(reconfig_task)
+    LOG.debug(_("Associated VM to instance %s"), instance['uuid'],
+               instance=instance)
+    # Invalidate the cache, so that it is refetched the next time
+    vm_ref_cache_delete(instance['uuid'])

nova/virt/vmwareapi/vmops.py

@@ -105,6 +105,7 @@ class VMwareVMOps(object):
         self._tmp_folder = 'vmware_temp'
         self._default_root_device = 'vda'
         self._rescue_suffix = '-rescue'
+        self._migrate_suffix = '-orig'
         self._poll_rescue_last_ran = None
         self._is_neutron = utils.is_neutron()
         self._datastore_dc_mapping = {}
@@ -1251,49 +1252,33 @@ class VMwareVMOps(object):
                                        step=1,
                                        total_steps=RESIZE_TOTAL_STEPS)
 
-        # 2. Rename the original VM with suffix '-orig'
+        # 2. Disassociate the linked vsphere VM from the instance
-        name_label = self._get_orig_vm_name_label(instance)
+        vm_util.disassociate_vmref_from_instance(self._session, instance,
-        LOG.debug(_("Renaming the VM to %s") % name_label,
+                                                 vm_ref,
-                  instance=instance)
+                                                 suffix=self._migrate_suffix)
-        rename_task = self._session._call_method(
-                            self._session._get_vim(),
-                            "Rename_Task", vm_ref, newName=name_label)
-        self._session._wait_for_task(rename_task)
-        LOG.debug(_("Renamed the VM to %s") % name_label,
-                  instance=instance)
         self._update_instance_progress(context, instance,
                                        step=2,
                                        total_steps=RESIZE_TOTAL_STEPS)
 
-        # Get the clone vm spec
         ds_ref = vm_util.get_datastore_ref_and_name(
                             self._session, self._cluster, host_ref,
                             datastore_regex=self._datastore_regex)[0]
-        client_factory = self._session._get_vim().client.factory
-        rel_spec = vm_util.relocate_vm_spec(client_factory, ds_ref, host_ref)
-        clone_spec = vm_util.clone_vm_spec(client_factory, rel_spec)
         dc_info = self.get_datacenter_ref_and_name(ds_ref)
-
+        # 3. Clone the VM for instance
-        # 3. Clone VM on ESX host
+        vm_util.clone_vmref_for_instance(self._session, instance, vm_ref,
-        LOG.debug(_("Cloning VM to host %s") % dest, instance=instance)
+                                         host_ref, ds_ref, dc_info.vmFolder)
-        vm_clone_task = self._session._call_method(
-                                self._session._get_vim(),
-                                "CloneVM_Task", vm_ref,
-                                folder=dc_info.vmFolder,
-                                name=instance['uuid'],
-                                spec=clone_spec)
-        self._session._wait_for_task(vm_clone_task)
-        LOG.debug(_("Cloned VM to host %s") % dest, instance=instance)
         self._update_instance_progress(context, instance,
                                        step=3,
                                        total_steps=RESIZE_TOTAL_STEPS)
 
     def confirm_migration(self, migration, instance, network_info):
         """Confirms a resize, destroying the source VM."""
-        instance_name = self._get_orig_vm_name_label(instance)
+        # Destroy the original VM. The vm_ref needs to be searched using the
-        # Destroy the original VM. The vm_ref is via the instance_name
+        # instance['uuid'] + self._migrate_suffix as the identifier. We will
-        # and not the UUID
+        # not get the vm when searched using the instanceUuid but rather will
-        vm_ref = vm_util.get_vm_ref_from_name(self._session, instance_name)
+        # be found using the uuid buried in the extraConfig
+        vm_ref = vm_util.search_vm_ref_by_identifier(self._session,
+                                    instance['uuid'] + self._migrate_suffix)
         if vm_ref is None:
             LOG.debug(_("instance not present"), instance=instance)
             return
@@ -1312,21 +1297,8 @@ class VMwareVMOps(object):
     def finish_revert_migration(self, context, instance, network_info,
                                 block_device_info, power_on=True):
         """Finish reverting a resize."""
-        # The original vm was suffixed with '-orig'; find it using
+        vm_util.associate_vmref_for_instance(self._session, instance,
-        # the old suffix, remove the suffix, then power it back on.
+                                             suffix=self._migrate_suffix)
-        name_label = self._get_orig_vm_name_label(instance)
-        vm_ref = vm_util.get_vm_ref_from_name(self._session, name_label)
-        if vm_ref is None:
-            raise exception.InstanceNotFound(instance_id=name_label)
-
-        LOG.debug(_("Renaming the VM from %s") % name_label,
-                  instance=instance)
-        rename_task = self._session._call_method(
-                            self._session._get_vim(),
-                            "Rename_Task", vm_ref, newName=instance['uuid'])
-        self._session._wait_for_task(rename_task)
-        LOG.debug(_("Renamed the VM from %s") % name_label,
-                  instance=instance)
         if power_on:
             self._power_on(instance)