Merge "VMware: Remove references to ebs_root from spawn()"

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

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)