diff --git a/manila/share/drivers/container/container_helper.py b/manila/share/drivers/container/container_helper.py index 2aa775879f..a8bd1f27e9 100644 --- a/manila/share/drivers/container/container_helper.py +++ b/manila/share/drivers/container/container_helper.py @@ -13,9 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. +import re import uuid from oslo_log import log +from oslo_utils import excutils from manila import exception from manila.i18n import _ @@ -62,39 +64,84 @@ class DockerExecHelper(driver.ExecuteMixin): cmd = ["docker", "run", "-d", "-i", "-t", "--privileged", "-v", "/dev:/dev", "--name=%s" % name, "-v", "/tmp/shares:/shares", image_name] - result = self._inner_execute(cmd) or ["", 1] - if result[1] != "": - raise exception.ManilaException( - _("Container %s has failed to start.") % name) + try: + result = self._inner_execute(cmd) + except (exception.ProcessExecutionError, OSError): + raise exception.ShareBackendException( + msg="Container %s has failed to start." % name) LOG.info("A container has been successfully started! Its id is " "%s.", result[0].rstrip('\n')) def stop_container(self, name): LOG.debug("Stopping container %s.", name) - cmd = ["docker", "stop", name] - result = self._inner_execute(cmd) or ['', 1] - if result[1] != '': - raise exception.ManilaException( - _("Container %s has failed to stop properly.") % name) + try: + self._inner_execute(["docker", "stop", name]) + except (exception.ProcessExecutionError, OSError): + raise exception.ShareBackendException( + msg="Container %s has failed to stop properly." % name) LOG.info("Container %s is successfully stopped.", name) - def execute(self, name=None, cmd=None): + def execute(self, name=None, cmd=None, ignore_errors=False): if name is None: raise exception.ManilaException(_("Container name not specified.")) if cmd is None or (type(cmd) is not list): raise exception.ManilaException(_("Missing or malformed command.")) LOG.debug("Executing inside a container %s.", name) cmd = ["docker", "exec", "-i", name] + cmd - result = self._inner_execute(cmd) - LOG.debug("Run result: %s.", result) + result = self._inner_execute(cmd, ignore_errors=ignore_errors) return result - def _inner_execute(self, cmd): + def _inner_execute(self, cmd, ignore_errors=False): LOG.debug("Executing command: %s.", " ".join(cmd)) try: result = self._execute(*cmd, run_as_root=True) - except Exception: - LOG.exception("Executing command failed.") - return None - LOG.debug("Execution result: %s.", result) - return result + except (exception.ProcessExecutionError, OSError) as e: + with excutils.save_and_reraise_exception( + reraise=not ignore_errors): + LOG.warning("Failed to run command %(cmd)s due to " + "%(reason)s.", {'cmd': cmd, 'reason': e}) + else: + LOG.debug("Execution result: %s.", result) + return result + + def fetch_container_address(self, name, address_family="inet6"): + result = self.execute( + name, + ["ip", "-oneline", + "-family", address_family, + "address", "show", "scope", "global", "dev", "eth0"], + ) + address_w_prefix = result[0].split()[3] + address = address_w_prefix.split('/')[0] + return address + + def find_container_veth(self, name): + interfaces = self._execute("ovs-vsctl", "list", "interface", + run_as_root=True)[0] + veths = set(re.findall("veth[0-9a-zA-Z]{7}", interfaces)) + manila_re = "manila-container=\".*\"" + for veth in veths: + try: + iface_data = self._execute("ovs-vsctl", "list", "interface", + veth, run_as_root=True)[0] + except (exception.ProcessExecutionError, OSError) as e: + LOG.debug("Error listing interface %(veth)s. " + "Reason: %(reason)s", {'veth': veth, + 'reason': e}) + continue + + container_id = re.findall(manila_re, iface_data) + if container_id == []: + continue + elif container_id[0].split("manila-container=")[-1].split( + "manila_")[-1].strip('"') == name.split("manila_")[-1]: + return veth + + def container_exists(self, name): + + result = self._execute("docker", "ps", "--no-trunc", + "--format='{{.Names}}'", run_as_root=True)[0] + for line in result.split('\n'): + if name == line.strip("'"): + return True + return False diff --git a/manila/share/drivers/container/driver.py b/manila/share/drivers/container/driver.py index a6be2b75a6..f679f503ac 100644 --- a/manila/share/drivers/container/driver.py +++ b/manila/share/drivers/container/driver.py @@ -120,61 +120,45 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): LOG.debug("Create share on server '%s'.", share_server["id"]) server_id = self._get_container_name(share_server["id"]) share_name = share.share_id - self.container.execute( - server_id, - ["mkdir", "-m", "750", "/shares/%s" % share_name] - ) - self.storage.provide_storage(share) - lv_device = self.storage._get_lv_device(share) - self.container.execute( - server_id, - ["mount", lv_device, "/shares/%s" % share_name] - ) - location = self._get_helper(share).create_share(server_id) + self.storage.provide_storage(share_name, share['size']) + + location = self._create_and_mount_share_links( + share, server_id, share_name) + return location @utils.synchronized('container_driver_delete_share_lock', external=True) def delete_share(self, context, share, share_server=None): LOG.debug("Deleting share %(share)s on server '%(server)s'.", {"server": share_server["id"], - "share": share.share_id}) + "share": self._get_share_name(share)}) server_id = self._get_container_name(share_server["id"]) - self._get_helper(share).delete_share(server_id) + share_name = self._get_share_name(share) - self.container.execute( - server_id, - ["umount", "/shares/%s" % share.share_id] - ) - # (aovchinnikov): bug 1621784 manifests itself here as well as in - # storage helper. There is a chance that we won't be able to remove - # this directory, despite the fact that it is not shared anymore and - # already contains nothing. In such case the driver should not fail - # share deletion, but issue a warning. - try: - self.container.execute( - server_id, - ["rm", "-fR", "/shares/%s" % share.share_id] - ) - except exception.ProcessExecutionError as e: - LOG.warning("Failed to remove /shares/%(share)s directory in " - "container %(cont)s.", {"share": share.share_id, - "cont": server_id}) - LOG.error(e) + self._delete_and_umount_share_links(share, server_id, share_name, + ignore_errors=True) - self.storage.remove_storage(share) - LOG.debug("Deletion of share %s is completed!", share.share_id) + self.storage.remove_storage(share_name) + LOG.debug("Deleted share %s successfully.", share_name) + + def _get_share_name(self, share): + if share.get('export_location'): + return share['export_location'].split('/')[-1] + else: + return share.share_id def extend_share(self, share, new_size, share_server=None): server_id = self._get_container_name(share_server["id"]) + share_name = self._get_share_name(share) self.container.execute( server_id, - ["umount", "/shares/%s" % share.share_id] + ["umount", "/shares/%s" % share_name] ) - self.storage.extend_share(share, new_size, share_server) - lv_device = self.storage._get_lv_device(share) + self.storage.extend_share(share_name, new_size, share_server) + lv_device = self.storage._get_lv_device(share_name) self.container.execute( server_id, - ["mount", lv_device, "/shares/%s" % share.share_id] + ["mount", lv_device, "/shares/%s" % share_name] ) def ensure_share(self, context, share, share_server=None): @@ -183,11 +167,12 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): def update_access(self, context, share, access_rules, add_rules, delete_rules, share_server=None): server_id = self._get_container_name(share_server["id"]) + share_name = self._get_share_name(share) LOG.debug("Updating access to share %(share)s at " "share server %(share_server)s.", {"share_server": share_server["id"], - "share": share.share_id}) - self._get_helper(share).update_access(server_id, + "share": share_name}) + self._get_helper(share).update_access(server_id, share_name, access_rules, add_rules, delete_rules) @@ -262,27 +247,17 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): def _teardown_server(self, *args, **kwargs): server_id = self._get_container_name(kwargs["server_details"]["id"]) self.container.stop_container(server_id) - interfaces = self._execute("ovs-vsctl", "list", "interface", - run_as_root=True)[0] - veths = set(re.findall("veth[0-9a-zA-Z]{7}", interfaces)) - manila_re = ("manila_[0-9a-f]{8}_[0-9a-f]{4}_[0-9a-f]{4}_[0-9a-f]{4}_" - "[0-9a-f]{12}") - for veth in veths: - iface_data = self._execute("ovs-vsctl", "list", "interface", veth, - run_as_root=True)[0] - container_id = re.findall(manila_re, iface_data) - if container_id == []: - continue - elif container_id[0] == server_id: - LOG.debug("Deleting veth %s.", veth) - try: - self._execute("ovs-vsctl", "--", "del-port", - self.configuration.container_ovs_bridge_name, - veth, run_as_root=True) - except exception.ProcessExecutionError as e: - LOG.warning("Failed to delete port %s: port " - "vanished.", veth) - LOG.error(e) + veth = self.container.find_container_veth(server_id) + if veth: + LOG.debug("Deleting veth %s.", veth) + try: + self._execute("ovs-vsctl", "--", "del-port", + self.configuration.container_ovs_bridge_name, + veth, run_as_root=True) + except exception.ProcessExecutionError as e: + LOG.warning("Failed to delete port %s: port " + "vanished.", veth) + LOG.error(e) def _get_veth_state(self): result = self._execute("brctl", "show", @@ -319,3 +294,38 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin): self._connect_to_network(server_id, network_info, veth) LOG.info("Container %s was created.", server_id) return {"id": network_info["server_id"]} + + def _delete_and_umount_share_links( + self, share, server_id, share_name, ignore_errors=False): + + self._get_helper(share).delete_share(server_id, share_name, + ignore_errors=ignore_errors) + + self.container.execute( + server_id, + ["umount", "/shares/%s" % share_name], + ignore_errors=ignore_errors + ) + # (aovchinnikov): bug 1621784 manifests itself here as well as in + # storage helper. There is a chance that we won't be able to remove + # this directory, despite the fact that it is not shared anymore and + # already contains nothing. In such case the driver should not fail + # share deletion, but issue a warning. + self.container.execute( + server_id, + ["rm", "-fR", "/shares/%s" % share_name], + ignore_errors=True + ) + + def _create_and_mount_share_links(self, share, server_id, share_name): + self.container.execute( + server_id, + ["mkdir", "-m", "750", "/shares/%s" % share_name] + ) + lv_device = self.storage._get_lv_device(share_name) + self.container.execute( + server_id, + ["mount", lv_device, "/shares/%s" % share_name] + ) + location = self._get_helper(share).create_share(server_id) + return location diff --git a/manila/share/drivers/container/protocol_helper.py b/manila/share/drivers/container/protocol_helper.py index 3a9aaa594c..aff9e6712d 100644 --- a/manila/share/drivers/container/protocol_helper.py +++ b/manila/share/drivers/container/protocol_helper.py @@ -49,24 +49,15 @@ class DockerCIFSHelper(object): ["net", "conf", "setparm", share_name, param, value] ) # TODO(tbarron): pass configured address family when we support IPv6 - address = self._fetch_container_address(server_id, 'inet') + address = self.container.fetch_container_address( + server_id, address_family='inet') return r"//%(addr)s/%(name)s" % {"addr": address, "name": share_name} - def _fetch_container_address(self, server_id, address_family="inet6"): - result = self.container.execute( - server_id, - ["ip", "-oneline", - "-family", address_family, - "address", "show", "scope", "global", "dev", "eth0"], - ) - address_w_prefix = result[0].split()[3] - address = address_w_prefix.split('/')[0] - return address - - def delete_share(self, server_id): + def delete_share(self, server_id, share_name, ignore_errors=False): self.container.execute( server_id, - ["net", "conf", "delshare", self.share.share_id] + ["net", "conf", "delshare", share_name], + ignore_errors=ignore_errors ) def _get_access_group(self, access_level): @@ -81,9 +72,13 @@ class DockerCIFSHelper(object): def _get_existing_users(self, server_id, share_name, access): result = self.container.execute( server_id, - ["net", "conf", "getparm", share_name, access] - )[0].rstrip('\n') - return result + ["net", "conf", "getparm", share_name, access], + ignore_errors=True + ) + if result: + return result[0].rstrip('\n') + else: + return "" def _set_users(self, server_id, share_name, access, users_to_set): self.container.execute( @@ -118,7 +113,7 @@ class DockerCIFSHelper(object): if allowed_users != existing_users: self._set_users(server_id, share_name, access, allowed_users) - def update_access(self, server_id, access_rules, + def update_access(self, server_id, share_name, access_rules, add_rules=None, delete_rules=None): def _rule_updater(rules, action, override_type_check=False): @@ -135,7 +130,6 @@ class DockerCIFSHelper(object): "driver.") % access_type raise exception.InvalidShareAccess(reason=msg) - share_name = self.share.share_id if not (add_rules or delete_rules): # clean all users first. self.container.execute( diff --git a/manila/share/drivers/container/storage_helper.py b/manila/share/drivers/container/storage_helper.py index 3803359247..48ac1d0793 100644 --- a/manila/share/drivers/container/storage_helper.py +++ b/manila/share/drivers/container/storage_helper.py @@ -72,43 +72,47 @@ class LVMHelper(driver.ExecuteMixin): 'reserved_percentage': 0, }, ] - def _get_lv_device(self, share): + def _get_lv_device(self, share_name): return os.path.join("/dev", self.configuration.container_volume_group, - share.share_id) + share_name) - def _get_lv_folder(self, share): + def _get_lv_folder(self, share_name): # Provides folder name in hosts /tmp to which logical volume is # mounted prior to providing access to it from a container. - return os.path.join("/tmp/shares", share.share_id) + return os.path.join("/tmp/shares", share_name) - def provide_storage(self, share): - share_name = share.share_id + def provide_storage(self, share_name, size): self._execute("lvcreate", "-p", "rw", "-L", - str(share.size) + "G", "-n", share_name, + str(size) + "G", "-n", share_name, self.configuration.container_volume_group, run_as_root=True) - self._execute("mkfs.ext4", self._get_lv_device(share), + self._execute("mkfs.ext4", self._get_lv_device(share_name), run_as_root=True) - def remove_storage(self, share): - to_remove = self._get_lv_device(share) + def _try_to_unmount_device(self, device): + # NOTE(ganso): We invoke this method to be sure volume was unmounted, + # and we swallow the exception in case it fails to. try: - self._execute("umount", to_remove, run_as_root=True) + self._execute("umount", device, run_as_root=True) except exception.ProcessExecutionError as e: - LOG.warning("Failed to umount helper directory %s.", - to_remove) - LOG.error(e) + LOG.warning("Failed to umount helper directory %(device)s due to " + "%(reason)s.", {'device': device, 'reason': e}) + + def remove_storage(self, share_name): + device = self._get_lv_device(share_name) + self._try_to_unmount_device(device) + # (aovchinnikov): bug 1621784 manifests itself in jamming logical # volumes, so try removing once and issue warning until it is fixed. try: self._execute("lvremove", "-f", "--autobackup", "n", - to_remove, run_as_root=True) + device, run_as_root=True) except exception.ProcessExecutionError as e: - LOG.warning("Failed to remove logical volume %s.", to_remove) - LOG.error(e) + LOG.warning("Failed to remove logical volume %(device)s due to " + "%(reason)s.", {'device': device, 'reason': e}) - def extend_share(self, share, new_size, share_server=None): - lv_device = self._get_lv_device(share) + 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) diff --git a/manila/tests/share/drivers/container/fakes.py b/manila/tests/share/drivers/container/fakes.py index a210431261..f77d07cad0 100644 --- a/manila/tests/share/drivers/container/fakes.py +++ b/manila/tests/share/drivers/container/fakes.py @@ -16,6 +16,56 @@ from manila.tests.db import fakes as db_fakes +FAKE_VSCTL_LIST_INTERFACES_X = ( + 'fake stuff\n' + 'foo not_a_veth something_fake bar\n' + 'foo veth11b2c34 something_fake bar\n' + 'foo veth25f6g7h manila-container="fake1" bar\n' + 'foo veth3jd83j7 manila-container="my_container" bar\n' + 'foo veth4i9j10k manila-container="fake2" bar\n' + 'more fake stuff\n' +) + +FAKE_VSCTL_LIST_INTERFACES = ( + 'fake stuff\n' + 'foo not_a_veth something_fake bar\n' + 'foo veth11b2c34 something_fake bar\n' + 'foo veth25f6g7h manila-container="fake1" bar\n' + 'foo veth3jd83j7 manila-container="manila_my_container" bar\n' + 'foo veth4i9j10k manila-container="fake2" bar\n' + 'more fake stuff\n' +) + +FAKE_VSCTL_LIST_INTERFACE_1 = ( + 'fake stuff\n' + 'foo veth11b2c34 something_fake bar\n' + 'more fake stuff\n' +) + +FAKE_VSCTL_LIST_INTERFACE_2 = ( + 'fake stuff\n' + 'foo veth25f6g7h manila-container="fake1" bar\n' + 'more fake stuff\n' +) + +FAKE_VSCTL_LIST_INTERFACE_3_X = ( + 'fake stuff\n' + 'foo veth3jd83j7 manila-container="my_container" bar\n' + 'more fake stuff\n' +) + +FAKE_VSCTL_LIST_INTERFACE_3 = ( + 'fake stuff\n' + 'foo veth3jd83j7 manila-container="manila_my_container" bar\n' + 'more fake stuff\n' +) + +FAKE_VSCTL_LIST_INTERFACE_4 = ( + 'fake stuff\n' + 'foo veth4i9j10k manila-container="fake2" bar\n' + 'more fake stuff\n' +) + def fake_share(**kwargs): share = { @@ -55,3 +105,23 @@ def fake_network(**kwargs): } network.update(kwargs) return db_fakes.FakeModel(network) + + +def fake_share_server(**kwargs): + share_server = { + 'id': 'fake' + } + share_server.update(kwargs) + return db_fakes.FakeModel(share_server) + + +def fake_identifier(): + return '7cf7c200-d3af-4e05-b87e-9167c95dfcad' + + +def fake_share_no_export_location(**kwargs): + share = { + 'share_id': 'fakeshareid', + } + share.update(kwargs) + return db_fakes.FakeModel(share) diff --git a/manila/tests/share/drivers/container/test_container_helper.py b/manila/tests/share/drivers/container/test_container_helper.py index 637c2ecee2..bbeac35bcc 100644 --- a/manila/tests/share/drivers/container/test_container_helper.py +++ b/manila/tests/share/drivers/container/test_container_helper.py @@ -14,6 +14,7 @@ # under the License. """Unit tests for the Container helper module.""" +import ddt import mock import uuid @@ -21,8 +22,10 @@ from manila import exception from manila.share import configuration from manila.share.drivers.container import container_helper from manila import test +from manila.tests.share.drivers.container import fakes +@ddt.ddt class DockerExecHelperTestCase(test.TestCase): """Tests DockerExecHelper""" @@ -47,9 +50,9 @@ class DockerExecHelperTestCase(test.TestCase): def test_start_container_impossible_failure(self): self.mock_object(self.DockerExecHelper, "_inner_execute", - mock.Mock(return_value=['', 'but how?!'])) + mock.Mock(side_effect=OSError())) - self.assertRaises(exception.ManilaException, + self.assertRaises(exception.ShareBackendException, self.DockerExecHelper.start_container) def test_stop_container(self): @@ -63,10 +66,9 @@ class DockerExecHelperTestCase(test.TestCase): def test_stop_container_oh_noes(self): self.mock_object(self.DockerExecHelper, "_inner_execute", - mock.Mock(return_value=['fake_output', - 'fake_problem'])) + mock.Mock(side_effect=OSError)) - self.assertRaises(exception.ManilaException, + self.assertRaises(exception.ShareBackendException, self.DockerExecHelper.stop_container, "manila-fake-container") @@ -77,7 +79,8 @@ class DockerExecHelperTestCase(test.TestCase): self.DockerExecHelper.execute("fake_container", ["fake_script"]) - self.DockerExecHelper._inner_execute.assert_called_once_with(expected) + self.DockerExecHelper._inner_execute.assert_called_once_with( + expected, ignore_errors=False) def test_execute_name_not_there(self): self.assertRaises(exception.ManilaException, @@ -102,9 +105,122 @@ class DockerExecHelperTestCase(test.TestCase): self.assertEqual(result, 'fake') def test__inner_execute_not_ok(self): - self.DockerExecHelper._execute = mock.Mock(return_value='fake') - self.DockerExecHelper._execute.side_effect = KeyError() + self.DockerExecHelper._execute = mock.Mock(side_effect=[OSError()]) - result = self.DockerExecHelper._inner_execute("fake_command") + self.assertRaises(OSError, + self.DockerExecHelper._inner_execute, "fake_command") + + def test__inner_execute_not_ok_ignore_errors(self): + self.DockerExecHelper._execute = mock.Mock(side_effect=OSError()) + + result = self.DockerExecHelper._inner_execute("fake_command", + ignore_errors=True) self.assertIsNone(result) + + @ddt.data(('inet', + "192.168.0.254", + ["5: br0 inet 192.168.0.254/24 brd 192.168.0.255 " + "scope global br0 valid_lft forever preferred_lft forever"]), + ("inet6", + "2001:470:8:c82:6600:6aff:fe84:8dda", + ["5: br0 inet6 2001:470:8:c82:6600:6aff:fe84:8dda/64 " + "scope global valid_lft forever preferred_lft forever"]), + ) + @ddt.unpack + def test_fetch_container_address(self, address_family, expected_address, + return_value): + fake_name = "fakeserver" + mock_execute = self.DockerExecHelper.execute = mock.Mock( + return_value=return_value) + + address = self.DockerExecHelper.fetch_container_address( + fake_name, + address_family) + + self.assertEqual(expected_address, address) + mock_execute.assert_called_once_with( + fake_name, ["ip", "-oneline", "-family", address_family, "address", + "show", "scope", "global", "dev", "eth0"] + ) + + @ddt.data('my_container', 'manila_my_container') + def test_find_container_veth(self, name): + + interfaces = [fakes.FAKE_VSCTL_LIST_INTERFACE_1, + fakes.FAKE_VSCTL_LIST_INTERFACE_2, + fakes.FAKE_VSCTL_LIST_INTERFACE_4] + + if 'manila_' in name: + list_interfaces = [fakes.FAKE_VSCTL_LIST_INTERFACES] + interfaces.append(fakes.FAKE_VSCTL_LIST_INTERFACE_3) + else: + list_interfaces = [fakes.FAKE_VSCTL_LIST_INTERFACES_X] + interfaces.append(fakes.FAKE_VSCTL_LIST_INTERFACE_3_X) + + def get_interface_data_according_to_veth(*args, **kwargs): + if len(args) == 4: + for interface in interfaces: + if args[3] in interface: + return [interface] + else: + return list_interfaces + + self.DockerExecHelper._execute = mock.Mock( + side_effect=get_interface_data_according_to_veth) + + result = self.DockerExecHelper.find_container_veth(name) + + self.assertEqual("veth3jd83j7", result) + + @ddt.data(True, False) + def test_find_container_veth_not_found(self, remove_veth): + + if remove_veth: + list_executes = [[fakes.FAKE_VSCTL_LIST_INTERFACES], + [fakes.FAKE_VSCTL_LIST_INTERFACE_1], + OSError, + [fakes.FAKE_VSCTL_LIST_INTERFACE_3], + [fakes.FAKE_VSCTL_LIST_INTERFACE_4]] + else: + list_executes = [[fakes.FAKE_VSCTL_LIST_INTERFACES], + [fakes.FAKE_VSCTL_LIST_INTERFACE_1], + [fakes.FAKE_VSCTL_LIST_INTERFACE_2], + [fakes.FAKE_VSCTL_LIST_INTERFACE_3], + [fakes.FAKE_VSCTL_LIST_INTERFACE_4]] + + self.DockerExecHelper._execute = mock.Mock( + side_effect=list_executes) + list_veths = ['veth11b2c34', 'veth25f6g7h', 'veth3jd83j7', + 'veth4i9j10k'] + + self.assertIsNone( + self.DockerExecHelper.find_container_veth("foo_bar")) + list_calls = [mock.call("ovs-vsctl", "list", "interface", + run_as_root=True)] + + for veth in list_veths: + list_calls.append( + mock.call("ovs-vsctl", "list", "interface", veth, + run_as_root=True) + ) + + self.DockerExecHelper._execute.assert_has_calls( + list_calls, any_order=True + ) + + @ddt.data((["wrong_name\nfake\nfake_container\nfake_name'"], True), + (["wrong_name\nfake_container\nfake'"], False), + ("\n", False)) + @ddt.unpack + def test_container_exists(self, fake_return_value, expected_result): + + self.DockerExecHelper._execute = mock.Mock( + return_value=fake_return_value) + + result = self.DockerExecHelper.container_exists("fake_name") + + self.DockerExecHelper._execute.assert_called_once_with( + "docker", "ps", "--no-trunc", "--format='{{.Names}}'", + run_as_root=True) + self.assertEqual(expected_result, result) diff --git a/manila/tests/share/drivers/container/test_driver.py b/manila/tests/share/drivers/container/test_driver.py index 787f026283..62263c8291 100644 --- a/manila/tests/share/drivers/container/test_driver.py +++ b/manila/tests/share/drivers/container/test_driver.py @@ -109,62 +109,142 @@ class ContainerShareDriverTestCase(test.TestCase): self.assertFalse(self._driver._stats['ipv6_support']) def test_create_share(self): - helper = mock.Mock() - self.mock_object(helper, 'create_share', - mock.Mock(return_value='export_location')) - self.mock_object(self._driver, "_get_helper", - mock.Mock(return_value=helper)) - self.mock_object(self._driver.storage, 'provide_storage') - self.mock_object(self._driver.container, 'execute') + + share_server = {'id': 'fake'} + fake_container_name = 'manila_fake_container' + + mock_provide_storage = self.mock_object(self._driver.storage, + 'provide_storage') + mock_get_container_name = self.mock_object( + 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', + mock.Mock(return_value='export_location')) self.assertEqual('export_location', self._driver.create_share(self._context, self.share, - {'id': 'fake'})) + share_server)) + mock_provide_storage.assert_called_once_with( + self.share.share_id, self.share.size + ) + 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( + share_server['id'] + ) - def test_delete_share_ok(self): + def test__create_and_mount_share_links(self): helper = mock.Mock() - self.mock_object(self._driver, "_get_helper", - mock.Mock(return_value=helper)) - self.mock_object(self._driver.container, 'execute') + server_id = 'fake_id' + share_name = 'fake_name' + + mock_create_share = self.mock_object( + helper, 'create_share', mock.Mock(return_value='export_location')) + mock__get_helper = self.mock_object( + self._driver, "_get_helper", mock.Mock(return_value=helper)) + self.mock_object(self._driver.storage, "_get_lv_device", + mock.Mock(return_value={})) + mock_execute = self.mock_object(self._driver.container, 'execute') + + self.assertEqual('export_location', + self._driver._create_and_mount_share_links( + self.share, server_id, share_name)) + mock_create_share.assert_called_once_with(server_id) + mock__get_helper.assert_called_once_with(self.share) + mock_execute.assert_has_calls([ + mock.call(server_id, ["mkdir", "-m", "750", + "/shares/%s" % share_name]), + mock.call(server_id, ["mount", {}, + "/shares/%s" % share_name]) + ]) + + def test__delete_and_umount_share_links(self): + helper = mock.Mock() + server_id = 'fake_id' + share_name = 'fake_name' + mock__get_helper = self.mock_object( + 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.share, server_id, share_name) + + mock__get_helper.assert_called_once_with(self.share) + mock_delete_share.assert_called_once_with( + server_id, share_name, ignore_errors=False) + mock_execute.assert_has_calls([ + mock.call(server_id, ["umount", "/shares/%s" % share_name], + ignore_errors=False), + mock.call(server_id, ["rm", "-fR", "/shares/%s" % share_name], + ignore_errors=True)] + ) + + def test_delete_share(self): + fake_server_id = "manila_container_name" + fake_share_name = "fake_share_name" + fake_share_server = {'id': 'fake'} + + mock_get_container_name = self.mock_object( + self._driver, '_get_container_name', + mock.Mock(return_value=fake_server_id)) + mock_get_share_name = self.mock_object( + self._driver, '_get_share_name', + 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_share(self._context, self.share, {'id': 'fake'}) + self._driver.delete_share(self._context, self.share, fake_share_server) - self._driver.container.execute.assert_called_with( - 'manila_fake', - ['rm', '-fR', '/shares/fakeshareid'] - ) + mock_get_container_name.assert_called_once_with( + fake_share_server['id'] + ) + mock_get_share_name.assert_called_with( + self.share + ) + mock_delete_and_umount.assert_called_once_with( + self.share, fake_server_id, fake_share_name, + ignore_errors=True + ) - def test_delete_share_rm_fails(self): - def fake_execute(*args): - if 'rm' in args[1]: - raise exception.ProcessExecutionError() - self.mock_object(driver.LOG, "warning") - self.mock_object(self._driver, "_get_helper") - self.mock_object(self._driver.container, "execute", fake_execute) - self.mock_object(self._driver.storage, 'remove_storage') + @ddt.data(True, False) + def test__get_share_name(self, has_export_location): - self._driver.delete_share(self._context, self.share, {'id': 'fake'}) + if not has_export_location: + fake_share = cont_fakes.fake_share_no_export_location() + expected_result = fake_share.share_id + else: + fake_share = cont_fakes.fake_share() + expected_result = fake_share['export_location'].split('/')[-1] - self.assertTrue(driver.LOG.warning.called) + result = self._driver._get_share_name(fake_share) + self.assertEqual(expected_result, result) def test_extend_share(self): + fake_new_size = 2 + fake_share_server = {'id': 'fake-server'} share = cont_fakes.fake_share() + share_name = self._driver._get_share_name(share) actual_arguments = [] expected_arguments = [ - ('manila_fake_server', ['umount', '/shares/fakeshareid']), + ('manila_fake_server', ['umount', '/shares/%s' % share_name]), ('manila_fake_server', - ['mount', '/dev/manila_docker_volumes/fakeshareid', - '/shares/fakeshareid']) + ['mount', '/dev/manila_docker_volumes/%s' % share_name, + '/shares/%s' % share_name]) ] - self.mock_object(self._driver.storage, "extend_share") + mock_extend_share = self.mock_object(self._driver.storage, + "extend_share") self._driver.container.execute = functools.partial( self.fake_exec_sync, execute_arguments=actual_arguments, ret_val='') - self._driver.extend_share(share, 2, {'id': 'fake-server'}) + self._driver.extend_share(share, fake_new_size, fake_share_server) self.assertEqual(expected_arguments, actual_arguments) + mock_extend_share.assert_called_once_with(share_name, fake_new_size, + fake_share_server) def test_ensure_share(self): # Does effectively nothing by design. @@ -172,6 +252,7 @@ class ContainerShareDriverTestCase(test.TestCase): def test_update_access_access_rules_ok(self): helper = mock.Mock() + fake_share_name = self._driver._get_share_name(self.share) self.mock_object(self._driver, "_get_helper", mock.Mock(return_value=helper)) @@ -179,7 +260,7 @@ class ContainerShareDriverTestCase(test.TestCase): [{'access_level': const.ACCESS_LEVEL_RW}], [], [], {"id": "fake"}) - helper.update_access.assert_called_with('manila_fake', + helper.update_access.assert_called_with('manila_fake', fake_share_name, [{'access_level': 'rw'}], [], []) @@ -229,75 +310,37 @@ class ContainerShareDriverTestCase(test.TestCase): self._driver._connect_to_network("fake-server", network_info, "fake-veth") - @ddt.data(['veth0000000'], ['veth0000000' * 2]) - def test__teardown_server(self, list_of_veths): - def fake_ovs_execute(*args, **kwargs): - kwargs['arguments'].append(args) - if len(args) == 3: - return list_of_veths - elif len(args) == 4: - return ('fake:manila_b5afb5c1_6011_43c4_8a37_29820e6951a7', '') - else: - return 0 - actual_arguments = [] - expected_arguments = [ - ('ovs-vsctl', 'list', 'interface'), - ('ovs-vsctl', 'list', 'interface', 'veth0000000'), - ('ovs-vsctl', '--', 'del-port', 'br-int', 'veth0000000') - ] - self.mock_object(self._driver.container, "stop_container", mock.Mock()) - self._driver._execute = functools.partial( - fake_ovs_execute, arguments=actual_arguments) + @ddt.data({'veth': "fake_veth", 'exception': None}, + {'veth': "fake_veth", 'exception': + exception.ProcessExecutionError('fake')}, + {'veth': None, 'exception': None}) + @ddt.unpack + def test__teardown_server(self, veth, exception): + fake_server_details = {"id": "b5afb5c1-6011-43c4-8a37-29820e6951a7"} + container_name = self._driver._get_container_name( + fake_server_details['id']) + mock_stop_container = self.mock_object( + self._driver.container, "stop_container") + mock_find_container = self.mock_object( + self._driver.container, "find_container_veth", + mock.Mock(return_value=veth)) + mock_execute = self.mock_object(self._driver, "_execute", + mock.Mock(side_effect=exception)) self._driver._teardown_server( - server_details={"id": "b5afb5c1-6011-43c4-8a37-29820e6951a7"}) + server_details=fake_server_details) - self.assertEqual(expected_arguments.sort(), actual_arguments.sort()) - - @ddt.data(['veth0000000'], ['veth0000000' * 2]) - def test__teardown_server_veth_disappeared_mysteriously(self, - list_of_veths): - def fake_ovs_execute(*args, **kwargs): - if len(args) == 3: - return list_of_veths - if len(args) == 4: - return ('fake:manila_b5afb5c1_6011_43c4_8a37_29820e6951a7', '') - if 'del-port' in args: - raise exception.ProcessExecutionError() - else: - return 0 - self.mock_object(driver.LOG, "warning") - self.mock_object(self._driver, "_execute", fake_ovs_execute) - - self._driver._teardown_server( - server_details={"id": "b5afb5c1-6011-43c4-8a37-29820e6951a7"}) - - self.assertTrue(driver.LOG.warning.called) - - @ddt.data(['veth0000000'], ['veth0000000' * 2]) - def test__teardown_server_check_continuation(self, list_of_veths): - def fake_ovs_execute(*args, **kwargs): - kwargs['arguments'].append(args) - if len(args) == 3: - return list_of_veths - elif len(args) == 4: - return ('fake:', '') - else: - return 0 - actual_arguments = [] - expected_arguments = [ - ('ovs-vsctl', 'list', 'interface'), - ('ovs-vsctl', 'list', 'interface', 'veth0000000'), - ('ovs-vsctl', '--', 'del-port', 'br-int', 'veth0000000') - ] - self.mock_object(self._driver.container, "stop_container", mock.Mock()) - self._driver._execute = functools.partial( - fake_ovs_execute, arguments=actual_arguments) - - self._driver._teardown_server( - server_details={"id": "b5afb5c1-6011-43c4-8a37-29820e6951a7"}) - - self.assertEqual(expected_arguments.sort(), actual_arguments.sort()) + mock_stop_container.assert_called_once_with( + container_name + ) + mock_find_container.assert_called_once_with( + container_name + ) + if exception is None and veth is not None: + mock_execute.assert_called_once_with( + "ovs-vsctl", "--", "del-port", + self._driver.configuration.container_ovs_bridge_name, veth, + run_as_root=True) def test__get_veth_state(self): retval = ('veth0000000\n', '') diff --git a/manila/tests/share/drivers/container/test_protocol_helper.py b/manila/tests/share/drivers/container/test_protocol_helper.py index 4eceaad652..0896f78e4d 100644 --- a/manila/tests/share/drivers/container/test_protocol_helper.py +++ b/manila/tests/share/drivers/container/test_protocol_helper.py @@ -98,11 +98,11 @@ class DockerCIFSHelperTestCase(test.TestCase): def test_delete_share(self): self.DockerCIFSHelper.share = fake_share() - self.DockerCIFSHelper.delete_share("fakeserver") + self.DockerCIFSHelper.delete_share("fakeserver", "fakeshareid") self.DockerCIFSHelper.container.execute.assert_called_with( "fakeserver", - ["net", "conf", "delshare", "fakeshareid"]) + ["net", "conf", "delshare", "fakeshareid"], ignore_errors=False) def test__get_access_group_ro(self): result = self.DockerCIFSHelper._get_access_group(const.ACCESS_LEVEL_RO) @@ -130,7 +130,8 @@ class DockerCIFSHelperTestCase(test.TestCase): self.assertEqual("fake_user", result) self.DockerCIFSHelper.container.execute.assert_called_once_with( "fake_server_id", - ["net", "conf", "getparm", "fake_share", "fake_access"]) + ["net", "conf", "getparm", "fake_share", "fake_access"], + ignore_errors=True) def test__set_users(self): self.DockerCIFSHelper.container.execute = mock.Mock() @@ -239,7 +240,7 @@ class DockerCIFSHelperTestCase(test.TestCase): self.assertRaises(exception.InvalidShareAccess, self.DockerCIFSHelper.update_access, - "fakeserver", allow_rules, [], []) + "fakeserver", "fakeshareid", allow_rules, [], []) def test_update_access_access_rules_ok(self): access_rules = [{ @@ -250,7 +251,7 @@ class DockerCIFSHelperTestCase(test.TestCase): self.mock_object(self.DockerCIFSHelper, "_allow_access") self.DockerCIFSHelper.container.execute = mock.Mock() - self.DockerCIFSHelper.update_access("fakeserver", + self.DockerCIFSHelper.update_access("fakeserver", "fakeshareid", access_rules, [], []) self.DockerCIFSHelper._allow_access.assert_called_once_with( @@ -270,7 +271,7 @@ class DockerCIFSHelperTestCase(test.TestCase): }] self.mock_object(self.DockerCIFSHelper, "_allow_access") - self.DockerCIFSHelper.update_access("fakeserver", [], + self.DockerCIFSHelper.update_access("fakeserver", "fakeshareid", [], add_rules, []) self.DockerCIFSHelper._allow_access.assert_called_once_with( @@ -287,30 +288,11 @@ class DockerCIFSHelperTestCase(test.TestCase): }] self.mock_object(self.DockerCIFSHelper, "_deny_access") - self.DockerCIFSHelper.update_access("fakeserver", [], - [], delete_rules) + self.DockerCIFSHelper.update_access("fakeserver", "fakeshareid", + [], [], delete_rules) self.DockerCIFSHelper._deny_access.assert_called_once_with( "fakeshareid", "fakeserver", "fakeuser", "ro") - - @ddt.data(('inet', - "192.168.0.254", - ["5: br0 inet 192.168.0.254/24 brd 192.168.0.255 " - "scope global br0 valid_lft forever preferred_lft forever"]), - ("inet6", - "2001:470:8:c82:6600:6aff:fe84:8dda", - ["5: br0 inet6 2001:470:8:c82:6600:6aff:fe84:8dda/64 " - "scope global valid_lft forever preferred_lft forever"]), - ) - @ddt.unpack - def test__fetch_container_address(self, address_family, expected_address, - return_value): - self.DockerCIFSHelper.container.execute = mock.Mock( - return_value=return_value) - address = self.DockerCIFSHelper._fetch_container_address( - "fakeserver", - address_family) - self.assertEqual(expected_address, address) diff --git a/manila/tests/share/drivers/container/test_storage_helper.py b/manila/tests/share/drivers/container/test_storage_helper.py index d5c8db0bb6..1d3eac765c 100644 --- a/manila/tests/share/drivers/container/test_storage_helper.py +++ b/manila/tests/share/drivers/container/test_storage_helper.py @@ -61,15 +61,18 @@ class LVMHelperTestCase(test.TestCase): self.assertEqual(expected_result, result) def test__get_lv_device(self): - self.assertEqual("/dev/manila_docker_volumes/fakeshareid", - self.LVMHelper._get_lv_device(self.share)) + fake_share_name = 'fakeshareid' + self.assertEqual("/dev/manila_docker_volumes/%s" % fake_share_name, + self.LVMHelper._get_lv_device(fake_share_name)) def test__get_lv_folder(self): - self.assertEqual("/tmp/shares/fakeshareid", - self.LVMHelper._get_lv_folder(self.share)) + fake_share_name = 'fakeshareid' + self.assertEqual("/tmp/shares/%s" % fake_share_name, + self.LVMHelper._get_lv_folder(fake_share_name)) def test_provide_storage(self): actual_arguments = [] + fake_share_name = 'fakeshareid' expected_arguments = [ ('lvcreate', '-p', 'rw', '-L', '1G', '-n', 'fakeshareid', 'manila_docker_volumes'), @@ -79,38 +82,49 @@ class LVMHelperTestCase(test.TestCase): self.fake_exec_sync, execute_arguments=actual_arguments, ret_val='') - self.LVMHelper.provide_storage(self.share) + self.LVMHelper.provide_storage(fake_share_name, 1) self.assertEqual(expected_arguments, actual_arguments) + @ddt.data(None, exception.ProcessExecutionError) + def test__try_to_unmount_device(self, side_effect): + device = {} + mock_warning = self.mock_object(storage_helper.LOG, 'warning') + mock_execute = self.mock_object(self.LVMHelper, '_execute', + mock.Mock(side_effect=side_effect)) + self.LVMHelper._try_to_unmount_device(device) + + mock_execute.assert_called_once_with( + "umount", device, run_as_root=True + ) + if side_effect is not None: + mock_warning.assert_called_once() + def test_remove_storage(self): - actual_arguments = [] - expected_arguments = [ - ('umount', '/dev/manila_docker_volumes/fakeshareid'), - ('lvremove', '-f', '--autobackup', 'n', - '/dev/manila_docker_volumes/fakeshareid') - ] - self.LVMHelper._execute = functools.partial( - self.fake_exec_sync, execute_arguments=actual_arguments, - ret_val='') + fake_share_name = 'fakeshareid' + fake_device = {} - self.LVMHelper.remove_storage(self.share) + mock_get_lv_device = self.mock_object( + self.LVMHelper, '_get_lv_device', + mock.Mock(return_value=fake_device)) + mock_try_to_umount = self.mock_object(self.LVMHelper, + '_try_to_unmount_device') + mock_execute = self.mock_object(self.LVMHelper, '_execute') - self.assertEqual(expected_arguments, actual_arguments) + self.LVMHelper.remove_storage(fake_share_name) - def test_remove_storage_umount_failed(self): - def fake_execute(*args, **kwargs): - if 'umount' in args: - raise exception.ProcessExecutionError() - - self.mock_object(storage_helper.LOG, "warning") - self.mock_object(self.LVMHelper, "_execute", fake_execute) - - self.LVMHelper.remove_storage(self.share) - - self.assertTrue(storage_helper.LOG.warning.called) + mock_get_lv_device.assert_called_once_with( + fake_share_name + ) + mock_try_to_umount.assert_called_once_with(fake_device) + mock_execute.assert_called_once_with( + 'lvremove', '-f', '--autobackup', 'n', fake_device, + run_as_root=True + ) def test_remove_storage_lvremove_failed(self): + fake_share_name = 'fakeshareid' + def fake_execute(*args, **kwargs): if 'lvremove' in args: raise exception.ProcessExecutionError() @@ -118,7 +132,7 @@ class LVMHelperTestCase(test.TestCase): self.mock_object(storage_helper.LOG, "warning") self.mock_object(self.LVMHelper, "_execute", fake_execute) - self.LVMHelper.remove_storage(self.share) + self.LVMHelper.remove_storage(fake_share_name) self.assertTrue(storage_helper.LOG.warning.called) @@ -130,10 +144,11 @@ class LVMHelperTestCase(test.TestCase): ('e2fsck', '-f', '-y', '/dev/manila_docker_volumes/fakeshareid'), ('resize2fs', '/dev/manila_docker_volumes/fakeshareid'), ] + fake_share_name = 'fakeshareid' self.LVMHelper._execute = functools.partial( self.fake_exec_sync, execute_arguments=actual_arguments, ret_val='') - self.LVMHelper.extend_share(self.share, 'share', 3) + self.LVMHelper.extend_share(fake_share_name, 'share', 3) self.assertEqual(expected_arguments, actual_arguments)