diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 438d43f91..60e2572d6 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -2303,21 +2303,28 @@ class ListServer(command.Lister): columns += ('image_name',) column_headers += ('Image',) - if parsed_args.long: - columns += ( - 'flavor_name', - 'flavor_id', - ) - column_headers += ( - 'Flavor Name', - 'Flavor ID', - ) - else: - if parsed_args.no_name_lookup: - columns += ('flavor_id',) - else: - columns += ('flavor_name',) + # microversion 2.47 puts the embedded flavor into the server response + # body but omits the id, so if not present we just expose the original + # flavor name in the output + if compute_client.api_version >= api_versions.APIVersion('2.47'): + columns += ('flavor_name',) column_headers += ('Flavor',) + else: + if parsed_args.long: + columns += ( + 'flavor_name', + 'flavor_id', + ) + column_headers += ( + 'Flavor Name', + 'Flavor ID', + ) + else: + if parsed_args.no_name_lookup: + columns += ('flavor_id',) + else: + columns += ('flavor_name',) + column_headers += ('Flavor',) if parsed_args.long: columns += ( @@ -2441,18 +2448,13 @@ class ListServer(command.Lister): s.image_name = IMAGE_STRING_FOR_BFV s.image_id = IMAGE_STRING_FOR_BFV - if 'id' in s.flavor: + if compute_client.api_version < api_versions.APIVersion('2.47'): flavor = flavors.get(s.flavor['id']) if flavor: s.flavor_name = flavor.name s.flavor_id = s.flavor['id'] else: - # TODO(mriedem): Fix this for microversion >= 2.47 where the - # flavor is embedded in the server response without the id. - # We likely need to drop the Flavor ID column in that case if - # --long is specified. - s.flavor_name = '' - s.flavor_id = '' + s.flavor_name = s.flavor['original_name'] table = ( column_headers, diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 2f83fa2bd..8ccec91bb 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -3709,7 +3709,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 = ( @@ -3737,7 +3737,7 @@ class TestServerList(TestServer): ) def setUp(self): - super(TestServerList, self).setUp() + super(_TestServerList, self).setUp() self.search_opts = { 'reservation_id': None, @@ -3776,7 +3776,7 @@ class TestServerList(TestServer): }, 'OS-EXT-AZ:availability_zone': 'availability-zone-xxx', 'OS-EXT-SRV-ATTR:host': 'host-name-xxx', - 'Metadata': '', + 'Metadata': format_columns.DictColumn({}), } # The servers to be listed. @@ -3795,10 +3795,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.return_value = [ @@ -3813,8 +3814,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, @@ -3822,34 +3823,8 @@ class TestServerList(TestServer): # Image will be an empty string if boot-from-volume self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, self.flavor.name, - )) - self.data_long.append(( - s.id, - s.name, - s.status, - getattr(s, 'OS-EXT-STS:task_state'), - server.PowerStateColumn( - getattr(s, 'OS-EXT-STS:power_state') - ), - format_columns.DictListColumn(s.networks), - # Image will be an empty string if boot-from-volume - self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, - s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, - self.flavor.name, - s.flavor['id'], - getattr(s, 'OS-EXT-AZ:availability_zone'), - getattr(s, 'OS-EXT-SRV-ATTR:host'), - format_columns.DictColumn({}), - )) - self.data_no_name_lookup.append(( - s.id, - s.name, - s.status, - format_columns.DictListColumn(s.networks), - # Image will be an empty string if boot-from-volume - s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, - s.flavor['id'] - )) + ) for s in self.servers + ) def test_server_list_no_option(self): arglist = [] @@ -3870,7 +3845,7 @@ class TestServerList(TestServer): self.assertFalse(self.flavors_mock.get.call_count) self.assertFalse(self.get_image_mock.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 = [] @@ -3889,9 +3864,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.PowerStateColumn( + getattr(s, 'OS-EXT-STS:power_state') + ), + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, + s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, + 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', ] @@ -3902,10 +3896,9 @@ 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.assertCountEqual(tuple(self.data_long), tuple(data)) + self.assertEqual(self.data, tuple(data)) def test_server_list_column_option(self): arglist = [ @@ -3927,6 +3920,18 @@ class TestServerList(TestServer): self.assertIn('Created At', columns) def test_server_list_no_name_lookup_option(self): + self.data = tuple( + ( + s.id, + s.name, + s.status, + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, + s.flavor['id'] + ) for s in self.servers + ) + arglist = [ '--no-name-lookup', ] @@ -3940,9 +3945,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, + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + s.image['id'] if s.image else server.IMAGE_STRING_FOR_BFV, + s.flavor['id'] + ) for s in self.servers + ) + arglist = [ '-n', ] @@ -3956,7 +3973,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 = [ @@ -3978,7 +3995,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): @@ -3999,81 +4016,7 @@ class TestServerList(TestServer): self.servers_mock.list.assert_called_with(**self.kwargs) self.assertEqual(self.columns, columns) - self.assertEqual(tuple(self.data), tuple(data)) - - def test_server_list_with_locked_pre_v273(self): - - arglist = [ - '--locked' - ] - verifylist = [ - ('locked', True) - ] - - 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_locked_v273(self): - - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') - arglist = [ - '--locked' - ] - verifylist = [ - ('locked', True) - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - 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)) - - def test_server_list_with_unlocked_v273(self): - - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') - arglist = [ - '--unlocked' - ] - verifylist = [ - ('unlocked', True) - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - 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)) - - def test_server_list_with_locked_and_unlocked_v273(self): - - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.73') - arglist = [ - '--locked', - '--unlocked' - ] - verifylist = [ - ('locked', True), - ('unlocked', True) - ] - - ex = self.assertRaises( - utils.ParserException, - self.check_parser, self.cmd, arglist, verifylist) - self.assertIn('Argument parse failed', str(ex)) + self.assertEqual(self.data, tuple(data)) def test_server_list_with_flavor(self): @@ -4093,7 +4036,7 @@ 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_changes_since(self): @@ -4114,7 +4057,7 @@ 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)) @mock.patch.object(iso8601, 'parse_date', side_effect=iso8601.ParseError) def test_server_list_with_invalid_changes_since(self, mock_parse_isotime): @@ -4137,123 +4080,6 @@ class TestServerList(TestServer): 'Invalid time value' ) - def test_server_list_v266_with_changes_before(self): - self.app.client_manager.compute.api_version = ( - api_versions.APIVersion('2.66')) - arglist = [ - '--changes-before', '2016-03-05T06:27:59Z', - '--deleted' - ] - verifylist = [ - ('changes_before', '2016-03-05T06:27:59Z'), - ('deleted', True), - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - columns, data = self.cmd.take_action(parsed_args) - - 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)) - - @mock.patch.object(iso8601, 'parse_date', side_effect=iso8601.ParseError) - def test_server_list_v266_with_invalid_changes_before( - self, mock_parse_isotime): - self.app.client_manager.compute.api_version = ( - api_versions.APIVersion('2.66')) - - arglist = [ - '--changes-before', 'Invalid time value', - ] - verifylist = [ - ('changes_before', 'Invalid time value'), - ] - - 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-before value: Invalid time ' - 'value', str(e)) - mock_parse_isotime.assert_called_once_with( - 'Invalid time value' - ) - - def test_server_with_changes_before_pre_v266(self): - self.app.client_manager.compute.api_version = ( - api_versions.APIVersion('2.65')) - - arglist = [ - '--changes-before', '2016-03-05T06:27:59Z', - '--deleted' - ] - verifylist = [ - ('changes_before', '2016-03-05T06:27:59Z'), - ('deleted', True), - ] - - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - self.assertRaises(exceptions.CommandError, - self.cmd.take_action, - parsed_args) - - def test_server_list_v269_with_partial_constructs(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.69') - - arglist = [] - verifylist = [] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) - - # include "partial results" from non-responsive part of - # infrastructure. - server_dict = { - "id": "server-id-95a56bfc4xxxxxx28d7e418bfd97813a", - "status": "UNKNOWN", - "tenant_id": "6f70656e737461636b20342065766572", - "created": "2018-12-03T21:06:18Z", - "links": [ - { - "href": "http://fake/v2.1/", - "rel": "self" - }, - { - "href": "http://fake", - "rel": "bookmark" - } - ], - # We need to pass networks as {} because its defined as a property - # of the novaclient Server class which gives {} by default. If not - # it will fail at formatting the networks info later on. - "networks": {} - } - _server = compute_fakes.fakes.FakeResource( - info=server_dict, - ) - self.servers.append(_server) - columns, data = self.cmd.take_action(parsed_args) - # get the first three servers out since our interest is in the partial - # server. - next(data) - next(data) - next(data) - partial_server = next(data) - expected_row = ( - 'server-id-95a56bfc4xxxxxx28d7e418bfd97813a', - '', - 'UNKNOWN', - format_columns.DictListColumn({}), - '', - '', - ) - self.assertEqual(expected_row, partial_server) - def test_server_list_with_tag(self): self.app.client_manager.compute.api_version = api_versions.APIVersion( '2.26') @@ -4274,7 +4100,7 @@ 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_tag_pre_v225(self): self.app.client_manager.compute.api_version = api_versions.APIVersion( @@ -4317,7 +4143,7 @@ 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_not_tag_pre_v226(self): self.app.client_manager.compute.api_version = api_versions.APIVersion( @@ -4478,6 +4304,258 @@ class TestServerList(TestServer): 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 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.extra_specs, + } + + # 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.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, + format_columns.DictListColumn(s.networks), + # Image will be an empty string if boot-from-volume + self.image.name if s.image else server.IMAGE_STRING_FOR_BFV, + self.flavor.name, + ) for s in self.servers) + + def test_server_list_with_locked_pre_v273(self): + + arglist = [ + '--locked' + ] + verifylist = [ + ('locked', True) + ] + + 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_locked(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--locked' + ] + verifylist = [ + ('locked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['locked'] = True + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) + + def test_server_list_with_unlocked_v273(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--unlocked' + ] + verifylist = [ + ('unlocked', True) + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + self.search_opts['locked'] = False + self.servers_mock.list.assert_called_with(**self.kwargs) + + self.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) + + def test_server_list_with_locked_and_unlocked(self): + + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.73') + arglist = [ + '--locked', + '--unlocked' + ] + verifylist = [ + ('locked', True), + ('unlocked', True) + ] + + ex = self.assertRaises( + utils.ParserException, + self.check_parser, self.cmd, arglist, verifylist) + self.assertIn('Argument parse failed', str(ex)) + + def test_server_list_with_changes_before(self): + self.app.client_manager.compute.api_version = ( + api_versions.APIVersion('2.66')) + arglist = [ + '--changes-before', '2016-03-05T06:27:59Z', + '--deleted' + ] + verifylist = [ + ('changes_before', '2016-03-05T06:27:59Z'), + ('deleted', True), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + columns, data = self.cmd.take_action(parsed_args) + + 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.assertItemsEqual(self.columns, columns) + self.assertItemsEqual(self.data, tuple(data)) + + @mock.patch.object(iso8601, 'parse_date', side_effect=iso8601.ParseError) + def test_server_list_with_invalid_changes_before( + self, mock_parse_isotime): + self.app.client_manager.compute.api_version = ( + api_versions.APIVersion('2.66')) + + arglist = [ + '--changes-before', 'Invalid time value', + ] + verifylist = [ + ('changes_before', 'Invalid time value'), + ] + + 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-before value: Invalid time ' + 'value', str(e)) + mock_parse_isotime.assert_called_once_with( + 'Invalid time value' + ) + + def test_server_with_changes_before_pre_v266(self): + self.app.client_manager.compute.api_version = ( + api_versions.APIVersion('2.65')) + + arglist = [ + '--changes-before', '2016-03-05T06:27:59Z', + '--deleted' + ] + verifylist = [ + ('changes_before', '2016-03-05T06:27:59Z'), + ('deleted', True), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + + def test_server_list_v269_with_partial_constructs(self): + self.app.client_manager.compute.api_version = \ + api_versions.APIVersion('2.69') + arglist = [] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + # include "partial results" from non-responsive part of + # infrastructure. + server_dict = { + "id": "server-id-95a56bfc4xxxxxx28d7e418bfd97813a", + "status": "UNKNOWN", + "tenant_id": "6f70656e737461636b20342065766572", + "created": "2018-12-03T21:06:18Z", + "links": [ + { + "href": "http://fake/v2.1/", + "rel": "self" + }, + { + "href": "http://fake", + "rel": "bookmark" + } + ], + # We need to pass networks as {} because its defined as a property + # of the novaclient Server class which gives {} by default. If not + # it will fail at formatting the networks info later on. + "networks": {} + } + server = compute_fakes.fakes.FakeResource( + info=server_dict, + ) + self.servers.append(server) + columns, data = self.cmd.take_action(parsed_args) + # get the first three servers out since our interest is in the partial + # server. + next(data) + next(data) + next(data) + partial_server = next(data) + expected_row = ( + 'server-id-95a56bfc4xxxxxx28d7e418bfd97813a', '', + 'UNKNOWN', format_columns.DictListColumn({}), '', '') + self.assertEqual(expected_row, partial_server) + + class TestServerLock(TestServer): def setUp(self): diff --git a/releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml b/releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml new file mode 100644 index 000000000..fdb37bbbc --- /dev/null +++ b/releasenotes/notes/fix-flavor-in-server-list-microversion-2.47-af200e9bb4747e2d.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Add support for compute API microversion 2.47, which changes how flavor + details are included in server detail responses. In 2.46 and below, + only the flavor ID was shown in the server detail response. Starting in + 2.47, flavor information is embedded in the server response. The newer + behavior is now supported.