Use RetrievePropertiesEx instead of RetrieveProperties

Fixes bug 1183654

The fix also adds a configuration variable enabling the user to
configure the maximum amount of objects that can be retrieved by
the aforementioned function.

When using RetrievePropertiesEx the server may allocate resources
in case additional calls are made. These are addressed by reading the
extra resources using ContinueRetrievePropertiesEx (if necessary)
or CancelRetrievePropertiesEx (when the necessary result has been found).

DocImpact

Change-Id: I894d9698461b0ce22b01211fd196f6c32899a8fd

etc/nova/nova.conf.sample

@@ -3118,6 +3118,20 @@
 #wsdl_location=<None>
 
 
+#
+# Options defined in nova.virt.vmwareapi.vim_util
+#
+
+# The maximum number of ObjectContent data objects that should
+# be returned in a single result. A positive value will cause
+# the operation to suspend the retrieval when the count of
+# objects reaches the specified maximum. The server may still
+# limit the count to something less than the configured value.
+# Any remaining objects may be retrieved with additional
+# requests. (integer value)
+#maximum_objects=100
+
+
 #
 # Options defined in nova.virt.vmwareapi.vmops
 #

nova/tests/virt/vmwareapi/test_vmwareapi.py

@@ -175,7 +175,7 @@ def _check_vm_record(self):
 
         # Get record for VM
         vms = vmwareapi_fake._get_objects("VirtualMachine")
-        vm = vms[0]
+        vm = vms.objects[0]
 
         # Check that m1.large above turned into the right thing.
         mem_kib = long(self.type_data['memory_mb']) << 10
@@ -521,10 +521,10 @@ def fake_get_orig_vm_name_label(instance):
 
         def fake_get_vm_ref_from_name(session, vm_name):
             self.assertEquals(self.vm_name, vm_name)
-            return vmwareapi_fake._get_objects("VirtualMachine")[0]
+            return vmwareapi_fake._get_objects("VirtualMachine").objects[0]
 
         def fake_get_vm_ref_from_uuid(session, vm_uuid):
-            return vmwareapi_fake._get_objects("VirtualMachine")[0]
+            return vmwareapi_fake._get_objects("VirtualMachine").objects[0]
 
         def fake_call_method(*args, **kwargs):
             pass

nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py

@@ -42,8 +42,10 @@ def tearDown(self):
         fake.reset()
 
     def test_get_datastore_ref_and_name(self):
+        fake_objects = fake.FakeRetrieveResult()
+        fake_objects.add_object(fake.Datastore())
         result = vm_util.get_datastore_ref_and_name(
-            fake_session([fake.Datastore()]))
+            fake_session(fake_objects))
 
         self.assertEquals(result[1], "fake-ds")
         self.assertEquals(result[2], 1024 * 1024 * 1024 * 1024)
@@ -66,9 +68,10 @@ def test_get_host_ref_from_id(self):
 
         fake_host_id = fake_host_sys.obj.value
         fake_host_name = "ha-host"
-
+        fake_objects = fake.FakeRetrieveResult()
+        fake_objects.add_object(fake_host_sys)
         ref = vm_util.get_host_ref_from_id(
-            fake_session([fake_host_sys]), fake_host_id, ['name'])
+            fake_session(fake_objects), fake_host_id, ['name'])
 
         self.assertIsInstance(ref, fake.HostSystem)
         self.assertEqual(fake_host_id, ref.obj.value)
@@ -84,9 +87,11 @@ def test_get_host_name_for_vm(self):
                 "vm-123", "VirtualMachine"))
         fake_vm.propSet.append(
             fake.Property('name', 'vm-123'))
+        fake_objects = fake.FakeRetrieveResult()
+        fake_objects.add_object(fake_vm)
 
         vm_ref = vm_util.get_vm_ref_from_name(
-                fake_session([fake_vm]), 'vm-123')
+                fake_session(fake_objects), 'vm-123')
 
         self.assertIsNotNone(vm_ref)
 
@@ -98,8 +103,12 @@ def test_get_host_name_for_vm(self):
                                 'host-123', 'HostSystem'))
                 ])]
 
