From c5c26716bff1078f4108e9b7c01524f4e52b14c4 Mon Sep 17 00:00:00 2001 From: Artom Lifshitz Date: Fri, 8 Apr 2016 10:14:10 +0000 Subject: [PATCH] libvirt: attach configdrive after instance XML In order to expose device tagging metadata in the configdrive, the latter needs to be attached after the instance XML has been generated and device addresses are known, but before the guest OS runs, as it may depend on cloud-init data on the config drive. This patch moves configdrive creation after the instance XML has been generated but before the guest runs. Partially implements: blueprint virt-device-role-tagging Co-authored-by: Vladik Romanovsky Change-Id: I931421ea688641e2ceb212c6dc099639c53433f2 --- nova/tests/unit/virt/libvirt/test_driver.py | 95 ++++++------- nova/virt/libvirt/driver.py | 144 ++++++++++++-------- 2 files changed, 123 insertions(+), 116 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 07bf9d3577..225a82b59a 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 6af7639cf1..6e76d6155a 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2534,6 +2534,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, @@ -2542,7 +2547,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. @@ -2579,6 +2584,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, @@ -2589,9 +2599,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(): @@ -2952,54 +2963,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 @@ -3099,8 +3067,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 @@ -3346,9 +3357,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'], @@ -4689,7 +4704,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 @@ -4699,6 +4714,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) @@ -4730,7 +4747,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( @@ -4772,7 +4790,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) @@ -7139,6 +7158,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. @@ -7206,7 +7229,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,