From 63e15ff405c46821d64676900edf020973c280de Mon Sep 17 00:00:00 2001 From: Khomesh Thakre Date: Fri, 6 Nov 2020 22:45:03 +0530 Subject: compute: Show flavor in 'server list' with API >= 2.47 Fix the issue where the flavor name was empty in server list output. This requires somewhat invasive unit test changes to reflect the changed API response from the server, but this has the upside of meaning we don't need new tests since what we have validates things. Also drop the flavor ID column as it is removed from the compute API. Change-Id: Ica3320242a38901c1180b2b29109c9474366fde0 Signed-off-by: Khomesh Thakre Story: 2008257 Task: 41113 (cherry picked from commit 8e362402dee07744668bcf7f6774af4fbe9a07e3) (cherry picked from commit 0873e7580eceab07c3be0824d2ea4163491f8d6e) (cherry picked from commit 4b7e777c0ce19aa67a9a33cbeb3b4ee2b052383f) (cherry picked from commit fef473390c7bb6874a38b98053e54cf18547b23c) (cherry picked from commit 0a6babc04ce53a9234521e0549ba11b725d20bf7) --- .../tests/unit/compute/v2/test_server.py | 316 +++++++++++++-------- 1 file changed, 201 insertions(+), 115 deletions(-) (limited to 'openstackclient/tests') diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 819fc354..b5071b6b 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -2337,7 +2337,7 @@ class TestServerDumpCreate(TestServer): self.run_method_with_servers('trigger_crash_dump', 3) -class TestServerList(TestServer): +class _TestServerList(TestServer): # Columns to be listed up. columns = ( @@ -2365,7 +2365,7 @@ class TestServerList(TestServer): ) def setUp(self): - super(TestServerList, self).setUp() + super(_TestServerList, self).setUp() self.search_opts = { 'reservation_id': None, @@ -2420,10 +2420,11 @@ class TestServerList(TestServer): # Get the command object to test self.cmd = server.ListServer(self.app, None) - # Prepare data returned by fake Nova API. - self.data = [] - self.data_long = [] - self.data_no_name_lookup = [] + +class TestServerList(_TestServerList): + + def setUp(self): + super(TestServerList, self).setUp() Image = collections.namedtuple('Image', 'id name') self.images_mock.list.return_value = [ @@ -2438,8 +2439,8 @@ class TestServerList(TestServer): for s in self.servers ] - for s in self.servers: - self.data.append(( + self.data = tuple( + ( s.id, s.name, s.status, @@ -2447,34 +2448,8 @@ class TestServerList(TestServer): # Image will be an empty string if boot-from-volume self.image.name if s.image else s.image, self.flavor.name, - )) - self.data_long.append(( - s.id, - s.name, - s.status, - getattr(s, 'OS-EXT-STS:task_state'), - server._format_servers_list_power_state( - getattr(s, 'OS-EXT-STS:power_state') - ), - server._format_servers_list_networks(s.networks), - # Image will be an empty string if boot-from-volume - self.image.name if s.image else s.image, - s.image['id'] if s.image else s.image, - self.flavor.name, - s.flavor['id'], - getattr(s, 'OS-EXT-AZ:availability_zone'), - getattr(s, 'OS-EXT-SRV-ATTR:host'), - s.Metadata, - )) - self.data_no_name_lookup.append(( - s.id, - s.name, - s.status, - server._format_servers_list_networks(s.networks), - # Image will be an empty string if boot-from-volume - s.image['id'] if s.image else s.image, - s.flavor['id'] - )) + ) for s in self.servers + ) def test_server_list_no_option(self): arglist = [] @@ -2495,7 +2470,7 @@ class TestServerList(TestServer): self.assertFalse(self.flavors_mock.get.call_count) self.assertFalse(self.images_mock.get.call_count) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_no_servers(self): arglist = [] @@ -2514,9 +2489,28 @@ class TestServerList(TestServer): self.assertEqual(0, self.images_mock.list.call_count) self.assertEqual(0, self.flavors_mock.list.call_count) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_long_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + getattr(s, 'OS-EXT-STS:task_state'), + server._format_servers_list_power_state( + getattr(s, 'OS-EXT-STS:power_state') + ), + server._format_servers_list_networks(s.networks), + # Image will be an empty string if boot-from-volume + self.image.name if s.image else s.image, + s.image['id'] if s.image else s.image, + self.flavor.name, + s.flavor['id'], + getattr(s, 'OS-EXT-AZ:availability_zone'), + getattr(s, 'OS-EXT-SRV-ATTR:host'), + s.Metadata, + ) for s in self.servers) arglist = [ '--long', ] @@ -2527,12 +2521,23 @@ class TestServerList(TestServer): parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns_long, columns) - self.assertEqual(tuple(self.data_long), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_no_name_lookup_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + server._format_servers_list_networks(s.networks), + # Image will be an empty string if boot-from-volume + s.image['id'] if s.image else s.image, + s.flavor['id'] + ) for s in self.servers + ) + arglist = [ '--no-name-lookup', ] @@ -2546,9 +2551,21 @@ class TestServerList(TestServer): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data_no_name_lookup), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_n_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + server._format_servers_list_networks(s.networks), + # Image will be an empty string if boot-from-volume + s.image['id'] if s.image else s.image, + s.flavor['id'] + ) for s in self.servers + ) + arglist = [ '-n', ] @@ -2562,7 +2579,7 @@ class TestServerList(TestServer): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data_no_name_lookup), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_name_lookup_one_by_one(self): arglist = [ @@ -2584,7 +2601,7 @@ class TestServerList(TestServer): self.flavors_mock.get.assert_called() self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_image(self): @@ -2604,145 +2621,213 @@ class TestServerList(TestServer): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) - def test_server_list_with_locked_pre_v273(self): + def test_server_list_with_flavor(self): arglist = [ - '--locked' + '--flavor', self.flavor.id ] verifylist = [ - ('locked', True) + ('flavor', self.flavor.id) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - ex = self.assertRaises(exceptions.CommandError, - self.cmd.take_action, - parsed_args) - self.assertIn( - '--os-compute-api-version 2.73 or greater is required', str(ex)) + columns, data = self.cmd.take_action(parsed_args) - def test_server_list_with_locked_v273(self): + self.flavors_mock.get.has_calls(self.flavor.id) + + self.search_opts['flavor'] = self.flavor.id + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertEqual(self.columns, columns) + self.assertEqual(self.data, tuple(data)) + + def test_server_list_with_changes_since(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') arglist = [ - '--locked' + '--changes-since', '2016-03-04T06:27:59Z', + '--deleted' ] verifylist = [ - ('locked', True) + ('changes_since', '2016-03-04T06:27:59Z'), + ('deleted', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.search_opts['locked'] = True + self.search_opts['changes-since'] = '2016-03-04T06:27:59Z' + self.search_opts['deleted'] = True self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertEqual(self.data, tuple(data)) - def test_server_list_with_unlocked_v273(self): + @mock.patch.object(timeutils, 'parse_isotime', side_effect=ValueError) + def test_server_list_with_invalid_changes_since(self, mock_parse_isotime): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') arglist = [ - '--unlocked' + '--changes-since', 'Invalid time value', ] verifylist = [ - ('unlocked', True) + ('changes_since', 'Invalid time value'), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) + try: + self.cmd.take_action(parsed_args) + self.fail('CommandError should be raised.') + except exceptions.CommandError as e: + self.assertEqual('Invalid changes-since value: Invalid time ' + 'value', str(e)) + mock_parse_isotime.assert_called_once_with( + 'Invalid time value' + ) - self.search_opts['locked'] = False - self.servers_mock.list.assert_called_with(**self.kwargs) - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) +class TestServerListV273(_TestServerList): + + # Columns to be listed up. + columns = ( + 'ID', + 'Name', + 'Status', + 'Networks', + 'Image', + 'Flavor', + ) + columns_long = ( + 'ID', + 'Name', + 'Status', + 'Task State', + 'Power State', + 'Networks', + 'Image Name', + 'Image ID', + 'Flavor', + 'Availability Zone', + 'Host', + 'Properties', + ) - def test_server_list_with_locked_and_unlocked_v273(self): + def setUp(self): + super(TestServerListV273, self).setUp() + + # The fake servers' attributes. Use the original attributes names in + # nova, not the ones printed by "server list" command. + self.attrs['flavor'] = { + 'vcpus': self.flavor.vcpus, + 'ram': self.flavor.ram, + 'disk': self.flavor.disk, + 'ephemeral': self.flavor.ephemeral, + 'swap': self.flavor.swap, + 'original_name': self.flavor.name, + 'extra_specs': self.flavor.properties, + } + + # The servers to be listed. + self.servers = self.setup_servers_mock(3) + self.servers_mock.list.return_value = self.servers + + Image = collections.namedtuple('Image', 'id name') + self.images_mock.list.return_value = [ + Image(id=s.image['id'], name=self.image.name) + # Image will be an empty string if boot-from-volume + for s in self.servers if s.image + ] + + # The flavor information is embedded, so now reason for this to be + # called + self.flavors_mock.list = mock.NonCallableMock() + + self.data = tuple( + ( + s.id, + s.name, + s.status, + server._format_servers_list_networks(s.networks), + # Image will be an empty string if boot-from-volume + self.image.name if s.image else s.image, + self.flavor.name, + ) for s in self.servers) + + def test_server_list_with_locked_pre_v273(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') arglist = [ - '--locked', - '--unlocked' + '--locked' ] verifylist = [ - ('locked', True), - ('unlocked', True) + ('locked', True) ] - ex = self.assertRaises( - utils.ParserException, - self.check_parser, self.cmd, arglist, verifylist) - self.assertIn('Argument parse failed', str(ex)) + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + ex = self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + self.assertIn( + '--os-compute-api-version 2.73 or greater is required', str(ex)) - def test_server_list_with_flavor(self): + def test_server_list_with_locked(self): + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') arglist = [ - '--flavor', self.flavor.id + '--locked' ] verifylist = [ - ('flavor', self.flavor.id) + ('locked', True) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.flavors_mock.get.has_calls(self.flavor.id) - - self.search_opts['flavor'] = self.flavor.id + self.search_opts['locked'] = True self.servers_mock.list.assert_called_with(**self.kwargs) - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) - def test_server_list_with_changes_since(self): + def test_server_list_with_unlocked_v273(self): + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') arglist = [ - '--changes-since', '2016-03-04T06:27:59Z', - '--deleted' + '--unlocked' ] verifylist = [ - ('changes_since', '2016-03-04T06:27:59Z'), - ('deleted', True), + ('unlocked', True) ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) columns, data = self.cmd.take_action(parsed_args) - self.search_opts['changes-since'] = '2016-03-04T06:27:59Z' - self.search_opts['deleted'] = True + self.search_opts['locked'] = False self.servers_mock.list.assert_called_with(**self.kwargs) - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) - @mock.patch.object(timeutils, 'parse_isotime', side_effect=ValueError) - def test_server_list_with_invalid_changes_since(self, mock_parse_isotime): + def test_server_list_with_locked_and_unlocked(self): + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') arglist = [ - '--changes-since', 'Invalid time value', + '--locked', + '--unlocked' ] verifylist = [ - ('changes_since', 'Invalid time value'), + ('locked', True), + ('unlocked', True) ] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - try: - self.cmd.take_action(parsed_args) - self.fail('CommandError should be raised.') - except exceptions.CommandError as e: - self.assertEqual('Invalid changes-since value: Invalid time ' - 'value', str(e)) - mock_parse_isotime.assert_called_once_with( - 'Invalid time value' - ) + ex = self.assertRaises( + utils.ParserException, + self.check_parser, self.cmd, arglist, verifylist) + self.assertIn('Argument parse failed', str(ex)) - def test_server_list_v266_with_changes_before(self): + def test_server_list_with_changes_before(self): self.app.client_manager.compute.api_version = ( api_versions.APIVersion('2.66')) arglist = [ @@ -2759,13 +2844,14 @@ class TestServerList(TestServer): self.search_opts['changes-before'] = '2016-03-05T06:27:59Z' self.search_opts['deleted'] = True + self.servers_mock.list.assert_called_with(**self.kwargs) - self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) @mock.patch.object(timeutils, 'parse_isotime', side_effect=ValueError) - def test_server_list_v266_with_invalid_changes_before( + def test_server_list_with_invalid_changes_before( self, mock_parse_isotime): self.app.client_manager.compute.api_version = ( api_versions.APIVersion('2.66')) -- cgit v1.2.1