Skip to content

Commit 95c8661

Browse files
nikitychstevemar
nikitych
authored and
stevemar
committed
Switch server create to block_device_mapping_v2
Current compute_client.servers.create() relies on block_device_mapping arg which is legacy[1]. "block_device_mapping" format require device_name which is leads to hard-coded hack in --volume key handler to KVM specific. "block_device_mapping_v2" format is more friendly to hypervisiors. Support of block_device_mapping_v2 appear in python-novaclient 2.16.0, openstackclient require at least 2.29.0 Makes options --volume and --block-device-mapping work simultaneously. Appends --block-device-mapping data even if --volume used. After bug 1383338 only --volume was taken when both are used. [1]http://docs.openstack.org/developer/nova/block_device_mapping.html NOTE(dtroyer): I moved the new test_boot_from_volume() functional test to Ie51b1c375c5940856ec76a5770df3c6bd18a3eba to test our previous behaviour. The only changes required to support the new behaviour should be that the empty_volume is now attached in that test. Change-Id: I7bac3d870dd9ca404093142f8bce22a62e49180d Closes-Bug: 1647406 Closes-Bug: 1497845
1 parent a05e276 commit 95c8661

4 files changed

Lines changed: 70 additions & 33 deletions

File tree

‎openstackclient/compute/v2/server.py‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -507,28 +507,40 @@ def take_action(self, parsed_args):
507507
"exception": e}
508508
)
509509

510-
block_device_mapping = {}
510+
block_device_mapping_v2 = []
511511
if volume:
512-
# When booting from volume, for now assume no other mappings
513-
# This device value is likely KVM-specific
514-
block_device_mapping = {'vda': volume}
515-
else:
516-
for dev_map in parsed_args.block_device_mapping:
517-
dev_key, dev_vol = dev_map.split('=', 1)
518-
block_volume = None
519-
if dev_vol:
520-
vol = dev_vol.split(':', 1)[0]
521-
if vol:
522-
vol_id = utils.find_resource(
512+
block_device_mapping_v2 = [{'uuid': volume,
513+
'boot_index': '0',
514+
'source_type': 'volume',
515+
'destination_type': 'volume'
516+
}]
517+
for dev_map in parsed_args.block_device_mapping:
518+
dev_name, dev_map = dev_map.split('=', 1)
519+
if dev_map:
520+
dev_map = dev_map.split(':')
521+
if len(dev_map) > 0:
522+
mapping = {
523+
'device_name': dev_name,
524+
'uuid': utils.find_resource(
523525
volume_client.volumes,
524-
vol,
525-
).id
526-
block_volume = dev_vol.replace(vol, vol_id)
526+
dev_map[0],
527+
).id}
528+
# Block device mapping v1 compatibility
529+
if len(dev_map) > 1 and \
530+
dev_map[1] in ('volume', 'snapshot'):
531+
mapping['source_type'] = dev_map[1]
527532
else:
528-
msg = _("Volume name or ID must be specified if "
529-
"--block-device-mapping is specified")
530-
raise exceptions.CommandError(msg)
531-
block_device_mapping.update({dev_key: block_volume})
533+
mapping['source_type'] = 'volume'
534+
mapping['destination_type'] = 'volume'
535+
if len(dev_map) > 2:
536+
mapping['volume_size'] = dev_map[2]
537+
if len(dev_map) > 3:
538+
mapping['delete_on_termination'] = dev_map[3]
539+
else:
540+
msg = _("Volume name or ID must be specified if "
541+
"--block-device-mapping is specified")
542+
raise exceptions.CommandError(msg)
543+
block_device_mapping_v2.append(mapping)
532544

533545
nics = []
534546
if parsed_args.nic in ('auto', 'none'):
@@ -598,7 +610,7 @@ def take_action(self, parsed_args):
598610
userdata=userdata,
599611
key_name=parsed_args.key_name,
600612
availability_zone=parsed_args.availability_zone,
601-
block_device_mapping=block_device_mapping,
613+
block_device_mapping_v2=block_device_mapping_v2,
602614
nics=nics,
603615
scheduler_hints=hints,
604616
config_drive=config_drive)

‎openstackclient/tests/functional/compute/v2/test_server.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,19 +417,23 @@ def test_server_boot_from_volume(self):
417417

