diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 07bf9d357715..225a82b59a8a 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -from collections import defaultdict from collections import deque import contextlib import copy @@ -9811,7 +9810,8 @@ class LibvirtConnTestCase(test.NoDBTestCase): ] self.assertEqual(wantFiles, gotFiles) - def _create_image_helper(self, callback, suffix=''): + def _create_image_helper(self, callback, suffix='', + test_create_configdrive=False): gotFiles = [] imported_files = [] @@ -9873,8 +9873,11 @@ class LibvirtConnTestCase(test.NoDBTestCase): disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, instance, image_meta) - drvr._create_image(context, instance, disk_info['mapping'], - suffix=suffix) + if test_create_configdrive: + drvr._create_configdrive(context, instance) + else: + drvr._create_image(context, instance, disk_info['mapping'], + suffix=suffix) drvr._get_guest_xml(self.context, instance, None, disk_info, image_meta) @@ -9896,61 +9899,42 @@ class LibvirtConnTestCase(test.NoDBTestCase): ] self.assertEqual(gotFiles, wantFiles) - def test_create_image_with_configdrive(self): + def test_create_configdrive(self): def enable_configdrive(instance_ref): instance_ref['config_drive'] = 'true' # Ensure that we create a config drive and then import it into the # image backend store - _, imported_files = self._create_image_helper(enable_configdrive) + _, imported_files = self._create_image_helper( + enable_configdrive, test_create_configdrive=True) self.assertTrue(imported_files[0][0].endswith('/disk.config')) self.assertEqual('disk.config', imported_files[0][1]) - def test_create_image_with_configdrive_rescue(self): - def enable_configdrive(instance_ref): - instance_ref['config_drive'] = 'true' - - # Ensure that we create a config drive and then import it into the - # image backend store - _, imported_files = self._create_image_helper(enable_configdrive, - suffix='.rescue') - self.assertTrue(imported_files[0][0].endswith('/disk.config.rescue')) - self.assertEqual('disk.config.rescue', imported_files[0][1]) - - @mock.patch.object(nova.virt.configdrive.ConfigDriveBuilder, 'make_drive') - @mock.patch.object(instance_metadata, 'InstanceMetadata') - def test_create_image_with_configdrive_and_config_drive_exists( - self, mock_instance_metadata, mock_make_drive): + @mock.patch('nova.virt.libvirt.LibvirtDriver._create_configdrive') + @mock.patch('nova.virt.libvirt.LibvirtDriver.get_info') + @mock.patch('nova.virt.libvirt.LibvirtDriver._create_image') + @mock.patch('nova.virt.libvirt.LibvirtDriver._get_guest_xml') + @mock.patch('nova.virt.libvirt.LibvirtDriver._get_instance_disk_info') + @mock.patch('nova.virt.libvirt.blockinfo.get_disk_info') + @mock.patch('nova.virt.libvirt.guest.Guest') + @mock.patch.object(loopingcall, 'FixedIntervalLoopingCall') + def test_spawn_with_config_drive(self, mock_timer, mock_guest, mock_disk, + mock_idisk, mock_guest_xml, + mock_create_image, mock_info, + mock_create): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - - def mock_disk_creator(): - exists = mock.Mock(return_value=True) - return mock.Mock(exists=exists) - - # The disks dictionary contains Mocks whose exists() method returns - # True - disks = defaultdict(mock_disk_creator) - - def fake_image(instance, disk_name, image_type=None): - return disks[disk_name] - - # image_backend returns mocks from the disks dict, keyed by disk name - drvr.image_backend.image = mock.Mock(side_effect=fake_image) - instance = objects.Instance(**self.test_instance) - instance.task_state = task_states.RESIZE_FINISH - instance.config_drive = True image_meta = objects.ImageMeta.from_dict(self.test_image_meta) - disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, - instance, - image_meta) - drvr._create_image(self.context, instance, disk_info['mapping']) - mock_make_drive.assert_not_called() - - disks['disk.config'].exists.return_value = False - drvr._create_image(self.context, instance, disk_info['mapping']) - self.assertTrue(mock_make_drive.called) + with test.nested( + mock.patch.object(drvr, '_create_images_and_backing'), + mock.patch.object(drvr, 'plug_vifs'), + mock.patch.object(drvr.firewall_driver, 'setup_basic_filtering'), + mock.patch.object(drvr.firewall_driver, 'prepare_instance_filter'), + mock.patch.object(drvr.firewall_driver, 'apply_instance_filter')): + drvr.spawn(self.context, instance, + image_meta, [], None) + self.assertTrue(mock_create.called) @mock.patch.object(nova.virt.libvirt.imagebackend.Image, 'cache', side_effect=exception.ImageNotFound(image_id='fake-id')) @@ -13596,7 +13580,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): network_info) pause = self._get_pause_flag(drvr, network_info) create_domain.assert_called_once_with( - fake_xml, pause=pause, power_on=True) + fake_xml, pause=pause, power_on=True, post_xml_callback=None) self.assertEqual(mock_dom, guest._domain) def test_get_guest_storage_config(self): @@ -14942,7 +14926,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): def fake_create_domain_and_network( context, xml, instance, network_info, disk_info, block_device_info=None, power_on=True, reboot=False, - vifs_already_plugged=False): + vifs_already_plugged=False, post_xml_callback=None): self.fake_create_domain_called = True self.assertEqual(powered_on, power_on) self.assertTrue(vifs_already_plugged) @@ -15649,7 +15633,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase): ).AndReturn(dummyxml) self.drvr._destroy(instance) - self.drvr._create_domain(mox.IgnoreArg()) + self.drvr._create_domain(mox.IgnoreArg(), + post_xml_callback=mox.IgnoreArg()) self.mox.ReplayAll() @@ -15711,7 +15696,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase): @mock.patch( 'nova.virt.configdrive.ConfigDriveBuilder.add_instance_metadata') @mock.patch('nova.virt.configdrive.ConfigDriveBuilder.make_drive') - def test_rescue_config_drive(self, mock_make, mock_add): + @mock.patch('nova.virt.libvirt.guest.Guest') + def test_rescue_config_drive(self, mock_guest, mock_make, mock_add): instance = self._create_instance() uuid = instance.uuid configdrive_path = uuid + '/disk.config.rescue' @@ -15735,8 +15721,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase): '__init__') self.mox.StubOutWithMock(self.drvr, '_get_guest_xml') self.mox.StubOutWithMock(self.drvr, '_destroy') - self.mox.StubOutWithMock(self.drvr, '_create_domain') - self.drvr._get_existing_domain_xml(mox.IgnoreArg(), mox.IgnoreArg()).MultipleTimes().AndReturn(dummyxml) libvirt_utils.write_to_file(mox.IgnoreArg(), mox.IgnoreArg()) @@ -15746,10 +15730,10 @@ class LibvirtDriverTestCase(test.NoDBTestCase): ).AndReturn(mock_backend.kernel) imagebackend.Backend.image(instance, 'ramdisk.rescue', 'raw' ).AndReturn(mock_backend.ramdisk) - imagebackend.Backend.image(instance, 'disk.config.rescue', 'raw' - ).AndReturn(mock_backend.config) imagebackend.Backend.image(instance, 'disk.rescue', 'default' ).AndReturn(mock_backend.root) + imagebackend.Backend.image(instance, 'disk.config.rescue', 'raw' + ).AndReturn(mock_backend.config) instance_metadata.InstanceMetadata.__init__(mox.IgnoreArg(), content=mox.IgnoreArg(), @@ -15764,7 +15748,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase): write_to_disk=mox.IgnoreArg() ).AndReturn(dummyxml) self.drvr._destroy(instance) - self.drvr._create_domain(mox.IgnoreArg()) self.mox.ReplayAll() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 43d1bc8dd648..160ce1736a2e 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2536,6 +2536,11 @@ class LibvirtDriver(driver.ComputeDriver): instance, image_meta, rescue=True) + gen_confdrive = functools.partial(self._create_configdrive, + context, instance, + admin_pass=rescue_password, + network_info=network_info, + suffix='.rescue') self._create_image(context, instance, disk_info['mapping'], suffix='.rescue', disk_images=rescue_images, network_info=network_info, @@ -2544,7 +2549,7 @@ class LibvirtDriver(driver.ComputeDriver): image_meta, rescue=rescue_images, write_to_disk=True) self._destroy(instance) - self._create_domain(xml) + self._create_domain(xml, post_xml_callback=gen_confdrive) def unrescue(self, instance, network_info): """Reboot the VM which is being rescued back into primary images. @@ -2581,6 +2586,11 @@ class LibvirtDriver(driver.ComputeDriver): instance, image_meta, block_device_info) + gen_confdrive = functools.partial(self._create_configdrive, + context, instance, + admin_pass=admin_password, + files=injected_files, + network_info=network_info) self._create_image(context, instance, disk_info['mapping'], network_info=network_info, @@ -2591,9 +2601,10 @@ class LibvirtDriver(driver.ComputeDriver): disk_info, image_meta, block_device_info=block_device_info, write_to_disk=True) - self._create_domain_and_network(context, xml, instance, network_info, - disk_info, - block_device_info=block_device_info) + self._create_domain_and_network( + context, xml, instance, network_info, disk_info, + block_device_info=block_device_info, + post_xml_callback=gen_confdrive) LOG.debug("Instance is running", instance=instance) def _wait_for_boot(): @@ -2954,54 +2965,11 @@ class LibvirtDriver(driver.ComputeDriver): image_id=disk_images['ramdisk_id']) inst_type = instance.get_flavor() + if CONF.libvirt.virt_type == 'uml': + libvirt_utils.chown(image('disk').path, 'root') - # Config drive - config_drive_image = None - if configdrive.required_by(instance): - LOG.info(_LI('Using config drive'), instance=instance) - - config_drive_image = self.image_backend.image( - instance, 'disk.config' + suffix, - self._get_disk_config_image_type()) - - # Don't overwrite an existing config drive - if not config_drive_image.exists(): - extra_md = {} - if admin_pass: - extra_md['admin_pass'] = admin_pass - - inst_md = instance_metadata.InstanceMetadata( - instance, content=files, extra_md=extra_md, - network_info=network_info) - - cdb = configdrive.ConfigDriveBuilder(instance_md=inst_md) - with cdb: - config_drive_local_path = self._get_disk_config_path( - instance, suffix) - LOG.info(_LI('Creating config drive at %(path)s'), - {'path': config_drive_local_path}, - instance=instance) - - try: - cdb.make_drive(config_drive_local_path) - except processutils.ProcessExecutionError as e: - with excutils.save_and_reraise_exception(): - LOG.error(_LE('Creating config drive failed ' - 'with error: %s'), - e, instance=instance) - - try: - config_drive_image.import_file( - instance, config_drive_local_path, - 'disk.config' + suffix) - finally: - # NOTE(mikal): if the config drive was imported into RBD, - # then we no longer need the local copy - if CONF.libvirt.images_type == 'rbd': - os.unlink(config_drive_local_path) - - need_inject = (config_drive_image is None and inject_files and - CONF.libvirt.inject_partition != -2) + # File injection only if needed + need_inject = inject_files and CONF.libvirt.inject_partition != -2 # NOTE(ndipanov): Even if disk_mapping was passed in, which # currently happens only on rescue - we still don't want to @@ -3101,8 +3069,51 @@ class LibvirtDriver(driver.ComputeDriver): size=size, swap_mb=swap_mb) - if CONF.libvirt.virt_type == 'uml': - libvirt_utils.chown(image('disk').path, 'root') + def _create_configdrive(self, context, instance, admin_pass=None, + files=None, network_info=None, suffix=''): + config_drive_image = None + if configdrive.required_by(instance): + LOG.info(_LI('Using config drive'), instance=instance) + + config_drive_image = self.image_backend.image( + instance, 'disk.config' + suffix, + self._get_disk_config_image_type()) + + # Don't overwrite an existing config drive + if not config_drive_image.exists(): + extra_md = {} + if admin_pass: + extra_md['admin_pass'] = admin_pass + + inst_md = instance_metadata.InstanceMetadata( + instance, content=files, extra_md=extra_md, + network_info=network_info) + + cdb = configdrive.ConfigDriveBuilder(instance_md=inst_md) + with cdb: + config_drive_local_path = self._get_disk_config_path( + instance, suffix) + LOG.info(_LI('Creating config drive at %(path)s'), + {'path': config_drive_local_path}, + instance=instance) + + try: + cdb.make_drive(config_drive_local_path) + except processutils.ProcessExecutionError as e: + with excutils.save_and_reraise_exception(): + LOG.error(_LE('Creating config drive failed ' + 'with error: %s'), + e, instance=instance) + + try: + config_drive_image.import_file( + instance, config_drive_local_path, + 'disk.config' + suffix) + finally: + # NOTE(mikal): if the config drive was imported into RBD, + # then we no longer need the local copy + if CONF.libvirt.images_type == 'rbd': + os.unlink(config_drive_local_path) def _prepare_pci_devices_for_use(self, pci_devices): # kvm , qemu support managed mode @@ -3348,9 +3359,13 @@ class LibvirtDriver(driver.ComputeDriver): # to rbd yet. Try to fall back on 'flat' image type. # TODO(melwitt): Add online migration of some sort so we can # remove this fall back once we know all config drives are in rbd. - image = self.image_backend.image(instance, name, 'flat') - LOG.debug('Config drive not found in RBD, falling back to the ' - 'instance directory', instance=instance) + # NOTE(vladikr): make sure that the flat image exist, otherwise + # the image will be created after the domain definition. + flat_image = self.image_backend.image(instance, name, 'flat') + if flat_image.exists(): + image = flat_image + LOG.debug('Config drive not found in RBD, falling back to the ' + 'instance directory', instance=instance) disk_info = disk_mapping[name] return image.libvirt_info(disk_info['bus'], disk_info['dev'], @@ -4691,7 +4706,7 @@ class LibvirtDriver(driver.ComputeDriver): # TODO(sahid): Consider renaming this to _create_guest. def _create_domain(self, xml=None, domain=None, - power_on=True, pause=False): + power_on=True, pause=False, post_xml_callback=None): """Create a domain. Either domain or xml must be passed in. If both are passed, then @@ -4701,6 +4716,8 @@ class LibvirtDriver(driver.ComputeDriver): """ if xml: guest = libvirt_guest.Guest.create(xml, self._host) + if post_xml_callback is not None: + post_xml_callback() else: guest = libvirt_guest.Guest(domain) @@ -4732,7 +4749,8 @@ class LibvirtDriver(driver.ComputeDriver): def _create_domain_and_network(self, context, xml, instance, network_info, disk_info, block_device_info=None, power_on=True, reboot=False, - vifs_already_plugged=False): + vifs_already_plugged=False, + post_xml_callback=None): """Do required network setup and create domain.""" block_device_mapping = driver.block_device_info_get_mapping( @@ -4774,7 +4792,8 @@ class LibvirtDriver(driver.ComputeDriver): with self._lxc_disk_handler(instance, instance.image_meta, block_device_info, disk_info): guest = self._create_domain( - xml, pause=pause, power_on=power_on) + xml, pause=pause, power_on=power_on, + post_xml_callback=post_xml_callback) self.firewall_driver.apply_instance_filter(instance, network_info) @@ -7141,6 +7160,10 @@ class LibvirtDriver(driver.ComputeDriver): block_device_info=None, inject_files=False, fallback_from_host=migration.source_compute) + gen_confdrive = functools.partial(self._create_configdrive, + context, instance, + network_info=network_info) + # Resize root disk and a single ephemeral disk called disk.local # Also convert raw disks to qcow2 if migrating to host which uses # qcow2 from host which uses raw. @@ -7208,7 +7231,8 @@ class LibvirtDriver(driver.ComputeDriver): block_disk_info, block_device_info=block_device_info, power_on=power_on, - vifs_already_plugged=True) + vifs_already_plugged=True, + post_xml_callback=gen_confdrive) if power_on: timer = loopingcall.FixedIntervalLoopingCall( self._wait_for_running,