VMware: Skip ESX hosts in maintenance mode

Volume creation will fail if the VMDK driver selects
an ESX host in maintenance mode for volume creation.
This may happen even if there are usable ESX hosts
in vCenter. This patch fixes the problem by ignoring
ESX hosts in maintenance mode during volume creation.

Closes-bug: #1492221
Change-Id: I8cb89f95d0411ea2140e3107941d4e5e04a0156d

diff --git a/cinder/tests/unit/test_vmware_datastore.py b/cinder/tests/unit/test_vmware_datastore.py
index d29eed61090127ac1db34b24804b31d0a3a8c5ac..56ee991bd8f838f8edcc2e2687075fc61419b5f6 100644 (file)
--- a/cinder/tests/unit/test_vmware_datastore.py
+++ b/cinder/tests/unit/test_vmware_datastore.py
@@ -432,3 +432,22 @@ class DatastoreTest(test.TestCase):
         get_profile_id_by_name.assert_called_once_with(self._session,
                                                        profile_name)
         filter_by_profile.assert_called_once_with([datastore], profile_id)
+
+    def test_get_all_hosts(self):
+        host_1 = self._create_host('host-1')
+        host_2 = self._create_host('host-2')
+        hosts = mock.Mock(objects=[mock.Mock(obj=host_1),
+                                   mock.Mock(obj=host_2)])
+
+        self._vops.get_hosts.return_value = hosts
+        self._vops.continue_retrieval.return_value = None
+        # host_1 is usable and host_2 is not usable
+        self._vops.is_host_usable.side_effect = [True, False]
+
+        ret = self._ds_sel._get_all_hosts()
+
+        self.assertEqual([host_1], ret)
+        self._vops.get_hosts.assert_called_once_with()
+        self._vops.continue_retrieval.assert_called_once_with(hosts)
+        exp_calls = [mock.call(host_1), mock.call(host_2)]
+        self.assertEqual(exp_calls, self._vops.is_host_usable.call_args_list)

diff --git a/cinder/tests/unit/test_vmware_vmdk.py b/cinder/tests/unit/test_vmware_vmdk.py
index f27031fed907cf29d576c8108852c3e7099092b7..a9d29ed62734203aecee1d803f45d18a8388e8e6 100644 (file)
--- a/cinder/tests/unit/test_vmware_vmdk.py
+++ b/cinder/tests/unit/test_vmware_vmdk.py
@@ -2658,8 +2658,26 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         delete_if_exists.assert_called_once_with(tmp)
 
     @mock.patch.object(VMDK_DRIVER, 'volumeops')
+    def test_get_hosts(self, vops):
+        host_1 = mock.sentinel.host_1
+        host_2 = mock.sentinel.host_2
+        host_3 = mock.sentinel.host_3
+        vops.get_cluster_hosts.side_effect = [[host_1, host_2], [host_3]]
+        # host_1 and host_3 are usable, host_2 is not usable
+        vops.is_host_usable.side_effect = [True, False, True]
+
+        cls_1 = mock.sentinel.cls_1
+        cls_2 = mock.sentinel.cls_2
+        self.assertEqual([host_1, host_3],
+                         self._driver._get_hosts([cls_1, cls_2]))
+        exp_calls = [mock.call(cls_1), mock.call(cls_2)]
+        self.assertEqual(exp_calls, vops.get_cluster_hosts.call_args_list)
+        exp_calls = [mock.call(host_1), mock.call(host_2), mock.call(host_3)]
+        self.assertEqual(exp_calls, vops.is_host_usable.call_args_list)
+
+    @mock.patch.object(VMDK_DRIVER, '_get_hosts')
     @mock.patch.object(VMDK_DRIVER, 'ds_sel')
-    def test_select_datastore(self, ds_sel, vops):
+    def test_select_datastore(self, ds_sel, get_hosts):
         cls_1 = mock.sentinel.cls_1
         cls_2 = mock.sentinel.cls_2
         self._driver._clusters = [cls_1, cls_2]
@@ -2667,23 +2685,20 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         host_1 = mock.sentinel.host_1
         host_2 = mock.sentinel.host_2
         host_3 = mock.sentinel.host_3
-        vops.get_cluster_hosts.side_effect = [[host_1, host_2], [host_3]]
+        get_hosts.return_value = [host_1, host_2, host_3]
 
         best_candidate = mock.sentinel.best_candidate
         ds_sel.select_datastore.return_value = best_candidate
 
         req = mock.sentinel.req
         self.assertEqual(best_candidate, self._driver._select_datastore(req))
-
-        exp_calls = [mock.call(cls_1), mock.call(cls_2)]
-        self.assertEqual(exp_calls, vops.get_cluster_hosts.call_args_list)
-
+        get_hosts.assert_called_once_with(self._driver._clusters)
         ds_sel.select_datastore.assert_called_once_with(
             req, hosts=[host_1, host_2, host_3])
 
