diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 9838ed5480..b2ae93c807 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -832,13 +832,12 @@ class CreateServer(command.ShowOne): action=parseractions.MultiKeyValueAction, dest='block_devices', default=[], - required_keys=[ - 'boot_index', - ], + required_keys=[], optional_keys=[ 'uuid', 'source_type', 'destination_type', - 'disk_bus', 'device_type', 'device_name', 'guest_format', - 'volume_size', 'volume_type', 'delete_on_termination', 'tag', + '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' @@ -1336,14 +1335,15 @@ 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 'boot_index' in mapping: + 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') @@ -1383,9 +1383,9 @@ class CreateServer(command.ShowOne): ) raise exceptions.CommandError(msg) else: - if mapping['source_type'] in ('image', 'blank'): + if mapping['source_type'] in ('blank',): mapping['destination_type'] = 'local' - else: # volume, snapshot + else: # volume, image, snapshot mapping['destination_type'] = 'volume' if 'delete_on_termination' in mapping: diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py index bad3f93d47..9cf2fc7f0f 100644 --- a/openstackclient/tests/functional/compute/v2/test_server.py +++ b/openstackclient/tests/functional/compute/v2/test_server.py @@ -574,7 +574,90 @@ class ServerTests(common.ComputeTestCase): cmd_output['status'], ) - def test_server_boot_with_bdm_snapshot(self): + def _test_server_boot_with_bdm_volume(self, use_legacy): + """Test server create from volume, server delete""" + # get volume status wait function + volume_wait_for = volume_common.BaseVolumeTests.wait_for_status + + # create source empty volume + volume_name = uuid.uuid4().hex + cmd_output = json.loads(self.openstack( + 'volume create -f json ' + + '--size 1 ' + + volume_name + )) + volume_id = cmd_output["id"] + self.assertIsNotNone(volume_id) + self.addCleanup(self.openstack, 'volume delete ' + volume_name) + self.assertEqual(volume_name, cmd_output['name']) + volume_wait_for("volume", volume_name, "available") + + if use_legacy: + bdm_arg = f'--block-device-mapping vdb={volume_name}' + else: + bdm_arg = ( + f'--block-device ' + f'device_name=vdb,source_type=volume,boot_index=1,' + f'uuid={volume_id}' + ) + + # create server + server_name = uuid.uuid4().hex + server = json.loads(self.openstack( + 'server create -f json ' + + '--flavor ' + self.flavor_name + ' ' + + '--image ' + self.image_name + ' ' + + bdm_arg + ' ' + + self.network_arg + ' ' + + '--wait ' + + server_name + )) + self.assertIsNotNone(server["id"]) + self.addCleanup(self.openstack, 'server delete --wait ' + server_name) + self.assertEqual( + server_name, + server['name'], + ) + + # check server volumes_attached, format is + # {"volumes_attached": "id='2518bc76-bf0b-476e-ad6b-571973745bb5'",} + cmd_output = json.loads(self.openstack( + 'server show -f json ' + + server_name + )) + volumes_attached = cmd_output['volumes_attached'] + self.assertIsNotNone(volumes_attached) + + # check volumes + cmd_output = json.loads(self.openstack( + 'volume show -f json ' + + volume_name + )) + attachments = cmd_output['attachments'] + self.assertEqual( + 1, + len(attachments), + ) + self.assertEqual( + server['id'], + attachments[0]['server_id'], + ) + self.assertEqual( + "in-use", + cmd_output['status'], + ) + + def test_server_boot_with_bdm_volume(self): + """Test server create from image with bdm volume, server delete""" + self._test_server_boot_with_bdm_volume(use_legacy=False) + + # TODO(stephenfin): Remove when we drop support for the + # '--block-device-mapping' option + def test_server_boot_with_bdm_volume_legacy(self): + """Test server create from image with bdm volume, server delete""" + self._test_server_boot_with_bdm_volume(use_legacy=True) + + def _test_server_boot_with_bdm_snapshot(self, use_legacy): """Test server create from image with bdm snapshot, server delete""" # get volume status wait function volume_wait_for = volume_common.BaseVolumeTests.wait_for_status @@ -588,12 +671,8 @@ class ServerTests(common.ComputeTestCase): empty_volume_name )) self.assertIsNotNone(cmd_output["id"]) - self.addCleanup(self.openstack, - 'volume delete ' + empty_volume_name) - self.assertEqual( - empty_volume_name, - cmd_output['name'], - ) + self.addCleanup(self.openstack, 'volume delete ' + empty_volume_name) + self.assertEqual(empty_volume_name, cmd_output['name']) volume_wait_for("volume", empty_volume_name, "available") # create snapshot of source empty volume @@ -603,7 +682,8 @@ class ServerTests(common.ComputeTestCase): '--volume ' + empty_volume_name + ' ' + empty_snapshot_name )) - self.assertIsNotNone(cmd_output["id"]) + empty_snapshot_id = cmd_output["id"] + self.assertIsNotNone(empty_snapshot_id) # Deleting volume snapshot take time, so we need to wait until the # snapshot goes. Entries registered by self.addCleanup will be called # in the reverse order, so we need to register wait_for_delete first. @@ -617,14 +697,26 @@ class ServerTests(common.ComputeTestCase): ) volume_wait_for("volume snapshot", empty_snapshot_name, "available") + if use_legacy: + bdm_arg = ( + f'--block-device-mapping ' + f'vdb={empty_snapshot_name}:snapshot:1:true' + ) + else: + bdm_arg = ( + f'--block-device ' + f'device_name=vdb,uuid={empty_snapshot_id},' + f'source_type=snapshot,volume_size=1,' + f'delete_on_termination=true,boot_index=1' + ) + # create server with bdm snapshot server_name = uuid.uuid4().hex server = json.loads(self.openstack( 'server create -f json ' + '--flavor ' + self.flavor_name + ' ' + '--image ' + self.image_name + ' ' + - '--block-device-mapping ' - 'vdb=' + empty_snapshot_name + ':snapshot:1:true ' + + bdm_arg + ' ' + self.network_arg + ' ' + '--wait ' + server_name @@ -681,7 +773,17 @@ class ServerTests(common.ComputeTestCase): # the attached volume had been deleted pass - def test_server_boot_with_bdm_image(self): + def test_server_boot_with_bdm_snapshot(self): + """Test server create from image with bdm snapshot, server delete""" + self._test_server_boot_with_bdm_snapshot(use_legacy=False) + + # TODO(stephenfin): Remove when we drop support for the + # '--block-device-mapping' option + def test_server_boot_with_bdm_snapshot_legacy(self): + """Test server create from image with bdm snapshot, server delete""" + self._test_server_boot_with_bdm_snapshot(use_legacy=True) + + def _test_server_boot_with_bdm_image(self, use_legacy): # Tests creating a server where the root disk is backed by the given # --image but a --block-device-mapping with type=image is provided so # that the compute service creates a volume from that image and @@ -689,6 +791,32 @@ class ServerTests(common.ComputeTestCase): # marked as delete_on_termination=True so it will be automatically # deleted when the server is deleted. + if use_legacy: + # This means create a 1GB volume from the specified image, attach + # it to the server at /dev/vdb and delete the volume when the + # server is deleted. + bdm_arg = ( + f'--block-device-mapping ' + f'vdb={self.image_name}:image:1:true ' + ) + else: + # get image ID + cmd_output = json.loads(self.openstack( + 'image show -f json ' + + self.image_name + )) + image_id = cmd_output['id'] + + # This means create a 1GB volume from the specified image, attach + # it to the server at /dev/vdb and delete the volume when the + # server is deleted. + bdm_arg = ( + f'--block-device ' + f'device_name=vdb,uuid={image_id},' + f'source_type=image,volume_size=1,' + f'delete_on_termination=true,boot_index=1' + ) + # create server with bdm type=image # NOTE(mriedem): This test is a bit unrealistic in that specifying the # same image in the block device as the --image option does not really @@ -700,11 +828,7 @@ class ServerTests(common.ComputeTestCase): 'server create -f json ' + '--flavor ' + self.flavor_name + ' ' + '--image ' + self.image_name + ' ' + - '--block-device-mapping ' - # This means create a 1GB volume from the specified image, attach - # it to the server at /dev/vdb and delete the volume when the - # server is deleted. - 'vdb=' + self.image_name + ':image:1:true ' + + bdm_arg + ' ' + self.network_arg + ' ' + '--wait ' + server_name @@ -768,6 +892,14 @@ class ServerTests(common.ComputeTestCase): # the attached volume had been deleted pass + def test_server_boot_with_bdm_image(self): + self._test_server_boot_with_bdm_image(use_legacy=False) + + # TODO(stephenfin): Remove when we drop support for the + # '--block-device-mapping' option + def test_server_boot_with_bdm_image_legacy(self): + self._test_server_boot_with_bdm_image(use_legacy=True) + def test_boot_from_volume(self): # Tests creating a server using --image and --boot-from-volume where # the compute service will create a root volume of the specified size diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index ced5d458c2..9666c5fdba 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -2029,7 +2029,7 @@ class TestServerCreate(TestServer): 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' + block_device = f'uuid={self.volume.id},source_type=volume' arglist = [ '--image', 'image1', '--flavor', self.flavor.id, @@ -2043,7 +2043,6 @@ class TestServerCreate(TestServer): { 'uuid': self.volume.id, 'source_type': 'volume', - 'boot_index': '1', }, ]), ('server_name', self.new_server.name), @@ -2069,7 +2068,6 @@ class TestServerCreate(TestServer): 'uuid': self.volume.id, 'source_type': 'volume', 'destination_type': 'volume', - 'boot_index': 1, }], 'nics': [], 'scheduler_hints': {}, @@ -2171,20 +2169,6 @@ class TestServerCreate(TestServer): 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' @@ -2201,7 +2185,7 @@ class TestServerCreate(TestServer): 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' + block_device = f'uuid={self.volume.name},source_type=foo' arglist = [ '--image', 'image1', '--flavor', self.flavor.id, @@ -2216,7 +2200,7 @@ class TestServerCreate(TestServer): def test_server_create_with_block_device_invalid_destination_type(self): block_device = \ - f'uuid={self.volume.name},destination_type=foo,boot_index=1' + f'uuid={self.volume.name},destination_type=foo' arglist = [ '--image', 'image1', '--flavor', self.flavor.id, @@ -2231,7 +2215,7 @@ class TestServerCreate(TestServer): def test_server_create_with_block_device_invalid_shutdown(self): block_device = \ - f'uuid={self.volume.name},delete_on_termination=foo,boot_index=1' + f'uuid={self.volume.name},delete_on_termination=foo' arglist = [ '--image', 'image1', '--flavor', self.flavor.id, @@ -2250,7 +2234,7 @@ class TestServerCreate(TestServer): '2.41') block_device = \ - f'uuid={self.volume.name},tag=foo,boot_index=1' + f'uuid={self.volume.name},tag=foo' arglist = [ '--image', 'image1', '--flavor', self.flavor.id, @@ -2269,7 +2253,7 @@ class TestServerCreate(TestServer): 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' + block_device = f'uuid={self.volume.name},volume_type=foo' arglist = [ '--image', 'image1', '--flavor', self.flavor.id,