diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 3db36f7262..b1a86a23df 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -801,6 +801,7 @@ class CreateServer(command.ShowOne): 'options.' ) ) + # TODO(stephenfin): Remove this in the v7.0 parser.add_argument( '--block-device-mapping', metavar='', @@ -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' '=:::\n' ': block device name, like: vdb, xvdc ' @@ -822,6 +823,49 @@ class CreateServer(command.ShowOne): '(optional)\n' ': 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 of the volume, snapshot or ID ' + '(required if using source image, snapshot or volume),\n' + 'source_type=: source type ' + '(one of: image, snapshot, volume, blank),\n' + 'destination_typ=: destination type ' + '(one of: volume, local) (optional),\n' + 'disk_bus=: device bus ' + '(one of: uml, lxc, virtio, ...) (optional),\n' + 'device_type=: device type ' + '(one of: disk, cdrom, etc. (optional),\n' + 'device_name=: name of the device (optional),\n' + 'volume_size=: size of the block device in MiB ' + '(for swap) or GiB (for everything else) (optional),\n' + 'guest_format=: format of device (optional),\n' + 'boot_index=: index of disk used to order boot ' + 'disk ' + '(required for volume-backed instances),\n' + 'delete_on_termination=: whether to delete the ' + 'volume upon deletion of server (optional),\n' + 'tag=: device metadata tag (optional),\n' + '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: diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index ce93f21ea3..0548924d27 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -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', diff --git a/releasenotes/notes/add-missing-server-create-opts-d5e32bd743e9e132.yaml b/releasenotes/notes/add-missing-server-create-opts-d5e32bd743e9e132.yaml index b82a9d30d1..951d944dd4 100644 --- a/releasenotes/notes/add-missing-server-create-opts-d5e32bd743e9e132.yaml +++ b/releasenotes/notes/add-missing-server-create-opts-d5e32bd743e9e132.yaml @@ -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.