libvirt: Re-initialise volumes, encryptors, and vifs on hard reboot
We call _hard_reboot during reboot, power_on, and
resume_state_on_host_boot. It functions essentially by tearing as much
of an instance as possible before recreating it, which additionally
makes it useful to operators for attempting automated recovery of
instances in an inconsistent state.
The Libvirt driver would previously only call _destroy and
_undefine_domain when hard rebooting an instance. This would leave vifs
plugged, volumes connected, and encryptors attached on the host. It
also means that when we try to restart the instance, we assume all
these things are correctly configured. If they are not, the instance
may fail to start at all, or may be incorrectly configured when
starting.
For example, consider an instance with an encrypted volume after a
compute host reboot. When we attempt to start the instance, power_on
will call _hard_reboot. The volume will be coincidentally re-attached
as a side-effect of calling _get_guest_xml(!), but when we call
_create_domain_and_network we pass reboot=True, which tells it not to
reattach the encryptor, as it is assumed to be already attached. We
are therefore left presenting the encrypted volume data directly to
the instance without decryption.
The approach in this patch is to ensure we recreate the instance as
fully as possible during hard reboot. This means not passing
vifs_already_plugged and reboot to _create_domain_and_network, which
in turn requires that we fully destroy the instance first. This
addresses the specific problem given in the example, but also a whole
class of potential volume and vif related issues of inconsistent
state.
Because we now always tear down volumes, encryptors, and vifs, we are
relying on the tear down of these things to be idempotent. This
highlighted that detach of the luks and cryptsetup encryptors were not
idempotent. We depend on the fixes for those os-brick drivers.
NOTE(melwitt): Instead of depending on the os-brick changes to handle
the "already detached" scenario during cleanup for the stable
backports, we handle it in the driver since we can't bump g-r for
stable branches.
Closes-bug: #1724573
Change-Id: Id188d48609f3d22d14e16c7f6114291d547a8986
(cherry picked from commit 3f8daf0804)
This commit is contained in:
Lee Yarwood
parent
b535f08085
commit
7da74a094f
|
|
@ -12050,16 +12050,15 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
|||
mock_hard_reboot.assert_called_once_with(self.context,
|
||||
instance, [], None)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver._undefine_domain')
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver.get_info')
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver._create_domain_and_network')
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver._get_guest_xml')
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver.'
|
||||
'_get_instance_disk_info_from_config')
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver._destroy')
|
||||
@mock.patch('nova.virt.libvirt.LibvirtDriver.destroy')
|
||||
def test_hard_reboot(self, mock_destroy, mock_get_disk_info,
|
||||
mock_get_guest_xml, mock_create_domain_and_network,
|
||||
mock_get_info, mock_undefine):
|
||||
mock_get_info):
|
||||
self.context.auth_token = True # any non-None value will suffice
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
network_info = _fake_network_info(self, 1)
|
||||
|
|
@ -12107,13 +12106,13 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
|||
for name in ('disk', 'disk.local'):
|
||||
self.assertTrue(disks[name].cache.called)
|
||||
|
||||
mock_destroy.assert_called_once_with(instance)
|
||||
mock_undefine.assert_called_once_with(instance)
|
||||
mock_destroy.assert_called_once_with(self.context, instance,
|
||||
network_info, destroy_disks=False,
|
||||
block_device_info=block_device_info)
|
||||
|
||||
mock_create_domain_and_network.assert_called_once_with(self.context,
|
||||
dummyxml, instance, network_info,
|
||||
block_device_info=block_device_info,
|
||||
reboot=True, vifs_already_plugged=True)
|
||||
block_device_info=block_device_info)
|
||||
|
||||
@mock.patch('oslo_utils.fileutils.ensure_tree')
|
||||
@mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall')
|
||||
|
|
@ -15076,6 +15075,49 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
|||
self.assertTrue(instance.cleaned)
|
||||
save.assert_called_once_with()
|
||||
|
||||
@mock.patch('os_brick.encryptors.get_encryption_metadata')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._undefine_domain')
|
||||
def _test_cleanup_encryption_process_execution_error(self, mock_undefine,
|
||||
mock_disconnect, mock_get_encryptor, mock_get_meta,
|
||||
not_found=True):
|
||||
mock_get_meta.return_value = {'fake': 'meta'}
|
||||
mock_encryptor = mock.Mock(spec=encryptors.nop.NoOpEncryptor)
|
||||
mock_get_encryptor.return_value = mock_encryptor
|
||||
# Exit code 4 from os-brick detach_volume means "not found" and we want
|
||||
# to verify we ignore it while we're trying a volume detach.
|
||||
exc = processutils.ProcessExecutionError(exit_code=4 if not_found
|
||||
else 1)
|
||||
mock_encryptor.detach_volume.side_effect = exc
|
||||
|
||||
instance = objects.Instance(self.context, **self.test_instance)
|
||||
connection_info = {'data': {'volume_id': 'fake'}}
|
||||
block_device_info = {'root_device_name': '/dev/vda',
|
||||
'ephemerals': [],
|
||||
'block_device_mapping': [
|
||||
{'connection_info': connection_info,
|
||||
'mount_device': '/dev/vda'}]}
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
|
||||
if not_found:
|
||||
drvr.cleanup(self.context, instance, network_info={},
|
||||
destroy_disks=False,
|
||||
block_device_info=block_device_info)
|
||||
mock_disconnect.assert_called_once_with(connection_info, 'vda',
|
||||
instance)
|
||||
mock_undefine.assert_called_once_with(instance)
|
||||
else:
|
||||
self.assertRaises(processutils.ProcessExecutionError, drvr.cleanup,
|
||||
self.context, instance, network_info={},
|
||||
destroy_disks=False,
|
||||
block_device_info=block_device_info)
|
||||
|
||||
def test_cleanup_encryption_volume_already_detached(self):
|
||||
self._test_cleanup_encryption_process_execution_error(not_found=True)
|
||||
|
||||
def test_cleanup_encryption_volume_detach_failed(self):
|
||||
self._test_cleanup_encryption_process_execution_error(not_found=False)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete',
|
||||
return_value=True)
|
||||
def _test_swap_volume(self, mock_is_job_complete, source_type,
|
||||
|
|
|
|||
|
|
@ -985,7 +985,20 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
# encryptor may report that the volume is still in use.
|
||||
encryptor = self._get_volume_encryptor(connection_info,
|
||||
encryption)
|
||||
encryptor.detach_volume(**encryption)
|
||||
try:
|
||||
encryptor.detach_volume(**encryption)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
if e.exit_code == 4:
|
||||
LOG.debug('Ignoring exit code 4, volume already '
|
||||
'destroyed')
|
||||
else:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.warning(
|
||||
'Could not disconnect encrypted volume '
|
||||
'%(volume)s. If the device is still '
|
||||
'active, it will have to destroyed '
|
||||
'manually for cleanup to succeed.',
|
||||
{'volume': disk_dev})
|
||||
|
||||
try:
|
||||
self._disconnect_volume(connection_info, disk_dev, instance)
|
||||
|
|
@ -2494,12 +2507,15 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
re-creates the domain to ensure the reboot happens, as the guest
|
||||
OS cannot ignore this action.
|
||||
"""
|
||||
|
||||
self._destroy(instance)
|
||||
# Domain XML will be redefined so we can safely undefine it
|
||||
# from libvirt. This ensure that such process as create serial
|
||||
# console for guest will run smoothly.
|
||||
self._undefine_domain(instance)
|
||||
# NOTE(mdbooth): In addition to performing a hard reboot of the domain,
|
||||
# the hard reboot operation is relied upon by operators to be an
|
||||
# automated attempt to fix as many things as possible about a
|
||||
# non-functioning instance before resorting to manual intervention.
|
||||
# With this goal in mind, we tear down all the aspects of an instance
|
||||
# we can here without losing data. This allows us to re-initialise from
|
||||
# scratch, and hopefully fix, most aspects of a non-functioning guest.
|
||||
self.destroy(context, instance, network_info, destroy_disks=False,
|
||||
block_device_info=block_device_info)
|
||||
|
||||
# Convert the system metadata to image metadata
|
||||
# NOTE(mdbooth): This is a workaround for stateless Nova compute
|
||||
|
|
@ -2535,9 +2551,7 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
# Initialize all the necessary networking, block devices and
|
||||
# start the instance.
|
||||
self._create_domain_and_network(context, xml, instance, network_info,
|
||||
block_device_info=block_device_info,
|
||||
reboot=True,
|
||||
vifs_already_plugged=True)
|
||||
block_device_info=block_device_info)
|
||||
self._prepare_pci_devices_for_use(
|
||||
pci_manager.get_instance_pci_devs(instance, 'all'))
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue