openstack/nova
Code Issues Proposed changes

virt: Add 'context', drop 'network_info' parameters for 'unrescue'

Browse Source 
In a future change, we'll want access to this so that it's possible to
retrieve vTPM data during the unrescue operation. While we're here, it
seems nothing is using the 'network_info' argument anymore, presumably
since the demise of nova-network, and this can and should be dropped.
Resolve both issues in one go, adding the 'context' parameter, dropping
the 'network_info' one, and updating the various callers and tests for
same.

Maintainers of out-of-tree drivers have been notified of these changes
[1].

[1] http://lists.openstack.org/pipermail/openstack-discuss/2020-July/015824.html

Part of blueprint add-emulated-virtual-tpm

Change-Id: Id5e4b0f26d5a2a93db6a7d96555a2cff29d9a2cf
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This commit is contained in:
Stephen Finucane 2020-05-18 13:45:41 +01:00
parent 72cf37bca0
commit 7e4d8afb95
16 changed files with 72 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
mypy-files.txt
View File

@ -1,3 +1,4 @@
nova/virt/driver.py
nova/virt/hardware.py
nova/virt/libvirt/__init__.py
nova/virt/libvirt/driver.py

4
nova/compute/manager.py
View File

@ -4127,6 +4127,7 @@ class ComputeManager(manager.Manager):
@wrap_instance_event(prefix='compute')
@wrap_instance_fault
def unrescue_instance(self, context, instance):
orig_context = context
context = context.elevated()
LOG.info('Unrescuing', instance=instance)
@ -4138,8 +4139,7 @@ class ComputeManager(manager.Manager):
phase=fields.NotificationPhase.START)
with self._error_out_instance_on_exception(context, instance):
self.driver.unrescue(instance,
network_info)
self.driver.unrescue(orig_context, instance)
instance.vm_state = vm_states.ACTIVE
instance.task_state = None

5
nova/test.py
View File

@ -625,8 +625,9 @@ class TestCase(base.BaseTestCase):
baseclass)
for name in sorted(implmethods.keys()):
baseargs = utils.getargspec(basemethods[name])
implargs = utils.getargspec(implmethods[name])
# NOTE(stephenfin): We ignore type annotations
baseargs = utils.getargspec(basemethods[name])[:-1]
implargs = utils.getargspec(implmethods[name])[:-1]
self.assertEqual(baseargs, implargs,
"%s args don't match base class %s" %

11
nova/tests/unit/compute/test_compute.py
View File

@ -2248,11 +2248,10 @@ class ComputeTestCase(BaseTestCase,
self.stub_out('nova.virt.fake.FakeDriver.rescue', fake_rescue)
def fake_unrescue(self, instance_ref, network_info):
def fake_unrescue(self, context, instance_ref):
called['unrescued'] = True
self.stub_out('nova.virt.fake.FakeDriver.unrescue',
fake_unrescue)
self.stub_out('nova.virt.fake.FakeDriver.unrescue', fake_unrescue)
instance = self._create_fake_instance_obj()
self.compute.build_and_run_instance(self.context, instance, {}, {}, {},
@ -2327,10 +2326,10 @@ class ComputeTestCase(BaseTestCase,
@mock.patch('nova.context.RequestContext.elevated')
def test_unrescue_notifications(self, mock_context, mock_notify):
# Ensure notifications on instance rescue.
def fake_unrescue(self, instance_ref, network_info):
def fake_unrescue(self, context, instance_ref):
pass
self.stub_out('nova.virt.fake.FakeDriver.unrescue',
fake_unrescue)
self.stub_out('nova.virt.fake.FakeDriver.unrescue', fake_unrescue)
context = self.context
mock_context.return_value = context

2
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -4443,7 +4443,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
]
notify_instance_usage.assert_has_calls(notify_calls)
driver_unrescue.assert_called_once_with(instance, fake_nw_info)
driver_unrescue.assert_called_once_with(self.context, instance)
mock_notify.assert_has_calls([
mock.call(self.context, instance, 'fake-mini',
action='unrescue', phase='start'),

11
nova/tests/unit/virt/ironic/test_driver.py
View File

@ -3063,7 +3063,7 @@ class IronicDriverSyncTestCase(IronicDriverTestCase):
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
self.driver.unrescue(instance, None)
self.driver.unrescue(self.ctx, instance)
mock_sps.assert_called_once_with(node.uuid, 'unrescue')
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@ -3078,8 +3078,7 @@ class IronicDriverSyncTestCase(IronicDriverTestCase):
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
self.assertRaises(exception.InstanceUnRescueFailure,
self.driver.unrescue,
instance, None)
self.driver.unrescue, self.ctx, instance)
@mock.patch.object(ironic_driver.IronicDriver,
'_validate_instance_and_node')
@ -3092,8 +3091,7 @@ class IronicDriverSyncTestCase(IronicDriverTestCase):
instance_id='fake')
self.assertRaises(exception.InstanceUnRescueFailure,
self.driver.unrescue,
instance, None)
self.driver.unrescue, self.ctx, instance)
@mock.patch.object(ironic_driver.IronicDriver,
'_validate_instance_and_node')
@ -3108,8 +3106,7 @@ class IronicDriverSyncTestCase(IronicDriverTestCase):
node=node.uuid)
self.assertRaises(exception.InstanceUnRescueFailure,
self.driver.unrescue,
instance, None)
self.driver.unrescue, self.ctx, instance)
def test__can_send_version(self):
self.assertIsNone(

2
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -23272,7 +23272,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
) as (mock_write, mock_destroy, mock_create, mock_del,
mock_rmtree, mock_isdir, mock_lvm_disks,
mock_remove_volumes, mock_glob):
drvr.unrescue(instance, None)
drvr.unrescue(self.context, instance)
mock_destroy.assert_called_once_with(instance)
mock_create.assert_called_once_with("fake_unrescue_xml",
fake_dom)

