diff --git a/contrib/ci/post_test_hook.sh b/contrib/ci/post_test_hook.sh index 873efed6bf..1a3110b4f1 100755 --- a/contrib/ci/post_test_hook.sh +++ b/contrib/ci/post_test_hook.sh @@ -260,7 +260,6 @@ elif [[ "$DRIVER" == "dummy" ]]; then iniset $TEMPEST_CONFIG share create_networks_when_multitenancy_enabled False iniset $TEMPEST_CONFIG share multi_backend True elif [[ "$DRIVER" == "container"* ]]; then - MANILA_TESTS="(^manila_tempest_tests.tests.api)(?=.*\[.*\bbackend\b.*\])" if [[ "$DRIVER" == "container_with_custom_image" ]]; then # TODO(vponomaryov): set scenario tests for run when # manila tempest plugin supports share protocol and rules that @@ -270,7 +269,7 @@ elif [[ "$DRIVER" == "container"* ]]; then fi MANILA_TEMPEST_CONCURRENCY=8 RUN_MANILA_SG_TESTS=False - RUN_MANILA_MANAGE_TESTS=False + RUN_MANILA_MANAGE_TESTS=True RUN_MANILA_QUOTA_TESTS=False RUN_MANILA_SHRINK_TESTS=False RUN_MANILA_SNAPSHOT_TESTS=False diff --git a/etc/manila/rootwrap.d/share.filters b/etc/manila/rootwrap.d/share.filters index c3609cd72a..d5e9f6a980 100644 --- a/etc/manila/rootwrap.d/share.filters +++ b/etc/manila/rootwrap.d/share.filters @@ -187,3 +187,9 @@ sha256sum: CommandFilter, sha256sum, root # manila/utils.py: 'tee', '%s' tee: CommandFilter, tee, root + +# manila/share/drivers/container/storage_helper.py: lvs -o lv_size --noheadings --nosuffix --units g +lvs: CommandFilter, lvs, root + +# manila/share/drivers/container/storage_helper.py: lvrename --autobackup n +lvrename: CommandFilter, lvrename, root diff --git a/manila/share/drivers/container/container_helper.py b/manila/share/drivers/container/container_helper.py index a8bd1f27e9..cea4462061 100644 --- a/manila/share/drivers/container/container_helper.py +++ b/manila/share/drivers/container/container_helper.py @@ -115,6 +115,34 @@ class DockerExecHelper(driver.ExecuteMixin): address = address_w_prefix.split('/')[0] return address + def rename_container(self, name, new_name): + veth_name = self.find_container_veth(name) + if not veth_name: + raise exception.ManilaException( + _("Could not find OVS information related to " + "container %s.") % name) + + try: + self._inner_execute(["docker", "rename", name, new_name]) + except (exception.ProcessExecutionError, OSError): + raise exception.ShareBackendException( + msg="Could not rename container %s." % name) + + cmd = ["ovs-vsctl", "set", "interface", veth_name, + "external-ids:manila-container=%s" % new_name] + try: + self._inner_execute(cmd) + except (exception.ProcessExecutionError, OSError): + try: + self._inner_execute(["docker", "rename", new_name, name]) + except (exception.ProcessExecutionError, OSError): + msg = _("Could not rename back container %s.") % name + LOG.exception(msg) + raise exception.ShareBackendException( + msg="Could not update OVS information %s." % name) + + LOG.info("Container %s has been successfully renamed.", name) + def find_container_veth(self, name): interfaces = self._execute("ovs-vsctl", "list", "interface", run_as_root=True)[0] diff --git a/manila/share/drivers/container/driver.py b/manila/share/drivers/container/driver.py index f679f503ac..251cf3beec 100644 --- a/manila/share/drivers/container/driver.py +++ b/manila/share/drivers/container/driver.py @@ -20,6 +20,7 @@ Current implementation suggests that a container when started by Docker will be plugged into a Linux bridge. Also it is suggested that all interfaces willing to talk to each other reside in an OVS bridge.""" +import math import re from oslo_config import cfg @@ -122,7 +123,7 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): share_name = share.share_id self.storage.provide_storage(share_name, share['size']) - location = self._create_and_mount_share_links( + location = self._create_export_and_mount_storage( share, server_id, share_name) return location @@ -135,8 +136,8 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): server_id = self._get_container_name(share_server["id"]) share_name = self._get_share_name(share) - self._delete_and_umount_share_links(share, server_id, share_name, - ignore_errors=True) + self._delete_export_and_umount_storage(share, server_id, share_name, + ignore_errors=True) self.storage.remove_storage(share_name) LOG.debug("Deleted share %s successfully.", share_name) @@ -295,7 +296,7 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): LOG.info("Container %s was created.", server_id) return {"id": network_info["server_id"]} - def _delete_and_umount_share_links( + def _delete_export_and_umount_storage( self, share, server_id, share_name, ignore_errors=False): self._get_helper(share).delete_share(server_id, share_name, @@ -317,7 +318,7 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): ignore_errors=True ) - def _create_and_mount_share_links(self, share, server_id, share_name): + def _create_export_and_mount_storage(self, share, server_id, share_name): self.container.execute( server_id, ["mkdir", "-m", "750", "/shares/%s" % share_name] @@ -329,3 +330,50 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): ) location = self._get_helper(share).create_share(server_id) return location + + def manage_existing_with_server( + self, share, driver_options, share_server=None): + if not share_server and self.driver_handles_share_servers: + raise exception.ShareBackendException( + "A share server object is needed to manage a share in this " + "driver mode of operation.") + server_id = self._get_container_name(share_server["id"]) + share_name = self._get_share_name(share) + size = int(math.ceil(float(self.storage.get_size(share_name)))) + + self._delete_export_and_umount_storage(share, server_id, share_name) + + new_share_name = share.share_id + self.storage.rename_storage(share_name, new_share_name) + + location = self._create_export_and_mount_storage( + share, server_id, new_share_name) + + result = {'size': size, 'export_locations': [location]} + LOG.info("Successfully managed share %(share)s, returning %(data)s", + {'share': share.id, 'data': result}) + return result + + def unmanage_with_server(self, share, share_server=None): + pass + + def get_share_server_network_info( + self, context, share_server, identifier, driver_options): + name = self._get_correct_container_old_name(identifier) + return [self.container.fetch_container_address(name, "inet")] + + def manage_server(self, context, share_server, identifier, driver_options): + new_name = self._get_container_name(share_server['id']) + old_name = self._get_correct_container_old_name(identifier) + self.container.rename_container(old_name, new_name) + return new_name, {'id': share_server['id']} + + def unmanage_server(self, server_details, security_services=None): + pass + + def _get_correct_container_old_name(self, name): + # Check if the container with the given name exists, else return + # the name based on the driver template + if not self.container.container_exists(name): + return self._get_container_name(name) + return name diff --git a/manila/share/drivers/container/storage_helper.py b/manila/share/drivers/container/storage_helper.py index 48ac1d0793..8ebafbb93b 100644 --- a/manila/share/drivers/container/storage_helper.py +++ b/manila/share/drivers/container/storage_helper.py @@ -111,9 +111,33 @@ class LVMHelper(driver.ExecuteMixin): LOG.warning("Failed to remove logical volume %(device)s due to " "%(reason)s.", {'device': device, 'reason': e}) + def rename_storage(self, share_name, new_share_name): + old_device = self._get_lv_device(share_name) + new_device = self._get_lv_device(new_share_name) + + self._try_to_unmount_device(old_device) + + try: + self._execute("lvrename", "--autobackup", "n", + old_device, new_device, run_as_root=True) + except exception.ProcessExecutionError as e: + msg = ("Failed to rename logical volume %(device)s due to " + "%(reason)s." % {'device': old_device, 'reason': e}) + LOG.exception(msg) + raise + def extend_share(self, share_name, new_size, share_server=None): lv_device = self._get_lv_device(share_name) cmd = ('lvextend', '-L', '%sG' % new_size, '-n', lv_device) self._execute(*cmd, run_as_root=True) self._execute("e2fsck", "-f", "-y", lv_device, run_as_root=True) self._execute('resize2fs', lv_device, run_as_root=True) + + def get_size(self, share_name): + device = self._get_lv_device(share_name) + size = self._execute( + "lvs", "-o", "lv_size", "--noheadings", "--nosuffix", + "--units", "g", device, run_as_root=True) + LOG.debug("Found size %(size)s for LVM device " + "%(lvm)s.", {'size': size[0], 'lvm': share_name}) + return size[0] diff --git a/manila/tests/share/drivers/container/test_container_helper.py b/manila/tests/share/drivers/container/test_container_helper.py index bbeac35bcc..618f8f8494 100644 --- a/manila/tests/share/drivers/container/test_container_helper.py +++ b/manila/tests/share/drivers/container/test_container_helper.py @@ -144,6 +144,63 @@ class DockerExecHelperTestCase(test.TestCase): "show", "scope", "global", "dev", "eth0"] ) + def test_rename_container(self): + + fake_old_name = "old_name" + fake_new_name = "new_name" + fake_veth_name = "veth_fake" + self.DockerExecHelper.find_container_veth = mock.Mock( + return_value=fake_veth_name) + mock__inner_execute = self.DockerExecHelper._inner_execute = mock.Mock( + return_value=['fake', '']) + + self.DockerExecHelper.rename_container(fake_old_name, fake_new_name) + self.DockerExecHelper.find_container_veth.assert_called_once_with( + fake_old_name + ) + mock__inner_execute.assert_has_calls([ + mock.call(["docker", "rename", fake_old_name, fake_new_name]), + mock.call(["ovs-vsctl", "set", "interface", fake_veth_name, + "external-ids:manila-container=%s" % fake_new_name]) + ]) + + def test_rename_container_exception_veth(self): + + self.DockerExecHelper.find_container_veth = mock.Mock( + return_value=None) + + self.assertRaises(exception.ManilaException, + self.DockerExecHelper.rename_container, + "old_name", "new_name") + + @ddt.data([['fake', ''], OSError, ['fake', '']], + [['fake', ''], OSError, OSError], + [OSError]) + def test_rename_container_exception_cmds(self, side_effect): + fake_old_name = "old_name" + fake_new_name = "new_name" + fake_veth_name = "veth_fake" + + self.DockerExecHelper.find_container_veth = mock.Mock( + return_value=fake_veth_name) + mock__inner_execute = self.DockerExecHelper._inner_execute = mock.Mock( + side_effect=side_effect) + + self.assertRaises(exception.ShareBackendException, + self.DockerExecHelper.rename_container, + fake_old_name, fake_new_name) + + if len(side_effect) > 1: + mock__inner_execute.assert_has_calls([ + mock.call(["docker", "rename", fake_old_name, fake_new_name]), + mock.call(["ovs-vsctl", "set", "interface", fake_veth_name, + "external-ids:manila-container=%s" % fake_new_name]) + ]) + else: + mock__inner_execute.assert_has_calls([ + mock.call(["docker", "rename", fake_old_name, fake_new_name]), + ]) + @ddt.data('my_container', 'manila_my_container') def test_find_container_veth(self, name): diff --git a/manila/tests/share/drivers/container/test_driver.py b/manila/tests/share/drivers/container/test_driver.py index 62263c8291..f8a623e0a1 100644 --- a/manila/tests/share/drivers/container/test_driver.py +++ b/manila/tests/share/drivers/container/test_driver.py @@ -119,7 +119,7 @@ class ContainerShareDriverTestCase(test.TestCase): self._driver, '_get_container_name', mock.Mock(return_value=fake_container_name)) mock_create_and_mount = self.mock_object( - self._driver, '_create_and_mount_share_links', + self._driver, '_create_export_and_mount_storage', mock.Mock(return_value='export_location')) self.assertEqual('export_location', @@ -135,7 +135,7 @@ class ContainerShareDriverTestCase(test.TestCase): share_server['id'] ) - def test__create_and_mount_share_links(self): + def test__create_export_and_mount_storage(self): helper = mock.Mock() server_id = 'fake_id' share_name = 'fake_name' @@ -149,7 +149,7 @@ class ContainerShareDriverTestCase(test.TestCase): mock_execute = self.mock_object(self._driver.container, 'execute') self.assertEqual('export_location', - self._driver._create_and_mount_share_links( + self._driver._create_export_and_mount_storage( self.share, server_id, share_name)) mock_create_share.assert_called_once_with(server_id) mock__get_helper.assert_called_once_with(self.share) @@ -160,7 +160,7 @@ class ContainerShareDriverTestCase(test.TestCase): "/shares/%s" % share_name]) ]) - def test__delete_and_umount_share_links(self): + def test__delete_export_and_umount_storage(self): helper = mock.Mock() server_id = 'fake_id' share_name = 'fake_name' @@ -168,7 +168,7 @@ class ContainerShareDriverTestCase(test.TestCase): self._driver, "_get_helper", mock.Mock(return_value=helper)) mock_delete_share = self.mock_object(helper, 'delete_share') mock_execute = self.mock_object(self._driver.container, 'execute') - self._driver._delete_and_umount_share_links( + self._driver._delete_export_and_umount_storage( self.share, server_id, share_name) mock__get_helper.assert_called_once_with(self.share) @@ -194,7 +194,7 @@ class ContainerShareDriverTestCase(test.TestCase): mock.Mock(return_value=fake_share_name)) self.mock_object(self._driver.storage, 'remove_storage') mock_delete_and_umount = self.mock_object( - self._driver, '_delete_and_umount_share_links') + self._driver, '_delete_export_and_umount_storage') self._driver.delete_share(self._context, self.share, fake_share_server) @@ -391,3 +391,133 @@ class ContainerShareDriverTestCase(test.TestCase): self._driver._connect_to_network.assert_called_once_with(server_id, network_info, 'veth0') + + def test_manage_existing(self): + + fake_container_name = "manila_fake_container" + fake_export_location = 'export_location' + expected_result = { + 'size': 1, + 'export_locations': [fake_export_location] + } + fake_share_server = cont_fakes.fake_share() + fake_share_name = self._driver._get_share_name(self.share) + mock_get_container_name = self.mock_object( + self._driver, '_get_container_name', + mock.Mock(return_value=fake_container_name)) + mock_get_share_name = self.mock_object( + self._driver, '_get_share_name', + mock.Mock(return_value=fake_share_name)) + mock_rename_storage = self.mock_object( + self._driver.storage, 'rename_storage') + mock_get_size = self.mock_object( + self._driver.storage, 'get_size', mock.Mock(return_value=1)) + mock_delete_and_umount = self.mock_object( + self._driver, '_delete_export_and_umount_storage') + mock_create_and_mount = self.mock_object( + self._driver, '_create_export_and_mount_storage', + mock.Mock(return_value=fake_export_location) + ) + + result = self._driver.manage_existing_with_server( + self.share, {}, fake_share_server) + + mock_rename_storage.assert_called_once_with( + fake_share_name, self.share.share_id + ) + mock_get_size.assert_called_once_with( + fake_share_name + ) + mock_delete_and_umount.assert_called_once_with( + self.share, fake_container_name, fake_share_name + ) + mock_create_and_mount.assert_called_once_with( + self.share, fake_container_name, self.share.share_id + ) + mock_get_container_name.assert_called_once_with( + fake_share_server['id'] + ) + mock_get_share_name.assert_called_with( + self.share + ) + self.assertEqual(expected_result, result) + + def test_manage_existing_no_share_server(self): + + self.assertRaises(exception.ShareBackendException, + self._driver.manage_existing_with_server, + self.share, {}) + + def test_unmanage(self): + self.assertIsNone(self._driver.unmanage_with_server(self.share)) + + def test_get_share_server_network_info(self): + + fake_share_server = cont_fakes.fake_share_server() + fake_id = cont_fakes.fake_identifier() + expected_result = ['veth11b2c34'] + + interfaces = [cont_fakes.FAKE_VSCTL_LIST_INTERFACE_1, + cont_fakes.FAKE_VSCTL_LIST_INTERFACE_2, + cont_fakes.FAKE_VSCTL_LIST_INTERFACE_4, + cont_fakes.FAKE_VSCTL_LIST_INTERFACE_3] + + self.mock_object(self._driver.container, 'execute', + mock.Mock(return_value=interfaces)) + + result = self._driver.get_share_server_network_info(self._context, + fake_share_server, + fake_id, {}) + self.assertEqual(expected_result, result) + + def test_manage_server(self): + + fake_id = cont_fakes.fake_identifier() + fake_share_server = cont_fakes.fake_share_server() + fake_container_name = "manila_fake_container" + fake_container_old_name = "fake_old_name" + + mock_get_container_name = self.mock_object( + self._driver, '_get_container_name', + mock.Mock(return_value=fake_container_name)) + mock_get_correct_container_old_name = self.mock_object( + self._driver, '_get_correct_container_old_name', + mock.Mock(return_value=fake_container_old_name) + ) + mock_rename_container = self.mock_object(self._driver.container, + 'rename_container') + expected_result = {'id': fake_share_server['id']} + + new_identifier, new_backend_details = self._driver.manage_server( + self._context, fake_share_server, fake_id, {}) + + self.assertEqual(expected_result, new_backend_details) + self.assertEqual(fake_container_name, new_identifier) + mock_rename_container.assert_called_once_with( + fake_container_old_name, fake_container_name) + mock_get_container_name.assert_called_with( + fake_share_server['id'] + ) + mock_get_correct_container_old_name.assert_called_once_with( + fake_id + ) + + @ddt.data(True, False) + def test__get_correct_container_old_name(self, container_exists): + + expected_name = 'fake-name' + fake_name = 'fake-name' + + mock_container_exists = self.mock_object( + self._driver.container, 'container_exists', + mock.Mock(return_value=container_exists)) + + if not container_exists: + expected_name = 'manila_fake_name' + + result = self._driver._get_correct_container_old_name(fake_name) + + self.assertEqual(expected_name, result) + mock_container_exists.assert_called_once_with( + fake_name + ) diff --git a/manila/tests/share/drivers/container/test_storage_helper.py b/manila/tests/share/drivers/container/test_storage_helper.py index 1d3eac765c..00fb5b9964 100644 --- a/manila/tests/share/drivers/container/test_storage_helper.py +++ b/manila/tests/share/drivers/container/test_storage_helper.py @@ -136,6 +136,39 @@ class LVMHelperTestCase(test.TestCase): self.assertTrue(storage_helper.LOG.warning.called) + @ddt.data(None, exception.ProcessExecutionError) + def test_rename_storage(self, side_effect): + fake_old_share_name = 'fake_old_name' + fake_new_share_name = 'fake_new_name' + fake_new_device = "/dev/new_device" + fake_old_device = "/dev/old_device" + + mock_get_lv_device = self.mock_object( + self.LVMHelper, '_get_lv_device', + mock.Mock(side_effect=[fake_old_device, fake_new_device])) + mock_try_to_umount = self.mock_object(self.LVMHelper, + '_try_to_unmount_device') + + mock_execute = self.mock_object(self.LVMHelper, '_execute', + mock.Mock(side_effect=side_effect)) + + if side_effect is None: + self.LVMHelper.rename_storage(fake_old_share_name, + fake_new_share_name) + else: + self.assertRaises(exception.ProcessExecutionError, + self.LVMHelper.rename_storage, + fake_old_share_name, fake_new_share_name) + mock_try_to_umount.assert_called_once_with(fake_old_device) + mock_execute.mock_assert_called_once_with( + "lvrename", "--autobackup", "n", fake_old_device, fake_new_device, + run_as_root=True + ) + mock_get_lv_device.assert_has_calls([ + mock.call(fake_old_share_name), + mock.call(fake_new_share_name) + ]) + def test_extend_share(self): actual_arguments = [] expected_arguments = [ @@ -152,3 +185,22 @@ class LVMHelperTestCase(test.TestCase): self.LVMHelper.extend_share(fake_share_name, 'share', 3) self.assertEqual(expected_arguments, actual_arguments) + + def test_get_size(self): + share_name = 'fakeshareid' + fake_old_device = {} + + mock_get_lv_device = self.mock_object( + self.LVMHelper, '_get_lv_device', + mock.Mock(return_value=fake_old_device)) + mock_execute = self.mock_object(self.LVMHelper, '_execute', + mock.Mock(return_value=[1, "args"])) + + result = self.LVMHelper.get_size(share_name) + + mock_execute.assert_called_once_with( + "lvs", "-o", "lv_size", "--noheadings", "--nosuffix", "--units", + "g", fake_old_device, run_as_root=True + ) + mock_get_lv_device.assert_called_once_with(share_name) + self.assertEqual(result, 1) diff --git a/releasenotes/notes/container-manage-unmanage-share-servers-880d889828ee7ce3.yaml b/releasenotes/notes/container-manage-unmanage-share-servers-880d889828ee7ce3.yaml new file mode 100644 index 0000000000..0f31294557 --- /dev/null +++ b/releasenotes/notes/container-manage-unmanage-share-servers-880d889828ee7ce3.yaml @@ -0,0 +1,6 @@ +--- +features: + - Added managing and unmanaging of share servers functionality + to the Container Driver, allowing for shares to be managed + and unmanaged. +