+        fake_objects = fake.FakeRetrieveResult()
+        for results in fake_results:
+            fake_objects.add_object(results)
+
         host_id = vm_util.get_host_id_from_vm_ref(
-            fake_session(fake_results), vm_ref)
+            fake_session(fake_objects), vm_ref)
 
         self.assertEqual('host-123', host_id)
 
@@ -109,6 +118,7 @@ def test_property_from_property_set(self):
         DynamicProperty = namedtuple('Property', ['name', 'val'])
         MoRef = namedtuple('Val', ['value'])
 
+        good_objects = fake.FakeRetrieveResult()
         results_good = [
             ObjectContent(propSet=[
                 DynamicProperty(name='name', val=MoRef(value='vm-123'))]),
@@ -121,7 +131,10 @@ def test_property_from_property_set(self):
             ObjectContent(propSet=[
                 DynamicProperty(
                     name='something', val=MoRef(value='thing'))]), ]
+        for result in results_good:
+            good_objects.add_object(result)
 
+        bad_objects = fake.FakeRetrieveResult()
         results_bad = [
             ObjectContent(propSet=[
                 DynamicProperty(name='name', val=MoRef(value='vm-123'))]),
@@ -131,22 +144,24 @@ def test_property_from_property_set(self):
             ObjectContent(propSet=[
                 DynamicProperty(
                     name='something', val=MoRef(value='thing'))]), ]
+        for result in results_bad:
+            bad_objects.add_object(result)
 
         prop = vm_util.property_from_property_set(
-                    'runtime.host', results_good)
+                    'runtime.host', good_objects)
         self.assertIsNotNone(prop)
         value = prop.val.value
         self.assertEqual('host-123', value)
 
         prop2 = vm_util.property_from_property_set(
-                    'runtime.host', results_bad)
+                    'runtime.host', bad_objects)
         self.assertIsNone(prop2)
 
-        prop3 = vm_util.property_from_property_set('foo', results_good)
+        prop3 = vm_util.property_from_property_set('foo', good_objects)
         self.assertIsNotNone(prop3)
         val3 = prop3.val.value
         self.assertEqual('bar1', val3)
 
-        prop4 = vm_util.property_from_property_set('foo', results_bad)
+        prop4 = vm_util.property_from_property_set('foo', bad_objects)
         self.assertIsNotNone(prop4)
         self.assertEqual('bar1', prop4.val)

nova/virt/vmwareapi/error_util.py

@@ -67,7 +67,7 @@ class FaultCheckers(object):
     @staticmethod
     def retrieveproperties_fault_checker(resp_obj):
         """
-        Checks the RetrieveProperties response for errors. Certain faults
+        Checks the RetrievePropertiesEx response for errors. Certain faults
         are sent as part of the SOAP body as property of missingSet.
         For example NotAuthenticated fault.
         """
@@ -91,5 +91,5 @@ def retrieveproperties_fault_checker(resp_obj):
         if fault_list:
             exc_msg_list = ', '.join(fault_list)
             raise VimFaultException(fault_list, Exception(_("Error(s) %s "
-                    "occurred in the call to RetrieveProperties") %
+                    "occurred in the call to RetrievePropertiesEx") %
                     exc_msg_list))

nova/virt/vmwareapi/fake.py

@@ -77,12 +77,22 @@ def _create_object(table, table_obj):
 
 def _get_objects(obj_type):
     """Get objects of the type."""
-    lst_objs = []
+    lst_objs = FakeRetrieveResult()
     for key in _db_content[obj_type]:
-        lst_objs.append(_db_content[obj_type][key])
+        lst_objs.add_object(_db_content[obj_type][key])
     return lst_objs
 
 
+class FakeRetrieveResult(object):
+    """Object to retrieve a ObjectContent list."""
+
+    def __init__(self):
+        self.objects = []
+
+    def add_object(self, object):
+        self.objects.append(object)
+
+
 class Property(object):
     """Property Object base class."""
 
@@ -317,16 +327,19 @@ def __init__(self, **kwargs):
         super(ClusterComputeResource, self).__init__("ClusterComputeResource",
                                                      value="domain-test")
         r_pool = DataObject()
-        r_pool.ManagedObjectReference = [_get_objects("ResourcePool")[0].obj]
+        obj = _get_objects("ResourcePool").objects[0].obj
+        r_pool.ManagedObjectReference = [obj]
         self.set("resourcePool", r_pool)
 
         host_sys = DataObject()
