Merge "compute: Add support for loading BDMs from files"

2 files changed, 153 insertions, 10 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index b2ae93c8..cd6e2c91 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -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 '
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 9666c5fd..775ad0d9 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -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'