From 3b8f24ea81caeac429c2c1cf4fd048cf741cab66 Mon Sep 17 00:00:00 2001
From: Valeriy Ponomaryov <vponomaryov@mirantis.com>
Date: Mon, 3 Oct 2016 19:28:22 +0300
Subject: [PATCH] Fix share-server-delete command
and add functional tests for it.
Which can be enabled/disabled using
'run_share_servers_tests' config option.
Change-Id: I07c5b03d576b41a61867a90d828a112c8a3f4f4b
Closes-Bug: #1629317
---
manilaclient/config.py | 6 +
manilaclient/tests/functional/client.py | 71 ++++++-
.../tests/functional/test_share_servers.py | 123 ++++++++++--
manilaclient/tests/unit/v2/fakes.py | 16 +-
manilaclient/tests/unit/v2/test_shell.py | 183 +++++++++++-------
manilaclient/v2/share_servers.py | 19 +-
manilaclient/v2/shell.py | 14 +-
7 files changed, 329 insertions(+), 103 deletions(-)
diff --git a/manilaclient/config.py b/manilaclient/config.py
index 419906bff..fa753ca20 100644
--- a/manilaclient/config.py
+++ b/manilaclient/config.py
@@ -130,6 +130,12 @@ share_opts = [
help="Defines whether to run tests that use share snapshots "
"or not. Disable this feature if used driver doesn't "
"support it."),
+ cfg.BoolOpt("run_share_servers_tests",
+ default=True,
+ help="Defines whether to run tests that use share servers "
+ "or not. Disable this feature if used driver doesn't "
+ "support it or when autodeletion of share servers "
+ "is enabled."),
]
# 2. Generate config
diff --git a/manilaclient/tests/functional/client.py b/manilaclient/tests/functional/client.py
index 5d5d50e27..e90240fa4 100644
--- a/manilaclient/tests/functional/client.py
+++ b/manilaclient/tests/functional/client.py
@@ -31,6 +31,7 @@ CONF = config.CONF
SHARE = 'share'
SHARE_TYPE = 'share_type'
SHARE_NETWORK = 'share_network'
+SHARE_SERVER = 'share_server'
def not_found_wrapper(f):
@@ -39,9 +40,10 @@ def not_found_wrapper(f):
try:
return f(self, *args, **kwargs)
except tempest_lib_exc.CommandFailed as e:
- if re.search('No (\w+) with a name or ID', e.stderr):
- # Raise appropriate 'NotFound' error
- raise tempest_lib_exc.NotFound()
+ for regexp in ('No (\w+) with a name or ID', 'not found'):
+ if re.search(regexp, e.stderr):
+ # Raise appropriate 'NotFound' error
+ raise tempest_lib_exc.NotFound()
raise
return wrapped_func
@@ -123,6 +125,8 @@ class ManilaCLIClient(base.CLIClient):
func = self.is_share_type_deleted
elif res_type == SHARE_NETWORK:
func = self.is_share_network_deleted
+ elif res_type == SHARE_SERVER:
+ func = self.is_share_server_deleted
elif res_type == SHARE:
func = self.is_share_deleted
elif res_type == SNAPSHOT:
@@ -1068,3 +1072,64 @@ class ManilaCLIClient(base.CLIClient):
microversion=microversion)
share = output_parser.details(share_raw)
return share
+
+ # Share servers
+
+ @not_found_wrapper
+ def get_share_server(self, share_server, microversion=None):
+ """Returns share server by its Name or ID."""
+ share_server_raw = self.manila(
+ 'share-server-show %s' % share_server, microversion=microversion)
+ share_server = output_parser.details(share_server_raw)
+ return share_server
+
+ def list_share_servers(self, filters=None, columns=None,
+ microversion=None):
+ """List share servers.
+
+ :param filters: dict -- filters for listing of share servers.
+ Example, input:
+ {'project_id': 'foo'}
+ {'-project_id': 'foo'}
+ {'--project_id': 'foo'}
+ {'project-id': 'foo'}
+ will be transformed to filter parameter "--project-id=foo"
+ :param columns: comma separated string of columns.
+ Example, "--columns id"
+ """
+ cmd = 'share-server-list '
+ if columns is not None:
+ cmd += ' --columns ' + columns
+ if filters and isinstance(filters, dict):
+ for k, v in filters.items():
+ cmd += '%(k)s=%(v)s ' % {
+ 'k': self._stranslate_to_cli_optional_param(k), 'v': v}
+ share_servers_raw = self.manila(cmd, microversion=microversion)
+ share_servers = utils.listing(share_servers_raw)
+ return share_servers
+
+ @not_found_wrapper
+ def delete_share_server(self, share_server, microversion=None):
+ """Deletes share server by its Name or ID."""
+ return self.manila('share-server-delete %s' % share_server,
+ microversion=microversion)
+
+ def is_share_server_deleted(self, share_server_id, microversion=None):
+ """Says whether share server is deleted or not.
+
+ :param share_server: text -- ID of the share server
+ """
+ servers = self.list_share_servers(microversion=microversion)
+ for list_element in servers:
+ if share_server_id == list_element['Id']:
+ return False
+ return True
+
+ def wait_for_share_server_deletion(self, share_server, microversion=None):
+ """Wait for share server deletion by its Name or ID.
+
+ :param share_server: text -- Name or ID of share server
+ """
+ self.wait_for_resource_deletion(
+ SHARE_SERVER, res_id=share_server, interval=3, timeout=60,
+ microversion=microversion)
diff --git a/manilaclient/tests/functional/test_share_servers.py b/manilaclient/tests/functional/test_share_servers.py
index b648df935..f7b9446f1 100644
--- a/manilaclient/tests/functional/test_share_servers.py
+++ b/manilaclient/tests/functional/test_share_servers.py
@@ -14,34 +14,133 @@
# under the License.
import ddt
+from tempest.lib.common.utils import data_utils
from tempest.lib import exceptions
+from manilaclient import config
from manilaclient.tests.functional import base
+CONF = config.CONF
+
@ddt.ddt
-class ManilaClientTestShareServersReadOnly(base.BaseTestCase):
+class ShareServersReadOnlyTest(base.BaseTestCase):
+
+ @classmethod
+ def setUpClass(cls):
+ super(ShareServersReadOnlyTest, cls).setUpClass()
+ cls.client = cls.get_admin_client()
def test_share_server_list(self):
- self.clients['admin'].manila('share-server-list')
+ self.client.list_share_servers()
def test_share_server_list_with_host_param(self):
- self.clients['admin'].manila('share-server-list', params='--host host')
+ self.client.list_share_servers(filters={'host': 'fake_host'})
def test_share_server_list_with_status_param(self):
- self.clients['admin'].manila(
- 'share-server-list', params='--status status')
+ self.client.list_share_servers(filters={'status': 'fake_status'})
def test_share_server_list_with_share_network_param(self):
- self.clients['admin'].manila(
- 'share-server-list', params='--share-network share-network')
+ self.client.list_share_servers(filters={'share_network': 'fake_sn'})
def test_share_server_list_with_project_id_param(self):
- self.clients['admin'].manila(
- 'share-server-list', params='--project-id project-id')
+ self.client.list_share_servers(
+ filters={'project_id': 'fake_project_id'})
+
+ @ddt.data(
+ 'host', 'status', 'project_id', 'share_network',
+ 'host,status,project_id,share_network',
+ )
+ def test_share_server_list_with_specified_columns(self, columns):
+ self.client.list_share_servers(columns=columns)
def test_share_server_list_by_user(self):
self.assertRaises(
- exceptions.CommandFailed,
- self.clients['user'].manila,
- 'share-server-list')
+ exceptions.CommandFailed, self.user_client.list_share_servers)
+
+
+@ddt.ddt
+class ShareServersReadWriteBase(base.BaseTestCase):
+
+ protocol = None
+
+ @classmethod
+ def setUpClass(cls):
+ super(ShareServersReadWriteBase, cls).setUpClass()
+ if not CONF.run_share_servers_tests:
+ message = "share-servers tests are disabled."
+ raise cls.skipException(message)
+ if cls.protocol not in CONF.enable_protocols:
+ message = "%s tests are disabled." % cls.protocol
+ raise cls.skipException(message)
+
+ cls.client = cls.get_admin_client()
+ if not cls.client.share_network:
+ message = "Can run only with DHSS=True mode"
+ raise cls.skipException(message)
+
+ def test_get_and_delete_share_server(self):
+ name = data_utils.rand_name('autotest_share_name')
+ description = data_utils.rand_name('autotest_share_description')
+
+ # We create separate share network to be able to delete share server
+ # further knowing that it is not used by any other concurrent test.
+ common_share_network = self.client.get_share_network(
+ self.client.share_network)
+ neutron_net_id = (
+ common_share_network['neutron_net_id']
+ if 'none' not in common_share_network['neutron_net_id'].lower()
+ else None)
+ neutron_subnet_id = (
+ common_share_network['neutron_subnet_id']
+ if 'none' not in common_share_network['neutron_subnet_id'].lower()
+ else None)
+ share_network = self.client.create_share_network(
+ neutron_net_id=neutron_net_id,
+ neutron_subnet_id=neutron_subnet_id,
+ )
+
+ self.share = self.create_share(
+ share_protocol=self.protocol,
+ size=1,
+ name=name,
+ description=description,
+ share_network=share_network['id'],
+ client=self.client,
+ )
+ share_server_id = self.client.get_share(
+ self.share['id'])['share_server_id']
+
+ # Get share server
+ server = self.client.get_share_server(share_server_id)
+ expected_keys = (
+ 'id', 'host', 'status', 'created_at', 'updated_at',
+ 'share_network_id', 'share_network_name', 'project_id',
+ )
+ for key in expected_keys:
+ self.assertIn(key, server)
+
+ # Delete share
+ self.client.delete_share(self.share['id'])
+ self.client.wait_for_share_deletion(self.share['id'])
+
+ # Delete share server
+ self.client.delete_share_server(share_server_id)
+ self.client.wait_for_share_server_deletion(share_server_id)
+
+
+class ShareServersReadWriteNFSTest(ShareServersReadWriteBase):
+ protocol = 'nfs'
+
+
+class ShareServersReadWriteCIFSTest(ShareServersReadWriteBase):
+ protocol = 'cifs'
+
+
+def load_tests(loader, tests, _):
+ result = []
+ for test_case in tests:
+ if type(test_case._tests[0]) is ShareServersReadWriteBase:
+ continue
+ result.append(test_case)
+ return loader.suiteClass(result)
diff --git a/manilaclient/tests/unit/v2/fakes.py b/manilaclient/tests/unit/v2/fakes.py
index 44caca06a..cf92467d1 100644
--- a/manilaclient/tests/unit/v2/fakes.py
+++ b/manilaclient/tests/unit/v2/fakes.py
@@ -169,7 +169,16 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
share_servers = {
'share_servers': {
'id': 1234,
- 'share_network_id': 'fake_network_id',
+ 'share_network_id': 'fake_network_id_1',
+ },
+ }
+ return (200, {}, share_servers)
+
+ def get_share_servers_5678(self, **kw):
+ share_servers = {
+ 'share_servers': {
+ 'id': 5678,
+ 'share_network_id': 'fake_network_id_2',
},
}
return (200, {}, share_servers)
@@ -254,6 +263,8 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
raise AssertionError("Unexpected action: %s" % action)
return (resp, {}, _body)
+ post_snapshots_5678_action = post_snapshots_1234_action
+
def post_snapshots_manage(self, body, **kw):
_body = {'snapshot': {'id': 'fake'}}
resp = 202
@@ -452,6 +463,9 @@ class FakeHTTPClient(fakes.FakeHTTPClient):
def delete_share_servers_1234(self, **kwargs):
return (202, {}, None)
+ def delete_share_servers_5678(self, **kwargs):
+ return (202, {}, None)
+
def delete_security_services_fake_security_service1(self, **kwargs):
return (202, {}, None)
diff --git a/manilaclient/tests/unit/v2/test_shell.py b/manilaclient/tests/unit/v2/test_shell.py
index cf7723633..510e5100b 100644
--- a/manilaclient/tests/unit/v2/test_shell.py
+++ b/manilaclient/tests/unit/v2/test_shell.py
@@ -29,7 +29,12 @@ from manilaclient import exceptions
from manilaclient import shell
from manilaclient.tests.unit import utils as test_utils
from manilaclient.tests.unit.v2 import fakes
+from manilaclient.v2 import security_services
from manilaclient.v2 import share_instances
+from manilaclient.v2 import share_networks
+from manilaclient.v2 import share_servers
+from manilaclient.v2 import share_snapshots
+from manilaclient.v2 import share_types
from manilaclient.v2 import shell as shell_v2
@@ -1974,93 +1979,125 @@ class ShellTest(test_utils.TestCase):
expected = {'reset_task_state': {'task_state': param}}
self.assert_called('POST', '/shares/1234/action', body=expected)
- @ddt.data('fake-security-service1',
- 'fake-security-service1 fake-security-service2')
- @mock.patch.object(shell_v2, '_find_security_service', mock.Mock())
- def test_security_service_delete(self, args):
- args_split = args.split()
- shell_v2._find_security_service.side_effect = args_split
+ @ddt.data(('fake_security_service1', ),
+ ('fake_security_service1', 'fake_security_service2'))
+ def test_security_service_delete(self, ss_ids):
+ fake_security_services = [
+ security_services.SecurityService('fake', {'id': ss_id}, True)
+ for ss_id in ss_ids
+ ]
+ self.mock_object(
+ shell_v2, '_find_security_service',
+ mock.Mock(side_effect=fake_security_services))
- self.run_command('security-service-delete %s' % args)
+ self.run_command('security-service-delete %s' % ' '.join(ss_ids))
- self.assert_called_anytime(
- 'DELETE', '/security-services/%s' % args_split[0],
- clear_callstack=False)
- self.assert_called_anytime(
- 'DELETE', '/security-services/%s' % args_split[-1])
+ shell_v2._find_security_service.assert_has_calls([
+ mock.call(self.shell.cs, ss_id) for ss_id in ss_ids
+ ])
+ for ss in fake_security_services:
+ self.assert_called_anytime(
+ 'DELETE', '/security-services/%s' % ss.id,
+ clear_callstack=False)
- @ddt.data('fake-share-network1',
- 'fake-share-network1 fake-share-network2')
- @mock.patch.object(shell_v2, '_find_share_network', mock.Mock())
- def test_share_network_delete(self, args):
- args_split = args.split()
- shell_v2._find_share_network.side_effect = args_split
+ @ddt.data(('fake_share_network1', ),
+ ('fake_share_network1', 'fake_share_network1'))
+ def test_share_network_delete(self, sn_ids):
+ fake_share_networks = [
+ share_networks.ShareNetwork('fake', {'id': sn_id}, True)
+ for sn_id in sn_ids
+ ]
+ self.mock_object(
+ shell_v2, '_find_share_network',
+ mock.Mock(side_effect=fake_share_networks))
- self.run_command('share-network-delete %s' % args)
+ self.run_command('share-network-delete %s' % ' '.join(sn_ids))
- self.assert_called_anytime(
- 'DELETE', '/share-networks/%s' % args_split[0],
- clear_callstack=False)
- self.assert_called_anytime(
- 'DELETE', '/share-networks/%s' % args_split[-1])
+ shell_v2._find_share_network.assert_has_calls([
+ mock.call(self.shell.cs, sn_id) for sn_id in sn_ids
+ ])
+ for sn in fake_share_networks:
+ self.assert_called_anytime(
+ 'DELETE', '/share-networks/%s' % sn.id,
+ clear_callstack=False)
- @ddt.data('fake-snapshot1',
- 'fake-snapshot1 fake-snapshot2')
- @mock.patch.object(shell_v2, '_find_share_snapshot', mock.Mock())
- def test_snapshot_delete(self, args):
- args_split = args.split()
- shell_v2._find_share_snapshot.side_effect = args_split
+ @ddt.data(('fake_snapshot1', ), ('fake_snapshot1', 'fake_snapshot2'))
+ def test_snapshot_delete(self, snapshot_ids):
+ fake_snapshots = [
+ share_snapshots.ShareSnapshot('fake', {'id': snapshot_id}, True)
+ for snapshot_id in snapshot_ids
+ ]
+ self.mock_object(
+ shell_v2, '_find_share_snapshot',
+ mock.Mock(side_effect=fake_snapshots))
- self.run_command('snapshot-delete %s' % args)
+ self.run_command('snapshot-delete %s' % ' '.join(snapshot_ids))
- self.assert_called_anytime(
- 'DELETE', '/snapshots/%s' % args_split[0],
- clear_callstack=False)
- self.assert_called_anytime(
- 'DELETE', '/snapshots/%s' % args_split[-1])
+ shell_v2._find_share_snapshot.assert_has_calls([
+ mock.call(self.shell.cs, s_id) for s_id in snapshot_ids
+ ])
+ for snapshot in fake_snapshots:
+ self.assert_called_anytime(
+ 'DELETE', '/snapshots/%s' % snapshot.id,
+ clear_callstack=False)
- @ddt.data('fake-snapshot-force1',
- 'fake-snapshot-force1 fake-snapshot-force2')
- @mock.patch.object(shell_v2, '_find_share_snapshot', mock.Mock())
- def test_snapshot_delete_force(self, args):
- args_split = args.split()
- shell_v2._find_share_snapshot.side_effect = args_split
+ @ddt.data(('1234', ), ('1234', '5678'))
+ def test_snapshot_force_delete(self, snapshot_ids):
+ fake_snapshots = [
+ share_snapshots.ShareSnapshot('fake', {'id': snapshot_id}, True)
+ for snapshot_id in snapshot_ids
+ ]
+ self.mock_object(
+ shell_v2, '_find_share_snapshot',
+ mock.Mock(side_effect=fake_snapshots))
- self.run_command('snapshot-force-delete %s' % args)
+ self.run_command('snapshot-force-delete %s' % ' '.join(snapshot_ids))
- self.assert_called_anytime(
- 'POST', '/snapshots/%s/action' % args_split[0],
- {'force_delete': None}, clear_callstack=False)
- self.assert_called_anytime(
- 'POST', '/snapshots/%s/action' % args_split[-1],
- {'force_delete': None})
+ shell_v2._find_share_snapshot.assert_has_calls([
+ mock.call(self.shell.cs, s_id) for s_id in snapshot_ids
+ ])
+ for snapshot in fake_snapshots:
+ self.assert_called_anytime(
+ 'POST', '/snapshots/%s/action' % snapshot.id,
+ {'force_delete': None},
+ clear_callstack=False)
- @ddt.data('fake-type1',
- 'fake-type1 fake-type2')
- @mock.patch.object(shell_v2, '_find_share_type', mock.Mock())
- def test_share_type_delete(self, args):
- args_split = args.split()
- shell_v2._find_share_type.side_effect = args_split
+ @ddt.data(('fake_type1', ), ('fake_type1', 'fake_type2'))
+ def test_share_type_delete(self, type_ids):
+ fake_share_types = [
+ share_types.ShareType('fake', {'id': type_id}, True)
+ for type_id in type_ids
+ ]
+ self.mock_object(
+ shell_v2, '_find_share_type',
+ mock.Mock(side_effect=fake_share_types))
- self.run_command('type-delete %s' % args)
+ self.run_command('type-delete %s' % ' '.join(type_ids))
- self.assert_called_anytime(
- 'DELETE', '/types/%s' % args_split[0],
- clear_callstack=False)
- self.assert_called_anytime(
- 'DELETE', '/types/%s' % args_split[-1])
+ shell_v2._find_share_type.assert_has_calls([
+ mock.call(self.shell.cs, t_id) for t_id in type_ids
+ ])
+ for fake_share_type in fake_share_types:
+ self.assert_called_anytime(
+ 'DELETE', '/types/%s' % fake_share_type.id,
+ clear_callstack=False)
- @ddt.data('fake-share-server1',
- 'fake-share-server1 fake-share-server2')
- @mock.patch.object(shell_v2, '_find_share_server', mock.Mock())
- def test_share_server_delete(self, args):
- args_split = args.split()
- shell_v2._find_share_server.side_effect = args_split
+ @ddt.data(('1234', ), ('1234', '5678'))
+ def test_share_server_delete(self, server_ids):
+ fake_share_servers = [
+ share_servers.ShareServer('fake', {'id': server_id}, True)
+ for server_id in server_ids
+ ]
+ self.mock_object(
+ shell_v2, '_find_share_server',
+ mock.Mock(side_effect=fake_share_servers))
- self.run_command('share-server-delete %s' % args)
+ self.run_command('share-server-delete %s' % ' '.join(server_ids))
- self.assert_called_anytime(
- 'DELETE', '/share-servers/%s' % args_split[0],
- clear_callstack=False)
- self.assert_called_anytime(
- 'DELETE', '/share-servers/%s' % args_split[-1])
+ shell_v2._find_share_server.assert_has_calls([
+ mock.call(self.shell.cs, s_id) for s_id in server_ids
+ ])
+ for server in fake_share_servers:
+ self.assert_called_anytime(
+ 'DELETE', '/share-servers/%s' % server.id,
+ clear_callstack=False)
diff --git a/manilaclient/v2/share_servers.py b/manilaclient/v2/share_servers.py
index c820e5ab1..d13f3d8f8 100644
--- a/manilaclient/v2/share_servers.py
+++ b/manilaclient/v2/share_servers.py
@@ -38,17 +38,22 @@ class ShareServer(common_base.Resource):
attr = 'share_network_name'
return super(ShareServer, self).__getattr__(attr)
+ def delete(self):
+ """Delete this share server."""
+ self.manager.delete(self)
+
class ShareServerManager(base.ManagerWithFind):
"""Manage :class:`ShareServer` resources."""
resource_class = ShareServer
- def get(self, server_id):
+ def get(self, server):
"""Get a share server.
- :param server_id: The ID of the share server to get.
+ :param server: ID of the :class:`ShareServer` to get.
:rtype: :class:`ShareServer`
"""
+ server_id = common_base.getid(server)
server = self._get("%s/%s" % (RESOURCES_PATH, server_id),
RESOURCE_NAME)
# Split big dict 'backend_details' to separated strings
@@ -62,20 +67,22 @@ class ShareServerManager(base.ManagerWithFind):
server._info["details:%s" % k] = v
return server
- def details(self, server_id):
+ def details(self, server):
"""Get a share server details.
- :param server_id: The ID of the share server to get details from.
+ :param server: ID of the :class:`ShareServer` to get details from.
:rtype: list of :class:`ShareServerBackendDetails
"""
+ server_id = common_base.getid(server)
return self._get("%s/%s/details" % (RESOURCES_PATH, server_id),
"details")
- def delete(self, server_id):
+ def delete(self, server):
"""Delete share server.
- :param server_id: id of share server to be deleted.
+ :param server: ID of the :class:`ShareServer` to delete.
"""
+ server_id = common_base.getid(server)
self._delete(RESOURCE_PATH % server_id)
def list(self, search_opts=None):
diff --git a/manilaclient/v2/shell.py b/manilaclient/v2/shell.py
index 37d2c7773..02fb1eb07 100644
--- a/manilaclient/v2/shell.py
+++ b/manilaclient/v2/shell.py
@@ -2605,15 +2605,14 @@ def do_share_server_delete(cs, args):
failure_count = 0
- for id in args.id:
+ for server_id in args.id:
try:
- id_ref = _find_share_server(
- cs, id)
+ id_ref = _find_share_server(cs, server_id)
cs.share_servers.delete(id_ref)
except Exception as e:
failure_count += 1
print("Delete for share server %s failed: %s" % (
- id, e), file=sys.stderr)
+ server_id, e), file=sys.stderr)
if failure_count == len(args.id):
raise exceptions.CommandError("Unable to delete any of the specified "
@@ -2950,15 +2949,14 @@ def do_type_delete(cs, args):
failure_count = 0
- for id in args.id:
+ for name_or_id in args.id:
try:
- id_ref = _find_share_type(
- cs, id)
+ id_ref = _find_share_type(cs, name_or_id)
cs.share_types.delete(id_ref)
except Exception as e:
failure_count += 1
print("Delete for share type %s failed: %s" % (
- id, e), file=sys.stderr)
+ name_or_id, e), file=sys.stderr)
if failure_count == len(args.id):
raise exceptions.CommandError("Unable to delete any of the specified "