diff --git a/openstackclient/compute/v2/server_volume.py b/openstackclient/compute/v2/server_volume.py index d53cec931d..45d604a521 100644 --- a/openstackclient/compute/v2/server_volume.py +++ b/openstackclient/compute/v2/server_volume.py @@ -15,6 +15,7 @@ """Compute v2 Server action implementations""" from novaclient import api_versions +from openstack import utils as sdk_utils from osc_lib.command import command from osc_lib import exceptions from osc_lib import utils @@ -34,27 +35,25 @@ class ListServerVolume(command.Lister): return parser def take_action(self, parsed_args): + compute_client = self.app.client_manager.sdk_connection.compute - compute_client = self.app.client_manager.compute - - server = utils.find_resource( - compute_client.servers, + server = compute_client.find_server( parsed_args.server, + ignore_missing=False, ) - - volumes = compute_client.volumes.get_server_volumes(server.id) + volumes = compute_client.volume_attachments(server) columns = () column_headers = () - if compute_client.api_version < api_versions.APIVersion('2.89'): + if not sdk_utils.supports_microversion(compute_client, '2.89'): columns += ('id',) column_headers += ('ID',) columns += ( 'device', - 'serverId', - 'volumeId', + 'server_id', + 'volume_id', ) column_headers += ( 'Device', @@ -62,25 +61,21 @@ class ListServerVolume(command.Lister): 'Volume ID', ) - if compute_client.api_version >= api_versions.APIVersion('2.70'): + if sdk_utils.supports_microversion(compute_client, '2.70'): columns += ('tag',) column_headers += ('Tag',) - if compute_client.api_version >= api_versions.APIVersion('2.79'): + if sdk_utils.supports_microversion(compute_client, '2.79'): columns += ('delete_on_termination',) column_headers += ('Delete On Termination?',) - if compute_client.api_version >= api_versions.APIVersion('2.89'): - columns += ('attachment_id', 'bdm_uuid') + if sdk_utils.supports_microversion(compute_client, '2.89'): + columns += ('attachment_id', 'bdm_id') column_headers += ('Attachment ID', 'BlockDeviceMapping UUID') return ( column_headers, - ( - utils.get_item_properties( - s, columns, mixed_case_fields=('serverId', 'volumeId') - ) for s in volumes - ), + (utils.get_item_properties(s, columns) for s in volumes), ) diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index 830b3543b6..1c909b6754 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -1291,10 +1291,8 @@ class ServerTests(common.ComputeTestCase): parse_output=True, ) - self.assertIsNotNone(cmd_output['ID']) self.assertEqual(server_id, cmd_output['Server ID']) self.assertEqual(volume_id, cmd_output['Volume ID']) - volume_attachment_id = cmd_output['ID'] cmd_output = self.openstack( 'server volume list ' + @@ -1302,7 +1300,6 @@ class ServerTests(common.ComputeTestCase): parse_output=True, ) - self.assertEqual(volume_attachment_id, cmd_output[0]['ID']) self.assertEqual(server_id, cmd_output[0]['Server ID']) self.assertEqual(volume_id, cmd_output[0]['Volume ID']) diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py index 7d8c29ad7c..c5f6ebe2f2 100644 --- a/openstackclient/tests/unit/compute/v2/fakes.py +++ b/openstackclient/tests/unit/compute/v2/fakes.py @@ -1609,122 +1609,49 @@ class FakeServerMigration(object): return migration -class FakeVolumeAttachment(object): - """Fake one or more volume attachments (BDMs).""" +def create_one_volume_attachment(attrs=None): + """Create a fake volume attachment. - @staticmethod - def create_one_volume_attachment(attrs=None, methods=None): - """Create a fake volume attachment. + :param dict attrs: A dictionary with all attributes + :return: A fake openstack.compute.v2.volume_attachment.VolumeAttachment + object + """ + attrs = attrs or {} - :param dict attrs: - A dictionary with all attributes - :param dict methods: - A dictionary with all methods - :return: - A FakeResource object, with id, device, and so on - """ - attrs = attrs or {} - methods = methods or {} + # Set default attributes. + volume_attachment_info = { + "id": uuid.uuid4().hex, + "device": "/dev/sdb", + "server_id": uuid.uuid4().hex, + "volume_id": uuid.uuid4().hex, + # introduced in API microversion 2.70 + "tag": "foo", + # introduced in API microversion 2.79 + "delete_on_termination": True, + # introduced in API microversion 2.89 + "attachment_id": uuid.uuid4().hex, + "bdm_id": uuid.uuid4().hex, + } - # Set default attributes. - volume_attachment_info = { - "id": uuid.uuid4().hex, - "device": "/dev/sdb", - "serverId": uuid.uuid4().hex, - "volumeId": uuid.uuid4().hex, - # introduced in API microversion 2.70 - "tag": "foo", - # introduced in API microversion 2.79 - "delete_on_termination": True, - # introduced in API microversion 2.89 - "attachment_id": uuid.uuid4().hex, - "bdm_uuid": uuid.uuid4().hex - } + # Overwrite default attributes. + volume_attachment_info.update(attrs) - # Overwrite default attributes. - volume_attachment_info.update(attrs) + return volume_attachment.VolumeAttachment(**volume_attachment_info) - volume_attachment = fakes.FakeResource( - info=copy.deepcopy(volume_attachment_info), - methods=methods, - loaded=True) - return volume_attachment - @staticmethod - def create_volume_attachments(attrs=None, methods=None, count=2): - """Create multiple fake volume attachments (BDMs). +def create_volume_attachments(attrs=None, count=2): + """Create multiple fake volume attachments. - :param dict attrs: - A dictionary with all attributes - :param dict methods: - A dictionary with all methods - :param int count: - The number of volume attachments to fake - :return: - A list of FakeResource objects faking the volume attachments. - """ - volume_attachments = [] - for i in range(0, count): - volume_attachments.append( - FakeVolumeAttachment.create_one_volume_attachment( - attrs, methods)) + :param dict attrs: A dictionary with all attributes + :param int count: The number of volume attachments to fake + :return: A list of fake + openstack.compute.v2.volume_attachment.VolumeAttachment objects + """ + volume_attachments = [] + for i in range(0, count): + volume_attachments.append(create_one_volume_attachment(attrs)) - return volume_attachments - - @staticmethod - def create_one_sdk_volume_attachment(attrs=None, methods=None): - """Create a fake sdk VolumeAttachment. - - :param dict attrs: - A dictionary with all attributes - :param dict methods: - A dictionary with all methods - :return: - A fake VolumeAttachment object, with id, device, and so on - """ - attrs = attrs or {} - methods = methods or {} - - # Set default attributes. - volume_attachment_info = { - "id": uuid.uuid4().hex, - "device": "/dev/sdb", - "server_id": uuid.uuid4().hex, - "volume_id": uuid.uuid4().hex, - # introduced in API microversion 2.70 - "tag": "foo", - # introduced in API microversion 2.79 - "delete_on_termination": True, - # introduced in API microversion 2.89 - "attachment_id": uuid.uuid4().hex, - "bdm_uuid": uuid.uuid4().hex - } - - # Overwrite default attributes. - volume_attachment_info.update(attrs) - - return volume_attachment.VolumeAttachment(**volume_attachment_info) - - @staticmethod - def create_sdk_volume_attachments(attrs=None, methods=None, count=2): - """Create multiple fake VolumeAttachment objects (BDMs). - - :param dict attrs: - A dictionary with all attributes - :param dict methods: - A dictionary with all methods - :param int count: - The number of volume attachments to fake - :return: - A list of VolumeAttachment objects faking the volume attachments. - """ - volume_attachments = [] - for i in range(0, count): - volume_attachments.append( - FakeVolumeAttachment.create_one_sdk_volume_attachment( - attrs, methods)) - - return volume_attachments + return volume_attachments def create_one_hypervisor(attrs=None): diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index d7e84ba381..5624986277 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -933,8 +933,7 @@ class TestServerVolume(TestServer): 'volume_id': self.volumes[0].id, } self.volume_attachment = \ - compute_fakes.FakeVolumeAttachment.\ - create_one_sdk_volume_attachment(attrs=attrs) + compute_fakes.create_one_volume_attachment(attrs=attrs) self.sdk_client.create_volume_attachment.return_value = \ self.volume_attachment diff --git a/openstackclient/tests/unit/compute/v2/test_server_volume.py b/openstackclient/tests/unit/compute/v2/test_server_volume.py index 02d378f82a..78b733a5fc 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_volume.py +++ b/openstackclient/tests/unit/compute/v2/test_server_volume.py @@ -11,7 +11,10 @@ # under the License. # +from unittest import mock + from novaclient import api_versions +from openstack import utils as sdk_utils from osc_lib import exceptions from openstackclient.compute.v2 import server_volume @@ -31,26 +34,31 @@ class TestServerVolume(compute_fakes.TestComputev2): self.servers_volumes_mock = self.app.client_manager.compute.volumes self.servers_volumes_mock.reset_mock() + self.app.client_manager.sdk_connection = mock.Mock() + self.app.client_manager.sdk_connection.compute = mock.Mock() + self.sdk_client = self.app.client_manager.sdk_connection.compute + class TestServerVolumeList(TestServerVolume): def setUp(self): super().setUp() - self.server = compute_fakes.FakeServer.create_one_server() - self.volume_attachments = ( - compute_fakes.FakeVolumeAttachment.create_volume_attachments()) + self.server = compute_fakes.FakeServer.create_one_sdk_server() + self.volume_attachments = compute_fakes.create_volume_attachments() - self.servers_mock.get.return_value = self.server - self.servers_volumes_mock.get_server_volumes.return_value = ( + self.sdk_client.find_server.return_value = self.server + self.sdk_client.volume_attachments.return_value = ( self.volume_attachments) # Get the command object to test self.cmd = server_volume.ListServerVolume(self.app, None) - def test_server_volume_list(self): + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_list(self, sm_mock): self.app.client_manager.compute.api_version = \ api_versions.APIVersion('2.1') + sm_mock.side_effect = [False, False, False, False] arglist = [ self.server.id, @@ -68,24 +76,23 @@ class TestServerVolumeList(TestServerVolume): ( self.volume_attachments[0].id, self.volume_attachments[0].device, - self.volume_attachments[0].serverId, - self.volume_attachments[0].volumeId, + self.volume_attachments[0].server_id, + self.volume_attachments[0].volume_id, ), ( self.volume_attachments[1].id, self.volume_attachments[1].device, - self.volume_attachments[1].serverId, - self.volume_attachments[1].volumeId, + self.volume_attachments[1].server_id, + self.volume_attachments[1].volume_id, ), ), tuple(data), ) - self.servers_volumes_mock.get_server_volumes.assert_called_once_with( - self.server.id) + self.sdk_client.volume_attachments.assert_called_once_with(self.server) - def test_server_volume_list_with_tags(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.70') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_list_with_tags(self, sm_mock): + sm_mock.side_effect = [False, True, False, False] arglist = [ self.server.id, @@ -105,27 +112,25 @@ class TestServerVolumeList(TestServerVolume): ( self.volume_attachments[0].id, self.volume_attachments[0].device, - self.volume_attachments[0].serverId, - self.volume_attachments[0].volumeId, + self.volume_attachments[0].server_id, + self.volume_attachments[0].volume_id, self.volume_attachments[0].tag, ), ( self.volume_attachments[1].id, self.volume_attachments[1].device, - self.volume_attachments[1].serverId, - self.volume_attachments[1].volumeId, + self.volume_attachments[1].server_id, + self.volume_attachments[1].volume_id, self.volume_attachments[1].tag, ), ), tuple(data), ) - self.servers_volumes_mock.get_server_volumes.assert_called_once_with( - self.server.id) - - def test_server_volume_list_with_delete_on_attachment(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.79') + self.sdk_client.volume_attachments.assert_called_once_with(self.server) + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_list_with_delete_on_attachment(self, sm_mock): + sm_mock.side_effect = [False, True, True, False] arglist = [ self.server.id, ] @@ -148,29 +153,28 @@ class TestServerVolumeList(TestServerVolume): ( self.volume_attachments[0].id, self.volume_attachments[0].device, - self.volume_attachments[0].serverId, - self.volume_attachments[0].volumeId, + self.volume_attachments[0].server_id, + self.volume_attachments[0].volume_id, self.volume_attachments[0].tag, self.volume_attachments[0].delete_on_termination, ), ( self.volume_attachments[1].id, self.volume_attachments[1].device, - self.volume_attachments[1].serverId, - self.volume_attachments[1].volumeId, + self.volume_attachments[1].server_id, + self.volume_attachments[1].volume_id, self.volume_attachments[1].tag, self.volume_attachments[1].delete_on_termination, ), ), tuple(data), ) - self.servers_volumes_mock.get_server_volumes.assert_called_once_with( - self.server.id) + self.sdk_client.volume_attachments.assert_called_once_with(self.server) - def test_server_volume_list_with_attachment_ids(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.89') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_list_with_attachment_ids(self, sm_mock): + sm_mock.side_effect = [True, True, True, True] arglist = [ self.server.id, ] @@ -193,28 +197,27 @@ class TestServerVolumeList(TestServerVolume): ( ( self.volume_attachments[0].device, - self.volume_attachments[0].serverId, - self.volume_attachments[0].volumeId, + self.volume_attachments[0].server_id, + self.volume_attachments[0].volume_id, self.volume_attachments[0].tag, self.volume_attachments[0].delete_on_termination, self.volume_attachments[0].attachment_id, - self.volume_attachments[0].bdm_uuid + self.volume_attachments[0].bdm_id ), ( self.volume_attachments[1].device, - self.volume_attachments[1].serverId, - self.volume_attachments[1].volumeId, + self.volume_attachments[1].server_id, + self.volume_attachments[1].volume_id, self.volume_attachments[1].tag, self.volume_attachments[1].delete_on_termination, self.volume_attachments[1].attachment_id, - self.volume_attachments[1].bdm_uuid + self.volume_attachments[1].bdm_id ), ), tuple(data), ) - self.servers_volumes_mock.get_server_volumes.assert_called_once_with( - self.server.id) + self.sdk_client.volume_attachments.assert_called_once_with(self.server) class TestServerVolumeUpdate(TestServerVolume): diff --git a/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml b/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml new file mode 100644 index 0000000000..20b8df49b5 --- /dev/null +++ b/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml @@ -0,0 +1,3 @@ +features: + - | + Switch the server volume list command from novaclient to SDK.