Merge "Replace "Display Name" by "Name" in volume list"

doc/source/backwards-incompatible.rst

@@ -27,6 +27,27 @@ Backwards Incompatible Changes
 ..   * Remove in: <5.0>
 ..   * Commit: <tbd>
 
+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
 -------------------
 

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

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"

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"

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):
 

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):
 

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):

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(

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 <https://bugs.launchpad.net/python-openstackclient/+bug/1657956>`_]