From: Vipin Balachandran <vbala@vmware.com>
Date: Wed, 10 Dec 2014 07:11:08 +0000 (+0530)
Subject: VMware: Fix datastore selection with single host
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=01b029e4d3afb181b43cce4b7f7c0185adb9eeac;p=openstack-build%2Fcinder-build.git

VMware: Fix datastore selection with single host

Currently, the select_datastore method is called with an empty list of
ESX hosts. This causes the method to retrieve all the hosts in vCenter
as candidates for selecting a datastore. The reference to a retrieved
host is stored in attribute 'obj'. Some of the future bug fixes will
need to invoke this method with reference to a single ESX host. Such
invocations will fail with attribute error: 'val instance has no
attribute 'obj''. This patch fixes the attribute error.

Closes-Bug: #1401052
Change-Id: Ib4f48d2ebfbfa3a9ce6a402300d41351548932ef
---

diff --git a/cinder/tests/test_vmware_datastore.py b/cinder/tests/test_vmware_datastore.py
index e5206369f..a118a52d0 100644
--- a/cinder/tests/test_vmware_datastore.py
+++ b/cinder/tests/test_vmware_datastore.py
@@ -60,6 +60,12 @@ class DatastoreTest(test.TestCase):
         return mock.Mock(datastore=ds, freeSpace=free_space, type=_type,
                          capacity=capacity)
 
+    def _create_host(self, value):
+        host = mock.Mock(spec=['_type', 'value'])
+        host._type = 'HostSystem'
+        host.value = value
+        return host
+
     @mock.patch('cinder.volume.drivers.vmware.datastore.DatastoreSelector.'
                 '_filter_by_profile')
     def test_filter_datastores(self, filter_by_profile):
@@ -181,13 +187,14 @@ class DatastoreTest(test.TestCase):
                                          free_space=units.Mi,
                                          capacity=4 * units.Mi)
 
-        host_1 = mock.sentinel.host_1
-        host_2 = mock.sentinel.host_3
-        host_3 = mock.sentinel.host_3
+        host_1 = self._create_host('host-1')
+        host_2 = self._create_host('host-2')
+        host_3 = self._create_host('host-3')
 
-        connected_hosts = {mock.sentinel.ds_1: [host_1],
-                           mock.sentinel.ds_2: [host_1, host_2],
-                           mock.sentinel.ds_3: [host_1, host_2, host_3]}
+        connected_hosts = {mock.sentinel.ds_1: [host_1.value],
+                           mock.sentinel.ds_2: [host_1.value, host_2.value],
+                           mock.sentinel.ds_3: [host_1.value, host_2.value,
+                                                host_3.value]}
         self._vops.get_connected_hosts.side_effect = (
             lambda summary: connected_hosts[summary])
 
@@ -228,7 +235,7 @@ class DatastoreTest(test.TestCase):
         self._vops.get_hosts.assert_called_once_with()
 
         # Test with single host with no valid datastores.
-        host_1 = mock.sentinel.host_1
+        host_1 = self._create_host('host-1')
         self._vops.get_hosts.return_value = mock.Mock(
             objects=[mock.Mock(obj=host_1)])
         self._vops.continue_retrieval.return_value = None
@@ -240,8 +247,8 @@ class DatastoreTest(test.TestCase):
         # Test with three hosts and vCenter connection problem while fetching
         # datastores for the second host.
         self._vops.get_dss_rp.reset_mock()
-        host_2 = mock.sentinel.host_2
-        host_3 = mock.sentinel.host_3
+        host_2 = self._create_host('host-2')
+        host_3 = self._create_host('host-3')
         self._vops.get_hosts.return_value = mock.Mock(
             objects=[mock.Mock(obj=host_1),
                      mock.Mock(obj=host_2),
@@ -347,6 +354,30 @@ class DatastoreTest(test.TestCase):
         # Clear side effects.
         self._vops.get_dss_rp.side_effect = None
 
+    @mock.patch('cinder.volume.drivers.vmware.datastore.DatastoreSelector.'
+                '_filter_datastores')
+    def test_select_datastore_with_single_host(self, filter_datastores):
+        host = self._create_host('host-1')
+        req = {self._ds_sel.SIZE_BYTES: units.Gi}
+
+        ds = mock.sentinel.ds
+        rp = mock.sentinel.rp
+        self._vops.get_dss_rp.return_value = ([ds], rp)
+
+        summary = self._create_summary(ds, free_space=2 * units.Gi,
+                                       capacity=3 * units.Gi)
+        filter_datastores.return_value = [summary]
+        self._vops.get_connected_hosts.return_value = [host.value]
+
+        self.assertEqual((host, rp, summary),
+                         self._ds_sel.select_datastore(req, [host]))
+
+        # reset mocks
+        self._vops.get_dss_rp.reset_mock()
+        self._vops.get_dss_rp.return_value = None
+        self._vops.get_connected_hosts.reset_mock()
+        self._vops.get_connected_hosts.return_value = None
+
     @mock.patch('cinder.volume.drivers.vmware.datastore.DatastoreSelector.'
                 '_filter_by_profile')
     def test_is_datastore_compliant(self, filter_by_profile):
diff --git a/cinder/volume/drivers/vmware/datastore.py b/cinder/volume/drivers/vmware/datastore.py
index f686596bd..0c052720b 100644
--- a/cinder/volume/drivers/vmware/datastore.py
+++ b/cinder/volume/drivers/vmware/datastore.py
@@ -122,7 +122,8 @@ class DatastoreSelector(object):
             if not hosts:
                 break
 
-            all_hosts.extend(hosts)
+            for host in hosts:
+                all_hosts.append(host.obj)
             retrieve_result = self._vops.continue_retrieval(
                 retrieve_result)
         return all_hosts
@@ -203,8 +204,7 @@ class DatastoreSelector(object):
                   "requirements: %(req)s.",
                   {'hosts': hosts,
                    'req': req})
-        for host in hosts:
-            host_ref = host.obj
+        for host_ref in hosts:
             try:
                 (datastores, rp) = self._vops.get_dss_rp(host_ref)
             except error_util.VimConnectionException: