diff --git a/nova/tests/virt/vmwareapi/test_vmops.py b/nova/tests/virt/vmwareapi/test_vmops.py index 4f92ccacfa..034bef43a8 100644 --- a/nova/tests/virt/vmwareapi/test_vmops.py +++ b/nova/tests/virt/vmwareapi/test_vmops.py @@ -520,30 +520,45 @@ class VMwareVMOpsTestCase(test.NoDBTestCase): def test_finish_revert_migration_power_off(self): self._test_finish_revert_migration(power_on=False) - def test_spawn_mask_block_device_info_password(self): + @mock.patch.object(vmops.LOG, 'debug') + @mock.patch('nova.virt.vmwareapi.volumeops.VMwareVolumeOps' + '.attach_root_volume') + def test_spawn_mask_block_device_info_password(self, + mock_attach_root_volume, + mock_debug): # Very simple test that just ensures block_device_info auth_password # is masked when logged; the rest of the test just fails out early. data = {'auth_password': 'scrubme'} bdm = [{'connection_info': {'data': data}}] bdi = {'block_device_mapping': bdm} + self.password_logged = False + # Tests that the parameters to the to_xml method are sanitized for # passwords when logged. def fake_debug(*args, **kwargs): if 'auth_password' in args[0]: + self.password_logged = True self.assertNotIn('scrubme', args[0]) - with mock.patch.object(vmops.LOG, 'debug', - side_effect=fake_debug) as debug_mock: - # the invalid disk format will cause an exception - image_meta = {'disk_format': 'fake'} - self.assertRaises(exception.InvalidDiskFormat, self._vmops.spawn, - self._context, self._instance, image_meta, - injected_files=None, admin_password=None, - network_info=[], block_device_info=bdi) - # we don't care what the log message is, we just want to make sure - # our stub method is called which asserts the password is scrubbed - self.assertTrue(debug_mock.called) + mock_debug.side_effect = fake_debug + self.flags(flat_injected=False, vnc_enabled=False) + mock_attach_root_volume.side_effect = Exception + + # Call spawn(). We don't care what it does as long as it generates + # the log message, which we check below. + try: + self._vmops.spawn( + self._context, self._instance, {}, + injected_files=None, admin_password=None, + network_info=[], block_device_info=bdi + ) + except Exception: + pass + + # Check that the relevant log message was generated, and therefore + # that we checked it was scrubbed + self.assertTrue(self.password_logged) def test_get_ds_browser(self): cache = self._vmops._datastore_browser_mapping diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py index 1c1c468109..642d035945 100644 --- a/nova/virt/vmwareapi/vmops.py +++ b/nova/virt/vmwareapi/vmops.py @@ -186,16 +186,11 @@ class VMwareVMOps(object): #. Power on the VM. """ - ebs_root = False - if block_device_info: - msg = "Block device information present: %s" % block_device_info - # NOTE(mriedem): block_device_info can contain an auth_password - # so we have to scrub the message before logging it. - LOG.debug(logging.mask_password(msg), instance=instance) - block_device_mapping = driver.block_device_info_get_mapping( - block_device_info) - if block_device_mapping: - ebs_root = True + + # NOTE(hartsocks): some of the logic below relies on instance_name + # even when it is not set by the caller. + if instance_name is None: + instance_name = instance.uuid client_factory = self._session._get_vim().client.factory datastore = ds_util.get_datastore( @@ -247,9 +242,34 @@ class VMwareVMOps(object): vnc_port = vm_util.get_vnc_port(self._session) self._set_vnc_config(client_factory, instance, vnc_port) - if not ebs_root: - # this logic allows for instances or images to decide - # for themselves which strategy is best for them. + block_device_mapping = [] + if block_device_info is not None: + block_device_mapping = driver.block_device_info_get_mapping( + block_device_info) + + # NOTE(mdbooth): the logic here is that we ignore the image if there + # are block device mappings. This behaviour is incorrect, and a bug in + # the driver. We should be able to accept an image and block device + # mappings. + if len(block_device_mapping) > 0: + msg = "Block device information present: %s" % block_device_info + # NOTE(mriedem): block_device_info can contain an auth_password + # so we have to scrub the message before logging it. + LOG.debug(logging.mask_password(msg), instance=instance) + + for root_disk in block_device_mapping: + connection_info = root_disk['connection_info'] + # TODO(hartsocks): instance is unnecessary, remove it + # we still use instance in many locations for no other purpose + # than logging, can we simplify this? + self._volumeops.attach_root_volume(connection_info, instance, + self._default_root_device, + datastore.ref) + else: + # TODO(hartsocks): Refactor this section image handling section. + # The next section handles manipulating various image types + # as well as preparing those image's virtual disks for mounting + # in our virtual machine. upload_name = instance.image_ref upload_folder = '%s/%s' % (self._base_folder, upload_name) @@ -514,13 +534,6 @@ class VMwareVMOps(object): datastore.ref, str(uploaded_iso_path)) - else: - # Attach the root disk to the VM. - for root_disk in block_device_mapping: - connection_info = root_disk['connection_info'] - self._volumeops.attach_root_volume(connection_info, instance, - self._default_root_device, - datastore.ref) if power_on: vm_util.power_on_instance(self._session, instance, vm_ref=vm_ref)