From 72728025a98e6880e0b7ff7eaffaa92940f0cdf2 Mon Sep 17 00:00:00 2001 From: Igor Malinovskiy Date: Wed, 3 Jun 2015 17:47:09 +0300 Subject: [PATCH] Implement extend_share() method in Generic driver - Add implementation for extend_share() method - Add appropriate unit tests Partially implements bp share-extend-api Change-Id: I81c4278a8caa75a2b475eaf5538a5ae6d3ed18d5 --- doc/source/devref/generic_driver.rst | 3 + manila/share/drivers/generic.py | 65 ++++++++++++++-- manila/tests/fake_volume.py | 3 + manila/tests/share/drivers/test_generic.py | 91 ++++++++++++++++++++++ manila/volume/cinder.py | 4 + 5 files changed, 159 insertions(+), 7 deletions(-) diff --git a/doc/source/devref/generic_driver.rst b/doc/source/devref/generic_driver.rst index 3fbb1532..fee2a4b5 100644 --- a/doc/source/devref/generic_driver.rst +++ b/doc/source/devref/generic_driver.rst @@ -83,6 +83,9 @@ Known restrictions - Juno version does not use security services data provided with share-network. These data will be just ignored. +- Liberty version adds a share extend capability. Share access will be briefly + interrupted during an extend operation. + The :mod:`manila.share.drivers.generic` Module ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index a78e1742..9a988ac0 100644 --- a/manila/share/drivers/generic.py +++ b/manila/share/drivers/generic.py @@ -59,6 +59,9 @@ share_opts = [ cfg.IntOpt('max_time_to_create_volume', default=180, help="Maximum time to wait for creating cinder volume."), + cfg.IntOpt('max_time_to_extend_volume', + default=180, + help="Maximum time to wait for extending cinder volume."), cfg.IntOpt('max_time_to_attach', default=120, help="Maximum time to wait for attaching cinder volume."), @@ -466,19 +469,29 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): self.private_storage.update( share['id'], {'volume_id': volume['id']}) + msg_error = _('Failed to create volume') + msg_timeout = ( + _('Volume has not been created in %ss. Giving up') % + self.configuration.max_time_to_create_volume + ) + + return self._wait_for_available_volume( + volume, self.configuration.max_time_to_create_volume, + msg_error=msg_error, msg_timeout=msg_timeout + ) + + def _wait_for_available_volume(self, volume, timeout, + msg_error, msg_timeout): t = time.time() - while time.time() - t < self.configuration.max_time_to_create_volume: + while time.time() - t < timeout: if volume['status'] == const.STATUS_AVAILABLE: break if volume['status'] == const.STATUS_ERROR: - raise exception.ManilaException(_('Failed to create volume')) + raise exception.ManilaException(msg_error) time.sleep(1) - volume = self.volume_api.get(context, volume['id']) + volume = self.volume_api.get(self.admin_context, volume['id']) else: - raise exception.ManilaException( - _('Volume have not been created ' - 'in %ss. Giving up') % - self.configuration.max_time_to_create_volume) + raise exception.ManilaException(msg_timeout) return volume @@ -528,6 +541,44 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver): share['name']) return location + @ensure_server + def extend_share(self, share, new_size, share_server=None): + server_details = share_server['backend_details'] + + self._unmount_device(share, server_details) + self._detach_volume(self.admin_context, share, server_details) + + volume = self._get_volume(self.admin_context, share['id']) + volume = self._extend_volume(self.admin_context, volume, new_size) + + volume = self._attach_volume( + self.admin_context, + share, + server_details['instance_id'], + volume) + self._resize_filesystem(server_details, volume) + self._mount_device(share, server_details, volume) + + def _extend_volume(self, context, volume, new_size): + self.volume_api.extend(context, volume['id'], new_size) + + msg_error = _('Failed to extend volume %s') % volume['id'] + msg_timeout = ( + _('Volume has not been extended in %ss. Giving up') % + self.configuration.max_time_to_extend_volume + ) + return self._wait_for_available_volume( + volume, self.configuration.max_time_to_extend_volume, + msg_error=msg_error, msg_timeout=msg_timeout + ) + + def _resize_filesystem(self, server_details, volume): + """Resize filesystem of provided volume.""" + check_command = ['sudo', 'fsck', '-pf', volume['mountpoint']] + self._ssh_exec(server_details, check_command) + command = ['sudo', 'resize2fs', volume['mountpoint']] + self._ssh_exec(server_details, command) + def _is_share_server_active(self, context, share_server): """Check if the share server is active.""" has_active_share_server = ( diff --git a/manila/tests/fake_volume.py b/manila/tests/fake_volume.py index c5383cb2..81262fde 100644 --- a/manila/tests/fake_volume.py +++ b/manila/tests/fake_volume.py @@ -61,6 +61,9 @@ class API(object): def create(self, *args, **kwargs): pass + def extend(self, *args, **kwargs): + pass + def get_all(self, search_opts): pass diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py index ff5ce64b..2cb75233 100644 --- a/manila/tests/share/drivers/test_generic.py +++ b/manila/tests/share/drivers/test_generic.py @@ -642,6 +642,31 @@ class GenericShareDriverTestCase(test.TestCase): self._context, self.share) + def test_wait_for_available_volume(self): + fake_volume = {'status': 'creating', 'id': 'fake'} + fake_available_volume = {'status': 'available', 'id': 'fake'} + self.mock_object(self._driver.volume_api, 'get', + mock.Mock(return_value=fake_available_volume)) + + actual_result = self._driver._wait_for_available_volume( + fake_volume, 5, "error", "timeout") + + self.assertEqual(fake_available_volume, actual_result) + self._driver.volume_api.get.assert_called_once_with( + mock.ANY, fake_volume['id']) + + @ddt.data(mock.Mock(return_value={'status': 'creating', 'id': 'fake'}), + mock.Mock(return_value={'status': 'error', 'id': 'fake'})) + def test_wait_for_available_volume_invalid(self, volume_get_mock): + fake_volume = {'status': 'creating', 'id': 'fake'} + self.mock_object(self._driver.volume_api, 'get', volume_get_mock) + + self.assertRaises( + exception.ManilaException, + self._driver._wait_for_available_volume, + fake_volume, 1, "error", "timeout" + ) + def test_deallocate_container(self): fake_vol = fake_volume.FakeVolume() self.mock_object(self._driver, '_get_volume', @@ -1269,6 +1294,72 @@ class GenericShareDriverTestCase(test.TestCase): self._driver._get_mounted_share_size, '/fake/path', {}) + def test_extend_share(self): + fake_volume = "fake" + fake_share = {'id': 'fake'} + new_size = 123 + srv_details = self.server['backend_details'] + self.mock_object( + self._driver.service_instance_manager, + 'get_common_server', + mock.Mock(return_value=self.server) + ) + self.mock_object(self._driver, '_unmount_device') + self.mock_object(self._driver, '_detach_volume') + self.mock_object(self._driver, '_extend_volume') + self.mock_object(self._driver, '_attach_volume') + self.mock_object(self._driver, '_mount_device') + self.mock_object(self._driver, '_resize_filesystem') + self.mock_object( + self._driver, '_get_volume', + mock.Mock(return_value=fake_volume) + ) + CONF.set_default('driver_handles_share_servers', False) + + self._driver.extend_share(fake_share, new_size) + + self.assertTrue( + self._driver.service_instance_manager.get_common_server.called) + self._driver._unmount_device.assert_called_once_with( + fake_share, srv_details) + self._driver._detach_volume.assert_called_once_with( + mock.ANY, fake_share, srv_details) + self._driver._get_volume.assert_called_once_with( + mock.ANY, fake_share['id']) + self._driver._extend_volume.assert_called_once_with( + mock.ANY, fake_volume, new_size) + self._driver._attach_volume.assert_called_once_with( + mock.ANY, fake_share, srv_details['instance_id'], mock.ANY) + self.assertTrue(self._driver._resize_filesystem.called) + + def test_extend_volume(self): + fake_volume = {'id': 'fake'} + new_size = 123 + self.mock_object(self._driver.volume_api, 'extend') + self.mock_object(self._driver, '_wait_for_available_volume') + + self._driver._extend_volume(self._context, fake_volume, new_size) + + self._driver.volume_api.extend.assert_called_once_with( + self._context, fake_volume['id'], new_size + ) + self._driver._wait_for_available_volume.assert_called_once_with( + fake_volume, mock.ANY, msg_timeout=mock.ANY, msg_error=mock.ANY + ) + + def test_resize_filesystem(self): + fake_server_details = {'fake': 'fake'} + fake_volume = {'mountpoint': '/dev/fake'} + self.mock_object(self._driver, '_ssh_exec') + + self._driver._resize_filesystem(fake_server_details, fake_volume) + + self._driver._ssh_exec.assert_any_call( + fake_server_details, ['sudo', 'fsck', '-pf', '/dev/fake']) + self._driver._ssh_exec.assert_any_call( + fake_server_details, ['sudo', 'resize2fs', '/dev/fake']) + self.assertEqual(2, self._driver._ssh_exec.call_count) + @generic.ensure_server def fake(driver_instance, context, share_server=None): diff --git a/manila/volume/cinder.py b/manila/volume/cinder.py index dafd429a..22b23c10 100644 --- a/manila/volume/cinder.py +++ b/manila/volume/cinder.py @@ -312,6 +312,10 @@ class API(base.Base): except Exception as e: raise exception.ManilaException(e.message) + @translate_volume_exception + def extend(self, context, volume_id, new_size): + cinderclient(context).volumes.extend(volume_id, new_size) + @translate_volume_exception def delete(self, context, volume_id): cinderclient(context).volumes.delete(volume_id)