-    @mock.patch.object(VMDK_DRIVER, 'volumeops')
+    @mock.patch.object(VMDK_DRIVER, '_get_hosts')
     @mock.patch.object(VMDK_DRIVER, 'ds_sel')
-    def test_select_datastore_with_no_best_candidate(self, ds_sel, vops):
+    def test_select_datastore_with_no_best_candidate(self, ds_sel, get_hosts):
         cls_1 = mock.sentinel.cls_1
         cls_2 = mock.sentinel.cls_2
         self._driver._clusters = [cls_1, cls_2]
@@ -2691,7 +2706,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         host_1 = mock.sentinel.host_1
         host_2 = mock.sentinel.host_2
         host_3 = mock.sentinel.host_3
-        vops.get_cluster_hosts.side_effect = [[host_1, host_2], [host_3]]
+        get_hosts.return_value = [host_1, host_2, host_3]
 
         ds_sel.select_datastore.return_value = ()
 
@@ -2699,35 +2714,26 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
         self.assertRaises(vmdk_exceptions.NoValidDatastoreException,
                           self._driver._select_datastore,
                           req)
-
-        exp_calls = [mock.call(cls_1), mock.call(cls_2)]
-        self.assertEqual(exp_calls, vops.get_cluster_hosts.call_args_list)
-
+        get_hosts.assert_called_once_with(self._driver._clusters)
         ds_sel.select_datastore.assert_called_once_with(
             req, hosts=[host_1, host_2, host_3])
 
-    @mock.patch.object(VMDK_DRIVER, 'volumeops')
+    @mock.patch.object(VMDK_DRIVER, '_get_hosts')
     @mock.patch.object(VMDK_DRIVER, 'ds_sel')
-    def test_select_datastore_with_single_host(self, ds_sel, vops):
-        cls_1 = mock.sentinel.cls_1
-        cls_2 = mock.sentinel.cls_2
-        self._driver._clusters = [cls_1, cls_2]
-
-        host_1 = mock.sentinel.host_1
-
+    def test_select_datastore_with_single_host(self, ds_sel, get_hosts):
         best_candidate = mock.sentinel.best_candidate
         ds_sel.select_datastore.return_value = best_candidate
 
         req = mock.sentinel.req
+        host_1 = mock.sentinel.host_1
         self.assertEqual(best_candidate,
                          self._driver._select_datastore(req, host_1))
-
         ds_sel.select_datastore.assert_called_once_with(req, hosts=[host_1])
-        self.assertFalse(vops.get_cluster_hosts.called)
+        self.assertFalse(get_hosts.called)
 
-    @mock.patch.object(VMDK_DRIVER, 'volumeops')
+    @mock.patch.object(VMDK_DRIVER, '_get_hosts')
     @mock.patch.object(VMDK_DRIVER, 'ds_sel')
-    def test_select_datastore_with_empty_clusters(self, ds_sel, vops):
+    def test_select_datastore_with_empty_clusters(self, ds_sel, get_hosts):
         self._driver._clusters = None
 
         best_candidate = mock.sentinel.best_candidate
@@ -2735,26 +2741,22 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase):
 
         req = mock.sentinel.req
         self.assertEqual(best_candidate, self._driver._select_datastore(req))
-
         ds_sel.select_datastore.assert_called_once_with(req, hosts=None)
-        self.assertFalse(vops.get_cluster_hosts.called)
+        self.assertFalse(get_hosts.called)
 
-    @mock.patch.object(VMDK_DRIVER, 'volumeops')
+    @mock.patch.object(VMDK_DRIVER, '_get_hosts')
     @mock.patch.object(VMDK_DRIVER, 'ds_sel')
-    def test_select_datastore_with_no_valid_host(self, ds_sel, vops):
+    def test_select_datastore_with_no_valid_host(self, ds_sel, get_hosts):
         cls_1 = mock.sentinel.cls_1
         cls_2 = mock.sentinel.cls_2
         self._driver._clusters = [cls_1, cls_2]
 
-        vops.get_cluster_hosts.side_effect = [[], []]
+        get_hosts.return_value = []
 
         req = mock.sentinel.req
         self.assertRaises(vmdk_exceptions.NoValidHostException,
                           self._driver._select_datastore, req)
-
-        exp_calls = [mock.call(cls_1), mock.call(cls_2)]
-        self.assertEqual(exp_calls, vops.get_cluster_hosts.call_args_list)
-
+        get_hosts.assert_called_once_with(self._driver._clusters)
         self.assertFalse(ds_sel.called)
 
     @mock.patch.object(VMDK_DRIVER, 'volumeops')

