compute: Add 'server create --block-device' option
One of the last big gaps with novaclient. As noted in the release note, the current '--block-device-mapping' format is based on the old BDM v1 format, even though it actually results in BDM v2-style requests to the server. It's time to replace that. Change-Id: If4eba38ccfb208ee186b90a0eec95e5fe6cf8415 Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane
parent
f2deabb136
commit
ace4bfb640
@ -801,6 +801,7 @@ class CreateServer(command.ShowOne):
|
||||
'options.'
|
||||
)
|
||||
)
|
||||
# TODO(stephenfin): Remove this in the v7.0
|
||||
parser.add_argument(
|
||||
'--block-device-mapping',
|
||||
metavar='<dev-name=mapping>',
|
||||
@ -809,7 +810,7 @@ class CreateServer(command.ShowOne):
|
||||
# NOTE(RuiChen): Add '\n' to the end of line to improve formatting;
|
||||
# see cliff's _SmartHelpFormatter for more details.
|
||||
help=_(
|
||||
'Create a block device on the server.\n'
|
||||
'**Deprecated** Create a block device on the server.\n'
|
||||
'Block device mapping in the format\n'
|
||||
'<dev-name>=<id>:<type>:<size(GB)>:<delete-on-terminate>\n'
|
||||
'<dev-name>: block device name, like: vdb, xvdc '
|
||||
@ -822,6 +823,49 @@ class CreateServer(command.ShowOne):
|
||||
'(optional)\n'
|
||||
'<delete-on-terminate>: true or false; default: false '
|
||||
'(optional)\n'
|
||||
'Replaced by --block-device'
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
'--block-device',
|
||||
metavar='',
|
||||
action=parseractions.MultiKeyValueAction,
|
||||
dest='block_devices',
|
||||
default=[],
|
||||
required_keys=[
|
||||
'boot_index',
|
||||
],
|
||||
optional_keys=[
|
||||
'uuid', 'source_type', 'destination_type',
|
||||
'disk_bus', 'device_type', 'device_name', 'guest_format',
|
||||
'volume_size', 'volume_type', 'delete_on_termination', 'tag',
|
||||
],
|
||||
help=_(
|
||||
'Create a block device on the server.\n'
|
||||
'Block device in the format:\n'
|
||||
'uuid=<uuid>: UUID of the volume, snapshot or ID '
|
||||
'(required if using source image, snapshot or volume),\n'
|
||||
'source_type=<source_type>: source type '
|
||||
'(one of: image, snapshot, volume, blank),\n'
|
||||
'destination_typ=<destination_type>: destination type '
|
||||
'(one of: volume, local) (optional),\n'
|
||||
'disk_bus=<disk_bus>: device bus '
|
||||
'(one of: uml, lxc, virtio, ...) (optional),\n'
|
||||
'device_type=<device_type>: device type '
|
||||
'(one of: disk, cdrom, etc. (optional),\n'
|
||||
'device_name=<device_name>: name of the device (optional),\n'
|
||||
'volume_size=<volume_size>: size of the block device in MiB '
|
||||
'(for swap) or GiB (for everything else) (optional),\n'
|
||||
'guest_format=<guest_format>: format of device (optional),\n'
|
||||
'boot_index=<boot_index>: index of disk used to order boot '
|
||||
'disk '
|
||||
'(required for volume-backed instances),\n'
|
||||
'delete_on_termination=<true|false>: whether to delete the '
|
||||
'volume upon deletion of server (optional),\n'
|
||||
'tag=<tag>: device metadata tag (optional),\n'
|
||||
'volume_type=<volume_type>: type of volume to create (name or '
|
||||
'ID) when source if blank, image or snapshot and dest is '
|
||||
'volume (optional)'
|
||||
),
|
||||
)
|
||||
parser.add_argument(
|
||||
@ -1250,6 +1294,8 @@ class CreateServer(command.ShowOne):
|
||||
|
||||
# Handle block device by device name order, like: vdb -> vdc -> vdd
|
||||
for mapping in parsed_args.block_device_mapping:
|
||||
# The 'uuid' field isn't necessarily a UUID yet; let's validate it
|
||||
# just in case
|
||||
if mapping['source_type'] == 'volume':
|
||||
volume_id = utils.find_resource(
|
||||
volume_client.volumes, mapping['uuid'],
|
||||
@ -1279,6 +1325,77 @@ class CreateServer(command.ShowOne):
|
||||
|
||||
block_device_mapping_v2.append(mapping)
|
||||
|
||||
for mapping in parsed_args.block_devices:
|
||||
try:
|
||||
mapping['boot_index'] = int(mapping['boot_index'])
|
||||
except ValueError:
|
||||
msg = _(
|
||||
'The boot_index key of --block-device should be an '
|
||||
'integer'
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
|
||||
if 'tag' in mapping and (
|
||||
compute_client.api_version < api_versions.APIVersion('2.42')
|
||||
):
|
||||
msg = _(
|
||||
'--os-compute-api-version 2.42 or greater is '
|
||||
'required to support the tag key of --block-device'
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
|
||||
if 'volume_type' in mapping and (
|
||||
compute_client.api_version < api_versions.APIVersion('2.67')
|
||||
):
|
||||
msg = _(
|
||||
'--os-compute-api-version 2.67 or greater is '
|
||||
'required to support the volume_type key of --block-device'
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
|
||||
if 'source_type' in mapping:
|
||||
if mapping['source_type'] not in (
|
||||
'volume', 'image', 'snapshot', 'blank',
|
||||
):
|
||||
msg = _(
|
||||
'The source_type key of --block-device should be one '
|
||||
'of: volume, image, snapshot, blank'
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
else:
|
||||
mapping['source_type'] = 'blank'
|
||||
|
||||
if 'destination_type' in mapping:
|
||||
if mapping['destination_type'] not in ('local', 'volume'):
|
||||
msg = _(
|
||||
'The destination_type key of --block-device should be '
|
||||
'one of: local, volume'
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
else:
|
||||
if mapping['source_type'] in ('image', 'blank'):
|
||||
mapping['destination_type'] = 'local'
|
||||
else: # volume, snapshot
|
||||
mapping['destination_type'] = 'volume'
|
||||
|
||||
if 'delete_on_termination' in mapping:
|
||||
try:
|
||||
value = strutils.bool_from_string(
|
||||
mapping['delete_on_termination'], strict=True)
|
||||
except ValueError:
|
||||
msg = _(
|
||||
'The delete_on_termination key of --block-device '
|
||||
'should be a boolean-like value'
|
||||
)
|
||||
raise exceptions.CommandError(msg)
|
||||
|
||||
mapping['delete_on_termination'] = value
|
||||
else:
|
||||
if mapping['destination_type'] == 'local':
|
||||
mapping['delete_on_termination'] = True
|
||||
|
||||
block_device_mapping_v2.append(mapping)
|
||||
|
||||
nics = parsed_args.nics
|
||||
|
||||
if 'auto' in nics or 'none' in nics:
|
||||
|
||||
@ -2028,6 +2028,262 @@ class TestServerCreate(TestServer):
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist(), data)
|
||||
|
||||
def test_server_create_with_block_device(self):
|
||||
block_device = f'uuid={self.volume.id},source_type=volume,boot_index=1'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
verifylist = [
|
||||
('image', 'image1'),
|
||||
('flavor', self.flavor.id),
|
||||
('block_devices', [
|
||||
{
|
||||
'uuid': self.volume.id,
|
||||
'source_type': 'volume',
|
||||
'boot_index': '1',
|
||||
},
|
||||
]),
|
||||
('server_name', self.new_server.name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# CreateServer.take_action() returns two tuples
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
# Set expected values
|
||||
kwargs = {
|
||||
'meta': None,
|
||||
'files': {},
|
||||
'reservation_id': None,
|
||||
'min_count': 1,
|
||||
'max_count': 1,
|
||||
'security_groups': [],
|
||||
'userdata': None,
|
||||
'key_name': None,
|
||||
'availability_zone': None,
|
||||
'admin_pass': None,
|
||||
'block_device_mapping_v2': [{
|
||||
'uuid': self.volume.id,
|
||||
'source_type': 'volume',
|
||||
'destination_type': 'volume',
|
||||
'boot_index': 1,
|
||||
}],
|
||||
'nics': [],
|
||||
'scheduler_hints': {},
|
||||
'config_drive': None,
|
||||
}
|
||||
# ServerManager.create(name, image, flavor, **kwargs)
|
||||
self.servers_mock.create.assert_called_with(
|
||||
self.new_server.name,
|
||||
self.image,
|
||||
self.flavor,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist(), data)
|
||||
|
||||
def test_server_create_with_block_device_full(self):
|
||||
self.app.client_manager.compute.api_version = api_versions.APIVersion(
|
||||
'2.67')
|
||||
|
||||
block_device = (
|
||||
f'uuid={self.volume.id},source_type=volume,'
|
||||
f'destination_type=volume,disk_bus=ide,device_type=disk,'
|
||||
f'device_name=sdb,guest_format=ext4,volume_size=64,'
|
||||
f'volume_type=foo,boot_index=1,delete_on_termination=true,'
|
||||
f'tag=foo'
|
||||
)
|
||||
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
verifylist = [
|
||||
('image', 'image1'),
|
||||
('flavor', self.flavor.id),
|
||||
('block_devices', [
|
||||
{
|
||||
'uuid': self.volume.id,
|
||||
'source_type': 'volume',
|
||||
'destination_type': 'volume',
|
||||
'disk_bus': 'ide',
|
||||
'device_type': 'disk',
|
||||
'device_name': 'sdb',
|
||||
'guest_format': 'ext4',
|
||||
'volume_size': '64',
|
||||
'volume_type': 'foo',
|
||||
'boot_index': '1',
|
||||
'delete_on_termination': 'true',
|
||||
'tag': 'foo',
|
||||
},
|
||||
]),
|
||||
('server_name', self.new_server.name),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
# CreateServer.take_action() returns two tuples
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
# Set expected values
|
||||
kwargs = {
|
||||
'meta': None,
|
||||
'files': {},
|
||||
'reservation_id': None,
|
||||
'min_count': 1,
|
||||
'max_count': 1,
|
||||
'security_groups': [],
|
||||
'userdata': None,
|
||||
'key_name': None,
|
||||
'availability_zone': None,
|
||||
'admin_pass': None,
|
||||
'block_device_mapping_v2': [{
|
||||
'uuid': self.volume.id,
|
||||
'source_type': 'volume',
|
||||
'destination_type': 'volume',
|
||||
'disk_bus': 'ide',
|
||||
'device_name': 'sdb',
|
||||
'volume_size': '64',
|
||||
'guest_format': 'ext4',
|
||||
'boot_index': 1,
|
||||
'device_type': 'disk',
|
||||
'delete_on_termination': True,
|
||||
'tag': 'foo',
|
||||
'volume_type': 'foo',
|
||||
}],
|
||||
'nics': 'auto',
|
||||
'scheduler_hints': {},
|
||||
'config_drive': None,
|
||||
}
|
||||
# ServerManager.create(name, image, flavor, **kwargs)
|
||||
self.servers_mock.create.assert_called_with(
|
||||
self.new_server.name,
|
||||
self.image,
|
||||
self.flavor,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist(), data)
|
||||
|
||||
def test_server_create_with_block_device_no_boot_index(self):
|
||||
block_device = \
|
||||
f'uuid={self.volume.name},source_type=volume'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
self.assertRaises(
|
||||
argparse.ArgumentTypeError,
|
||||
self.check_parser,
|
||||
self.cmd, arglist, [])
|
||||
|
||||
def test_server_create_with_block_device_invalid_boot_index(self):
|
||||
block_device = \
|
||||
f'uuid={self.volume.name},source_type=volume,boot_index=foo'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, [])
|
||||
ex = self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
self.assertIn('The boot_index key of --block-device ', str(ex))
|
||||
|
||||
def test_server_create_with_block_device_invalid_source_type(self):
|
||||
block_device = f'uuid={self.volume.name},source_type=foo,boot_index=1'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, [])
|
||||
ex = self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
self.assertIn('The source_type key of --block-device ', str(ex))
|
||||
|
||||
def test_server_create_with_block_device_invalid_destination_type(self):
|
||||
block_device = \
|
||||
f'uuid={self.volume.name},destination_type=foo,boot_index=1'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, [])
|
||||
ex = self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
self.assertIn('The destination_type key of --block-device ', str(ex))
|
||||
|
||||
def test_server_create_with_block_device_invalid_shutdown(self):
|
||||
block_device = \
|
||||
f'uuid={self.volume.name},delete_on_termination=foo,boot_index=1'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, [])
|
||||
ex = self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
self.assertIn(
|
||||
'The delete_on_termination key of --block-device ', str(ex))
|
||||
|
||||
def test_server_create_with_block_device_tag_pre_v242(self):
|
||||
self.app.client_manager.compute.api_version = api_versions.APIVersion(
|
||||
'2.41')
|
||||
|
||||
block_device = \
|
||||
f'uuid={self.volume.name},tag=foo,boot_index=1'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, [])
|
||||
ex = self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
self.assertIn(
|
||||
'--os-compute-api-version 2.42 or greater is required',
|
||||
str(ex))
|
||||
|
||||
def test_server_create_with_block_device_volume_type_pre_v267(self):
|
||||
self.app.client_manager.compute.api_version = api_versions.APIVersion(
|
||||
'2.66')
|
||||
|
||||
block_device = f'uuid={self.volume.name},volume_type=foo,boot_index=1'
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', block_device,
|
||||
self.new_server.name,
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, [])
|
||||
ex = self.assertRaises(
|
||||
exceptions.CommandError,
|
||||
self.cmd.take_action, parsed_args)
|
||||
self.assertIn(
|
||||
'--os-compute-api-version 2.67 or greater is required',
|
||||
str(ex))
|
||||
|
||||
def test_server_create_with_block_device_mapping(self):
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
|
||||
@ -6,3 +6,12 @@ features:
|
||||
- ``--snapshot``
|
||||
- ``--ephemeral``
|
||||
- ``--swap``
|
||||
- ``--block-device``
|
||||
deprecations:
|
||||
- |
|
||||
The ``--block-device-mapping`` option of the ``server create`` command
|
||||
has been deprecated in favour of ``--block-device``. The format of the
|
||||
``--block-device-mapping`` option is based on the limited "BDM v1"
|
||||
format for block device maps introduced way back in the v1 nova API. The
|
||||
``--block-device`` option instead exposes the richer key-value based
|
||||
"BDM v2" format.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user