6
nova/tests/unit/virt/test_virt_drivers.py
View File

@ -297,8 +297,8 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
@mock.patch('os.unlink')
@mock.patch('nova.virt.libvirt.utils.load_file', return_value='')
def test_unrescue_unrescued_instance(self, mock_load_file, mock_unlink):
instance_ref, network_info = self._get_running_instance()
self.connection.unrescue(instance_ref, network_info)
instance_ref, _ = self._get_running_instance()
self.connection.unrescue(self.ctxt, instance_ref)
@catch_notimplementederror
@mock.patch('os.unlink')
@ -307,7 +307,7 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
instance_ref, network_info = self._get_running_instance()
self.connection.rescue(self.ctxt, instance_ref, network_info,
image_meta, '', None)
self.connection.unrescue(instance_ref, network_info)
self.connection.unrescue(self.ctxt, instance_ref)
@catch_notimplementederror
def test_poll_rebooting_instances(self):

6
nova/tests/unit/virt/xenapi/test_xenapi.py
View File

@ -1365,14 +1365,14 @@ class XenAPIVMTestCase(stubs.XenAPITestBase,
# Unrescue expects the original instance to be powered off
conn.power_off(instance)
xenapi_fake.create_vm(instance['name'] + '-rescue', 'Running')
conn.unrescue(instance, None)
conn.unrescue(self.context, instance)
def test_unrescue_not_in_rescue(self):
instance = self._create_instance(obj=True)
conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False)
# Ensure that it will not unrescue a non-rescued instance.
self.assertRaises(exception.InstanceNotInRescueMode, conn.unrescue,
instance, None)
self.assertRaises(exception.InstanceNotInRescueMode,
conn.unrescue, self.context, instance)
def test_finish_revert_migration(self):
instance = self._create_instance()

10
nova/virt/driver.py
View File

