From 331c40ce345968b40289a58accb129909137c4b4 Mon Sep 17 00:00:00 2001 From: Anthony Lee Date: Fri, 12 Dec 2014 11:32:15 -0800 Subject: [PATCH] Remove locks from LeftHand driver By removing synchronization locks from the LeftHand driver, support for many simultaneous volume create/deletes and volume attach/detachs will be added. This is useful for high availability environments where performance with locks in place would be reduced. Removing the locks requires the creation of a new LeftHand client when a request is sent to the driver now. This allows requests to no longer be blocked if a request is already in process by the driver. This results in a performance increase for the driver and better stability in a high availability environment. Closes-Bug: #1395953 Change-Id: I39732d944a79bbfc597077fe0873d4cfcaace211 --- cinder/tests/fake_hp_lefthand_client.py | 2 +- cinder/tests/test_hplefthand.py | 803 ++++++++++-------- .../drivers/san/hp/hp_lefthand_iscsi.py | 41 +- .../drivers/san/hp/hp_lefthand_rest_proxy.py | 152 ++-- 4 files changed, 593 insertions(+), 405 deletions(-) diff --git a/cinder/tests/fake_hp_lefthand_client.py b/cinder/tests/fake_hp_lefthand_client.py index 604cc888b..4dfa87d7e 100644 --- a/cinder/tests/fake_hp_lefthand_client.py +++ b/cinder/tests/fake_hp_lefthand_client.py @@ -22,7 +22,7 @@ import mock from cinder.tests import fake_hp_client_exceptions as hpexceptions hplefthand = mock.Mock() -hplefthand.version = "3.0.0" +hplefthand.version = "1.0.3" hplefthand.exceptions = hpexceptions sys.modules['hplefthandclient'] = hplefthand diff --git a/cinder/tests/test_hplefthand.py b/cinder/tests/test_hplefthand.py index 0e3010d7b..029436fd2 100644 --- a/cinder/tests/test_hplefthand.py +++ b/cinder/tests/test_hplefthand.py @@ -679,6 +679,17 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): self.driver.do_setup(None) return _mock_client.return_value + @mock.patch('hplefthandclient.version', "1.0.0") + def test_unsupported_client_version(self): + + self.assertRaises(exception.InvalidInput, + self.setup_driver) + + @mock.patch('hplefthandclient.version', "3.0.0") + def test_supported_client_version(self): + + self.setup_driver() + def test_create_volume(self): # setup drive with default configuration @@ -689,26 +700,33 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.createVolume.return_value = { 'iscsiIqn': self.connector['initiator']} - # execute driver - volume_info = self.driver.create_volume(self.volume) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + # execute driver + volume_info = self.driver.create_volume(self.volume) - self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', - volume_info['provider_location']) + self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', + volume_info['provider_location']) - expected = self.driver_startup_call_stack + [ - mock.call.createVolume( - 'fakevolume', - 1, - units.Gi, - {'isThinProvisioned': True, 'clusterName': 'CloudCluster1'})] + expected = self.driver_startup_call_stack + [ + mock.call.createVolume( + 'fakevolume', + 1, + units.Gi, + {'isThinProvisioned': True, + 'clusterName': 'CloudCluster1'}), + mock.call.logout()] - mock_client.assert_has_calls(expected) + mock_client.assert_has_calls(expected) - # mock HTTPServerError - mock_client.createVolume.side_effect = hpexceptions.HTTPServerError() - # ensure the raised exception is a cinder exception - self.assertRaises(exception.VolumeBackendAPIException, - self.driver.create_volume, self.volume) + # mock HTTPServerError + mock_client.createVolume.side_effect =\ + hpexceptions.HTTPServerError() + # ensure the raised exception is a cinder exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, self.volume) @mock.patch.object( volume_types, @@ -727,20 +745,26 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.createVolume.return_value = { 'iscsiIqn': self.connector['initiator']} - # execute creat_volume - volume_info = self.driver.create_volume(volume_with_vt) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + # execute create_volume + volume_info = self.driver.create_volume(volume_with_vt) - self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', - volume_info['provider_location']) + self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', + volume_info['provider_location']) - expected = self.driver_startup_call_stack + [ - mock.call.createVolume( - 'fakevolume', - 1, - units.Gi, - {'isThinProvisioned': False, 'clusterName': 'CloudCluster1'})] + expected = self.driver_startup_call_stack + [ + mock.call.createVolume( + 'fakevolume', + 1, + units.Gi, + {'isThinProvisioned': False, + 'clusterName': 'CloudCluster1'}), + mock.call.logout()] - mock_client.assert_has_calls(expected) + mock_client.assert_has_calls(expected) def test_delete_volume(self): @@ -751,25 +775,31 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): # mock return value of getVolumeByName mock_client.getVolumeByName.return_value = {'id': self.volume_id} - # execute delete_volume - self.driver.delete_volume(self.volume) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.deleteVolume(self.volume_id)] + # execute delete_volume + self.driver.delete_volume(self.volume) - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.deleteVolume(self.volume_id), + mock.call.logout()] - # mock HTTPNotFound (volume not found) - mock_client.getVolumeByName.side_effect = hpexceptions.HTTPNotFound() - # no exception should escape method - self.driver.delete_volume(self.volume) + mock_client.assert_has_calls(expected) - # mock HTTPConflict - mock_client.deleteVolume.side_effect = hpexceptions.HTTPConflict() - # ensure the raised exception is a cinder exception - self.assertRaises(exception.VolumeBackendAPIException, - self.driver.delete_volume, self.volume_id) + # mock HTTPNotFound (volume not found) + mock_client.getVolumeByName.side_effect =\ + hpexceptions.HTTPNotFound() + # no exception should escape method + self.driver.delete_volume(self.volume) + + # mock HTTPConflict + mock_client.deleteVolume.side_effect = hpexceptions.HTTPConflict() + # ensure the raised exception is a cinder exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.delete_volume, self.volume_id) def test_extend_volume(self): @@ -780,21 +810,27 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): # mock return value of getVolumeByName mock_client.getVolumeByName.return_value = {'id': self.volume_id} - # execute extend_volume - self.driver.extend_volume(self.volume, 2) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.modifyVolume(1, {'size': 2 * units.Gi})] + # execute extend_volume + self.driver.extend_volume(self.volume, 2) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.modifyVolume(1, {'size': 2 * units.Gi}), + mock.call.logout()] - # mock HTTPServerError (array failure) - mock_client.modifyVolume.side_effect = hpexceptions.HTTPServerError() - # ensure the raised exception is a cinder exception - self.assertRaises(exception.VolumeBackendAPIException, - self.driver.extend_volume, self.volume, 2) + # validate call chain + mock_client.assert_has_calls(expected) + + # mock HTTPServerError (array failure) + mock_client.modifyVolume.side_effect =\ + hpexceptions.HTTPServerError() + # ensure the raised exception is a cinder exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.extend_volume, self.volume, 2) def test_initialize_connection(self): @@ -810,37 +846,43 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): 'iscsiSessions': None } - # execute initialize_connection - result = self.driver.initialize_connection( - self.volume, - self.connector) - - # validate - self.assertEqual(result['driver_volume_type'], 'iscsi') - self.assertEqual(result['data']['target_discovered'], False) - self.assertEqual(result['data']['volume_id'], self.volume_id) - self.assertTrue('auth_method' not in result['data']) - - expected = self.driver_startup_call_stack + [ - mock.call.getServerByName('fakehost'), - mock.call.createServer - ( - 'fakehost', - 'iqn.1993-08.org.debian:01:222', - None - ), - mock.call.getVolumeByName('fakevolume'), - mock.call.addServerAccess(1, 0)] - - # validate call chain - mock_client.assert_has_calls(expected) - - # mock HTTPServerError (array failure) - mock_client.createServer.side_effect = hpexceptions.HTTPServerError() - # ensure the raised exception is a cinder exception - self.assertRaises( - exception.VolumeBackendAPIException, - self.driver.initialize_connection, self.volume, self.connector) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + # execute initialize_connection + result = self.driver.initialize_connection( + self.volume, + self.connector) + + # validate + self.assertEqual('iscsi', result['driver_volume_type']) + self.assertEqual(False, result['data']['target_discovered']) + self.assertEqual(self.volume_id, result['data']['volume_id']) + self.assertTrue('auth_method' not in result['data']) + + expected = self.driver_startup_call_stack + [ + mock.call.getServerByName('fakehost'), + mock.call.createServer + ( + 'fakehost', + 'iqn.1993-08.org.debian:01:222', + None + ), + mock.call.getVolumeByName('fakevolume'), + mock.call.addServerAccess(1, 0), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) + + # mock HTTPServerError (array failure) + mock_client.createServer.side_effect =\ + hpexceptions.HTTPServerError() + # ensure the raised exception is a cinder exception + self.assertRaises( + exception.VolumeBackendAPIException, + self.driver.initialize_connection, self.volume, self.connector) def test_initialize_connection_session_exists(self): @@ -856,29 +898,34 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): 'iscsiSessions': [{'server': {'uri': self.server_uri}}] } - # execute initialize_connection - result = self.driver.initialize_connection( - self.volume, - self.connector) - - # validate - self.assertEqual(result['driver_volume_type'], 'iscsi') - self.assertEqual(result['data']['target_discovered'], False) - self.assertEqual(result['data']['volume_id'], self.volume_id) - self.assertTrue('auth_method' not in result['data']) - - expected = self.driver_startup_call_stack + [ - mock.call.getServerByName('fakehost'), - mock.call.createServer - ( - 'fakehost', - 'iqn.1993-08.org.debian:01:222', - None - ), - mock.call.getVolumeByName('fakevolume')] - - # validate call chain - mock_client.assert_has_calls(expected) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + # execute initialize_connection + result = self.driver.initialize_connection( + self.volume, + self.connector) + + # validate + self.assertEqual('iscsi', result['driver_volume_type']) + self.assertEqual(False, result['data']['target_discovered']) + self.assertEqual(self.volume_id, result['data']['volume_id']) + self.assertTrue('auth_method' not in result['data']) + + expected = self.driver_startup_call_stack + [ + mock.call.getServerByName('fakehost'), + mock.call.createServer + ( + 'fakehost', + 'iqn.1993-08.org.debian:01:222', + None + ), + mock.call.getVolumeByName('fakevolume'), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_initialize_connection_with_chaps(self): @@ -897,30 +944,35 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): 'iscsiSessions': None } - # execute initialize_connection - result = self.driver.initialize_connection( - self.volume, - self.connector) - - # validate - self.assertEqual(result['driver_volume_type'], 'iscsi') - self.assertEqual(result['data']['target_discovered'], False) - self.assertEqual(result['data']['volume_id'], self.volume_id) - self.assertEqual(result['data']['auth_method'], 'CHAP') - - expected = self.driver_startup_call_stack + [ - mock.call.getServerByName('fakehost'), - mock.call.createServer - ( - 'fakehost', - 'iqn.1993-08.org.debian:01:222', - None - ), - mock.call.getVolumeByName('fakevolume'), - mock.call.addServerAccess(1, 0)] - - # validate call chain - mock_client.assert_has_calls(expected) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + # execute initialize_connection + result = self.driver.initialize_connection( + self.volume, + self.connector) + + # validate + self.assertEqual('iscsi', result['driver_volume_type']) + self.assertEqual(False, result['data']['target_discovered']) + self.assertEqual(self.volume_id, result['data']['volume_id']) + self.assertEqual('CHAP', result['data']['auth_method']) + + expected = self.driver_startup_call_stack + [ + mock.call.getServerByName('fakehost'), + mock.call.createServer + ( + 'fakehost', + 'iqn.1993-08.org.debian:01:222', + None + ), + mock.call.getVolumeByName('fakevolume'), + mock.call.addServerAccess(1, 0), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_terminate_connection(self): @@ -931,24 +983,30 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getVolumeByName.return_value = {'id': self.volume_id} mock_client.getServerByName.return_value = {'id': self.server_id} - # execute terminate_connection - self.driver.terminate_connection(self.volume, self.connector) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.getServerByName('fakehost'), - mock.call.removeServerAccess(1, 0)] + # execute terminate_connection + self.driver.terminate_connection(self.volume, self.connector) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.getServerByName('fakehost'), + mock.call.removeServerAccess(1, 0), + mock.call.logout()] - mock_client.getVolumeByName.side_effect = hpexceptions.HTTPNotFound() - # ensure the raised exception is a cinder exception - self.assertRaises( - exception.VolumeBackendAPIException, - self.driver.terminate_connection, - self.volume, - self.connector) + # validate call chain + mock_client.assert_has_calls(expected) + + mock_client.getVolumeByName.side_effect =\ + hpexceptions.HTTPNotFound() + # ensure the raised exception is a cinder exception + self.assertRaises( + exception.VolumeBackendAPIException, + self.driver.terminate_connection, + self.volume, + self.connector) def test_create_snapshot(self): @@ -958,25 +1016,31 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getVolumeByName.return_value = {'id': self.volume_id} - # execute create_snapshot - self.driver.create_snapshot(self.snapshot) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.createSnapshot( - 'fakeshapshot', - 1, - {'inheritAccess': True})] + # execute create_snapshot + self.driver.create_snapshot(self.snapshot) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.createSnapshot( + 'fakeshapshot', + 1, + {'inheritAccess': True}), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) - # mock HTTPServerError (array failure) - mock_client.getVolumeByName.side_effect = hpexceptions.HTTPNotFound() - # ensure the raised exception is a cinder exception - self.assertRaises( - exception.VolumeBackendAPIException, - self.driver.create_snapshot, self.snapshot) + # mock HTTPServerError (array failure) + mock_client.getVolumeByName.side_effect =\ + hpexceptions.HTTPNotFound() + # ensure the raised exception is a cinder exception + self.assertRaises( + exception.VolumeBackendAPIException, + self.driver.create_snapshot, self.snapshot) def test_delete_snapshot(self): @@ -986,39 +1050,46 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getSnapshotByName.return_value = {'id': self.snapshot_id} - # execute delete_snapshot - self.driver.delete_snapshot(self.snapshot) - - expected = self.driver_startup_call_stack + [ - mock.call.getSnapshotByName('fakeshapshot'), - mock.call.deleteSnapshot(3)] - - # validate call chain - mock_client.assert_has_calls(expected) - - mock_client.getSnapshotByName.side_effect = hpexceptions.HTTPNotFound() - # no exception is thrown, just error msg is logged - self.driver.delete_snapshot(self.snapshot) - - # mock HTTPServerError (array failure) - ex = hpexceptions.HTTPServerError({'message': 'Some message.'}) - mock_client.getSnapshotByName.side_effect = ex - # ensure the raised exception is a cinder exception - self.assertRaises( - exception.VolumeBackendAPIException, - self.driver.delete_snapshot, - self.snapshot) - - # mock HTTPServerError because the snap is in use - ex = hpexceptions.HTTPServerError({ - 'message': - 'Hey, dude cannot be deleted because it is a clone point duh.'}) - mock_client.getSnapshotByName.side_effect = ex - # ensure the raised exception is a cinder exception - self.assertRaises( - exception.SnapshotIsBusy, - self.driver.delete_snapshot, - self.snapshot) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + # execute delete_snapshot + self.driver.delete_snapshot(self.snapshot) + + expected = self.driver_startup_call_stack + [ + mock.call.getSnapshotByName('fakeshapshot'), + mock.call.deleteSnapshot(3), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) + + mock_client.getSnapshotByName.side_effect =\ + hpexceptions.HTTPNotFound() + # no exception is thrown, just error msg is logged + self.driver.delete_snapshot(self.snapshot) + + # mock HTTPServerError (array failure) + ex = hpexceptions.HTTPServerError({'message': 'Some message.'}) + mock_client.getSnapshotByName.side_effect = ex + # ensure the raised exception is a cinder exception + self.assertRaises( + exception.VolumeBackendAPIException, + self.driver.delete_snapshot, + self.snapshot) + + # mock HTTPServerError because the snap is in use + ex = hpexceptions.HTTPServerError({ + 'message': + 'Hey, dude cannot be deleted because it is a clone point' + ' duh.'}) + mock_client.getSnapshotByName.side_effect = ex + # ensure the raised exception is a cinder exception + self.assertRaises( + exception.SnapshotIsBusy, + self.driver.delete_snapshot, + self.snapshot) def test_create_volume_from_snapshot(self): @@ -1030,20 +1101,26 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.cloneSnapshot.return_value = { 'iscsiIqn': self.connector['initiator']} - # execute create_volume_from_snapshot - model_update = self.driver.create_volume_from_snapshot( - self.volume, self.snapshot) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected_iqn = 'iqn.1993-08.org.debian:01:222 0' - expected_location = "10.0.1.6:3260,1 %s" % expected_iqn - self.assertEqual(model_update['provider_location'], expected_location) + # execute create_volume_from_snapshot + model_update = self.driver.create_volume_from_snapshot( + self.volume, self.snapshot) - expected = self.driver_startup_call_stack + [ - mock.call.getSnapshotByName('fakeshapshot'), - mock.call.cloneSnapshot('fakevolume', 3)] + expected_iqn = 'iqn.1993-08.org.debian:01:222 0' + expected_location = "10.0.1.6:3260,1 %s" % expected_iqn + self.assertEqual(expected_location, + model_update['provider_location']) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getSnapshotByName('fakeshapshot'), + mock.call.cloneSnapshot('fakevolume', 3), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_create_cloned_volume(self): @@ -1053,16 +1130,21 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getVolumeByName.return_value = {'id': self.volume_id} - # execute create_cloned_volume - self.driver.create_cloned_volume( - self.cloned_volume, self.volume) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.cloneVolume('clone_volume', 1)] + # execute create_cloned_volume + self.driver.create_cloned_volume( + self.cloned_volume, self.volume) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.cloneVolume('clone_volume', 1), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) @mock.patch.object(volume_types, 'get_volume_type') def test_extra_spec_mapping(self, _mock_get_volume_type): @@ -1147,13 +1229,18 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): volume['host'] = host new_type = volume_types.get_volume_type(ctxt, new_type_ref['id']) - self.driver.retype(ctxt, volume, new_type, diff, host) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume')] + self.driver.retype(ctxt, volume, new_type, diff, host) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_retype_with_only_LH_extra_specs(self): # setup drive with default configuration @@ -1178,17 +1265,22 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): volume['host'] = host new_type = volume_types.get_volume_type(ctxt, new_type_ref['id']) - self.driver.retype(ctxt, volume, new_type, diff, host) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.modifyVolume( - 1, { - 'isThinProvisioned': False, - 'isAdaptiveOptimizationEnabled': True})] + self.driver.retype(ctxt, volume, new_type, diff, host) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.modifyVolume( + 1, { + 'isThinProvisioned': False, + 'isAdaptiveOptimizationEnabled': True}), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_retype_with_both_extra_specs(self): # setup drive with default configuration @@ -1213,14 +1305,19 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): volume['host'] = host new_type = volume_types.get_volume_type(ctxt, new_type_ref['id']) - self.driver.retype(ctxt, volume, new_type, diff, host) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.modifyVolume(1, {'isThinProvisioned': True})] + self.driver.retype(ctxt, volume, new_type, diff, host) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.modifyVolume(1, {'isThinProvisioned': True}), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_retype_same_extra_specs(self): # setup drive with default configuration @@ -1245,16 +1342,21 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): volume['host'] = host new_type = volume_types.get_volume_type(ctxt, new_type_ref['id']) - self.driver.retype(ctxt, volume, new_type, diff, host) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - expected = self.driver_startup_call_stack + [ - mock.call.getVolumeByName('fakevolume'), - mock.call.modifyVolume( - 1, - {'isAdaptiveOptimizationEnabled': False})] + self.driver.retype(ctxt, volume, new_type, diff, host) - # validate call chain - mock_client.assert_has_calls(expected) + expected = self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.modifyVolume( + 1, + {'isAdaptiveOptimizationEnabled': False}), + mock.call.logout()] + + # validate call chain + mock_client.assert_has_calls(expected) def test_migrate_no_location(self): # setup drive with default configuration @@ -1262,18 +1364,19 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client = self.setup_driver() host = {'host': self.serverName, 'capabilities': {}} - (migrated, update) = self.driver.migrate_volume( - None, - self.volume, - host) - self.assertFalse(migrated) - - # only startup code is called - mock_client.assert_has_calls(self.driver_startup_call_stack) - # and nothing else - self.assertEqual( - len(self.driver_startup_call_stack), - len(mock_client.method_calls)) + + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + (migrated, update) = self.driver.migrate_volume( + None, + self.volume, + host) + self.assertFalse(migrated) + + mock_client.assert_has_calls([]) + self.assertEqual(0, len(mock_client.method_calls)) def test_migrate_incorrect_vip(self): # setup drive with default configuration @@ -1282,7 +1385,8 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getClusterByName.return_value = { "virtualIPAddresses": [{ "ipV4Address": "10.10.10.10", - "ipV4NetMask": "255.255.240.0"}]} + "ipV4NetMask": "255.255.240.0"}], + "id": self.cluster_id} mock_client.getVolumeByName.return_value = {'id': self.volume_id} @@ -1293,20 +1397,26 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): host = { 'host': self.serverName, 'capabilities': {'location_info': location}} - (migrated, update) = self.driver.migrate_volume( - None, - self.volume, - host) - self.assertFalse(migrated) - expected = self.driver_startup_call_stack + [ - mock.call.getClusterByName('New_CloudCluster')] + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client - mock_client.assert_has_calls(expected) - # and nothing else - self.assertEqual( - len(expected), - len(mock_client.method_calls)) + (migrated, update) = self.driver.migrate_volume( + None, + self.volume, + host) + self.assertFalse(migrated) + + expected = self.driver_startup_call_stack + [ + mock.call.getClusterByName('New_CloudCluster'), + mock.call.logout()] + + mock_client.assert_has_calls(expected) + # and nothing else + self.assertEqual( + len(expected), + len(mock_client.method_calls)) def test_migrate_with_location(self): # setup drive with default configuration @@ -1315,7 +1425,8 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getClusterByName.return_value = { "virtualIPAddresses": [{ "ipV4Address": "10.10.10.111", - "ipV4NetMask": "255.255.240.0"}]} + "ipV4NetMask": "255.255.240.0"}], + "id": self.cluster_id} mock_client.getVolumeByName.return_value = {'id': self.volume_id, 'iscsiSessions': None} @@ -1329,25 +1440,32 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): host = { 'host': self.serverName, 'capabilities': {'location_info': location}} - (migrated, update) = self.driver.migrate_volume( - None, - self.volume, - host) - self.assertTrue(migrated) - - expected = self.driver_startup_call_stack + [ - mock.call.getClusterByName('New_CloudCluster'), - mock.call.getVolumeByName('fakevolume'), - mock.call.getVolume( - 1, - 'fields=snapshots,snapshots[resource[members[name]]]'), - mock.call.modifyVolume(1, {'clusterName': 'New_CloudCluster'})] - - mock_client.assert_has_calls(expected) - # and nothing else - self.assertEqual( - len(expected), - len(mock_client.method_calls)) + + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + (migrated, update) = self.driver.migrate_volume( + None, + self.volume, + host) + self.assertTrue(migrated) + + expected = self.driver_startup_call_stack + [ + mock.call.getClusterByName('New_CloudCluster'), + mock.call.logout()] + self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.getVolume( + 1, + 'fields=snapshots,snapshots[resource[members[name]]]'), + mock.call.modifyVolume(1, {'clusterName': 'New_CloudCluster'}), + mock.call.logout()] + + mock_client.assert_has_calls(expected) + # and nothing else + self.assertEqual( + len(expected), + len(mock_client.method_calls)) def test_migrate_with_Snapshots(self): # setup drive with default configuration @@ -1356,7 +1474,8 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.getClusterByName.return_value = { "virtualIPAddresses": [{ "ipV4Address": "10.10.10.111", - "ipV4NetMask": "255.255.240.0"}]} + "ipV4NetMask": "255.255.240.0"}], + "id": self.cluster_id} mock_client.getVolumeByName.return_value = { 'id': self.volume_id, @@ -1371,24 +1490,31 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): host = { 'host': self.serverName, 'capabilities': {'location_info': location}} - (migrated, update) = self.driver.migrate_volume( - None, - self.volume, - host) - self.assertFalse(migrated) - - expected = self.driver_startup_call_stack + [ - mock.call.getClusterByName('New_CloudCluster'), - mock.call.getVolumeByName('fakevolume'), - mock.call.getVolume( - 1, - 'fields=snapshots,snapshots[resource[members[name]]]')] - - mock_client.assert_has_calls(expected) - # and nothing else - self.assertEqual( - len(expected), - len(mock_client.method_calls)) + + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + (migrated, update) = self.driver.migrate_volume( + None, + self.volume, + host) + self.assertFalse(migrated) + + expected = self.driver_startup_call_stack + [ + mock.call.getClusterByName('New_CloudCluster'), + mock.call.logout()] + self.driver_startup_call_stack + [ + mock.call.getVolumeByName('fakevolume'), + mock.call.getVolume( + 1, + 'fields=snapshots,snapshots[resource[members[name]]]'), + mock.call.logout()] + + mock_client.assert_has_calls(expected) + # and nothing else + self.assertEqual( + len(expected), + len(mock_client.method_calls)) @mock.patch.object(volume_types, 'get_volume_type', return_value={'extra_specs': {'hplh:ao': 'true'}}) @@ -1405,21 +1531,27 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.createVolume.return_value = { 'iscsiIqn': self.connector['initiator']} - volume_info = self.driver.create_volume(volume_with_vt) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + volume_info = self.driver.create_volume(volume_with_vt) - self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', - volume_info['provider_location']) + self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', + volume_info['provider_location']) - # make sure createVolume is called without - # isAdaptiveOptimizationEnabled == true - expected = self.driver_startup_call_stack + [ - mock.call.createVolume( - 'fakevolume', - 1, - units.Gi, - {'isThinProvisioned': True, 'clusterName': 'CloudCluster1'})] + # make sure createVolume is called without + # isAdaptiveOptimizationEnabled == true + expected = self.driver_startup_call_stack + [ + mock.call.createVolume( + 'fakevolume', + 1, + units.Gi, + {'isThinProvisioned': True, + 'clusterName': 'CloudCluster1'}), + mock.call.logout()] - mock_client.assert_has_calls(expected) + mock_client.assert_has_calls(expected) @mock.patch.object(volume_types, 'get_volume_type', return_value={'extra_specs': {'hplh:ao': 'false'}}) @@ -1436,20 +1568,25 @@ class TestHPLeftHandRESTISCSIDriver(HPLeftHandBaseDriver, test.TestCase): mock_client.createVolume.return_value = { 'iscsiIqn': self.connector['initiator']} - volume_info = self.driver.create_volume(volume_with_vt) + with mock.patch.object(hp_lefthand_rest_proxy.HPLeftHandRESTProxy, + '_create_client') as mock_do_setup: + mock_do_setup.return_value = mock_client + + volume_info = self.driver.create_volume(volume_with_vt) - self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', - volume_info['provider_location']) + self.assertEqual('10.0.1.6:3260,1 iqn.1993-08.org.debian:01:222 0', + volume_info['provider_location']) - # make sure createVolume is called with - # isAdaptiveOptimizationEnabled == false - expected = self.driver_startup_call_stack + [ - mock.call.createVolume( - 'fakevolume', - 1, - units.Gi, - {'isThinProvisioned': True, - 'clusterName': 'CloudCluster1', - 'isAdaptiveOptimizationEnabled': False})] + # make sure createVolume is called with + # isAdaptiveOptimizationEnabled == false + expected = self.driver_startup_call_stack + [ + mock.call.createVolume( + 'fakevolume', + 1, + units.Gi, + {'isThinProvisioned': True, + 'clusterName': 'CloudCluster1', + 'isAdaptiveOptimizationEnabled': False}), + mock.call.logout()] - mock_client.assert_has_calls(expected) + mock_client.assert_has_calls(expected) diff --git a/cinder/volume/drivers/san/hp/hp_lefthand_iscsi.py b/cinder/volume/drivers/san/hp/hp_lefthand_iscsi.py index 430dffefa..342d2c57e 100644 --- a/cinder/volume/drivers/san/hp/hp_lefthand_iscsi.py +++ b/cinder/volume/drivers/san/hp/hp_lefthand_iscsi.py @@ -32,15 +32,16 @@ hplefthand_password for credentials to talk to the REST service on the LeftHand array. """ from cinder import exception -from cinder.i18n import _LI +from cinder.i18n import _LE, _LI from cinder.openstack.common import log as logging -from cinder import utils from cinder.volume.driver import VolumeDriver from cinder.volume.drivers.san.hp import hp_lefthand_cliq_proxy as cliq_proxy from cinder.volume.drivers.san.hp import hp_lefthand_rest_proxy as rest_proxy LOG = logging.getLogger(__name__) +MIN_CLIENT_VERSION = '1.0.3' + class HPLeftHandISCSIDriver(VolumeDriver): """Executes commands relating to HP/LeftHand SAN ISCSI volumes. @@ -50,9 +51,10 @@ class HPLeftHandISCSIDriver(VolumeDriver): 1.0.1 - Added support for retype 1.0.2 - Added support for volume migrate 1.0.3 - Fix for no handler for logger during tests + 1.0.4 - Removing locks bug #1395953 """ - VERSION = "1.0.3" + VERSION = "1.0.4" def __init__(self, *args, **kwargs): super(HPLeftHandISCSIDriver, self).__init__(*args, **kwargs) @@ -68,88 +70,85 @@ class HPLeftHandISCSIDriver(VolumeDriver): return proxy - @utils.synchronized('lefthand', external=True) def check_for_setup_error(self): self.proxy.check_for_setup_error() - @utils.synchronized('lefthand', external=True) def do_setup(self, context): self.proxy = self._create_proxy(*self.args, **self.kwargs) - self.proxy.do_setup(context) LOG.info(_LI("HPLeftHand driver %(driver_ver)s, " "proxy %(proxy_ver)s") % { "driver_ver": self.VERSION, "proxy_ver": self.proxy.get_version_string()}) - @utils.synchronized('lefthand', external=True) + if isinstance(self.proxy, cliq_proxy.HPLeftHandCLIQProxy): + self.proxy.do_setup(context) + else: + # Check minimum client version for REST proxy + client_version = rest_proxy.hplefthandclient.version + + if (client_version < MIN_CLIENT_VERSION): + ex_msg = (_LE("Invalid hplefthandclient version found (" + "%(found)s). Version %(minimum)s or greater " + "required.") + % {'found': client_version, + 'minimum': MIN_CLIENT_VERSION}) + LOG.error(ex_msg) + raise exception.InvalidInput(reason=ex_msg) + def create_volume(self, volume): """Creates a volume.""" return self.proxy.create_volume(volume) - @utils.synchronized('lefthand', external=True) def extend_volume(self, volume, new_size): """Extend the size of an existing volume.""" self.proxy.extend_volume(volume, new_size) - @utils.synchronized('lefthand', external=True) def create_volume_from_snapshot(self, volume, snapshot): """Creates a volume from a snapshot.""" return self.proxy.create_volume_from_snapshot(volume, snapshot) - @utils.synchronized('lefthand', external=True) def create_snapshot(self, snapshot): """Creates a snapshot.""" self.proxy.create_snapshot(snapshot) - @utils.synchronized('lefthand', external=True) def delete_volume(self, volume): """Deletes a volume.""" self.proxy.delete_volume(volume) - @utils.synchronized('lefthand', external=True) def delete_snapshot(self, snapshot): """Deletes a snapshot.""" self.proxy.delete_snapshot(snapshot) - @utils.synchronized('lefthand', external=True) def initialize_connection(self, volume, connector): """Assigns the volume to a server.""" return self.proxy.initialize_connection(volume, connector) - @utils.synchronized('lefthand', external=True) def terminate_connection(self, volume, connector, **kwargs): """Unassign the volume from the host.""" self.proxy.terminate_connection(volume, connector) - @utils.synchronized('lefthand', external=True) def get_volume_stats(self, refresh): data = self.proxy.get_volume_stats(refresh) data['driver_version'] = self.VERSION return data - @utils.synchronized('lefthand', external=True) def create_cloned_volume(self, volume, src_vref): return self.proxy.create_cloned_volume(volume, src_vref) - @utils.synchronized('lefthand', external=True) def create_export(self, context, volume): return self.proxy.create_export(context, volume) - @utils.synchronized('lefthand', external=True) def ensure_export(self, context, volume): return self.proxy.ensure_export(context, volume) - @utils.synchronized('lefthand', external=True) def remove_export(self, context, volume): return self.proxy.remove_export(context, volume) - @utils.synchronized('lefthand', external=True) def retype(self, context, volume, new_type, diff, host): """Convert the volume to be of the new type.""" return self.proxy.retype(context, volume, new_type, diff, host) - @utils.synchronized('lefthand', external=True) def migrate_volume(self, ctxt, volume, host): """Migrate directly if source and dest are managed by same storage.""" return self.proxy.migrate_volume(ctxt, volume, host) diff --git a/cinder/volume/drivers/san/hp/hp_lefthand_rest_proxy.py b/cinder/volume/drivers/san/hp/hp_lefthand_rest_proxy.py index 3f0071df0..f295d8255 100644 --- a/cinder/volume/drivers/san/hp/hp_lefthand_rest_proxy.py +++ b/cinder/volume/drivers/san/hp/hp_lefthand_rest_proxy.py @@ -30,7 +30,7 @@ LOG = logging.getLogger(__name__) try: import hplefthandclient - from hplefthandclient import client + from hplefthandclient import client as hp_lh_client from hplefthandclient import exceptions as hpexceptions except ImportError: import cinder.tests.fake_hp_lefthand_client as hplefthandclient @@ -93,9 +93,10 @@ class HPLeftHandRESTProxy(ISCSIDriver): improvement 1.0.5 - Fixed bug #1311350, Live-migration of an instance when attached to a volume was causing an error. + 1.0.6 - Removing locks bug #1395953 """ - VERSION = "1.0.5" + VERSION = "1.0.6" device_stats = {} @@ -109,24 +110,36 @@ class HPLeftHandRESTProxy(ISCSIDriver): # so we need to use it as a separator self.DRIVER_LOCATION = self.__class__.__name__ + ' %(cluster)s %(vip)s' + def _login(self): + client = self.do_setup(None) + return client + + def _logout(self, client): + client.logout() + + def _create_client(self): + return hp_lh_client.HPLeftHandClient( + self.configuration.hplefthand_api_url) + def do_setup(self, context): """Set up LeftHand client.""" try: - self.client = client.HPLeftHandClient( - self.configuration.hplefthand_api_url) - self.client.login( + client = self._create_client() + client.login( self.configuration.hplefthand_username, self.configuration.hplefthand_password) if self.configuration.hplefthand_debug: - self.client.debug_rest(True) + client.debug_rest(True) - cluster_info = self.client.getClusterByName( + cluster_info = client.getClusterByName( self.configuration.hplefthand_clustername) self.cluster_id = cluster_info['id'] virtual_ips = cluster_info['virtualIPAddresses'] self.cluster_vip = virtual_ips[0]['ipV4Address'] - self._update_backend_status() + self._update_backend_status(client) + + return client except hpexceptions.HTTPNotFound: raise exception.DriverNotInitialized( _('LeftHand cluster not found')) @@ -143,6 +156,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): def create_volume(self, volume): """Creates a volume.""" + client = self._login() try: # get the extra specs of interest from this volume's volume type volume_extra_specs = self._get_volume_extra_specs(volume) @@ -170,7 +184,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): clusterName = self.configuration.hplefthand_clustername optional['clusterName'] = clusterName - volume_info = self.client.createVolume( + volume_info = client.createVolume( volume['name'], self.cluster_id, volume['size'] * units.Gi, optional) @@ -178,45 +192,57 @@ class HPLeftHandRESTProxy(ISCSIDriver): return self._update_provider(volume_info) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def delete_volume(self, volume): """Deletes a volume.""" + client = self._login() try: - volume_info = self.client.getVolumeByName(volume['name']) - self.client.deleteVolume(volume_info['id']) + volume_info = client.getVolumeByName(volume['name']) + client.deleteVolume(volume_info['id']) except hpexceptions.HTTPNotFound: LOG.error(_LE("Volume did not exist. It will not be deleted")) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def extend_volume(self, volume, new_size): """Extend the size of an existing volume.""" + client = self._login() try: - volume_info = self.client.getVolumeByName(volume['name']) + volume_info = client.getVolumeByName(volume['name']) # convert GB to bytes options = {'size': int(new_size) * units.Gi} - self.client.modifyVolume(volume_info['id'], options) + client.modifyVolume(volume_info['id'], options) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def create_snapshot(self, snapshot): """Creates a snapshot.""" + client = self._login() try: - volume_info = self.client.getVolumeByName(snapshot['volume_name']) + volume_info = client.getVolumeByName(snapshot['volume_name']) option = {'inheritAccess': True} - self.client.createSnapshot(snapshot['name'], - volume_info['id'], - option) + client.createSnapshot(snapshot['name'], + volume_info['id'], + option) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def delete_snapshot(self, snapshot): """Deletes a snapshot.""" + client = self._login() try: - snap_info = self.client.getSnapshotByName(snapshot['name']) - self.client.deleteSnapshot(snap_info['id']) + snap_info = client.getSnapshotByName(snapshot['name']) + client.deleteSnapshot(snap_info['id']) except hpexceptions.HTTPNotFound: LOG.error(_LE("Snapshot did not exist. It will not be deleted")) except hpexceptions.HTTPServerError as ex: @@ -228,15 +254,21 @@ class HPLeftHandRESTProxy(ISCSIDriver): except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def get_volume_stats(self, refresh): """Gets volume stats.""" - if refresh: - self._update_backend_status() + client = self._login() + try: + if refresh: + self._update_backend_status(client) - return self.device_stats + return self.device_stats + finally: + self._logout(client) - def _update_backend_status(self): + def _update_backend_status(self, client): data = {} backend_name = self.configuration.safe_get('volume_backend_name') data['volume_backend_name'] = backend_name or self.__class__.__name__ @@ -247,7 +279,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): 'cluster': self.configuration.hplefthand_clustername, 'vip': self.cluster_vip}) - cluster_info = self.client.getCluster(self.cluster_id) + cluster_info = client.getCluster(self.cluster_id) total_capacity = cluster_info['spaceTotal'] free_capacity = cluster_info['spaceAvailable'] @@ -265,9 +297,10 @@ class HPLeftHandRESTProxy(ISCSIDriver): used from that host. HP VSA requires a volume to be assigned to a server. """ + client = self._login() try: - server_info = self._create_server(connector) - volume_info = self.client.getVolumeByName(volume['name']) + server_info = self._create_server(connector, client) + volume_info = client.getVolumeByName(volume['name']) access_already_enabled = False if volume_info['iscsiSessions'] is not None: @@ -280,7 +313,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): break if not access_already_enabled: - self.client.addServerAccess( + client.addServerAccess( volume_info['id'], server_info['id']) @@ -296,35 +329,46 @@ class HPLeftHandRESTProxy(ISCSIDriver): return {'driver_volume_type': 'iscsi', 'data': iscsi_properties} except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def terminate_connection(self, volume, connector, **kwargs): """Unassign the volume from the host.""" + client = self._login() try: - volume_info = self.client.getVolumeByName(volume['name']) - server_info = self.client.getServerByName(connector['host']) - self.client.removeServerAccess( + volume_info = client.getVolumeByName(volume['name']) + server_info = client.getServerByName(connector['host']) + client.removeServerAccess( volume_info['id'], server_info['id']) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def create_volume_from_snapshot(self, volume, snapshot): """Creates a volume from a snapshot.""" + client = self._login() try: - snap_info = self.client.getSnapshotByName(snapshot['name']) - volume_info = self.client.cloneSnapshot( + snap_info = client.getSnapshotByName(snapshot['name']) + volume_info = client.cloneSnapshot( volume['name'], snap_info['id']) return self._update_provider(volume_info) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def create_cloned_volume(self, volume, src_vref): + client = self._login() try: - volume_info = self.client.getVolumeByName(src_vref['name']) - self.client.cloneVolume(volume['name'], volume_info['id']) + volume_info = client.getVolumeByName(src_vref['name']) + client.cloneVolume(volume['name'], volume_info['id']) except Exception as ex: raise exception.VolumeBackendAPIException(ex) + finally: + self._logout(client) def _get_volume_extra_specs(self, volume): """Get extra specs from a volume.""" @@ -370,11 +414,11 @@ class HPLeftHandRESTProxy(ISCSIDriver): return {'provider_location': ( "%s %s %s" % (iscsi_portal, volume_info['iscsiIqn'], 0))} - def _create_server(self, connector): + def _create_server(self, connector, client): server_info = None chap_enabled = self.configuration.hplefthand_iscsi_chap_enabled try: - server_info = self.client.getServerByName(connector['host']) + server_info = client.getServerByName(connector['host']) chap_secret = server_info['chapTargetSecret'] if not chap_enabled and chap_secret: LOG.warning(_LW('CHAP secret exists for host %s but CHAP is ' @@ -394,9 +438,10 @@ class HPLeftHandRESTProxy(ISCSIDriver): 'chapTargetSecret': chap_secret, 'chapAuthenticationRequired': True } - server_info = self.client.createServer(connector['host'], - connector['initiator'], - optional) + + server_info = client.createServer(connector['host'], + connector['initiator'], + optional) return server_info def create_export(self, context, volume): @@ -426,12 +471,10 @@ class HPLeftHandRESTProxy(ISCSIDriver): 'new_type': new_type, 'diff': diff, 'host': host}) + client = self._login() try: - volume_info = self.client.getVolumeByName(volume['name']) - except hpexceptions.HTTPNotFound: - raise exception.VolumeNotFound(volume_id=volume['id']) + volume_info = client.getVolumeByName(volume['name']) - try: # pick out the LH extra specs new_extra_specs = dict(new_type).get('extra_specs') lh_extra_specs = self._get_lh_extra_specs( @@ -450,11 +493,14 @@ class HPLeftHandRESTProxy(ISCSIDriver): # map extra specs to LeftHand options options = self._map_extra_specs(changed_extra_specs) if len(options) > 0: - self.client.modifyVolume(volume_info['id'], options) + client.modifyVolume(volume_info['id'], options) return True - + except hpexceptions.HTTPNotFound: + raise exception.VolumeNotFound(volume_id=volume['id']) except Exception as ex: LOG.warning("%s" % ex) + finally: + self._logout(client) return False @@ -491,10 +537,11 @@ class HPLeftHandRESTProxy(ISCSIDriver): host_location = host['capabilities']['location_info'] (driver, cluster, vip) = host_location.split(' ') + client = self._login() try: # get the cluster info, if it exists and compare - cluster_info = self.client.getClusterByName(cluster) - LOG.debug('Clister info: %s' % cluster_info) + cluster_info = client.getClusterByName(cluster) + LOG.debug('Cluster info: %s' % cluster_info) virtual_ips = cluster_info['virtualIPAddresses'] if driver != self.__class__.__name__: @@ -513,9 +560,12 @@ class HPLeftHandRESTProxy(ISCSIDriver): "volume: %s because cluster exists in different " "management group.") % volume['name']) return false_ret + finally: + self._logout(client) + client = self._login() try: - volume_info = self.client.getVolumeByName(volume['name']) + volume_info = client.getVolumeByName(volume['name']) LOG.debug('Volume info: %s' % volume_info) # can't migrate if server is attached @@ -526,7 +576,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): return false_ret # can't migrate if volume has snapshots - snap_info = self.client.getVolume( + snap_info = client.getVolume( volume_info['id'], 'fields=snapshots,snapshots[resource[members[name]]]') LOG.debug('Snapshot info: %s' % snap_info) @@ -537,7 +587,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): return false_ret options = {'clusterName': cluster} - self.client.modifyVolume(volume_info['id'], options) + client.modifyVolume(volume_info['id'], options) except hpexceptions.HTTPNotFound: LOG.info(_LI("Cannot provide backend assisted migration for " "volume: %s because volume does not exist in this " @@ -546,5 +596,7 @@ class HPLeftHandRESTProxy(ISCSIDriver): except hpexceptions.HTTPServerError as ex: LOG.error(ex) return false_ret + finally: + self._logout(client) return (True, None) -- 2.45.2