From 25b4714f1cd12c20ff7653f05dddb5486a037fcc Mon Sep 17 00:00:00 2001 From: Ritvik Vinodkumar Date: Tue, 14 Dec 2021 16:40:13 +0000 Subject: [PATCH] Switch server volume update to sdk Switch the server volume update command from novaclient to SDK. Change-Id: Ib9876775bcf8268344da1a58ab0dd1695cb83ece --- openstackclient/compute/v2/server_volume.py | 35 +++--- .../unit/compute/v2/test_server_volume.py | 115 ++++++++++-------- ...e-list-update-to-sdk-95b1d3063e46f813.yaml | 2 +- requirements.txt | 2 +- 4 files changed, 84 insertions(+), 70 deletions(-) diff --git a/openstackclient/compute/v2/server_volume.py b/openstackclient/compute/v2/server_volume.py index 45d604a521..7d0cd33654 100644 --- a/openstackclient/compute/v2/server_volume.py +++ b/openstackclient/compute/v2/server_volume.py @@ -14,7 +14,6 @@ """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 @@ -83,14 +82,14 @@ class UpdateServerVolume(command.Command): """Update a volume attachment on the server.""" def get_parser(self, prog_name): - parser = super(UpdateServerVolume, self).get_parser(prog_name) + parser = super().get_parser(prog_name) parser.add_argument( 'server', help=_('Server to update volume for (name or ID)'), ) parser.add_argument( 'volume', - help=_('Volume (ID)'), + help=_('Volume to update attachment for (name or ID)'), ) termination_group = parser.add_mutually_exclusive_group() termination_group.add_argument( @@ -115,31 +114,29 @@ class UpdateServerVolume(command.Command): return parser def take_action(self, parsed_args): - - compute_client = self.app.client_manager.compute + compute_client = self.app.client_manager.sdk_connection.compute + volume_client = self.app.client_manager.sdk_connection.volume if parsed_args.delete_on_termination is not None: - if compute_client.api_version < api_versions.APIVersion('2.85'): + if not sdk_utils.supports_microversion(compute_client, '2.85'): msg = _( '--os-compute-api-version 2.85 or greater is required to ' - 'support the --(no-)delete-on-termination option' + 'support the -delete-on-termination or ' + '--preserve-on-termination option' ) raise exceptions.CommandError(msg) - server = utils.find_resource( - compute_client.servers, + server = compute_client.find_server( parsed_args.server, + ignore_missing=False, + ) + volume = volume_client.find_volume( + parsed_args.volume, + ignore_missing=False, ) - # NOTE(stephenfin): This may look silly, and that's because it is. - # This API was originally used only for the swapping volumes, which - # is an internal operation that should only be done by - # orchestration software rather than a human. We're not going to - # expose that, but we are going to expose the ability to change the - # delete on termination behavior. - compute_client.volumes.update_server_volume( - server.id, - parsed_args.volume, - parsed_args.volume, + compute_client.update_volume_attachment( + server, + volume, delete_on_termination=parsed_args.delete_on_termination, ) diff --git a/openstackclient/tests/unit/compute/v2/test_server_volume.py b/openstackclient/tests/unit/compute/v2/test_server_volume.py index 78b733a5fc..f86bc7ddbe 100644 --- a/openstackclient/tests/unit/compute/v2/test_server_volume.py +++ b/openstackclient/tests/unit/compute/v2/test_server_volume.py @@ -19,6 +19,7 @@ from osc_lib import exceptions from openstackclient.compute.v2 import server_volume from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes +from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes class TestServerVolume(compute_fakes.TestComputev2): @@ -26,17 +27,11 @@ class TestServerVolume(compute_fakes.TestComputev2): def setUp(self): super().setUp() - # Get a shortcut to the compute client ServerManager Mock - self.servers_mock = self.app.client_manager.compute.servers - self.servers_mock.reset_mock() - - # Get a shortcut to the compute client VolumeManager mock - 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 + self.app.client_manager.sdk_connection.volume = mock.Mock() + self.compute_client = self.app.client_manager.sdk_connection.compute + self.volume_client = self.app.client_manager.sdk_connection.volume class TestServerVolumeList(TestServerVolume): @@ -47,8 +42,8 @@ class TestServerVolumeList(TestServerVolume): self.server = compute_fakes.FakeServer.create_one_sdk_server() self.volume_attachments = compute_fakes.create_volume_attachments() - self.sdk_client.find_server.return_value = self.server - self.sdk_client.volume_attachments.return_value = ( + self.compute_client.find_server.return_value = self.server + self.compute_client.volume_attachments.return_value = ( self.volume_attachments) # Get the command object to test @@ -88,7 +83,9 @@ class TestServerVolumeList(TestServerVolume): ), tuple(data), ) - self.sdk_client.volume_attachments.assert_called_once_with(self.server) + self.compute_client.volume_attachments.assert_called_once_with( + self.server, + ) @mock.patch.object(sdk_utils, 'supports_microversion') def test_server_volume_list_with_tags(self, sm_mock): @@ -126,7 +123,9 @@ class TestServerVolumeList(TestServerVolume): ), tuple(data), ) - self.sdk_client.volume_attachments.assert_called_once_with(self.server) + self.compute_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): @@ -169,7 +168,9 @@ class TestServerVolumeList(TestServerVolume): ), tuple(data), ) - self.sdk_client.volume_attachments.assert_called_once_with(self.server) + self.compute_client.volume_attachments.assert_called_once_with( + self.server, + ) @mock.patch.object(sdk_utils, 'supports_microversion') def test_server_volume_list_with_attachment_ids(self, sm_mock): @@ -217,7 +218,9 @@ class TestServerVolumeList(TestServerVolume): ), tuple(data), ) - self.sdk_client.volume_attachments.assert_called_once_with(self.server) + self.compute_client.volume_attachments.assert_called_once_with( + self.server, + ) class TestServerVolumeUpdate(TestServerVolume): @@ -225,21 +228,23 @@ class TestServerVolumeUpdate(TestServerVolume): def setUp(self): super().setUp() - self.server = compute_fakes.FakeServer.create_one_server() - self.servers_mock.get.return_value = self.server + self.server = compute_fakes.FakeServer.create_one_sdk_server() + self.compute_client.find_server.return_value = self.server + + self.volume = volume_fakes.create_one_sdk_volume() + self.volume_client.find_volume.return_value = self.volume # Get the command object to test self.cmd = server_volume.UpdateServerVolume(self.app, None) def test_server_volume_update(self): - arglist = [ self.server.id, - 'foo', + self.volume.id, ] verifylist = [ ('server', self.server.id), - ('volume', 'foo'), + ('volume', self.volume.id), ('delete_on_termination', None), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -247,67 +252,73 @@ class TestServerVolumeUpdate(TestServerVolume): result = self.cmd.take_action(parsed_args) # This is a no-op - self.servers_volumes_mock.update_server_volume.assert_not_called() + self.compute_client.update_volume_attachment.assert_not_called() self.assertIsNone(result) - def test_server_volume_update_with_delete_on_termination(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.85') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_update_with_delete_on_termination(self, sm_mock): + sm_mock.return_value = True arglist = [ self.server.id, - 'foo', + self.volume.id, '--delete-on-termination', ] verifylist = [ ('server', self.server.id), - ('volume', 'foo'), + ('volume', self.volume.id), ('delete_on_termination', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.servers_volumes_mock.update_server_volume.assert_called_once_with( - self.server.id, 'foo', 'foo', - delete_on_termination=True) + self.compute_client.update_volume_attachment.assert_called_once_with( + self.server, + self.volume, + delete_on_termination=True, + ) self.assertIsNone(result) - def test_server_volume_update_with_preserve_on_termination(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.85') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_update_with_preserve_on_termination(self, sm_mock): + sm_mock.return_value = True arglist = [ self.server.id, - 'foo', + self.volume.id, '--preserve-on-termination', ] verifylist = [ ('server', self.server.id), - ('volume', 'foo'), + ('volume', self.volume.id), ('delete_on_termination', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.servers_volumes_mock.update_server_volume.assert_called_once_with( - self.server.id, 'foo', 'foo', - delete_on_termination=False) + self.compute_client.update_volume_attachment.assert_called_once_with( + self.server, + self.volume, + delete_on_termination=False + ) self.assertIsNone(result) - def test_server_volume_update_with_delete_on_termination_pre_v285(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.84') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_update_with_delete_on_termination_pre_v285( + self, sm_mock, + ): + sm_mock.return_value = False arglist = [ self.server.id, - 'foo', + self.volume.id, '--delete-on-termination', ] verifylist = [ ('server', self.server.id), - ('volume', 'foo'), + ('volume', self.volume.id), ('delete_on_termination', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -315,20 +326,24 @@ class TestServerVolumeUpdate(TestServerVolume): self.assertRaises( exceptions.CommandError, self.cmd.take_action, - parsed_args) + parsed_args, + ) + self.compute_client.update_volume_attachment.assert_not_called() - def test_server_volume_update_with_preserve_on_termination_pre_v285(self): - self.app.client_manager.compute.api_version = \ - api_versions.APIVersion('2.84') + @mock.patch.object(sdk_utils, 'supports_microversion') + def test_server_volume_update_with_preserve_on_termination_pre_v285( + self, sm_mock, + ): + sm_mock.return_value = False arglist = [ self.server.id, - 'foo', + self.volume.id, '--preserve-on-termination', ] verifylist = [ ('server', self.server.id), - ('volume', 'foo'), + ('volume', self.volume.id), ('delete_on_termination', False), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -336,4 +351,6 @@ class TestServerVolumeUpdate(TestServerVolume): self.assertRaises( exceptions.CommandError, self.cmd.take_action, - parsed_args) + parsed_args, + ) + self.compute_client.update_volume_attachment.assert_not_called() 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 index 20b8df49b5..6194fb1b04 100644 --- a/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml +++ b/releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml @@ -1,3 +1,3 @@ features: - | - Switch the server volume list command from novaclient to SDK. + Switch the server volume list and server volume update command from novaclient to SDK. diff --git a/requirements.txt b/requirements.txt index c787ef7627..1ae8cec422 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,7 +6,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 cliff>=3.5.0 # Apache-2.0 iso8601>=0.1.11 # MIT -openstacksdk>=0.102.0 # Apache-2.0 +openstacksdk>=0.103.0 # Apache-2.0 osc-lib>=2.3.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0