openstack/python-openstackclient
Code Issues Proposed changes

Merge "volume: Migrate 'volume delete' to SDK"

Browse Source
This commit is contained in:
Zuul
2025-05-19 15:54:41 +00:00
committed by Gerrit Code Review
parent bea4d834ce 082aca89d6
commit de7762f9b8
4 changed files with 173 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

121
openstackclient/tests/unit/volume/v2/test_volume.py
View File

@@ -13,6 +13,9 @@
from unittest import mock from unittest import mock
from openstack.block_storage.v2 import volume as _volume
from openstack import exceptions as sdk_exceptions
from openstack.test import fakes as sdk_fakes
from osc_lib.cli import format_columns from osc_lib.cli import format_columns
from osc_lib import exceptions from osc_lib import exceptions
from osc_lib import utils from osc_lib import utils
@@ -46,12 +49,6 @@ class TestVolume(volume_fakes.TestVolume):
self.consistencygroups_mock = self.volume_client.consistencygroups self.consistencygroups_mock = self.volume_client.consistencygroups
self.consistencygroups_mock.reset_mock() self.consistencygroups_mock.reset_mock()
def setup_volumes_mock(self, count):
volumes = volume_fakes.create_volumes(count=count)
self.volumes_mock.get = volume_fakes.get_volumes(volumes, 0)
return volumes
class TestVolumeCreate(TestVolume): class TestVolumeCreate(TestVolume):
project = identity_fakes.FakeProject.create_one_project() project = identity_fakes.FakeProject.create_one_project()
@@ -662,37 +659,37 @@ class TestVolumeCreate(TestVolume):
self.assertCountEqual(self.datalist, data) self.assertCountEqual(self.datalist, data)
class TestVolumeDelete(TestVolume): class TestVolumeDelete(volume_fakes.TestVolume):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.volumes_mock.delete.return_value = None self.volumes = list(sdk_fakes.generate_fake_resources(_volume.Volume))
self.volume_sdk_client.find_volume.side_effect = self.volumes
self.volume_sdk_client.delete_volume.return_value = None
# Get the command object to mock
self.cmd = volume.DeleteVolume(self.app, None) self.cmd = volume.DeleteVolume(self.app, None)
def test_volume_delete_one_volume(self): def test_volume_delete_one_volume(self):
volumes = self.setup_volumes_mock(count=1) arglist = [self.volumes[0].id]
arglist = [volumes[0].id]
verifylist = [ verifylist = [
("force", False), ("force", False),
("purge", False), ("purge", False),
("volumes", [volumes[0].id]), ("volumes", [self.volumes[0].id]),
] ]
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.volumes_mock.delete.assert_called_once_with(
volumes[0].id, cascade=False
)
self.assertIsNone(result) self.assertIsNone(result)
def test_volume_delete_multi_volumes(self): self.volume_sdk_client.find_volume.assert_called_once_with(
volumes = self.setup_volumes_mock(count=3) self.volumes[0].id, ignore_missing=False
)
self.volume_sdk_client.delete_volume.assert_called_once_with(
self.volumes[0].id, cascade=False, force=False
)
arglist = [v.id for v in volumes] def test_volume_delete_multi_volumes(self):
arglist = [v.id for v in self.volumes]
verifylist = [ verifylist = [
('force', False), ('force', False),
('purge', False), ('purge', False),
@@ -701,83 +698,95 @@ class TestVolumeDelete(TestVolume):
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)
calls = [mock.call(v.id, cascade=False) for v in volumes]
self.volumes_mock.delete.assert_has_calls(calls)
self.assertIsNone(result) self.assertIsNone(result)
self.volume_sdk_client.find_volume.assert_has_calls(
[mock.call(v.id, ignore_missing=False) for v in self.volumes]
)
self.volume_sdk_client.delete_volume.assert_has_calls(
[mock.call(v.id, cascade=False, force=False) for v in self.volumes]
)
def test_volume_delete_multi_volumes_with_exception(self): def test_volume_delete_multi_volumes_with_exception(self):
volumes = self.setup_volumes_mock(count=2) self.volume_sdk_client.find_volume.side_effect = [
self.volumes[0],
sdk_exceptions.NotFoundException(),
]
arglist = [ arglist = [
volumes[0].id, self.volumes[0].id,
'unexist_volume', 'unexist_volume',
] ]
verifylist = [ verifylist = [
('force', False), ('force', False),
('purge', False), ('purge', False),
('volumes', arglist), ('volumes', [self.volumes[0].id, 'unexist_volume']),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
find_mock_result = [volumes[0], exceptions.CommandError] exc = self.assertRaises(
with mock.patch.object( exceptions.CommandError,
utils, 'find_resource', side_effect=find_mock_result self.cmd.take_action,
) as find_mock: parsed_args,
try: )
self.cmd.take_action(parsed_args) self.assertEqual('1 of 2 volumes failed to delete.', str(exc))
self.fail('CommandError should be raised.')
except exceptions.CommandError as e:
self.assertEqual('1 of 2 volumes failed to delete.', str(e))
find_mock.assert_any_call(self.volumes_mock, volumes[0].id) self.volume_sdk_client.find_volume.assert_has_calls(
find_mock.assert_any_call(self.volumes_mock, 'unexist_volume') [
mock.call(self.volumes[0].id, ignore_missing=False),
self.assertEqual(2, find_mock.call_count) mock.call('unexist_volume', ignore_missing=False),
self.volumes_mock.delete.assert_called_once_with( ]
volumes[0].id, cascade=False )
) self.volume_sdk_client.delete_volume.assert_has_calls(
[
mock.call(self.volumes[0].id, cascade=False, force=False),
]
)
def test_volume_delete_with_purge(self): def test_volume_delete_with_purge(self):
volumes = self.setup_volumes_mock(count=1)
arglist = [ arglist = [
'--purge', '--purge',
volumes[0].id, self.volumes[0].id,
] ]
verifylist = [ verifylist = [
('force', False), ('force', False),
('purge', True), ('purge', True),
('volumes', [volumes[0].id]), ('volumes', [self.volumes[0].id]),
] ]
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.volumes_mock.delete.assert_called_once_with(
volumes[0].id, cascade=True
)
self.assertIsNone(result) self.assertIsNone(result)
def test_volume_delete_with_force(self): self.volume_sdk_client.find_volume.assert_called_once_with(
volumes = self.setup_volumes_mock(count=1) self.volumes[0].id, ignore_missing=False
)
self.volume_sdk_client.delete_volume.assert_called_once_with(
self.volumes[0].id, cascade=True, force=False
)
def test_volume_delete_with_force(self):
arglist = [ arglist = [
'--force', '--force',
volumes[0].id, self.volumes[0].id,
] ]
verifylist = [ verifylist = [
('force', True), ('force', True),
('purge', False), ('purge', False),
('volumes', [volumes[0].id]), ('volumes', [self.volumes[0].id]),
] ]
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.volumes_mock.force_delete.assert_called_once_with(volumes[0].id)
self.assertIsNone(result) self.assertIsNone(result)
self.volume_sdk_client.find_volume.assert_called_once_with(
self.volumes[0].id, ignore_missing=False
)
self.volume_sdk_client.delete_volume.assert_called_once_with(
self.volumes[0].id, cascade=False, force=True
)
class TestVolumeList(TestVolume): class TestVolumeList(TestVolume):
project = identity_fakes.FakeProject.create_one_project() project = identity_fakes.FakeProject.create_one_project()

156
openstackclient/tests/unit/volume/v3/test_volume.py
View File

@@ -17,6 +17,7 @@ from unittest import mock
from openstack.block_storage.v3 import block_storage_summary as _summary from openstack.block_storage.v3 import block_storage_summary as _summary
from openstack.block_storage.v3 import snapshot as _snapshot from openstack.block_storage.v3 import snapshot as _snapshot
from openstack.block_storage.v3 import volume as _volume from openstack.block_storage.v3 import volume as _volume
from openstack import exceptions as sdk_exceptions
from openstack.test import fakes as sdk_fakes from openstack.test import fakes as sdk_fakes
from osc_lib.cli import format_columns from osc_lib.cli import format_columns
from osc_lib import exceptions from osc_lib import exceptions
@@ -954,16 +955,17 @@ class TestVolumeCreate(volume_fakes.TestVolume):
) )
class TestVolumeDeleteLegacy(volume_fakes.TestVolume): class TestVolumeDelete(volume_fakes.TestVolume):
def setUp(self): def setUp(self):
super().setUp() super().setUp()
self.volumes_mock = self.volume_client.volumes self.volumes_mock = self.volume_client.volumes
self.volumes_mock.reset_mock() self.volumes_mock.reset_mock()
self.volumes_mock.delete.return_value = None self.volumes = list(sdk_fakes.generate_fake_resources(_volume.Volume))
self.volumes = volume_fakes.create_volumes(count=1) self.volume_sdk_client.find_volume.side_effect = self.volumes
self.volumes_mock.get = volume_fakes.get_volumes(self.volumes, 0) self.volume_sdk_client.delete_volume.return_value = None
self.volume_sdk_client.unmanage_volume.return_value = None
# Get the command object to mock # Get the command object to mock
self.cmd = volume.DeleteVolume(self.app, None) self.cmd = volume.DeleteVolume(self.app, None)
@@ -978,12 +980,15 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume):
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.volumes_mock.delete.assert_called_once_with(
self.volumes[0].id, cascade=False
)
self.assertIsNone(result) self.assertIsNone(result)
self.volume_sdk_client.find_volume.assert_called_once_with(
self.volumes[0].id, ignore_missing=False
)
self.volume_sdk_client.delete_volume.assert_called_once_with(
self.volumes[0].id, cascade=False, force=False
)
def test_volume_delete_multi_volumes(self): def test_volume_delete_multi_volumes(self):
arglist = [v.id for v in self.volumes] arglist = [v.id for v in self.volumes]
verifylist = [ verifylist = [
@@ -994,12 +999,21 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume):
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)
calls = [mock.call(v.id, cascade=False) for v in self.volumes]
self.volumes_mock.delete.assert_has_calls(calls)
self.assertIsNone(result) self.assertIsNone(result)
self.volume_sdk_client.find_volume.assert_has_calls(
[mock.call(v.id, ignore_missing=False) for v in self.volumes]
)
self.volume_sdk_client.delete_volume.assert_has_calls(
[mock.call(v.id, cascade=False, force=False) for v in self.volumes]
)
def test_volume_delete_multi_volumes_with_exception(self): def test_volume_delete_multi_volumes_with_exception(self):
self.volume_sdk_client.find_volume.side_effect = [
self.volumes[0],
sdk_exceptions.NotFoundException(),
]
arglist = [ arglist = [
self.volumes[0].id, self.volumes[0].id,
'unexist_volume', 'unexist_volume',
@@ -1007,27 +1021,28 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume):
verifylist = [ verifylist = [
('force', False), ('force', False),
('purge', False), ('purge', False),
('volumes', arglist), ('volumes', [self.volumes[0].id, 'unexist_volume']),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
find_mock_result = [self.volumes[0], exceptions.CommandError] exc = self.assertRaises(
with mock.patch.object( exceptions.CommandError,
utils, 'find_resource', side_effect=find_mock_result self.cmd.take_action,
) as find_mock: parsed_args,
try: )
self.cmd.take_action(parsed_args) self.assertEqual('1 of 2 volumes failed to delete.', str(exc))
self.fail('CommandError should be raised.')
except exceptions.CommandError as e:
self.assertEqual('1 of 2 volumes failed to delete.', str(e))
find_mock.assert_any_call(self.volumes_mock, self.volumes[0].id) self.volume_sdk_client.find_volume.assert_has_calls(
find_mock.assert_any_call(self.volumes_mock, 'unexist_volume') [
mock.call(self.volumes[0].id, ignore_missing=False),
self.assertEqual(2, find_mock.call_count) mock.call('unexist_volume', ignore_missing=False),
self.volumes_mock.delete.assert_called_once_with( ]
self.volumes[0].id, cascade=False )
) self.volume_sdk_client.delete_volume.assert_has_calls(
[
mock.call(self.volumes[0].id, cascade=False, force=False),
]
)
def test_volume_delete_with_purge(self): def test_volume_delete_with_purge(self):
arglist = [ arglist = [
@@ -1042,12 +1057,15 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume):
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.volumes_mock.delete.assert_called_once_with(
self.volumes[0].id, cascade=True
)
self.assertIsNone(result) self.assertIsNone(result)
self.volume_sdk_client.find_volume.assert_called_once_with(
self.volumes[0].id, ignore_missing=False
)
self.volume_sdk_client.delete_volume.assert_called_once_with(
self.volumes[0].id, cascade=True, force=False
)
def test_volume_delete_with_force(self): def test_volume_delete_with_force(self):
arglist = [ arglist = [
'--force', '--force',
@@ -1061,49 +1079,38 @@ class TestVolumeDeleteLegacy(volume_fakes.TestVolume):
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.volumes_mock.force_delete.assert_called_once_with(
self.volumes[0].id
)
self.assertIsNone(result) self.assertIsNone(result)
self.volume_sdk_client.find_volume.assert_called_once_with(
class TestVolumeDelete(volume_fakes.TestVolume): self.volumes[0].id, ignore_missing=False
def setUp(self): )
super().setUp() self.volume_sdk_client.delete_volume.assert_called_once_with(
self.volumes[0].id, cascade=False, force=True
self.volumes_mock = self.volume_client.volumes )
self.volumes_mock.reset_mock()
self.volume_sdk_client.unmanage_volume.return_value = None
# Get the command object to mock
self.cmd = volume.DeleteVolume(self.app, None)
def test_volume_delete_remote(self): def test_volume_delete_remote(self):
vol = sdk_fakes.generate_fake_resource(_volume.Volume, **{'size': 1}) arglist = ['--remote', self.volumes[0].id]
self.volumes_mock.get.return_value = vol
arglist = ['--remote', vol.id]
verifylist = [ verifylist = [
("remote", True), ("remote", True),
("force", False), ("force", False),
("purge", False), ("purge", False),
("volumes", [vol.id]), ("volumes", [self.volumes[0].id]),
] ]
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.volume_sdk_client.unmanage_volume.assert_called_once_with(vol.id)
self.assertIsNone(result) self.assertIsNone(result)
def test_volume_delete_multi_volumes_remote(self): self.volume_sdk_client.find_volume.assert_called_once_with(
volumes = sdk_fakes.generate_fake_resources( self.volumes[0].id, ignore_missing=False
_volume.Volume, count=3, attrs={'size': 1} )
self.volume_sdk_client.delete_volume.assert_not_called()
self.volume_sdk_client.unmanage_volume.assert_called_once_with(
self.volumes[0].id
) )
arglist = ['--remote'] def test_volume_delete_multi_volumes_remote(self):
arglist += [v.id for v in volumes] arglist = ['--remote'] + [v.id for v in self.volumes]
verifylist = [ verifylist = [
('remote', True), ('remote', True),
('force', False), ('force', False),
@@ -1113,24 +1120,27 @@ class TestVolumeDelete(volume_fakes.TestVolume):
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)
calls = [mock.call(v.id) for v in volumes]
self.volume_sdk_client.unmanage_volume.assert_has_calls(calls)
self.assertIsNone(result) self.assertIsNone(result)
def test_volume_delete_remote_with_purge(self): self.volume_sdk_client.find_volume.assert_has_calls(
vol = sdk_fakes.generate_fake_resource(_volume.Volume, **{'size': 1}) [mock.call(v.id, ignore_missing=False) for v in self.volumes]
)
self.volume_sdk_client.delete_volume.assert_not_called()
self.volume_sdk_client.unmanage_volume.assert_has_calls(
[mock.call(v.id) for v in self.volumes]
)
def test_volume_delete_remote_with_purge(self):
arglist = [ arglist = [
'--remote', '--remote',
'--purge', '--purge',
vol.id, self.volumes[0].id,
] ]
verifylist = [ verifylist = [
('remote', True), ('remote', True),
('force', False), ('force', False),
('purge', True), ('purge', True),
('volumes', [vol.id]), ('volumes', [self.volumes[0].id]),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1143,19 +1153,21 @@ class TestVolumeDelete(volume_fakes.TestVolume):
str(exc), str(exc),
) )
def test_volume_delete_remote_with_force(self): self.volume_sdk_client.find_volume.assert_not_called()
vol = sdk_fakes.generate_fake_resource(_volume.Volume, **{'size': 1}) self.volume_sdk_client.delete_volume.assert_not_called()
self.volume_sdk_client.unmanage_volume.assert_not_called()
def test_volume_delete_remote_with_force(self):
arglist = [ arglist = [
'--remote', '--remote',
'--force', '--force',
vol.id, self.volumes[0].id,
] ]
verifylist = [ verifylist = [
('remote', True), ('remote', True),
('force', True), ('force', True),
('purge', False), ('purge', False),
('volumes', [vol.id]), ('volumes', [self.volumes[0].id]),
] ]
parsed_args = self.check_parser(self.cmd, arglist, verifylist) parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -1168,6 +1180,10 @@ class TestVolumeDelete(volume_fakes.TestVolume):
str(exc), str(exc),
) )
self.volume_sdk_client.find_volume.assert_not_called()
self.volume_sdk_client.delete_volume.assert_not_called()
self.volume_sdk_client.unmanage_volume.assert_not_called()
class TestVolumeList(volume_fakes.TestVolume): class TestVolumeList(volume_fakes.TestVolume):
project = identity_fakes.FakeProject.create_one_project() project = identity_fakes.FakeProject.create_one_project()

21
openstackclient/volume/v2/volume.py
View File

@@ -364,18 +364,19 @@ class DeleteVolume(command.Command):
return parser return parser
def take_action(self, parsed_args): def take_action(self, parsed_args):
volume_client = self.app.client_manager.volume volume_client = self.app.client_manager.sdk_connection.volume
result = 0 result = 0
for i in parsed_args.volumes: for volume in parsed_args.volumes:
try: try:
volume_obj = utils.find_resource(volume_client.volumes, i) volume_obj = volume_client.find_volume(
if parsed_args.force: volume, ignore_missing=False
volume_client.volumes.force_delete(volume_obj.id) )
else: volume_client.delete_volume(
volume_client.volumes.delete( volume_obj.id,
volume_obj.id, cascade=parsed_args.purge force=parsed_args.force,
) cascade=parsed_args.purge,
)
except Exception as e: except Exception as e:
result += 1 result += 1
LOG.error( LOG.error(
@@ -383,7 +384,7 @@ class DeleteVolume(command.Command):
"Failed to delete volume with " "Failed to delete volume with "
"name or ID '%(volume)s': %(e)s" "name or ID '%(volume)s': %(e)s"
), ),
{'volume': i, 'e': e}, {'volume': volume, 'e': e},
) )
if result > 0: if result > 0:

21
openstackclient/volume/v3/volume.py
View File

@@ -505,8 +505,7 @@ class DeleteVolume(command.Command):
return parser return parser
def take_action(self, parsed_args): def take_action(self, parsed_args):
volume_client = self.app.client_manager.volume volume_client = self.app.client_manager.sdk_connection.volume
volume_client_sdk = self.app.client_manager.sdk_connection.volume
result = 0 result = 0
if parsed_args.remote and (parsed_args.force or parsed_args.purge): if parsed_args.remote and (parsed_args.force or parsed_args.purge):
@@ -516,16 +515,18 @@ class DeleteVolume(command.Command):
) )
raise exceptions.CommandError(msg) raise exceptions.CommandError(msg)
for i in parsed_args.volumes: for volume in parsed_args.volumes:
try: try:
volume_obj = utils.find_resource(volume_client.volumes, i) volume_obj = volume_client.find_volume(
volume, ignore_missing=False
)
if parsed_args.remote: if parsed_args.remote:
volume_client_sdk.unmanage_volume(volume_obj.id) volume_client.unmanage_volume(volume_obj.id)
elif parsed_args.force:
volume_client.volumes.force_delete(volume_obj.id)
else: else:
volume_client.volumes.delete( volume_client.delete_volume(
volume_obj.id, cascade=parsed_args.purge volume_obj.id,
force=parsed_args.force,
cascade=parsed_args.purge,
) )
except Exception as e: except Exception as e:
result += 1 result += 1
@@ -534,7 +535,7 @@ class DeleteVolume(command.Command):
"Failed to delete volume with " "Failed to delete volume with "
"name or ID '%(volume)s': %(e)s" "name or ID '%(volume)s': %(e)s"
), ),
{'volume': i, 'e': e}, {'volume': volume, 'e': e},
) )
if result > 0: if result > 0: