openstack
/
python-openstackclient
Code Issues Proposed changes

Merge "Switch server volume update to sdk"

This commit is contained in:
Zuul 2023-02-21 13:28:37 +00:00 committed by Gerrit Code Review
parent fa90ad1392 25b4714f1c
commit 2249d0026d
3 changed files with 83 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
openstackclient/compute/v2/server_volume.py
View File

@ -14,7 +14,6 @@
"""Compute v2 Server action implementations""" """Compute v2 Server action implementations"""
from novaclient import api_versions
from openstack import utils as sdk_utils from openstack import utils as sdk_utils
from osc_lib.command import command from osc_lib.command import command
from osc_lib import exceptions from osc_lib import exceptions
@ -83,14 +82,14 @@ class UpdateServerVolume(command.Command):
"""Update a volume attachment on the server.""" """Update a volume attachment on the server."""
def get_parser(self, prog_name): def get_parser(self, prog_name):
parser = super(UpdateServerVolume, self).get_parser(prog_name) parser = super().get_parser(prog_name)
parser.add_argument( parser.add_argument(
'server', 'server',
help=_('Server to update volume for (name or ID)'), help=_('Server to update volume for (name or ID)'),
) )
parser.add_argument( parser.add_argument(
'volume', 'volume',
help=_('Volume (ID)'), help=_('Volume to update attachment for (name or ID)'),
) )
termination_group = parser.add_mutually_exclusive_group() termination_group = parser.add_mutually_exclusive_group()
termination_group.add_argument( termination_group.add_argument(
@ -115,31 +114,29 @@ class UpdateServerVolume(command.Command):
return parser return parser
def take_action(self, parsed_args): def take_action(self, parsed_args):
compute_client = self.app.client_manager.sdk_connection.compute
compute_client = self.app.client_manager.compute volume_client = self.app.client_manager.sdk_connection.volume
if parsed_args.delete_on_termination is not None: 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 = _( msg = _(
'--os-compute-api-version 2.85 or greater is required to ' '--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) raise exceptions.CommandError(msg)
server = utils.find_resource( server = compute_client.find_server(
compute_client.servers,
parsed_args.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. compute_client.update_volume_attachment(
# This API was originally used only for the swapping volumes, which server,
# is an internal operation that should only be done by volume,
# 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,
delete_on_termination=parsed_args.delete_on_termination, delete_on_termination=parsed_args.delete_on_termination,
) )

115
openstackclient/tests/unit/compute/v2/test_server_volume.py
View File

@ -19,6 +19,7 @@ from osc_lib import exceptions
from openstackclient.compute.v2 import server_volume from openstackclient.compute.v2 import server_volume
from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes 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): class TestServerVolume(compute_fakes.TestComputev2):
@ -26,17 +27,11 @@ class TestServerVolume(compute_fakes.TestComputev2):
def setUp(self): def setUp(self):
super().setUp() 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 = mock.Mock()
self.app.client_manager.sdk_connection.compute = 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): class TestServerVolumeList(TestServerVolume):
@ -47,8 +42,8 @@ class TestServerVolumeList(TestServerVolume):
self.server = compute_fakes.FakeServer.create_one_sdk_server() self.server = compute_fakes.FakeServer.create_one_sdk_server()
self.volume_attachments = compute_fakes.create_volume_attachments() self.volume_attachments = compute_fakes.create_volume_attachments()
self.sdk_client.find_server.return_value = self.server self.compute_client.find_server.return_value = self.server
self.sdk_client.volume_attachments.return_value = ( self.compute_client.volume_attachments.return_value = (
self.volume_attachments) self.volume_attachments)
# Get the command object to test # Get the command object to test
@ -88,7 +83,9 @@ class TestServerVolumeList(TestServerVolume):
), ),
tuple(data), 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') @mock.patch.object(sdk_utils, 'supports_microversion')
def test_server_volume_list_with_tags(self, sm_mock): def test_server_volume_list_with_tags(self, sm_mock):
@ -126,7 +123,9 @@ class TestServerVolumeList(TestServerVolume):
), ),
tuple(data), 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') @mock.patch.object(sdk_utils, 'supports_microversion')
def test_server_volume_list_with_delete_on_attachment(self, sm_mock): def test_server_volume_list_with_delete_on_attachment(self, sm_mock):
@ -169,7 +168,9 @@ class TestServerVolumeList(TestServerVolume):
), ),
tuple(data), 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') @mock.patch.object(sdk_utils, 'supports_microversion')
def test_server_volume_list_with_attachment_ids(self, sm_mock): def test_server_volume_list_with_attachment_ids(self, sm_mock):
@ -217,7 +218,9 @@ class TestServerVolumeList(TestServerVolume):
), ),
tuple(data), 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): class TestServerVolumeUpdate(TestServerVolume):
@ -225,21 +228,23 @@ class TestServerVolumeUpdate(TestServerVolume):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.server = compute_fakes.FakeServer.create_one_server() self.server = compute_fakes.FakeServer.create_one_sdk_server()
self.servers_mock.get.return_value = self.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 # Get the command object to test
self.cmd = server_volume.UpdateServerVolume(self.app, None) self.cmd = server_volume.UpdateServerVolume(self.app, None)
def test_server_volume_update(self): def test_server_volume_update(self):
arglist = [ arglist = [
self.server.id, self.server.id,
'foo', self.volume.id,
] ]
verifylist = [ verifylist = [
('server', self.server.id), ('server', self.server.id),
('volume', 'foo'), ('volume', self.volume.id),
('delete_on_termination', None), ('delete_on_termination', None),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -247,67 +252,73 @@ class TestServerVolumeUpdate(TestServerVolume):
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
# This is a no-op # 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) self.assertIsNone(result)
def test_server_volume_update_with_delete_on_termination(self): @mock.patch.object(sdk_utils, 'supports_microversion')
self.app.client_manager.compute.api_version = \ def test_server_volume_update_with_delete_on_termination(self, sm_mock):
api_versions.APIVersion('2.85') sm_mock.return_value = True
arglist = [ arglist = [
self.server.id, self.server.id,
'foo', self.volume.id,
'--delete-on-termination', '--delete-on-termination',
] ]
verifylist = [ verifylist = [
('server', self.server.id), ('server', self.server.id),
('volume', 'foo'), ('volume', self.volume.id),
('delete_on_termination', True), ('delete_on_termination', True),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
self.servers_volumes_mock.update_server_volume.assert_called_once_with( self.compute_client.update_volume_attachment.assert_called_once_with(
self.server.id, 'foo', 'foo', self.server,
delete_on_termination=True) self.volume,
delete_on_termination=True,
)
self.assertIsNone(result) self.assertIsNone(result)
def test_server_volume_update_with_preserve_on_termination(self): @mock.patch.object(sdk_utils, 'supports_microversion')
self.app.client_manager.compute.api_version = \ def test_server_volume_update_with_preserve_on_termination(self, sm_mock):
api_versions.APIVersion('2.85') sm_mock.return_value = True
arglist = [ arglist = [
self.server.id, self.server.id,
'foo', self.volume.id,
'--preserve-on-termination', '--preserve-on-termination',
] ]
verifylist = [ verifylist = [
('server', self.server.id), ('server', self.server.id),
('volume', 'foo'), ('volume', self.volume.id),
('delete_on_termination', False), ('delete_on_termination', False),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
result = self.cmd.take_action(parsed_args) result = self.cmd.take_action(parsed_args)
self.servers_volumes_mock.update_server_volume.assert_called_once_with( self.compute_client.update_volume_attachment.assert_called_once_with(
self.server.id, 'foo', 'foo', self.server,
delete_on_termination=False) self.volume,
delete_on_termination=False
)
self.assertIsNone(result) self.assertIsNone(result)
def test_server_volume_update_with_delete_on_termination_pre_v285(self): @mock.patch.object(sdk_utils, 'supports_microversion')
self.app.client_manager.compute.api_version = \ def test_server_volume_update_with_delete_on_termination_pre_v285(
api_versions.APIVersion('2.84') self, sm_mock,
):
sm_mock.return_value = False
arglist = [ arglist = [
self.server.id, self.server.id,
'foo', self.volume.id,
'--delete-on-termination', '--delete-on-termination',
] ]
verifylist = [ verifylist = [
('server', self.server.id), ('server', self.server.id),
('volume', 'foo'), ('volume', self.volume.id),
('delete_on_termination', True), ('delete_on_termination', True),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -315,20 +326,24 @@ class TestServerVolumeUpdate(TestServerVolume):
self.assertRaises( self.assertRaises(
exceptions.CommandError, exceptions.CommandError,
self.cmd.take_action, 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): @mock.patch.object(sdk_utils, 'supports_microversion')
self.app.client_manager.compute.api_version = \ def test_server_volume_update_with_preserve_on_termination_pre_v285(
api_versions.APIVersion('2.84') self, sm_mock,
):
sm_mock.return_value = False
arglist = [ arglist = [
self.server.id, self.server.id,
'foo', self.volume.id,
'--preserve-on-termination', '--preserve-on-termination',
] ]
verifylist = [ verifylist = [
('server', self.server.id), ('server', self.server.id),
('volume', 'foo'), ('volume', self.volume.id),
('delete_on_termination', False), ('delete_on_termination', False),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -336,4 +351,6 @@ class TestServerVolumeUpdate(TestServerVolume):
self.assertRaises( self.assertRaises(
exceptions.CommandError, exceptions.CommandError,
self.cmd.take_action, self.cmd.take_action,
parsed_args) parsed_args,
)
self.compute_client.update_volume_attachment.assert_not_called()

2
releasenotes/notes/migrate-server-volume-list-update-to-sdk-95b1d3063e46f813.yaml
View File

@ -1,3 +1,3 @@
features: 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.