From ea987d3aba36177b0705c8a8c88365ebf4b40c17 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Wed, 26 Aug 2015 17:00:40 +0530 Subject: [PATCH] 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 --- cinder/tests/unit/test_vmware_datastore.py | 19 ++++++ cinder/tests/unit/test_vmware_vmdk.py | 70 +++++++++++----------- cinder/tests/unit/test_vmware_volumeops.py | 31 ++++++++++ cinder/volume/drivers/vmware/datastore.py | 3 +- cinder/volume/drivers/vmware/vmdk.py | 5 +- cinder/volume/drivers/vmware/volumeops.py | 17 ++++++ 6 files changed, 109 insertions(+), 36 deletions(-) diff --git a/cinder/tests/unit/test_vmware_datastore.py b/cinder/tests/unit/test_vmware_datastore.py index d29eed610..56ee991bd 100644 --- 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 f27031fed..a9d29ed62 100644 --- 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 903150ff1..f21712623 100644 --- 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 512b3b5b2..61d15d28e 100644 --- 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 39308ded4..72de97cba 100644 --- 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 fb84b84a6..9e8fd80dd 100644 --- 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. -- 2.45.2