-        host_sys.ManagedObjectReference = [_get_objects("HostSystem")[0].obj]
+        obj = _get_objects("HostSystem").objects[0].obj
+        host_sys.ManagedObjectReference = [obj]
         self.set("host", host_sys)
         self.set("name", "test_cluster")
 
         datastore = DataObject()
-        datastore.ManagedObjectReference = [_get_objects("Datastore")[0].obj]
+        obj = _get_objects("Datastore").objects[0].obj
+        datastore.ManagedObjectReference = [obj]
         self.set("datastore", datastore)
 
 
@@ -773,6 +786,14 @@ def _set_power_state(self, method, vm_ref, pwr_state="poweredOn"):
         task_mdo = create_task(method, "success")
         return task_mdo.obj
 
+    def _retrieve_properties_continue(self, method, *args, **kwargs):
+        """Continues the retrieve."""
+        return FakeRetrieveResult()
+
+    def _retrieve_properties_cancel(self, method, *args, **kwargs):
+        """Cancels the retrieve."""
+        return None
+
     def _retrieve_properties(self, method, *args, **kwargs):
         """Retrieves properties based on the type."""
         spec_set = kwargs.get("specSet")[0]
@@ -781,7 +802,7 @@ def _retrieve_properties(self, method, *args, **kwargs):
         if not isinstance(properties, list):
             properties = properties.split()
         objs = spec_set.objectSet
-        lst_ret_objs = []
+        lst_ret_objs = FakeRetrieveResult()
         for obj in objs:
             try:
                 obj_ref = obj.obj
@@ -797,14 +818,14 @@ def _retrieve_properties(self, method, *args, **kwargs):
                         temp_mdo = ManagedObject(mdo.objName, mdo.obj)
                         for prop in properties:
                             temp_mdo.set(prop, mdo.get(prop))
-                        lst_ret_objs.append(temp_mdo)
+                        lst_ret_objs.add_object(temp_mdo)
                 else:
                     if obj_ref in _db_content[type]:
                         mdo = _db_content[type][obj_ref]
                         temp_mdo = ManagedObject(mdo.objName, obj_ref)
                         for prop in properties:
                             temp_mdo.set(prop, mdo.get(prop))
-                        lst_ret_objs.append(temp_mdo)
+                        lst_ret_objs.add_object(temp_mdo)
             except Exception as exc:
                 LOG.exception(exc)
                 continue
@@ -872,9 +893,15 @@ def __getattr__(self, attr_name):
         elif attr_name == "MakeDirectory":
             return lambda *args, **kwargs: self._make_dir(attr_name,
                                                 *args, **kwargs)
-        elif attr_name == "RetrieveProperties":
+        elif attr_name == "RetrievePropertiesEx":
             return lambda *args, **kwargs: self._retrieve_properties(
                                                 attr_name, *args, **kwargs)
+        elif attr_name == "ContinueRetrievePropertiesEx":
+            return lambda *args, **kwargs: self._retrieve_properties_continue(
+                                                attr_name, *args, **kwargs)
+        elif attr_name == "CancelRetrievePropertiesEx":
+            return lambda *args, **kwargs: self._retrieve_properties_cancel(
+                                                attr_name, *args, **kwargs)
         elif attr_name == "AcquireCloneTicket":
             return lambda *args, **kwargs: self._just_return()
         elif attr_name == "AddPortGroup":

nova/virt/vmwareapi/host.py

@@ -35,8 +35,7 @@ def __init__(self, session):
 
     def host_power_action(self, host, action):
         """Reboots or shuts down the host."""
