From 7a791177e86b8ce84267fb860ada36d6daf0417b Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Mon, 20 May 2019 10:32:34 +1200 Subject: [PATCH] Get all the database instances by admin Currently, there is no way for the admin user to get databases of all the projects, which is very important for auditing or billing purpose. With this change, admin user can get all the database instances by running: $ openstack database instances list --all-projects Change-Id: I318907b1d4a63017f4a6f7967312770f8784c1f4 Story: #2005735 Task: #33393 --- troveclient/common.py | 2 +- troveclient/osc/v1/database_instances.py | 34 +++++++++--- troveclient/tests/fakes.py | 48 ++++++++++------- .../tests/osc/v1/test_database_instances.py | 54 ++++++++++++++----- troveclient/tests/test_common.py | 7 +++ troveclient/tests/test_management.py | 19 ++++--- troveclient/tests/test_v1_shell.py | 2 +- troveclient/v1/client.py | 6 +-- troveclient/v1/management.py | 25 +++------ 9 files changed, 124 insertions(+), 73 deletions(-) diff --git a/troveclient/common.py b/troveclient/common.py index 9c99b53f..df481c48 100644 --- a/troveclient/common.py +++ b/troveclient/common.py @@ -28,7 +28,7 @@ def append_query_strings(url, **query_strings): if not query_strings: return url query = '&'.join('{0}={1}'.format(key, val) - for key, val in query_strings.items() if val) + for key, val in query_strings.items() if val is not None) return url + ('?' + query if query else "") diff --git a/troveclient/osc/v1/database_instances.py b/troveclient/osc/v1/database_instances.py index 3bb6a742..d71434b6 100644 --- a/troveclient/osc/v1/database_instances.py +++ b/troveclient/osc/v1/database_instances.py @@ -69,10 +69,13 @@ def set_attributes_for_print_detail(instance): class ListDatabaseInstances(command.Lister): - _description = _("List database instances") columns = ['ID', 'Name', 'Datastore', 'Datastore Version', 'Status', 'Flavor ID', 'Size', 'Region'] + admin_columns = [ + 'ID', 'Name', 'Tenant ID', 'Datastore', 'Datastore Version', 'Status', + 'Flavor ID', 'Size' + ] def get_parser(self, prog_name): parser = super(ListDatabaseInstances, self).get_parser(prog_name) @@ -103,19 +106,34 @@ class ListDatabaseInstances(command.Lister): "deprecated in the future, retaining just " "--include_clustered.") ) + parser.add_argument( + '--all-projects', + dest='all_projects', + action="store_true", + default=False, + help=_("Include database instances of all projects (admin only)") + ) return parser def take_action(self, parsed_args): - db_instances = self.app.client_manager.database.instances - instances = db_instances.list(limit=parsed_args.limit, - marker=parsed_args.marker, - include_clustered=(parsed_args. - include_clustered)) + if parsed_args.all_projects: + db_instances = self.app.client_manager.database.mgmt_instances + cols = self.admin_columns + else: + db_instances = self.app.client_manager.database.instances + cols = self.columns + + instances = db_instances.list( + limit=parsed_args.limit, + marker=parsed_args.marker, + include_clustered=parsed_args.include_clustered + ) if instances: instances = set_attributes_for_print(instances) - instances = [osc_utils.get_item_properties(i, self.columns) + instances = [osc_utils.get_item_properties(i, cols) for i in instances] - return self.columns, instances + + return cols, instances class ShowDatabaseInstance(command.ShowOne): diff --git a/troveclient/tests/fakes.py b/troveclient/tests/fakes.py index 55d13f7f..a863623e 100644 --- a/troveclient/tests/fakes.py +++ b/troveclient/tests/fakes.py @@ -158,25 +158,37 @@ class FakeHTTPClient(base_client.HTTPClient): return r, body def get_instances(self, **kw): - return (200, {}, {"instances": [ + return ( + 200, + {}, { - "id": "1234", - "name": "test-member-1", - "status": "ACTIVE", - "ip": ["10.0.0.13"], - "volume": {"size": 2}, - "flavor": {"id": "02"}, - "region": "regionOne", - "datastore": {"version": "5.6", "type": "mysql"}}, - { - "id": "5678", - "name": "test-member-2", - "status": "ACTIVE", - "ip": ["10.0.0.14"], - "volume": {"size": 2}, - "flavor": {"id": "2"}, - "region": "regionOne", - "datastore": {"version": "5.6", "type": "mysql"}}]}) + "instances": [ + { + "id": "1234", + "name": "test-member-1", + "status": "ACTIVE", + "ip": ["10.0.0.13"], + "volume": {"size": 2}, + "flavor": {"id": "02"}, + "region": "regionOne", + "datastore": {"version": "5.6", "type": "mysql"}, + "tenant_id": "fake_tenant_id", + }, + { + "id": "5678", + "name": "test-member-2", + "status": "ACTIVE", + "ip": ["10.0.0.14"], + "volume": {"size": 2}, + "flavor": {"id": "2"}, + "region": "regionOne", + "datastore": {"version": "5.6", "type": "mysql"}, + "tenant_id": "fake_tenant_id", + }, + + ] + } + ) def get_instance_counts(self, **kw): return (200, {}, {"instances": [ diff --git a/troveclient/tests/osc/v1/test_database_instances.py b/troveclient/tests/osc/v1/test_database_instances.py index 488a835d..9adeae87 100644 --- a/troveclient/tests/osc/v1/test_database_instances.py +++ b/troveclient/tests/osc/v1/test_database_instances.py @@ -21,44 +21,71 @@ from troveclient.tests.osc.v1 import fakes class TestInstances(fakes.TestDatabasev1): - fake_instances = fakes.FakeInstances() - def setUp(self): super(TestInstances, self).setUp() + + self.fake_instances = fakes.FakeInstances() self.instance_client = self.app.client_manager.database.instances + self.mgmt_client = self.app.client_manager.database.mgmt_instances class TestInstanceList(TestInstances): - defaults = { 'include_clustered': False, 'limit': None, 'marker': None } - columns = database_instances.ListDatabaseInstances.columns - values = [('1234', 'test-member-1', 'mysql', '5.6', 'ACTIVE', '02', 2, - 'regionOne'), ('5678', 'test-member-2', 'mysql', '5.6', - 'ACTIVE', '2', 2, 'regionOne')] - def setUp(self): super(TestInstanceList, self).setUp() self.cmd = database_instances.ListDatabaseInstances(self.app, None) self.data = self.fake_instances.get_instances() - self.instance_client.list.return_value = common.Paginated(self.data) def test_instance_list_defaults(self): + self.instance_client.list.return_value = common.Paginated(self.data) + parsed_args = self.check_parser(self.cmd, [], []) columns, data = self.cmd.take_action(parsed_args) + self.instance_client.list.assert_called_once_with(**self.defaults) - self.assertEqual(self.columns, columns) - self.assertEqual(self.values, data) + self.assertEqual( + database_instances.ListDatabaseInstances.columns, + columns + ) + + values = [ + ('1234', 'test-member-1', 'mysql', '5.6', 'ACTIVE', '02', 2, + 'regionOne'), + ('5678', 'test-member-2', 'mysql', '5.6', 'ACTIVE', '2', 2, + 'regionOne') + ] + self.assertEqual(values, data) + + def test_instance_list_all_projects(self): + self.mgmt_client.list.return_value = common.Paginated(self.data) + + parsed_args = self.check_parser(self.cmd, ["--all-projects"], + [("all_projects", True)]) + columns, instances = self.cmd.take_action(parsed_args) + + self.mgmt_client.list.assert_called_once_with(**self.defaults) + self.assertEqual( + database_instances.ListDatabaseInstances.admin_columns, + columns + ) + + expected_instances = [ + ('1234', 'test-member-1', 'fake_tenant_id', 'mysql', '5.6', + 'ACTIVE', '02', 2), + ('5678', 'test-member-2', 'fake_tenant_id', 'mysql', '5.6', + 'ACTIVE', '2', 2) + ] + self.assertEqual(expected_instances, instances) class TestInstanceShow(TestInstances): - values = ('mysql', '5.6', '02', '1234', '10.0.0.13', - 'test-member-1', 'regionOne', 'ACTIVE', 2) + 'test-member-1', 'regionOne', 'ACTIVE', 'fake_tenant_id', 2) def setUp(self): super(TestInstanceShow, self).setUp() @@ -74,6 +101,7 @@ class TestInstanceShow(TestInstances): 'name', 'region', 'status', + 'tenant_id', 'volume', ) diff --git a/troveclient/tests/test_common.py b/troveclient/tests/test_common.py index 9c8ee5ac..0b6518ac 100644 --- a/troveclient/tests/test_common.py +++ b/troveclient/tests/test_common.py @@ -60,6 +60,13 @@ class CommonTest(testtools.TestCase): self.assertIn("key1=%s" % opts['key1'], result) self.assertNotIn("key2=%s" % opts['key2'], result) + opts = {'key1': 'value1', 'key2': None, 'key3': False} + result = common.append_query_strings(url, **opts) + self.assertTrue(result.startswith(url + '?')) + self.assertIn("key1=value1", result) + self.assertNotIn("key2", result) + self.assertIn("key3=False", result) + class PaginatedTest(testtools.TestCase): diff --git a/troveclient/tests/test_management.py b/troveclient/tests/test_management.py index 104bcc41..afe32e1c 100644 --- a/troveclient/tests/test_management.py +++ b/troveclient/tests/test_management.py @@ -73,18 +73,17 @@ class ManagementTest(testtools.TestCase): p, i = self.management.show(1) self.assertEqual(('/mgmt/instances/instance1', 'instance'), (p, i)) - def test_index(self): + def test_list(self): page_mock = mock.Mock() self.management._paginated = page_mock - self.management.index(deleted=True) - page_mock.assert_called_with('/mgmt/instances?deleted=true', - 'instances', None, None) - self.management.index(deleted=False) - page_mock.assert_called_with('/mgmt/instances?deleted=false', - 'instances', None, None) - self.management.index(deleted=True, limit=10, marker="foo") - page_mock.assert_called_with('/mgmt/instances?deleted=true', - 'instances', 10, "foo") + + self.management.list(deleted=True) + page_mock.assert_called_with('/mgmt/instances', 'instances', None, + None, query_strings={'deleted': True}) + + self.management.list(deleted=False, limit=10, marker="foo") + page_mock.assert_called_with('/mgmt/instances', 'instances', 10, "foo", + query_strings={"deleted": False}) def test_root_enabled_history(self): self.management.api.client.get = mock.Mock(return_value=('resp', None)) diff --git a/troveclient/tests/test_v1_shell.py b/troveclient/tests/test_v1_shell.py index f15c5991..e6adb83e 100644 --- a/troveclient/tests/test_v1_shell.py +++ b/troveclient/tests/test_v1_shell.py @@ -162,7 +162,7 @@ class ShellTest(utils.TestCase): def test_instance_list(self): self.run_command('list') - self.assert_called('GET', '/instances') + self.assert_called('GET', '/instances?include_clustered=False') def test_instance_show(self): self.run_command('show 1234') diff --git a/troveclient/v1/client.py b/troveclient/v1/client.py index 2fc7a971..9db46ba6 100644 --- a/troveclient/v1/client.py +++ b/troveclient/v1/client.py @@ -23,7 +23,7 @@ from troveclient.v1 import datastores from troveclient.v1 import flavors from troveclient.v1 import instances from troveclient.v1 import limits -# from troveclient.v1 import management +from troveclient.v1 import management from troveclient.v1 import metadata from troveclient.v1 import modules from troveclient.v1 import quota @@ -82,11 +82,11 @@ class Client(object): self.configuration_parameters = config_parameters self.metadata = metadata.Metadata(self) self.modules = modules.Modules(self) + self.quota = quota.Quotas(self) + self.mgmt_instances = management.Management(self) # self.hosts = Hosts(self) - self.quota = quota.Quotas(self) # self.storage = StorageInfo(self) - # self.management = Management(self) # self.management = MgmtClusters(self) # self.mgmt_flavor = MgmtFlavors(self) # self.accounts = Accounts(self) diff --git a/troveclient/v1/management.py b/troveclient/v1/management.py index 01ebf0a7..da7ca9d2 100644 --- a/troveclient/v1/management.py +++ b/troveclient/v1/management.py @@ -35,10 +35,6 @@ class Management(base.ManagerWithFind): """Manage :class:`Instances` resources.""" resource_class = instances.Instance - # Appease the abc gods - def list(self): - pass - def show(self, instance): """Get details of one instance. @@ -48,22 +44,13 @@ class Management(base.ManagerWithFind): return self._get("/mgmt/instances/%s" % base.getid(instance), 'instance') - def index(self, deleted=None, limit=None, marker=None): - """Show an overview of all local instances. + def list(self, limit=None, marker=None, deleted=False, **kwargs): + """Get all the database instances.""" + url = "/mgmt/instances" + kwargs["deleted"] = deleted - Optionally, filter by deleted status. - - :rtype: list of :class:`Instance`. - """ - form = '' - if deleted is not None: - if deleted: - form = "?deleted=true" - else: - form = "?deleted=false" - - url = "/mgmt/instances%s" % form - return self._paginated(url, "instances", limit, marker) + return self._paginated(url, "instances", limit, marker, + query_strings=kwargs) def root_enabled_history(self, instance): """Get root access history of one instance."""