@ -29,7 +29,9 @@ from oslo_utils import importutils
import six
import nova.conf
from nova import context as nova_context
from nova.i18n import _
from nova import objects
from nova.virt import event as virtevent
CONF = nova.conf.CONF
@ -879,12 +881,16 @@ class ComputeDriver(object):
"""
raise NotImplementedError()
def unrescue(self, instance, network_info):
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
"""Unrescue the specified instance.
:param context: security context
:param instance: nova.objects.instance.Instance
"""
# TODO(Vek): Need to pass context in for access to auth_token
raise NotImplementedError()
def power_off(self, instance, timeout=0, retry_interval=0):

8
nova/virt/fake.py
View File

@ -39,7 +39,9 @@ from nova.compute import task_states
from nova.compute import vm_states
import nova.conf
from nova.console import type as ctype
from nova import context as nova_context
from nova import exception
from nova import objects
from nova.objects import diagnostics as diagnostics_obj
from nova.objects import fields as obj_fields
from nova.objects import migrate_data
@ -238,7 +240,11 @@ class FakeDriver(driver.ComputeDriver):
rescue_password, block_device_info):
pass
def unrescue(self, instance, network_info):
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
self.instances[instance.uuid].state = power_state.RUNNING
def poll_rebooting_instances(self, timeout, instances):

9
nova/virt/hyperv/driver.py
View File

@ -26,7 +26,9 @@ from os_win import utilsfactory
from oslo_log import log as logging
import six
from nova import context as nova_context
from nova import exception
from nova import objects
from nova.virt import driver
from nova.virt.hyperv import eventhandler
from nova.virt.hyperv import hostops
@ -39,6 +41,7 @@ from nova.virt.hyperv import snapshotops
from nova.virt.hyperv import vmops
from nova.virt.hyperv import volumeops
LOG = logging.getLogger(__name__)
@ -361,7 +364,11 @@ class HyperVDriver(driver.ComputeDriver):
self._vmops.rescue_instance(context, instance, network_info,
image_meta, rescue_password)
def unrescue(self, instance, network_info):
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
self._vmops.unrescue_instance(instance)
def update_provider_tree(self, provider_tree, nodename, allocations=None):

11
nova/virt/ironic/driver.py
View File

@ -18,6 +18,7 @@
A driver wrapping the Ironic API, such that Nova may provision
bare metal resources.
"""
import base64
from distutils import version
import gzip
@ -2166,13 +2167,15 @@ class IronicDriver(virt_driver.ComputeDriver):
LOG.info('Successfully rescued Ironic node %(node)s',
{'node': node_uuid}, instance=instance)
def unrescue(self, instance, network_info):
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
"""Unrescue the specified instance.
:param context: security context
:param instance: nova.objects.instance.Instance
:param nova.network.model.NetworkInfo network_info:
Necessary network information for the unrescue. Ignored by this
driver.
"""
LOG.debug('Unrescue called for instance', instance=instance)

9
nova/virt/libvirt/driver.py
View File

@ -3568,9 +3568,12 @@ class LibvirtDriver(driver.ComputeDriver):
self._destroy(instance)
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.
"""
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
"""Reboot the VM which is being rescued back into primary images."""
instance_dir = libvirt_utils.get_instance_path(instance)
unrescue_xml_path = os.path.join(instance_dir, 'unrescue.xml')
xml = libvirt_utils.load_file(unrescue_xml_path)

7
nova/virt/vmwareapi/driver.py
View File

@ -37,6 +37,7 @@ from nova.compute import power_state
from nova.compute import task_states
from nova.compute import utils as compute_utils
import nova.conf
from nova import context as nova_context
from nova import exception
from nova.i18n import _
from nova import objects
@ -650,7 +651,11 @@ class VMwareVCDriver(driver.ComputeDriver):
"""Rescue the specified instance."""
self._vmops.rescue(context, instance, network_info, image_meta)
def unrescue(self, instance, network_info):
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
"""Unrescue the specified instance."""
self._vmops.unrescue(instance)

9
nova/virt/xenapi/driver.py
View File

@ -29,12 +29,13 @@ from oslo_log import log as logging
from oslo_serialization import jsonutils
from oslo_utils import units
from oslo_utils import versionutils
import six.moves.urllib.parse as urlparse
import nova.conf
from nova import context as nova_context
from nova import exception
from nova.i18n import _
from nova import objects
from nova.virt import driver
from nova.virt.xenapi import host
from nova.virt.xenapi import pool
@ -316,7 +317,11 @@ class XenAPIDriver(driver.ComputeDriver):
"""Set the ability to power on/off an instance."""
self._vmops.set_bootable(instance, is_bootable)
def unrescue(self, instance, network_info):
def unrescue(
self,
context: nova_context.RequestContext,
instance: 'objects.Instance',
):
"""Unrescue the specified instance."""
self._vmops.unrescue(instance)