-        host_mor = self._session._call_method(vim_util, "get_objects",
-                                              "HostSystem")[0].obj
+        host_mor = vm_util.get_host_ref(self._session)
         LOG.debug(_("%(action)s %(host)s"), {'action': action, 'host': host})
         if action == "reboot":
             host_task = self._session._call_method(
@@ -59,8 +58,7 @@ def host_maintenance_mode(self, host, mode):
         """Start/Stop host maintenance window. On start, it triggers
         guest VMs evacuation.
         """
-        host_mor = self._session._call_method(vim_util, "get_objects",
-                                              "HostSystem")[0].obj
+        host_mor = vm_util.get_host_ref(self._session)
         LOG.debug(_("Set maintenance mod on %(host)s to %(mode)s"),
                   {'host': host, 'mode': mode})
         if mode:
@@ -103,8 +101,7 @@ def get_host_stats(self, refresh=False):
     def update_status(self):
         """Update the current state of the host.
         """
-        host_mor = self._session._call_method(vim_util, "get_objects",
-                                              "HostSystem")[0].obj
+        host_mor = vm_util.get_host_ref(self._session)
         summary = self._session._call_method(vim_util,
                                              "get_dynamic_property",
                                              host_mor,

nova/virt/vmwareapi/vim_util.py

@@ -19,6 +19,22 @@
 The VMware API utility module.
 """
 
+from oslo.config import cfg
+
+
+vmware_opts = cfg.IntOpt('maximum_objects', default=100,
+                         help='The maximum number of ObjectContent data '
+                              'objects that should be returned in a single '
+                              'result. A positive value will cause the '
+                              'operation to suspend the retrieval when the '
+                              'count of objects reaches the specified '
+                              'maximum. The server may still limit the count '
+                              'to something less than the configured value. '
+                              'Any remaining objects may be retrieved with '
+                              'additional requests.')
+CONF = cfg.CONF
+CONF.register_opt(vmware_opts, 'vmware')
+
 
 def build_selection_spec(client_factory, name):
     """Builds the selection spec."""
@@ -143,15 +159,20 @@ def get_object_properties(vim, collector, mobj, type, properties):
     object_spec.skip = False
     property_filter_spec.propSet = [property_spec]
     property_filter_spec.objectSet = [object_spec]
-    return vim.RetrieveProperties(usecoll, specSet=[property_filter_spec])
+    options = client_factory.create('ns0:RetrieveOptions')
+    options.maxObjects = CONF.vmware.maximum_objects
+    return vim.RetrievePropertiesEx(usecoll, specSet=[property_filter_spec],
+                                    options=options)
 
 
 def get_dynamic_property(vim, mobj, type, property_name):
     """Gets a particular property of the Managed Object."""
     obj_content = get_object_properties(vim, None, mobj, type, [property_name])
+    if hasattr(obj_content, 'token'):
+        vim.CancelRetrievePropertiesEx(token=obj_content.token)
     property_value = None
-    if obj_content:
-        dynamic_property = obj_content[0].propSet
+    if obj_content.objects:
+        dynamic_property = obj_content.objects[0].propSet
         if dynamic_property:
             property_value = dynamic_property[0].val
     return property_value
@@ -172,8 +193,25 @@ def get_objects(vim, type, properties_to_collect=None, all=False):
     property_filter_spec = build_property_filter_spec(client_factory,
                                 [property_spec],
                                 [object_spec])
-    return vim.RetrieveProperties(vim.get_service_content().propertyCollector,
-                                specSet=[property_filter_spec])
+    options = client_factory.create('ns0:RetrieveOptions')
+    options.maxObjects = CONF.vmware.maximum_objects
+    return vim.RetrievePropertiesEx(
+            vim.get_service_content().propertyCollector,
+            specSet=[property_filter_spec], options=options)
+
+
+def cancel_retrieve(vim, token):
+    """Cancels the retrieve operation."""
+    return vim.CancelRetrievePropertiesEx(
+            vim.get_service_content().propertyCollector,
+            token=token)
+
+
+def continue_to_get_objects(vim, token):
+    """Continues to get the list of objects of the type specified."""
+    return vim.ContinueRetrievePropertiesEx(
+            vim.get_service_content().propertyCollector,
+            token=token)
 
 
 def get_prop_spec(client_factory, spec_type, properties):
@@ -217,5 +255,8 @@ def get_properties_for_a_collection_of_objects(vim, type,
         lst_obj_specs.append(get_obj_spec(client_factory, obj))
     prop_filter_spec = get_prop_filter_spec(client_factory,
                                             lst_obj_specs, [prop_spec])
-    return vim.RetrieveProperties(vim.get_service_content().propertyCollector,
-                                   specSet=[prop_filter_spec])
+    options = client_factory.create('ns0:RetrieveOptions')
+    options.maxObjects = CONF.vmware.maximum_objects
+    return vim.RetrievePropertiesEx(
+            vim.get_service_content().propertyCollector,
+            specSet=[prop_filter_spec], options=options)