From 25b4714f1cd12c20ff7653f05dddb5486a037fcc Mon Sep 17 00:00:00 2001
From: Ritvik Vinodkumar <vinodkumar.r@northeastern.edu>
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