compute: Add support for loading BDMs from files
The syntax of the '--block-device' parameter is complex and easily screwed up. Allow users to load a block device config from a file. For example: $ openstack server create ... --block-device file:///tmp/bdm.json ... This should alleviate the pain that is BDMv2 somewhat. No functional tests are provided since we already have tests for the CSV style of passing parameters and the unit tests show that the net result is the same. Change-Id: I3e3299bbdbbb343863b4c14fb4d9196ff3e1698d Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane
parent
7c1d6f769c
commit
d3bd0146ae
@ -18,8 +18,10 @@
|
||||
import argparse
|
||||
import getpass
|
||||
import io
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import urllib.parse
|
||||
|
||||
from cliff import columns as cliff_columns
|
||||
import iso8601
|
||||
@ -681,7 +683,7 @@ class NICAction(argparse.Action):
|
||||
class BDMLegacyAction(argparse.Action):
|
||||
|
||||
def __call__(self, parser, namespace, values, option_string=None):
|
||||
# Make sure we have an empty dict rather than None
|
||||
# Make sure we have an empty list rather than None
|
||||
if getattr(namespace, self.dest, None) is None:
|
||||
setattr(namespace, self.dest, [])
|
||||
|
||||
@ -723,6 +725,68 @@ class BDMLegacyAction(argparse.Action):
|
||||
getattr(namespace, self.dest).append(mapping)
|
||||
|
||||
|
||||
class BDMAction(parseractions.MultiKeyValueAction):
|
||||
|
||||
def __init__(self, option_strings, dest, **kwargs):
|
||||
required_keys = []
|
||||
optional_keys = [
|
||||
'uuid', 'source_type', 'destination_type',
|
||||
'disk_bus', 'device_type', 'device_name', 'volume_size',
|
||||
'guest_format', 'boot_index', 'delete_on_termination', 'tag',
|
||||
'volume_type',
|
||||
]
|
||||
super().__init__(
|
||||
option_strings, dest, required_keys=required_keys,
|
||||
optional_keys=optional_keys, **kwargs,
|
||||
)
|
||||
|
||||
# TODO(stephenfin): Remove once I549d0897ef3704b7f47000f867d6731ad15d3f2b
|
||||
# or similar lands in a release
|
||||
def validate_keys(self, keys):
|
||||
"""Validate the provided keys.
|
||||
|
||||
:param keys: A list of keys to validate.
|
||||
"""
|
||||
valid_keys = self.required_keys | self.optional_keys
|
||||
invalid_keys = [k for k in keys if k not in valid_keys]
|
||||
if invalid_keys:
|
||||
msg = _(
|
||||
"Invalid keys %(invalid_keys)s specified.\n"
|
||||
"Valid keys are: %(valid_keys)s"
|
||||
)
|
||||
raise argparse.ArgumentTypeError(msg % {
|
||||
'invalid_keys': ', '.join(invalid_keys),
|
||||
'valid_keys': ', '.join(valid_keys),
|
||||
})
|
||||
|
||||
missing_keys = [k for k in self.required_keys if k not in keys]
|
||||
if missing_keys:
|
||||
msg = _(
|
||||
"Missing required keys %(missing_keys)s.\n"
|
||||
"Required keys are: %(required_keys)s"
|
||||
)
|
||||
raise argparse.ArgumentTypeError(msg % {
|
||||
'missing_keys': ', '.join(missing_keys),
|
||||
'required_keys': ', '.join(self.required_keys),
|
||||
})
|
||||
|
||||
def __call__(self, parser, namespace, values, option_string=None):
|
||||
if getattr(namespace, self.dest, None) is None:
|
||||
setattr(namespace, self.dest, [])
|
||||
|
||||
if values.startswith('file://'):
|
||||
path = urllib.parse.urlparse(values).path
|
||||
with open(path) as fh:
|
||||
data = json.load(fh)
|
||||
|
||||
# Validate the keys - other validation is left to later
|
||||
self.validate_keys(list(data))
|
||||
|
||||
getattr(namespace, self.dest, []).append(data)
|
||||
else:
|
||||
super().__call__(parser, namespace, values, option_string)
|
||||
|
||||
|
||||
class CreateServer(command.ShowOne):
|
||||
_description = _("Create a new server")
|
||||
|
||||
@ -829,19 +893,15 @@ class CreateServer(command.ShowOne):
|
||||
parser.add_argument(
|
||||
'--block-device',
|
||||
metavar='',
|
||||
action=parseractions.MultiKeyValueAction,
|
||||
action=BDMAction,
|
||||
dest='block_devices',
|
||||
default=[],
|
||||
required_keys=[],
|
||||
optional_keys=[
|
||||
'uuid', 'source_type', 'destination_type',
|
||||
'disk_bus', 'device_type', 'device_name', 'volume_size',
|
||||
'guest_format', 'boot_index', 'delete_on_termination', 'tag',
|
||||
'volume_type',
|
||||
],
|
||||
help=_(
|
||||
'Create a block device on the server.\n'
|
||||
'Block device in the format:\n'
|
||||
'Either a URI-style path (\'file:\\\\{path}\') to a JSON file '
|
||||
'or a CSV-serialized string describing the block device '
|
||||
'mapping.\n'
|
||||
'The following keys are accepted:\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 '
|
||||
|
||||
@ -16,6 +16,8 @@ import argparse
|
||||
import collections
|
||||
import copy
|
||||
import getpass
|
||||
import json
|
||||
import tempfile
|
||||
from unittest import mock
|
||||
from unittest.mock import call
|
||||
|
||||
@ -2169,6 +2171,87 @@ class TestServerCreate(TestServer):
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist(), data)
|
||||
|
||||
def test_server_create_with_block_device_from_file(self):
|
||||
self.app.client_manager.compute.api_version = api_versions.APIVersion(
|
||||
'2.67')
|
||||
|
||||
block_device = {
|
||||
'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',
|
||||
}
|
||||
|
||||
with tempfile.NamedTemporaryFile(mode='w+') as fp:
|
||||
json.dump(block_device, fp=fp)
|
||||
fp.flush()
|
||||
|
||||
arglist = [
|
||||
'--image', 'image1',
|
||||
'--flavor', self.flavor.id,
|
||||
'--block-device', f'file://{fp.name}',
|
||||
self.new_server.name,
|
||||
]
|
||||
verifylist = [
|
||||
('image', 'image1'),
|
||||
('flavor', self.flavor.id),
|
||||
('block_devices', [block_device]),
|
||||
('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_invalid_boot_index(self):
|
||||
block_device = \
|
||||
f'uuid={self.volume.name},source_type=volume,boot_index=foo'
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user