From 923c38c51fcd858daa4e909121d0142bd1fc3f08 Mon Sep 17 00:00:00 2001 From: Sabari Kumar Murugesan Date: Fri, 18 Oct 2013 15:42:49 -0700 Subject: [PATCH] VMware: fix list_instances for multi-node driver VMwareVCDriver should only list instances in the nodes managed by it. Currently, it uses the an implementation that lists instances in the vCenter server inventory even if they are not in the nodes managed by the driver. Closes-bug: #1272286 Change-Id: I56c81a759eacc8c595e97ac5ca372834b675ebff --- nova/tests/virt/vmwareapi/test_driver_api.py | 16 +++++ nova/tests/virt/vmwareapi/test_vmops.py | 22 +++++++ nova/virt/vmwareapi/driver.py | 9 +++ nova/virt/vmwareapi/fake.py | 9 ++- nova/virt/vmwareapi/vmops.py | 65 +++++++++++++------- 5 files changed, 99 insertions(+), 22 deletions(-) diff --git a/nova/tests/virt/vmwareapi/test_driver_api.py b/nova/tests/virt/vmwareapi/test_driver_api.py index 01ec2d1ace30..8317dda15a86 100644 --- a/nova/tests/virt/vmwareapi/test_driver_api.py +++ b/nova/tests/virt/vmwareapi/test_driver_api.py @@ -1834,6 +1834,22 @@ class VMwareAPIVCDriverTestCase(VMwareAPIVMTestCase): super(VMwareAPIVCDriverTestCase, self).tearDown() vmwareapi_fake.cleanup() + def test_list_instances(self): + instances = self.conn.list_instances() + self.assertEqual(0, len(instances)) + + def test_list_instances_from_nodes(self): + # Create instance on node1 + self._create_vm(self.node_name) + # Create instances on the other node + self._create_vm(self.node_name2, num_instances=2) + self._create_vm(self.node_name2, num_instances=3) + node1_vmops = self.conn._get_vmops_for_compute_node(self.node_name) + node2_vmops = self.conn._get_vmops_for_compute_node(self.node_name2) + self.assertEqual(1, len(node1_vmops.list_instances())) + self.assertEqual(2, len(node2_vmops.list_instances())) + self.assertEqual(3, len(self.conn.list_instances())) + def _setup_mocks_for_session(self, mock_init): mock_init.return_value = None diff --git a/nova/tests/virt/vmwareapi/test_vmops.py b/nova/tests/virt/vmwareapi/test_vmops.py index 6df14fa774ce..ef21a50356f9 100644 --- a/nova/tests/virt/vmwareapi/test_vmops.py +++ b/nova/tests/virt/vmwareapi/test_vmops.py @@ -180,6 +180,28 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): "folder", "some_file") ops._create_folder_if_missing.assert_called_once() + def test_get_valid_vms_from_retrieve_result(self): + ops = vmops.VMwareVMOps(mock.Mock(), mock.Mock(), mock.Mock()) + fake_objects = vmwareapi_fake.FakeRetrieveResult() + fake_objects.add_object(vmwareapi_fake.VirtualMachine()) + fake_objects.add_object(vmwareapi_fake.VirtualMachine()) + fake_objects.add_object(vmwareapi_fake.VirtualMachine()) + vms = ops._get_valid_vms_from_retrieve_result(fake_objects) + self.assertEqual(3, len(vms)) + + def test_get_valid_vms_from_retrieve_result_with_invalid(self): + ops = vmops.VMwareVMOps(mock.Mock(), mock.Mock(), mock.Mock()) + fake_objects = vmwareapi_fake.FakeRetrieveResult() + fake_objects.add_object(vmwareapi_fake.VirtualMachine()) + invalid_vm1 = vmwareapi_fake.VirtualMachine() + invalid_vm1.set('runtime.connectionState', 'orphaned') + invalid_vm2 = vmwareapi_fake.VirtualMachine() + invalid_vm2.set('runtime.connectionState', 'inaccessible') + fake_objects.add_object(invalid_vm1) + fake_objects.add_object(invalid_vm2) + vms = ops._get_valid_vms_from_retrieve_result(fake_objects) + self.assertEqual(1, len(vms)) + def test_delete_vm_snapshot(self): def fake_call_method(module, method, *args, **kwargs): self.assertEqual('RemoveSnapshot_Task', method) diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py index 79d41cf7ee4e..04b689218cd2 100644 --- a/nova/virt/vmwareapi/driver.py +++ b/nova/virt/vmwareapi/driver.py @@ -412,6 +412,15 @@ class VMwareVCDriver(VMwareESXDriver): self._volumeops = self._resources.get(first_cluster).get('volumeops') self._vc_state = self._resources.get(first_cluster).get('vcstate') + def list_instances(self): + """List VM instances from all nodes.""" + instances = [] + nodes = self.get_available_nodes() + for node in nodes: + vmops = self._get_vmops_for_compute_node(node) + instances.extend(vmops.list_instances()) + return instances + def migrate_disk_and_power_off(self, context, instance, dest, flavor, network_info, block_device_info=None): diff --git a/nova/virt/vmwareapi/fake.py b/nova/virt/vmwareapi/fake.py index 044c6cf0d726..ca82aba49d84 100644 --- a/nova/virt/vmwareapi/fake.py +++ b/nova/virt/vmwareapi/fake.py @@ -492,6 +492,7 @@ class ResourcePool(ManagedObject): memoryAllocation = DataObject() cpuAllocation = DataObject() + vm_list = DataObject() memory.maxUsage = 1000 * units.Mi memory.overallUsage = 500 * units.Mi @@ -505,9 +506,11 @@ class ResourcePool(ManagedObject): memoryAllocation.reservation = 1024 config.memoryAllocation = memoryAllocation config.cpuAllocation = cpuAllocation + vm_list.ManagedObjectReference = [] self.set("summary", summary) self.set("summary.runtime.memory", memory) self.set("config", config) + self.set("vm", vm_list) parent = ManagedObjectReference(value=value, name=name) owner = ManagedObjectReference(value=value, @@ -862,6 +865,7 @@ def create_datastore(name, capacity, free): def create_res_pool(): res_pool = ResourcePool() _create_object('ResourcePool', res_pool) + return res_pool.obj def create_network(): @@ -874,7 +878,7 @@ def create_cluster(name, ds_ref): cluster._add_host(_get_object_refs("HostSystem")[0]) cluster._add_host(_get_object_refs("HostSystem")[1]) cluster._add_datastore(ds_ref) - cluster._add_root_resource_pool(_get_object_refs("ResourcePool")[0]) + cluster._add_root_resource_pool(create_res_pool()) _create_object('ClusterComputeResource', cluster) @@ -1050,6 +1054,7 @@ class FakeVim(object): def _create_vm(self, method, *args, **kwargs): """Creates and registers a VM object with the Host System.""" config_spec = kwargs.get("config") + pool = kwargs.get('pool') ds = _db_content["Datastore"].keys()[0] host = _db_content["HostSystem"].keys()[0] vm_dict = {"name": config_spec.name, @@ -1064,6 +1069,8 @@ class FakeVim(object): "instanceUuid": config_spec.instanceUuid} virtual_machine = VirtualMachine(**vm_dict) _create_object("VirtualMachine", virtual_machine) + res_pool = _get_object(pool) + res_pool.vm.ManagedObjectReference.append(virtual_machine.obj) task_mdo = create_task(method, "success") return task_mdo.obj diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py index cee1680843a3..05a7c926ac07 100644 --- a/nova/virt/vmwareapi/vmops.py +++ b/nova/virt/vmwareapi/vmops.py @@ -118,27 +118,7 @@ class VMwareVMOps(object): vms = self._session._call_method(vim_util, "get_objects", "VirtualMachine", ["name", "runtime.connectionState"]) - lst_vm_names = [] - - while vms: - token = vm_util._get_token(vms) - for vm in vms.objects: - vm_name = None - conn_state = None - for prop in vm.propSet: - if prop.name == "name": - vm_name = prop.val - elif prop.name == "runtime.connectionState": - conn_state = prop.val - # Ignoring the orphaned or inaccessible VMs - if conn_state not in ["orphaned", "inaccessible"]: - lst_vm_names.append(vm_name) - if token: - vms = self._session._call_method(vim_util, - "continue_to_get_objects", - token) - else: - break + lst_vm_names = self._get_valid_vms_from_retrieve_result(vms) LOG.debug(_("Got total of %s instances") % str(len(lst_vm_names))) return lst_vm_names @@ -1667,6 +1647,31 @@ class VMwareVMOps(object): datastores_info.append((ds, ds_info)) self._imagecache.update(context, instances, datastores_info) + def _get_valid_vms_from_retrieve_result(self, retrieve_result): + """Returns list of valid vms from RetrieveResult object.""" + lst_vm_names = [] + + while retrieve_result: + token = vm_util._get_token(retrieve_result) + for vm in retrieve_result.objects: + vm_name = None + conn_state = None + for prop in vm.propSet: + if prop.name == "name": + vm_name = prop.val + elif prop.name == "runtime.connectionState": + conn_state = prop.val + # Ignoring the orphaned or inaccessible VMs + if conn_state not in ["orphaned", "inaccessible"]: + lst_vm_names.append(vm_name) + if token: + retrieve_result = self._session._call_method(vim_util, + "continue_to_get_objects", + token) + else: + break + return lst_vm_names + class VMwareVCVMOps(VMwareVMOps): """Management class for VM-related tasks. @@ -1725,3 +1730,21 @@ class VMwareVCVMOps(VMwareVMOps): self._update_datacenter_cache_from_objects(dcs) dc_info = self._datastore_dc_mapping.get(ds_ref.value) return dc_info + + def list_instances(self): + """Lists the VM instances that are registered with vCenter cluster.""" + properties = ['name', 'runtime.connectionState'] + LOG.debug(_("Getting list of instances from cluster %s"), + self._cluster) + vms = [] + root_res_pool = self._session._call_method( + vim_util, "get_dynamic_property", self._cluster, + 'ClusterComputeResource', 'resourcePool') + if root_res_pool: + vms = self._session._call_method( + vim_util, 'get_inner_objects', root_res_pool, 'vm', + 'VirtualMachine', properties) + lst_vm_names = self._get_valid_vms_from_retrieve_result(vms) + + LOG.debug(_("Got total of %s instances") % str(len(lst_vm_names))) + return lst_vm_names