diff --git a/doc/source/backwards-incompatible.rst b/doc/source/backwards-incompatible.rst index 6516e794a..033860d34 100644 --- a/doc/source/backwards-incompatible.rst +++ b/doc/source/backwards-incompatible.rst @@ -27,6 +27,27 @@ Backwards Incompatible Changes .. * Remove in: <5.0> .. * Commit: +Release 3.12.0 +-------------- + +1. Replace ``Display Name`` by ``Name`` in volume list. + + Change column name ``Display Name`` to ``Name`` in ``volume list`` output. + Current ``volume list --name`` command uses ``display_name`` as search_opts + to send to cinder API, and show the result table with ``Display Name`` + as column title. Replace all ``Display Name`` by ``Name`` to be consistent + with other list commands. + + Support a mapping for volume list -c ``Display Name`` (Volume v1 and v2) + and volume create/show -c ``display_name`` (Volume v1) to maintain backward + compatibility until the next major release. + + * In favor of: ``openstack volume list -c Name`` + * As of: 3.12.0 + * Removed in: n/a + * Bug: https://bugs.launchpad.net/python-openstackclient/+bug/1657956 + * Commit: https://review.openstack.org/#/c/423081/ + Release 3.10 ------------ @@ -62,6 +83,7 @@ Release 3.0 * Bug: n/a * Commit: https://review.openstack.org/332938 + Releases Before 3.0 ------------------- diff --git a/openstackclient/tests/functional/volume/v1/test_transfer_request.py b/openstackclient/tests/functional/volume/v1/test_transfer_request.py index 498c90567..bd6128295 100644 --- a/openstackclient/tests/functional/volume/v1/test_transfer_request.py +++ b/openstackclient/tests/functional/volume/v1/test_transfer_request.py @@ -27,7 +27,7 @@ class TransferRequestTests(common.BaseVolumeTests): super(TransferRequestTests, cls).setUpClass() cmd_output = json.loads(cls.openstack( 'volume create -f json --size 1 ' + cls.VOLUME_NAME)) - cls.assertOutput(cls.VOLUME_NAME, cmd_output['display_name']) + cls.assertOutput(cls.VOLUME_NAME, cmd_output['name']) cmd_output = json.loads(cls.openstack( 'volume transfer request create -f json ' + @@ -51,7 +51,7 @@ class TransferRequestTests(common.BaseVolumeTests): # create a volume cmd_output = json.loads(self.openstack( 'volume create -f json --size 1 ' + volume_name)) - self.assertEqual(volume_name, cmd_output['display_name']) + self.assertEqual(volume_name, cmd_output['name']) # create volume transfer request for the volume # and get the auth_key of the new transfer request diff --git a/openstackclient/tests/functional/volume/v1/test_volume.py b/openstackclient/tests/functional/volume/v1/test_volume.py index 3f04e0715..6eddf2132 100644 --- a/openstackclient/tests/functional/volume/v1/test_volume.py +++ b/openstackclient/tests/functional/volume/v1/test_volume.py @@ -81,7 +81,7 @@ class VolumeTests(common.BaseVolumeTests): cmd_output = json.loads(self.openstack( 'volume list -f json ' )) - names = [x["Display Name"] for x in cmd_output] + names = [x["Name"] for x in cmd_output] self.assertIn(name1, names) self.assertIn(name2, names) @@ -97,7 +97,7 @@ class VolumeTests(common.BaseVolumeTests): 'volume list -f json ' + '--name ' + name1 )) - names = [x["Display Name"] for x in cmd_output] + names = [x["Name"] for x in cmd_output] self.assertIn(name1, names) self.assertNotIn(name2, names) @@ -113,7 +113,7 @@ class VolumeTests(common.BaseVolumeTests): )) self.assertEqual( name, - cmd_output["display_name"], + cmd_output["name"], ) self.assertEqual( 1, @@ -155,7 +155,7 @@ class VolumeTests(common.BaseVolumeTests): )) self.assertEqual( new_name, - cmd_output["display_name"], + cmd_output["name"], ) self.assertEqual( 2, @@ -191,6 +191,49 @@ class VolumeTests(common.BaseVolumeTests): cmd_output["properties"], ) + def test_volume_create_and_list_and_show_backward_compatibility(self): + """Test backward compatibility of create, list, show""" + name1 = uuid.uuid4().hex + json_output = json.loads(self.openstack( + 'volume create -f json ' + + '-c display_name -c id ' + + '--size 1 ' + + name1 + )) + self.assertIn('display_name', json_output) + self.assertEqual(name1, json_output['display_name']) + self.assertIn('id', json_output) + volume_id = json_output['id'] + self.assertIsNotNone(volume_id) + self.assertNotIn('name', json_output) + self.addCleanup(self.openstack, 'volume delete ' + volume_id) + + self.wait_for("volume", name1, "available") + + json_output = json.loads(self.openstack( + 'volume list -f json ' + + '-c "Display Name"' + )) + for each_volume in json_output: + self.assertIn('Display Name', each_volume) + + json_output = json.loads(self.openstack( + 'volume list -f json ' + + '-c "Name"' + )) + for each_volume in json_output: + self.assertIn('Name', each_volume) + + json_output = json.loads(self.openstack( + 'volume show -f json ' + + '-c display_name -c id ' + + name1 + )) + self.assertIn('display_name', json_output) + self.assertEqual(name1, json_output['display_name']) + self.assertIn('id', json_output) + self.assertNotIn('name', json_output) + def wait_for(self, check_type, check_name, desired_status, wait=120, interval=5, failures=['ERROR']): status = "notset" diff --git a/openstackclient/tests/functional/volume/v2/test_volume.py b/openstackclient/tests/functional/volume/v2/test_volume.py index 94ac792f9..f936907ca 100644 --- a/openstackclient/tests/functional/volume/v2/test_volume.py +++ b/openstackclient/tests/functional/volume/v2/test_volume.py @@ -90,7 +90,7 @@ class VolumeTests(common.BaseVolumeTests): 'volume list -f json ' + '--long' )) - names = [x["Display Name"] for x in cmd_output] + names = [x["Name"] for x in cmd_output] self.assertIn(name1, names) self.assertIn(name2, names) @@ -99,7 +99,7 @@ class VolumeTests(common.BaseVolumeTests): 'volume list -f json ' + '--status error' )) - names = [x["Display Name"] for x in cmd_output] + names = [x["Name"] for x in cmd_output] self.assertNotIn(name1, names) self.assertIn(name2, names) @@ -249,6 +249,37 @@ class VolumeTests(common.BaseVolumeTests): 'volume snapshot delete ' + snapshot_name) self.assertOutput('', raw_output) + def test_volume_list_backward_compatibility(self): + """Test backward compatibility of list command""" + name1 = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume create -f json ' + + '--size 1 ' + + name1 + )) + self.addCleanup(self.openstack, 'volume delete ' + name1) + self.assertEqual( + 1, + cmd_output["size"], + ) + self.wait_for("volume", name1, "available") + + # Test list -c "Display Name" + cmd_output = json.loads(self.openstack( + 'volume list -f json ' + + '-c "Display Name"' + )) + for each_volume in cmd_output: + self.assertIn('Display Name', each_volume) + + # Test list -c "Name" + cmd_output = json.loads(self.openstack( + 'volume list -f json ' + + '-c "Name"' + )) + for each_volume in cmd_output: + self.assertIn('Name', each_volume) + def wait_for(self, check_type, check_name, desired_status, wait=120, interval=5, failures=['ERROR']): status = "notset" diff --git a/openstackclient/tests/unit/volume/v1/test_volume.py b/openstackclient/tests/unit/volume/v1/test_volume.py index d46a7ba95..6b7937735 100644 --- a/openstackclient/tests/unit/volume/v1/test_volume.py +++ b/openstackclient/tests/unit/volume/v1/test_volume.py @@ -68,8 +68,8 @@ class TestVolumeCreate(TestVolume): 'bootable', 'created_at', 'display_description', - 'display_name', 'id', + 'name', 'properties', 'size', 'snapshot_id', @@ -86,8 +86,8 @@ class TestVolumeCreate(TestVolume): self.new_volume.bootable, self.new_volume.created_at, self.new_volume.display_description, - self.new_volume.display_name, self.new_volume.id, + self.new_volume.display_name, utils.format_dict(self.new_volume.metadata), self.new_volume.size, self.new_volume.snapshot_id, @@ -598,6 +598,38 @@ class TestVolumeCreate(TestVolume): self.assertRaises(tests_utils.ParserException, self.check_parser, self.cmd, arglist, verifylist) + def test_volume_create_backward_compatibility(self): + arglist = [ + '-c', 'display_name', + '--size', str(self.new_volume.size), + self.new_volume.display_name, + ] + verifylist = [ + ('columns', ['display_name']), + ('size', self.new_volume.size), + ('name', self.new_volume.display_name), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.volumes_mock.create.assert_called_with( + self.new_volume.size, + None, + None, + self.new_volume.display_name, + None, + None, + None, + None, + None, + None, + None, + ) + self.assertIn('display_name', columns) + self.assertNotIn('name', columns) + self.assertIn(self.new_volume.display_name, data) + class TestVolumeDelete(TestVolume): @@ -695,7 +727,7 @@ class TestVolumeList(TestVolume): _volume = volume_fakes.FakeVolume.create_one_volume() columns = ( 'ID', - 'Display Name', + 'Name', 'Status', 'Size', 'Attached to', @@ -806,7 +838,7 @@ class TestVolumeList(TestVolume): collist = ( 'ID', - 'Display Name', + 'Name', 'Status', 'Size', 'Type', @@ -863,6 +895,27 @@ class TestVolumeList(TestVolume): self.assertRaises(argparse.ArgumentTypeError, self.check_parser, self.cmd, arglist, verifylist) + def test_volume_list_backward_compatibility(self): + arglist = [ + '-c', 'Display Name', + ] + verifylist = [ + ('columns', ['Display Name']), + ('long', False), + ('all_projects', False), + ('name', None), + ('status', None), + ('limit', None), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.assertIn('Display Name', columns) + self.assertNotIn('Name', columns) + for each_volume in data: + self.assertIn(self._volume.display_name, each_volume) + class TestVolumeMigrate(TestVolume): @@ -1178,8 +1231,8 @@ class TestVolumeShow(TestVolume): 'bootable', 'created_at', 'display_description', - 'display_name', 'id', + 'name', 'properties', 'size', 'snapshot_id', @@ -1196,8 +1249,8 @@ class TestVolumeShow(TestVolume): self._volume.bootable, self._volume.created_at, self._volume.display_description, - self._volume.display_name, self._volume.id, + self._volume.display_name, utils.format_dict(self._volume.metadata), self._volume.size, self._volume.snapshot_id, @@ -1223,6 +1276,25 @@ class TestVolumeShow(TestVolume): self.assertEqual(self.columns, columns) self.assertEqual(self.datalist, data) + def test_volume_show_backward_compatibility(self): + arglist = [ + '-c', 'display_name', + self._volume.id, + ] + verifylist = [ + ('columns', ['display_name']), + ('volume', self._volume.id), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + self.volumes_mock.get.assert_called_with(self._volume.id) + + self.assertIn('display_name', columns) + self.assertNotIn('name', columns) + self.assertIn(self._volume.display_name, data) + class TestVolumeUnset(TestVolume): diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py index fbe719f3e..71e4eceac 100644 --- a/openstackclient/tests/unit/volume/v2/test_volume.py +++ b/openstackclient/tests/unit/volume/v2/test_volume.py @@ -790,7 +790,7 @@ class TestVolumeList(TestVolume): columns = [ 'ID', - 'Display Name', + 'Name', 'Status', 'Size', 'Attached to', @@ -827,7 +827,7 @@ class TestVolumeList(TestVolume): 'all_tenants': False, 'project_id': None, 'user_id': None, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -870,7 +870,7 @@ class TestVolumeList(TestVolume): 'all_tenants': True, 'project_id': self.project.id, 'user_id': None, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -915,7 +915,7 @@ class TestVolumeList(TestVolume): 'all_tenants': True, 'project_id': self.project.id, 'user_id': None, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -958,7 +958,7 @@ class TestVolumeList(TestVolume): 'all_tenants': False, 'project_id': None, 'user_id': self.user.id, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -1002,7 +1002,7 @@ class TestVolumeList(TestVolume): 'all_tenants': False, 'project_id': None, 'user_id': self.user.id, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -1045,7 +1045,7 @@ class TestVolumeList(TestVolume): 'all_tenants': False, 'project_id': None, 'user_id': None, - 'display_name': self.mock_volume.name, + 'name': self.mock_volume.name, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -1088,7 +1088,7 @@ class TestVolumeList(TestVolume): 'all_tenants': False, 'project_id': None, 'user_id': None, - 'display_name': None, + 'name': None, 'status': self.mock_volume.status, } self.volumes_mock.list.assert_called_once_with( @@ -1131,7 +1131,7 @@ class TestVolumeList(TestVolume): 'all_tenants': True, 'project_id': None, 'user_id': None, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -1175,7 +1175,7 @@ class TestVolumeList(TestVolume): 'all_tenants': False, 'project_id': None, 'user_id': None, - 'display_name': None, + 'name': None, 'status': None, } self.volumes_mock.list.assert_called_once_with( @@ -1186,7 +1186,7 @@ class TestVolumeList(TestVolume): collist = [ 'ID', - 'Display Name', + 'Name', 'Status', 'Size', 'Type', @@ -1248,7 +1248,7 @@ class TestVolumeList(TestVolume): 'status': None, 'project_id': None, 'user_id': None, - 'display_name': None, + 'name': None, 'all_tenants': False, } ) self.assertEqual(datalist, tuple(data)) @@ -1263,6 +1263,42 @@ class TestVolumeList(TestVolume): self.assertRaises(argparse.ArgumentTypeError, self.check_parser, self.cmd, arglist, verifylist) + def test_volume_list_backward_compatibility(self): + arglist = [ + '-c', 'Display Name', + ] + verifylist = [ + ('columns', ['Display Name']), + ('long', False), + ('all_projects', False), + ('name', None), + ('status', None), + ('marker', None), + ('limit', None), + ] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + columns, data = self.cmd.take_action(parsed_args) + + search_opts = { + 'all_tenants': False, + 'project_id': None, + 'user_id': None, + 'name': None, + 'status': None, + } + self.volumes_mock.list.assert_called_once_with( + search_opts=search_opts, + marker=None, + limit=None, + ) + + self.assertIn('Display Name', columns) + self.assertNotIn('Name', columns) + + for each_volume in data: + self.assertIn(self.mock_volume.name, each_volume) + class TestVolumeMigrate(TestVolume): diff --git a/openstackclient/volume/v1/volume.py b/openstackclient/volume/v1/volume.py index 0121e0596..b29429ef8 100644 --- a/openstackclient/volume/v1/volume.py +++ b/openstackclient/volume/v1/volume.py @@ -211,8 +211,14 @@ class CreateVolume(command.ShowOne): 'type': volume._info.pop('volume_type'), }, ) + # Replace "display_name" by "name", keep consistent in v1 and v2 + if 'display_name' in volume._info: + volume._info.update({'name': volume._info.pop('display_name')}) + volume_info = utils.backward_compat_col_showone( + volume._info, parsed_args.columns, {'display_name': 'name'} + ) - return zip(*sorted(six.iteritems(volume._info))) + return zip(*sorted(six.iteritems(volume_info))) class DeleteVolume(command.Command): @@ -330,7 +336,7 @@ class ListVolume(command.Lister): ) column_headers = ( 'ID', - 'Display Name', + 'Name', 'Status', 'Size', 'Type', @@ -348,7 +354,7 @@ class ListVolume(command.Lister): ) column_headers = ( 'ID', - 'Display Name', + 'Name', 'Status', 'Size', 'Attached to', @@ -373,6 +379,8 @@ class ListVolume(command.Lister): search_opts=search_opts, limit=parsed_args.limit, ) + column_headers = utils.backward_compat_col_lister( + column_headers, parsed_args.columns, {'Display Name': 'Name'}) return (column_headers, (utils.get_item_properties( @@ -576,7 +584,15 @@ class ShowVolume(command.ShowOne): {'project_id': volume._info.pop( 'os-vol-tenant-attr:tenant_id')} ) - return zip(*sorted(six.iteritems(volume._info))) + # Replace "display_name" by "name", keep consistent in v1 and v2 + if 'display_name' in volume._info: + volume._info.update({'name': volume._info.pop('display_name')}) + + volume_info = utils.backward_compat_col_showone( + volume._info, parsed_args.columns, {'display_name': 'name'} + ) + + return zip(*sorted(six.iteritems(volume_info))) class UnsetVolume(command.Command): diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py index 2b6c966d7..61f846b02 100644 --- a/openstackclient/volume/v2/volume.py +++ b/openstackclient/volume/v2/volume.py @@ -387,7 +387,6 @@ class ListVolume(command.Lister): 'Metadata', ] column_headers = copy.deepcopy(columns) - column_headers[1] = 'Display Name' column_headers[4] = 'Type' column_headers[6] = 'Attached to' column_headers[7] = 'Properties' @@ -400,7 +399,6 @@ class ListVolume(command.Lister): 'Attachments', ] column_headers = copy.deepcopy(columns) - column_headers[1] = 'Display Name' column_headers[4] = 'Attached to' # Cache the server list @@ -432,7 +430,7 @@ class ListVolume(command.Lister): 'all_tenants': all_projects, 'project_id': project_id, 'user_id': user_id, - 'display_name': parsed_args.name, + 'name': parsed_args.name, 'status': parsed_args.status, } @@ -441,6 +439,8 @@ class ListVolume(command.Lister): marker=parsed_args.marker, limit=parsed_args.limit, ) + column_headers = utils.backward_compat_col_lister( + column_headers, parsed_args.columns, {'Display Name': 'Name'}) return (column_headers, (utils.get_item_properties( diff --git a/releasenotes/notes/bug-1657956-977a615f01775288.yaml b/releasenotes/notes/bug-1657956-977a615f01775288.yaml new file mode 100644 index 000000000..3a392bed6 --- /dev/null +++ b/releasenotes/notes/bug-1657956-977a615f01775288.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Change column name ``Display Name`` to ``Name`` in ``volume list`` output. + Current ``volume list --name`` command uses ``display_name`` as search_opts + to send to cinder API, and show the result table with ``Display Name`` + as column title. Replace all ``Display Name`` by ``Name`` to be consistent + with other list commands. + + Support a mapping for volume list -c ``Display Name`` (Volume v1 and v2) + and volume create/show -c ``display_name`` (Volume v1) to maintain backward + compatibility until the next major release. + [Bug `1657956 `_]