openstack/python-openstackclient
Code Issues Proposed changes

volume: Add v3-specific volume module

Browse Source 
This makes testing easier.

Change-Id: I6b31026ae3c9dc66d828744534b35bb0a0d2ffbe
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2024-06-12 16:02:09 +01:00
parent ca91c826e3
commit 65cce3943a
7 changed files with 3067 additions and 442 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
openstackclient/tests/unit/volume/v2/fakes.py
View File

@ -25,7 +25,6 @@ from openstack.block_storage.v3 import capabilities as _capabilities
from openstack.block_storage.v3 import stats as _stats
from openstack.block_storage.v3 import volume as _volume
from openstack.image.v2 import _proxy as image_v2_proxy
from osc_lib.cli import format_columns
from openstackclient.tests.unit import fakes
from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
@ -472,43 +471,6 @@ def get_volumes(volumes=None, count=2):
return mock.Mock(side_effect=volumes)
def get_volume_columns(volume=None):
"""Get the volume columns from a faked volume object.
:param volume:
A FakeResource objects faking volume
:return
A tuple which may include the following keys:
('id', 'name', 'description', 'status', 'size', 'volume_type',
'metadata', 'snapshot', 'availability_zone', 'attachments')
"""
if volume is not None:
return tuple(k for k in sorted(volume.keys()))
return tuple([])
def get_volume_data(volume=None):
"""Get the volume data from a faked volume object.
:param volume:
A FakeResource objects faking volume
:return
A tuple which may include the following values:
('ce26708d', 'fake_volume', 'fake description', 'available',
20, 'fake_lvmdriver-1', "Alpha='a', Beta='b', Gamma='g'",
1, 'nova', [{'device': '/dev/ice', 'server_id': '1233'}])
"""
data_list = []
if volume is not None:
for x in sorted(volume.keys()):
if x == 'tags':
# The 'tags' should be format_list
data_list.append(format_columns.ListColumn(volume.info.get(x)))
else:
data_list.append(volume.info.get(x))
return tuple(data_list)
def create_one_backup(attrs=None):
"""Create a fake backup.

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

@ -12,9 +12,7 @@
# under the License.
from unittest import mock
from unittest.mock import call
from cinderclient import api_versions
from osc_lib.cli import format_columns
from osc_lib import exceptions
from osc_lib import utils
@ -42,9 +40,6 @@ class TestVolume(volume_fakes.TestVolume):
self.snapshots_mock = self.volume_client.volume_snapshots
self.snapshots_mock.reset_mock()
self.backups_mock = self.volume_client.backups
self.backups_mock.reset_mock()
self.types_mock = self.volume_client.volume_types
self.types_mock.reset_mock()
@ -126,7 +121,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -178,7 +172,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=consistency_group.id,
scheduler_hints={'k': 'v'},
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -218,7 +211,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -259,7 +251,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -300,7 +291,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -339,74 +329,11 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
self.assertCountEqual(self.datalist, data)
def test_volume_create_with_backup(self):
backup = volume_fakes.create_one_backup()
self.new_volume.backup_id = backup.id
arglist = [
'--backup',
self.new_volume.backup_id,
self.new_volume.name,
]
verifylist = [
('backup', self.new_volume.backup_id),
('name', self.new_volume.name),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.backups_mock.get.return_value = backup
self.volume_client.api_version = api_versions.APIVersion('3.47')
# In base command class ShowOne in cliff, abstract method take_action()
# returns a two-part tuple with a tuple of column names and a tuple of
# data to be shown.
columns, data = self.cmd.take_action(parsed_args)
self.volumes_mock.create.assert_called_once_with(
size=backup.size,
snapshot_id=None,
name=self.new_volume.name,
description=None,
volume_type=None,
availability_zone=None,
metadata=None,
imageRef=None,
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=backup.id,
)
self.assertEqual(self.columns, columns)
self.assertCountEqual(self.datalist, data)
def test_volume_create_with_backup_pre_347(self):
backup = volume_fakes.create_one_backup()
self.new_volume.backup_id = backup.id
arglist = [
'--backup',
self.new_volume.backup_id,
self.new_volume.name,
]
verifylist = [
('backup', self.new_volume.backup_id),
('name', self.new_volume.name),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.backups_mock.get.return_value = backup
exc = self.assertRaises(
exceptions.CommandError, self.cmd.take_action, parsed_args
)
self.assertIn("--os-volume-api-version 3.47 or greater", str(exc))
def test_volume_create_with_source_volume(self):
source_vol = "source_vol"
arglist = [
@ -439,7 +366,6 @@ class TestVolumeCreate(TestVolume):
source_volid=self.new_volume.id,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -479,7 +405,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -525,7 +450,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -580,7 +504,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(2, mock_error.call_count)
@ -632,7 +555,6 @@ class TestVolumeCreate(TestVolume):
source_volid=None,
consistencygroup_id=None,
scheduler_hints=None,
backup_id=None,
)
self.assertEqual(2, mock_error.call_count)
@ -742,7 +664,6 @@ class TestVolumeCreate(TestVolume):
'local_to_instance': 'v6',
'different_host': ['v5', 'v7'],
},
backup_id=None,
)
self.assertEqual(self.columns, columns)
@ -789,7 +710,7 @@ class TestVolumeDelete(TestVolume):
result = self.cmd.take_action(parsed_args)
calls = [call(v.id, cascade=False) for v in volumes]
calls = [mock.call(v.id, cascade=False) for v in volumes]
self.volumes_mock.delete.assert_has_calls(calls)
self.assertIsNone(result)
@ -1721,11 +1642,23 @@ class TestVolumeShow(TestVolume):
self.volumes_mock.get.assert_called_with(self._volume.id)
self.assertEqual(
volume_fakes.get_volume_columns(self._volume),
tuple(sorted(self._volume.keys())),
columns,
)
self.assertCountEqual(
volume_fakes.get_volume_data(self._volume),
self.assertTupleEqual(
(
self._volume.attachments,
self._volume.availability_zone,
self._volume.bootable,
self._volume.description,
self._volume.id,
self._volume.name,
format_columns.DictColumn(self._volume.metadata),
self._volume.size,
self._volume.snapshot_id,
self._volume.status,
self._volume.volume_type,
),
data,
)

140
openstackclient/tests/unit/volume/v3/fakes.py
View File

@ -10,6 +10,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import copy
import random
from unittest import mock
import uuid
@ -21,6 +22,7 @@ from openstack.block_storage.v3 import backup as _backup
from openstack.block_storage.v3 import extension as _extension
from openstack.block_storage.v3 import resource_filter as _filters
from openstack.block_storage.v3 import volume as _volume
from openstack.image.v2 import _proxy as _image_proxy
from openstackclient.tests.unit import fakes
from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
@ -34,12 +36,14 @@ class FakeVolumeClient:
self.management_url = kwargs['endpoint']
self.api_version = api_versions.APIVersion('3.0')
self.attachments = mock.Mock()
self.attachments.resource_class = fakes.FakeResource(None, {})
self.availability_zones = mock.Mock()
self.availability_zones.resource_class = fakes.FakeResource(None, {})
self.backups = mock.Mock()
self.backups.resource_class = fakes.FakeResource(None, {})
self.attachments = mock.Mock()
self.attachments.resource_class = fakes.FakeResource(None, {})
self.consistencygroups = mock.Mock()
self.consistencygroups.resource_class = fakes.FakeResource(None, {})
self.clusters = mock.Mock()
self.clusters.resource_class = fakes.FakeResource(None, {})
self.groups = mock.Mock()
@ -106,10 +110,14 @@ class TestVolume(
)
self.compute_client = self.app.client_manager.compute
# avoid circular imports by defining this manually rather than using
# openstackclient.tests.unit.image.v2.fakes.FakeClientMixin
self.app.client_manager.image = mock.Mock(spec=_image_proxy.Proxy)
self.image_client = self.app.client_manager.image
# TODO(stephenfin): Check if the responses are actually the same
create_one_snapshot = volume_v2_fakes.create_one_snapshot
create_one_volume = volume_v2_fakes.create_one_volume
create_one_volume_type = volume_v2_fakes.create_one_volume_type
@ -153,6 +161,54 @@ def create_availability_zones(attrs=None, count=2):
return availability_zones
def create_one_consistency_group(attrs=None):
"""Create a fake consistency group.
:param dict attrs:
A dictionary with all attributes
:return:
A FakeResource object with id, name, description, etc.
"""
attrs = attrs or {}
# Set default attributes.
consistency_group_info = {
"id": 'backup-id-' + uuid.uuid4().hex,
"name": 'backup-name-' + uuid.uuid4().hex,
"description": 'description-' + uuid.uuid4().hex,
"status": "error",
"availability_zone": 'zone' + uuid.uuid4().hex,
"created_at": 'time-' + uuid.uuid4().hex,
"volume_types": ['volume-type1'],
}
# Overwrite default attributes.
consistency_group_info.update(attrs)
consistency_group = fakes.FakeResource(
info=copy.deepcopy(consistency_group_info), loaded=True
)
return consistency_group
def create_consistency_groups(attrs=None, count=2):
"""Create multiple fake consistency groups.
:param dict attrs:
A dictionary with all attributes
:param int count:
The number of consistency groups to fake
:return:
A list of FakeResource objects faking the consistency groups
"""
consistency_groups = []
for i in range(0, count):
consistency_group = create_one_consistency_group(attrs)
consistency_groups.append(consistency_group)
return consistency_groups
def create_one_extension(attrs=None):
"""Create a fake extension.
@ -349,6 +405,84 @@ def create_resource_filters(attrs=None, count=2):
return resource_filters
def create_one_volume(attrs=None):
"""Create a fake volume.
:param dict attrs:
A dictionary with all attributes of volume
:return:
A FakeResource object with id, name, status, etc.
"""
attrs = attrs or {}
# Set default attribute
volume_info = {
'id': 'volume-id' + uuid.uuid4().hex,
'name': 'volume-name' + uuid.uuid4().hex,
'description': 'description' + uuid.uuid4().hex,
'status': random.choice(['available', 'in_use']),
'size': random.randint(1, 20),
'volume_type': random.choice(['fake_lvmdriver-1', 'fake_lvmdriver-2']),
'bootable': random.randint(0, 1),
'metadata': {
'key' + uuid.uuid4().hex: 'val' + uuid.uuid4().hex,
'key' + uuid.uuid4().hex: 'val' + uuid.uuid4().hex,
'key' + uuid.uuid4().hex: 'val' + uuid.uuid4().hex,
},
'snapshot_id': random.randint(1, 5),
'availability_zone': 'zone' + uuid.uuid4().hex,
'attachments': [
{
'device': '/dev/' + uuid.uuid4().hex,
'server_id': uuid.uuid4().hex,
},
],
}
# Overwrite default attributes if there are some attributes set
volume_info.update(attrs)
volume = fakes.FakeResource(None, volume_info, loaded=True)
return volume
def create_volumes(attrs=None, count=2):
"""Create multiple fake volumes.
:param dict attrs:
A dictionary with all attributes of volume
:param Integer count:
The number of volumes to be faked
:return:
A list of FakeResource objects
"""
volumes = []
for n in range(0, count):
volumes.append(create_one_volume(attrs))
return volumes
def get_volumes(volumes=None, count=2):
"""Get an iterable MagicMock object with a list of faked volumes.
If volumes list is provided, then initialize the Mock object with the
list. Otherwise create one.
:param List volumes:
A list of FakeResource objects faking volumes
:param Integer count:
The number of volumes to be faked
:return
An iterable Mock object with side_effect set to a list of faked
volumes
"""
if volumes is None:
volumes = create_volumes(count)
return mock.Mock(side_effect=volumes)
def create_one_sdk_volume(attrs=None):
"""Create a fake volume.

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

File diff suppressed because it is too large Load Diff

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

@ -99,12 +99,10 @@ class CreateVolume(command.ShowOne):
volume is not specified.
"""
if (
args.snapshot or args.source or args.backup
) is None and args.size is None:
if (args.snapshot or args.source) is None and args.size is None:
msg = _(
"--size is a required option if snapshot, backup "
"or source volume are not specified."
"--size is a required option if --snapshot or --source are "
"not specified"
)
raise exceptions.CommandError(msg)
@ -121,8 +119,8 @@ class CreateVolume(command.ShowOne):
metavar="<size>",
type=int,
help=_(
"Volume size in GB (required unless --snapshot, "
"--source or --backup is specified)"
"Volume size in GB (required unless --snapshot or "
"--source specified)"
),
)
parser.add_argument(
@ -146,14 +144,6 @@ class CreateVolume(command.ShowOne):
metavar="<volume>",
help=_("Volume to clone (name or ID)"),
)
source_group.add_argument(
"--backup",
metavar="<backup>",
help=_(
"Restore backup to a volume (name or ID) "
"(supported by --os-volume-api-version 3.47 or later)"
),
)
source_group.add_argument(
"--source-replicated",
metavar="<replicated-volume>",
@ -222,26 +212,17 @@ class CreateVolume(command.ShowOne):
parser, _ = self._get_parser(prog_name)
return parser
def _take_action(self, parsed_args):
def take_action(self, parsed_args):
CreateVolume._check_size_arg(parsed_args)
# size is validated in the above call to
# _check_size_arg where we check that size
# should be passed if we are not creating a
# volume from snapshot, backup or source volume
# volume from snapshot or source volume
size = parsed_args.size
volume_client = self.app.client_manager.volume
image_client = self.app.client_manager.image
if parsed_args.backup and not (
volume_client.api_version.matches('3.47')
):
msg = _(
"--os-volume-api-version 3.47 or greater is required "
"to create a volume from backup."
)
raise exceptions.CommandError(msg)
source_volume = None
if parsed_args.source:
source_volume_obj = utils.find_resource(
@ -276,15 +257,6 @@ class CreateVolume(command.ShowOne):
# snapshot size.
size = max(size or 0, snapshot_obj.size)
backup = None
if parsed_args.backup:
backup_obj = utils.find_resource(
volume_client.backups, parsed_args.backup
)
backup = backup_obj.id
# As above
size = max(size or 0, backup_obj.size)
volume = volume_client.volumes.create(
size=size,
snapshot_id=snapshot,
@ -297,7 +269,6 @@ class CreateVolume(command.ShowOne):
source_volid=source_volume,
consistencygroup_id=consistency_group,
scheduler_hints=parsed_args.hint,
backup_id=backup,
)
if parsed_args.bootable or parsed_args.non_bootable:
@ -359,9 +330,6 @@ class CreateVolume(command.ShowOne):
volume._info.pop("links", None)
return zip(*sorted(volume._info.items()))
def take_action(self, parsed_args):
return self._take_action(parsed_args)
class DeleteVolume(command.Command):
_description = _("Delete volume(s)")
@ -784,10 +752,7 @@ class SetVolume(command.Command):
_("New size must be greater than %s GB") % volume.size
)
raise exceptions.CommandError(msg)
if (
volume.status != 'available'
and not volume_client.api_version.matches('3.42')
):
if volume.status != 'available':
msg = (
_(
"Volume is in %s state, it must be available "

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

File diff suppressed because it is too large Load Diff

10
setup.cfg
View File

@ -767,11 +767,11 @@ openstack.volume.v3 =
volume_create = openstackclient.volume.v3.volume:CreateVolume
volume_delete = openstackclient.volume.v3.volume:DeleteVolume
volume_list = openstackclient.volume.v2.volume:ListVolume
volume_migrate = openstackclient.volume.v2.volume:MigrateVolume
volume_set = openstackclient.volume.v2.volume:SetVolume
volume_show = openstackclient.volume.v2.volume:ShowVolume
volume_unset = openstackclient.volume.v2.volume:UnsetVolume
volume_list = openstackclient.volume.v3.volume:ListVolume
volume_migrate = openstackclient.volume.v3.volume:MigrateVolume
volume_set = openstackclient.volume.v3.volume:SetVolume
volume_show = openstackclient.volume.v3.volume:ShowVolume
volume_unset = openstackclient.volume.v3.volume:UnsetVolume
volume_attachment_create = openstackclient.volume.v3.volume_attachment:CreateVolumeAttachment
volume_attachment_delete = openstackclient.volume.v3.volume_attachment:DeleteVolumeAttachment