diff --git a/cinder/tests/unit/test_vmware_volumeops.py b/cinder/tests/unit/test_vmware_volumeops.py
index 903150ff1be1949deb9a25e2a1b79b8ec229fa55..f21712623cdf97c3de54f156b0734ab8769485ef 100644 (file)
--- a/cinder/tests/unit/test_vmware_volumeops.py
+++ b/cinder/tests/unit/test_vmware_volumeops.py
@@ -126,6 +126,37 @@ class VolumeOpsTestCase(test.TestCase):
                                                         instance,
                                                         'runtime.host')
 
+    def _host_runtime_info(
+            self, connection_state='connected', in_maintenance=False):
+        return mock.Mock(connectionState=connection_state,
+                         inMaintenanceMode=in_maintenance)
+
+    def test_is_host_usable(self):
+        self.session.invoke_api.return_value = self._host_runtime_info()
+
+        self.assertTrue(self.vops.is_host_usable(mock.sentinel.host))
+        self.session.invoke_api.assert_called_once_with(
+            vim_util, 'get_object_property', self.session.vim,
+            mock.sentinel.host, 'runtime')
+
+    def test_is_host_usable_with_disconnected_host(self):
+        self.session.invoke_api.return_value = self._host_runtime_info(
+            connection_state='disconnected')
+
+        self.assertFalse(self.vops.is_host_usable(mock.sentinel.host))
+        self.session.invoke_api.assert_called_once_with(
+            vim_util, 'get_object_property', self.session.vim,
+            mock.sentinel.host, 'runtime')
+
+    def test_is_host_usable_with_host_in_maintenance(self):
+        self.session.invoke_api.return_value = self._host_runtime_info(
+            in_maintenance=True)
+
+        self.assertFalse(self.vops.is_host_usable(mock.sentinel.host))
+        self.session.invoke_api.assert_called_once_with(
+            vim_util, 'get_object_property', self.session.vim,
+            mock.sentinel.host, 'runtime')
+
     def test_get_hosts(self):
         hosts = mock.sentinel.hosts
         self.session.invoke_api.return_value = hosts

diff --git a/cinder/volume/drivers/vmware/datastore.py b/cinder/volume/drivers/vmware/datastore.py
index 512b3b5b2ceebe050a81b20921b2f19ffc1dbebe..61d15d28e698e0a8a0f897f9bcbc8c12720edba5 100644 (file)
--- a/cinder/volume/drivers/vmware/datastore.py
+++ b/cinder/volume/drivers/vmware/datastore.py
@@ -131,7 +131,8 @@ class DatastoreSelector(object):
                 break
 
             for host in hosts:
-                all_hosts.append(host.obj)
+                if self._vops.is_host_usable(host.obj):
+                    all_hosts.append(host.obj)
             retrieve_result = self._vops.continue_retrieval(
                 retrieve_result)
         return all_hosts

diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py
index 39308ded4a7199ea64bef84c12e9282edf54253e..72de97cbad2d062a8ce648ad6b2d14a35f2d3163 100644 (file)
--- a/cinder/volume/drivers/vmware/vmdk.py
+++ b/cinder/volume/drivers/vmware/vmdk.py
@@ -486,7 +486,10 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver):
         hosts = []
         if clusters:
             for cluster in clusters:
-                hosts.extend(self.volumeops.get_cluster_hosts(cluster))
+                cluster_hosts = self.volumeops.get_cluster_hosts(cluster)
+                for host in cluster_hosts:
+                    if self.volumeops.is_host_usable(host):
+                        hosts.append(host)
         return hosts
 
     def _select_datastore(self, req, host=None):

diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py
index fb84b84a6f86eb3aead48639476eb9749163a3f4..9e8fd80dd9b03cd69c5e58767bfcc1397ccecec9 100644 (file)
--- a/cinder/volume/drivers/vmware/volumeops.py
+++ b/cinder/volume/drivers/vmware/volumeops.py
@@ -322,6 +322,23 @@ class VMwareVolumeOps(object):
                                         self._session.vim, instance,
                                         'runtime.host')
 
+    def is_host_usable(self, host):
+        """Check if the given ESX host is usable.
+
+        A host is usable if it is connected to vCenter server and not in
+        maintenance mode.
+
+        :param host: Managed object reference to the ESX host
+        :return: True if host is usable, False otherwise
+        """
+        runtime_info = self._session.invoke_api(vim_util,
+                                                'get_object_property',
+                                                self._session.vim,
+                                                host,
+                                                'runtime')
+        return (runtime_info.connectionState == 'connected' and
+                not runtime_info.inMaintenanceMode)
+
     def get_hosts(self):
         """Get all host from the inventory.