418418
# NOTE(dtroyer): Prior to https://review.openstack.org/#/c/407111
419419
# --block-device-mapping was ignored if --volume
420-
# present on the command line, so this volume should
421-
# not be attached.
420+
# present on the command line. Now we should see the
421+
# attachment.
422422
cmd_output = json.loads(self.openstack(
423423
'volume show -f json ' +
424424
empty_volume_name
425425
))
426426
attachments = cmd_output['attachments']
427427
self.assertEqual(
428-
0,
428+
1,
429429
len(attachments),
430430
)
431431
self.assertEqual(
432-
"available",
432+
server['id'],
433+
attachments[0]['server_id'],
434+
)
435+
self.assertEqual(
436+
"in-use",
433437
cmd_output['status'],
434438
)
435439

‎openstackclient/tests/unit/compute/v2/test_server.py‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ def test_server_create_minimal(self):
308308
userdata=None,
309309
key_name=None,
310310
availability_zone=None,
311-
block_device_mapping={},
311+
block_device_mapping_v2=[],
312312
nics=[],
313313
scheduler_hints={},
314314
config_drive=None,
@@ -363,7 +363,7 @@ def test_server_create_with_options(self):
363363
userdata=None,
364364
key_name='keyname',
365365
availability_zone=None,
366-
block_device_mapping={},
366+
block_device_mapping_v2=[],
367367
nics=[],
368368
scheduler_hints={'a': ['b', 'c']},
369369
config_drive=None,
@@ -443,7 +443,7 @@ def test_server_create_with_network(self):
443443
userdata=None,
444444
key_name=None,
445445
availability_zone=None,
446-
block_device_mapping={},
446+
block_device_mapping_v2=[],
447447
nics=[{'net-id': 'net1_uuid',
448448
'v4-fixed-ip': '',
449449
'v6-fixed-ip': '',
@@ -500,7 +500,7 @@ def test_server_create_with_wait_ok(self, mock_wait_for_status):
500500
userdata=None,
501501
key_name=None,
502502
availability_zone=None,
503-
block_device_mapping={},
503+
block_device_mapping_v2=[],
504504
nics=[],
505505
scheduler_hints={},
506506
config_drive=None,
@@ -550,7 +550,7 @@ def test_server_create_with_wait_fails(self, mock_wait_for_status):
550550
userdata=None,
551551
key_name=None,
552552
availability_zone=None,
553-
block_device_mapping={},
553+
block_device_mapping_v2=[],
554554
nics=[],
555555
scheduler_hints={},
556556
config_drive=None,
@@ -605,7 +605,7 @@ def test_server_create_userdata(self, mock_open):
605605
userdata=mock_file,
606606
key_name=None,
607607
availability_zone=None,
608-
block_device_mapping={},
608+
block_device_mapping_v2=[],
609609
nics=[],
610610
scheduler_hints={},
611611
config_drive=None,
@@ -656,9 +656,14 @@ def test_server_create_with_block_device_mapping(self):
656656
userdata=None,
657657
key_name=None,
658658
availability_zone=None,
659-
block_device_mapping={
660-
'vda': real_volume_mapping
661-
},
659+
block_device_mapping_v2=[{
660+
'device_name': 'vda',
661+
'uuid': real_volume_mapping.split(':', 1)[0],
662+
'destination_type': 'volume',
663+
'source_type': 'volume',
664+
'delete_on_termination': '0',
665+
'volume_size': ''
666+
}],
662667
nics=[],
663668
scheduler_hints={},
664669
config_drive=None,

‎releasenotes/notes/bug-1647406-c936581034a1b6e4.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
fixes:
3+
- |
4+
Allow ``--block-device-mapping`` option to work correctly with
5+
``--volume`` option in ``server create`` command.
6+
After :lpbug:`1383338` ``--block-device-mapping`` was ignored if
7+
``--volume`` was present. Block device mappings are now appended
8+
to the mapping created by the ``--volume`` option if it is present.
9+
The device name of the boot volume specificed in the ``--volume`` option
10+
is no longer assumed to be *'vda'* but now uses the hypervisor's boot
11+
index to obtain the device name. This maintains the status quo for
12+
**QEMU/KVM** hypervisors but **XEN**, **parallels** and others
13+
*virt types* that have device naming is different from ``vd*``
14+
should now also work correctly.
15+
[:lpbug:`1497845`]
16+
[:lpbug:`1647406`]

0 commit comments

Comments
 (0)