diff --git a/nova/api/openstack/compute/evacuate.py b/nova/api/openstack/compute/evacuate.py index 22ae35df8f45..5e074548f6b1 100644 --- a/nova/api/openstack/compute/evacuate.py +++ b/nova/api/openstack/compute/evacuate.py @@ -146,6 +146,8 @@ class EvacuateController(wsgi.Controller): raise exc.HTTPBadRequest(explanation=e.format_message()) except exception.ForbiddenWithAccelerators as e: raise exc.HTTPForbidden(explanation=e.format_message()) + except exception.OperationNotSupportedForVTPM as e: + raise exc.HTTPConflict(explanation=e.format_message()) if (not api_version_request.is_supported(req, min_version='2.14') and CONF.api.enable_instance_password): diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index 05a5a09dc2f2..373ef9334822 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/nova/api/openstack/compute/migrate_server.py @@ -78,9 +78,12 @@ class MigrateServerController(wsgi.Controller): except (exception.TooManyInstances, exception.QuotaError, exception.ForbiddenWithAccelerators) as e: raise exc.HTTPForbidden(explanation=e.format_message()) - except (exception.InstanceIsLocked, - exception.InstanceNotReady, - exception.ServiceUnavailable) as e: + except ( + exception.InstanceIsLocked, + exception.InstanceNotReady, + exception.OperationNotSupportedForVTPM, + exception.ServiceUnavailable, + ) as e: raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, @@ -169,7 +172,10 @@ class MigrateServerController(wsgi.Controller): "'%(ex)s'", {'ex': ex}) else: raise exc.HTTPBadRequest(explanation=ex.format_message()) - except exception.OperationNotSupportedForSEV as e: + except ( + exception.OperationNotSupportedForSEV, + exception.OperationNotSupportedForVTPM, + ) as e: raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceIsLocked as e: raise exc.HTTPConflict(explanation=e.format_message()) diff --git a/nova/api/openstack/compute/rescue.py b/nova/api/openstack/compute/rescue.py index 0b9d524476af..242bb7d0ed9a 100644 --- a/nova/api/openstack/compute/rescue.py +++ b/nova/api/openstack/compute/rescue.py @@ -63,19 +63,20 @@ class RescueController(wsgi.Controller): rescue_password=password, rescue_image_ref=rescue_image_ref, allow_bfv_rescue=allow_bfv_rescue) - except exception.InstanceIsLocked as e: + except ( + exception.InstanceIsLocked, + exception.OperationNotSupportedForVTPM, + exception.InvalidVolume, + ) as e: raise exc.HTTPConflict(explanation=e.format_message()) - except exception.InstanceInvalidState as state_error: - common.raise_http_conflict_for_instance_invalid_state(state_error, - 'rescue', id) - except exception.InvalidVolume as volume_error: - raise exc.HTTPConflict(explanation=volume_error.format_message()) + except exception.InstanceInvalidState as e: + common.raise_http_conflict_for_instance_invalid_state( + e, 'rescue', id) except ( exception.InstanceNotRescuable, exception.UnsupportedRescueImage, - ) as non_rescuable: - raise exc.HTTPBadRequest( - explanation=non_rescuable.format_message()) + ) as e: + raise exc.HTTPBadRequest(explanation=e.format_message()) if CONF.api.enable_instance_password: return {'adminPass': password} diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 086a16daed6e..2c0d058ff7e8 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -961,10 +961,13 @@ class ServersController(wsgi.Controller): exception.ForbiddenWithAccelerators) as error: raise exc.HTTPForbidden( explanation=error.format_message()) - except (exception.InstanceIsLocked, - exception.InstanceNotReady, - exception.ServiceUnavailable, - exception.MixedInstanceNotSupportByComputeService) as e: + except ( + exception.InstanceIsLocked, + exception.InstanceNotReady, + exception.MixedInstanceNotSupportByComputeService, + exception.OperationNotSupportedForVTPM, + exception.ServiceUnavailable, + ) as e: raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, @@ -1112,7 +1115,10 @@ class ServersController(wsgi.Controller): image_href, password, **kwargs) - except exception.InstanceIsLocked as e: + except ( + exception.InstanceIsLocked, + exception.OperationNotSupportedForVTPM, + ) as e: raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, diff --git a/nova/api/openstack/compute/shelve.py b/nova/api/openstack/compute/shelve.py index 3dfda153303a..4aef6bf3efa4 100644 --- a/nova/api/openstack/compute/shelve.py +++ b/nova/api/openstack/compute/shelve.py @@ -51,8 +51,11 @@ class ShelveController(wsgi.Controller): 'project_id': instance.project_id}) try: self.compute_api.shelve(context, instance) - except (exception.InstanceIsLocked, - exception.UnexpectedTaskStateError) as e: + except ( + exception.InstanceIsLocked, + exception.OperationNotSupportedForVTPM, + exception.UnexpectedTaskStateError, + ) as e: raise exc.HTTPConflict(explanation=e.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, diff --git a/nova/compute/api.py b/nova/compute/api.py index 9eb1215160ec..d99273bd790b 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -229,12 +229,13 @@ def check_instance_lock(function): def reject_sev_instances(operation): - """Decorator. Raise OperationNotSupportedForSEV if instance has SEV - enabled. + """Reject requests to decorated function if instance has SEV enabled. + + Raise OperationNotSupportedForSEV if instance has SEV enabled. """ def outer(f): - @six.wraps(f) + @functools.wraps(f) def inner(self, context, instance, *args, **kw): if hardware.get_mem_encryption_constraint(instance.flavor, instance.image_meta): @@ -246,6 +247,25 @@ def reject_sev_instances(operation): return outer +def reject_vtpm_instances(operation): + """Reject requests to decorated function if instance has vTPM enabled. + + Raise OperationNotSupportedForVTPM if instance has vTPM enabled. + """ + + def outer(f): + @functools.wraps(f) + def inner(self, context, instance, *args, **kw): + if hardware.get_vtpm_constraint( + instance.flavor, instance.image_meta, + ): + raise exception.OperationNotSupportedForVTPM( + instance_uuid=instance.uuid, operation=operation) + return f(self, context, instance, *args, **kw) + return inner + return outer + + def _diff_dict(orig, new): """Return a dict describing how to change orig to new. The keys correspond to values that have changed; the value will be a list @@ -3372,6 +3392,7 @@ class API(base.Base): if img_arch: fields_obj.Architecture.canonicalize(img_arch) + @reject_vtpm_instances(instance_actions.REBUILD) @block_accelerators # TODO(stephenfin): We should expand kwargs out to named args @check_instance_lock @@ -3868,6 +3889,10 @@ class API(base.Base): return node + # TODO(stephenfin): This logic would be so much easier to grok if we + # finally split resize and cold migration into separate code paths + # TODO(stephenfin): The 'block_accelerators' decorator doesn't take into + # account the accelerators requested in the new flavor @block_accelerators @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED]) @@ -3932,6 +3957,31 @@ class API(base.Base): if same_instance_type and flavor_id: raise exception.CannotResizeToSameFlavor() + # NOTE(stephenfin): We use this instead of the 'reject_vtpm_instances' + # decorator since the operation can differ depending on args, and for + # resize we have two flavors to worry about + if same_instance_type: + if hardware.get_vtpm_constraint( + current_instance_type, instance.image_meta, + ): + raise exception.OperationNotSupportedForVTPM( + instance_uuid=instance.uuid, + operation=instance_actions.MIGRATE) + else: + if hardware.get_vtpm_constraint( + current_instance_type, instance.image_meta, + ): + raise exception.OperationNotSupportedForVTPM( + instance_uuid=instance.uuid, + operation=instance_actions.RESIZE) + + if hardware.get_vtpm_constraint( + new_instance_type, instance.image_meta, + ): + raise exception.OperationNotSupportedForVTPM( + instance_uuid=instance.uuid, + operation=instance_actions.RESIZE) + # ensure there is sufficient headroom for upsizes if flavor_id: self._check_quota_for_upsize(context, instance, @@ -4071,6 +4121,7 @@ class API(base.Base): allow_same_host = CONF.allow_resize_to_same_host return allow_same_host + @reject_vtpm_instances(instance_actions.SHELVE) @block_accelerators @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, @@ -4256,6 +4307,7 @@ class API(base.Base): self._record_action_start(context, instance, instance_actions.RESUME) self.compute_rpcapi.resume_instance(context, instance) + @reject_vtpm_instances(instance_actions.RESCUE) @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, vm_states.ERROR]) @@ -4940,6 +4992,7 @@ class API(base.Base): return _metadata @block_accelerators + @reject_vtpm_instances(instance_actions.LIVE_MIGRATION) @reject_sev_instances(instance_actions.LIVE_MIGRATION) @check_instance_lock @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED]) @@ -5069,6 +5122,7 @@ class API(base.Base): self.compute_rpcapi.live_migration_abort(context, instance, migration.id) + @reject_vtpm_instances(instance_actions.EVACUATE) @block_accelerators @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, vm_states.ERROR]) diff --git a/nova/exception.py b/nova/exception.py index 82e5f1610d30..c4706a4fc7ed 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -533,6 +533,12 @@ class OperationNotSupportedForSEV(NovaException): code = 409 +class OperationNotSupportedForVTPM(NovaException): + msg_fmt = _("Operation '%(operation)s' not supported for vTPM-enabled " + "instance (%(instance_uuid)s).") + code = 409 + + class InvalidHypervisorType(Invalid): msg_fmt = _("The supplied hypervisor type of is invalid.") diff --git a/nova/tests/unit/api/openstack/compute/test_evacuate.py b/nova/tests/unit/api/openstack/compute/test_evacuate.py index ac1e2f81ca20..bf74e55db795 100644 --- a/nova/tests/unit/api/openstack/compute/test_evacuate.py +++ b/nova/tests/unit/api/openstack/compute/test_evacuate.py @@ -112,6 +112,14 @@ class EvacuateTestV21(test.NoDBTestCase): 'adminPass': 'MyNewPass'}, uuid='BAD_UUID') + @mock.patch('nova.compute.api.API.evacuate') + def test_evacuate__with_vtpm(self, mock_evacuate): + mock_evacuate.side_effect = exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo') + self._check_evacuate_failure( + webob.exc.HTTPConflict, + {'host': 'foo', 'onSharedStorage': 'False', 'adminPass': 'bar'}) + def test_evacuate_with_active_service(self): def fake_evacuate(*args, **kwargs): raise exception.ComputeServiceInUse("Service still in use") diff --git a/nova/tests/unit/api/openstack/compute/test_migrate_server.py b/nova/tests/unit/api/openstack/compute/test_migrate_server.py index 1f3a52893305..2cfc7b38977e 100644 --- a/nova/tests/unit/api/openstack/compute/test_migrate_server.py +++ b/nova/tests/unit/api/openstack/compute/test_migrate_server.py @@ -138,6 +138,11 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): allowed=0) self._test_migrate_exception(exc_info, webob.exc.HTTPForbidden) + def test_migrate_vtpm_not_supported(self): + exc_info = exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo') + self._test_migrate_exception(exc_info, webob.exc.HTTPConflict) + def _test_migrate_live_succeeded(self, param): instance = self._stub_instance_get() @@ -284,6 +289,13 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): expected_exc=webob.exc.HTTPConflict, check_response=False) + def test_migrate_live_vtpm_not_supported(self): + self._test_migrate_live_failed_with_exception( + exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo'), + expected_exc=webob.exc.HTTPConflict, + check_response=False) + def test_migrate_live_pre_check_error(self): self._test_migrate_live_failed_with_exception( exception.MigrationPreCheckError(reason='')) @@ -601,8 +613,10 @@ class MigrateServerTestsV268(MigrateServerTestsV256): @mock.patch('nova.virt.hardware.get_mem_encryption_constraint', new=mock.Mock(return_value=True)) - @mock.patch.object(objects.instance.Instance, 'image_meta') - def test_live_migrate_sev_rejected(self, mock_image): + @mock.patch.object( + objects.instance.Instance, 'image_meta', + new=objects.ImageMeta.from_dict({})) + def test_live_migrate_sev_rejected(self): instance = self._stub_instance_get() body = {'os-migrateLive': {'host': 'hostname', 'block_migration': 'auto'}} diff --git a/nova/tests/unit/api/openstack/compute/test_rescue.py b/nova/tests/unit/api/openstack/compute/test_rescue.py index 93b93d6be408..3868d326ca86 100644 --- a/nova/tests/unit/api/openstack/compute/test_rescue.py +++ b/nova/tests/unit/api/openstack/compute/test_rescue.py @@ -13,9 +13,10 @@ # under the License. import mock -import webob +import ddt from oslo_utils.fixture import uuidsentinel as uuids +import webob from nova.api.openstack import api_version_request from nova.api.openstack.compute import rescue as rescue_v21 @@ -44,6 +45,7 @@ def fake_compute_get(*args, **kwargs): uuid=UUID, **kwargs) +@ddt.ddt class RescueTestV21(test.NoDBTestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' @@ -63,11 +65,16 @@ class RescueTestV21(test.NoDBTestCase): def _allow_bfv_rescue(self): return api_version_request.is_supported(self.fake_req, '2.87') - @mock.patch.object(compute.api.API, "rescue") - def test_rescue_from_locked_server(self, mock_rescue): - mock_rescue.side_effect = exception.InstanceIsLocked( - instance_uuid=UUID) - + @ddt.data( + exception.InstanceIsLocked(instance_uuid=uuids.instance), + exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo'), + exception.InvalidVolume(reason='foo'), + ) + @mock.patch.object(compute.api.API, 'rescue') + def test_rescue__http_conflict_error(self, exc, mock_rescue): + """Test that exceptions are translated into HTTP Conflict errors.""" + mock_rescue.side_effect = exc body = {"rescue": {"adminPass": "AABBCC112233"}} self.assertRaises(webob.exc.HTTPConflict, self.controller._rescue, diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index e707b8e18184..bec9058d7cc2 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt import fixtures import mock from oslo_utils.fixture import uuidsentinel as uuids @@ -48,6 +49,7 @@ class MockSetAdminPassword(object): self.password = password +@ddt.ddt class ServerActionsControllerTestV21(test.TestCase): image_uuid = '76fa36fc-c930-4bf3-8c8a-ea2a2420deb6' image_base_url = 'http://localhost:9292/images/' @@ -69,6 +71,7 @@ class ServerActionsControllerTestV21(test.TestCase): fakes.stub_out_compute_api_snapshot(self) fake.stub_out_image_service(self) self.flags(enable_instance_password=True, group='api') + # TODO(stephenfin): Use uuidsentinel instead of this self._image_href = '155d900f-4e14-4e4c-a73d-069cbf4541e6' self.controller = self._get_controller() @@ -87,7 +90,7 @@ class ServerActionsControllerTestV21(test.TestCase): self.image_api = glance.API() # Assume that anything that hits the compute API and looks for a # RequestSpec doesn't care about it, since testing logic that deep - # should be done in nova.tests.unit.compute.test_compute_api. + # should be done in nova.tests.unit.compute.test_api. mock_reqspec = mock.patch('nova.objects.RequestSpec') mock_reqspec.start() self.addCleanup(mock_reqspec.stop) @@ -379,12 +382,22 @@ class ServerActionsControllerTestV21(test.TestCase): # pep3333 requires applications produces headers which are str self.assertEqual(str, type(robj['location'])) + @ddt.data( + exception.InstanceIsLocked(instance_uuid=uuids.instance), + exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo'), + ) + @mock.patch('nova.compute.api.API.rebuild') + def test_rebuild__http_conflict_error(self, exc, mock_rebuild): + mock_rebuild.side_effect = exc + self.assertRaises( + webob.exc.HTTPConflict, + self.controller._action_rebuild, + self.req, uuids.instance, + body={'rebuild': {'imageRef': uuids.image}}) + def test_rebuild_raises_conflict_on_invalid_state(self): - body = { - "rebuild": { - "imageRef": self._image_href, - }, - } + body = {'rebuild': {'imageRef': uuids.image}} def fake_rebuild(*args, **kwargs): raise exception.InstanceInvalidState(attr='fake_attr', @@ -843,6 +856,17 @@ class ServerActionsControllerTestV21(test.TestCase): self.controller._action_resize, self.req, FAKE_UUID, body=body) + @mock.patch('nova.compute.api.API.resize') + def test_resize__vtpm_rejected(self, mock_resize): + """Test that 'OperationNotSupportedForVTPM' exception is translated.""" + mock_resize.side_effect = exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo') + body = {'resize': {'flavorRef': 'http://localhost/3'}} + self.assertRaises( + webob.exc.HTTPConflict, + self.controller._action_resize, + self.req, FAKE_UUID, body=body) + def test_confirm_resize_server(self): body = dict(confirmResize=None) diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_servers.py similarity index 99% rename from nova/tests/unit/api/openstack/compute/test_serversV21.py rename to nova/tests/unit/api/openstack/compute/test_servers.py index d13bca8e4b8c..bb8b4915db42 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_servers.py @@ -258,7 +258,7 @@ class ControllerTest(test.TestCase): self.ips_controller = ips.IPsController() # Assume that anything that hits the compute API and looks for a # RequestSpec doesn't care about it, since testing logic that deep - # should be done in nova.tests.unit.compute.test_compute_api. + # should be done in nova.tests.unit.compute.test_api. mock_reqspec = mock.patch('nova.objects.RequestSpec') mock_reqspec.start() self.addCleanup(mock_reqspec.stop) diff --git a/nova/tests/unit/api/openstack/compute/test_shelve.py b/nova/tests/unit/api/openstack/compute/test_shelve.py index 911e8f776cd8..cdbc2dd6dd53 100644 --- a/nova/tests/unit/api/openstack/compute/test_shelve.py +++ b/nova/tests/unit/api/openstack/compute/test_shelve.py @@ -13,8 +13,10 @@ # under the License. import mock + +import ddt from oslo_serialization import jsonutils -from oslo_utils.fixture import uuidsentinel +from oslo_utils.fixture import uuidsentinel as uuids import six import webob @@ -28,38 +30,35 @@ from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_instance -class ShelvePolicyTestV21(test.NoDBTestCase): +@ddt.ddt +class ShelveControllerTest(test.NoDBTestCase): plugin = shelve_v21 def setUp(self): - super(ShelvePolicyTestV21, self).setUp() + super().setUp() self.controller = self.plugin.ShelveController() self.req = fakes.HTTPRequest.blank('') + @ddt.data( + exception.InstanceIsLocked(instance_uuid=uuids.instance), + exception.OperationNotSupportedForVTPM( + instance_uuid=uuids.instance, operation='foo'), + exception.UnexpectedTaskStateError( + instance_uuid=uuids.instance, expected=None, + actual=task_states.SHELVING), + ) + @mock.patch('nova.compute.api.API.shelve') @mock.patch('nova.api.openstack.common.get_instance') - def test_shelve_locked_server(self, get_instance_mock): - get_instance_mock.return_value = ( + def test_shelve__http_conflict_error( + self, exc, mock_get_instance, mock_shelve, + ): + mock_get_instance.return_value = ( fake_instance.fake_instance_obj(self.req.environ['nova.context'])) - self.stub_out('nova.compute.api.API.shelve', - fakes.fake_actions_to_locked_server) - self.assertRaises(webob.exc.HTTPConflict, self.controller._shelve, - self.req, uuidsentinel.fake, {}) + mock_shelve.side_effect = exc - @mock.patch('nova.api.openstack.common.get_instance') - @mock.patch('nova.objects.instance.Instance.save') - def test_shelve_task_state_race(self, mock_save, get_instance_mock): - instance = fake_instance.fake_instance_obj( - self.req.environ['nova.context'], - vm_state=vm_states.ACTIVE, task_state=None) - instance.launched_at = instance.created_at - get_instance_mock.return_value = instance - mock_save.side_effect = exception.UnexpectedTaskStateError( - instance_uuid=instance.uuid, expected=None, - actual=task_states.SHELVING) - ex = self.assertRaises(webob.exc.HTTPConflict, self.controller._shelve, - self.req, uuidsentinel.fake, body={'shelve': {}}) - self.assertIn('Conflict updating instance', six.text_type(ex)) - mock_save.assert_called_once_with(expected_task_state=[None]) + self.assertRaises( + webob.exc.HTTPConflict, self.controller._shelve, + self.req, uuids.fake, {}) @mock.patch('nova.api.openstack.common.get_instance') def test_unshelve_locked_server(self, get_instance_mock): @@ -68,7 +67,7 @@ class ShelvePolicyTestV21(test.NoDBTestCase): self.stub_out('nova.compute.api.API.unshelve', fakes.fake_actions_to_locked_server) self.assertRaises(webob.exc.HTTPConflict, self.controller._unshelve, - self.req, uuidsentinel.fake, body={'unshelve': {}}) + self.req, uuids.fake, body={'unshelve': {}}) @mock.patch('nova.api.openstack.common.get_instance') def test_shelve_offload_locked_server(self, get_instance_mock): @@ -78,7 +77,7 @@ class ShelvePolicyTestV21(test.NoDBTestCase): fakes.fake_actions_to_locked_server) self.assertRaises(webob.exc.HTTPConflict, self.controller._shelve_offload, - self.req, uuidsentinel.fake, {}) + self.req, uuids.fake, {}) class UnshelveServerControllerTestV277(test.NoDBTestCase): diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_api.py similarity index 99% rename from nova/tests/unit/compute/test_compute_api.py rename to nova/tests/unit/compute/test_api.py index d6ff7b2c5299..7c3c6e8e5ac2 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -66,6 +66,7 @@ from nova.tests.unit import matchers from nova.tests.unit.objects import test_flavor from nova.tests.unit.objects import test_migration from nova import utils +from nova.virt import hardware from nova.volume import cinder @@ -1836,6 +1837,8 @@ class _ComputeAPIUnitTestMixIn(object): mock_inst_save.assert_called_once_with(expected_task_state=[None]) mock_get_requested_resources.assert_not_called() + # TODO(stephenfin): This is a terrible, terrible function and should be + # broken up into its constituent parts @mock.patch('nova.compute.api.API.get_instance_host_status', new=mock.Mock(return_value=fields_obj.HostStatus.UP)) @mock.patch('nova.virt.hardware.numa_get_constraints') @@ -2050,9 +2053,6 @@ class _ComputeAPIUnitTestMixIn(object): else: mock_resize.assert_not_called() - def _test_migrate(self, *args, **kwargs): - self._test_resize(*args, flavor_id_passed=False, **kwargs) - def test_resize(self): self._test_resize() @@ -2104,6 +2104,38 @@ class _ComputeAPIUnitTestMixIn(object): project_values={'cores': 1, 'ram': 2560}, project_id=fake_inst.project_id, user_id=fake_inst.user_id) + @mock.patch( + 'nova.compute.api.API.get_instance_host_status', + new=mock.Mock(return_value=fields_obj.HostStatus.UP)) + @mock.patch( + 'nova.compute.utils.is_volume_backed_instance', + new=mock.Mock(return_value=False)) + @mock.patch.object(flavors, 'get_flavor_by_flavor_id') + def test_resize__with_vtpm(self, mock_get_flavor): + """Ensure resizes are rejected if either flavor requests vTPM.""" + fake_inst = self._create_instance_obj() + current_flavor = fake_inst.flavor + new_flavor = self._create_flavor( + id=200, flavorid='new-flavor-id', name='new_flavor', + disabled=False, extra_specs={'hw:tpm_version': '2.0'}) + mock_get_flavor.return_value = new_flavor + + orig_get_vtpm_constraint = hardware.get_vtpm_constraint + with mock.patch.object(hardware, 'get_vtpm_constraint') as get_vtpm: + get_vtpm.side_effect = orig_get_vtpm_constraint + self.assertRaises( + exception.OperationNotSupportedForVTPM, + self.compute_api.resize, + self.context, fake_inst, flavor_id=new_flavor.flavorid) + + get_vtpm.assert_has_calls([ + mock.call(current_flavor, mock.ANY), + mock.call(new_flavor, mock.ANY), + ]) + + def _test_migrate(self, *args, **kwargs): + self._test_resize(*args, flavor_id_passed=False, **kwargs) + def test_migrate(self): self._test_migrate() @@ -2130,6 +2162,28 @@ class _ComputeAPIUnitTestMixIn(object): self._test_migrate(host_name='target_host', allow_cross_cell_resize=True) + @mock.patch( + 'nova.compute.api.API.get_instance_host_status', + new=mock.Mock(return_value=fields_obj.HostStatus.UP)) + @mock.patch( + 'nova.compute.utils.is_volume_backed_instance', + new=mock.Mock(return_value=False)) + def test_migrate__with_vtpm(self): + """Ensure migrations are rejected if instance uses vTPM.""" + flavor = self._create_flavor( + extra_specs={'hw:tpm_version': '2.0'}) + instance = self._create_instance_obj(flavor=flavor) + + orig_get_vtpm_constraint = hardware.get_vtpm_constraint + with mock.patch.object(hardware, 'get_vtpm_constraint') as get_vtpm: + get_vtpm.side_effect = orig_get_vtpm_constraint + self.assertRaises( + exception.OperationNotSupportedForVTPM, + self.compute_api.resize, + self.context, instance) + + get_vtpm.assert_called_once_with(flavor, mock.ANY) + @mock.patch('nova.compute.api.API.get_instance_host_status', new=mock.Mock(return_value=fields_obj.HostStatus.UP)) @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host', @@ -6763,6 +6817,8 @@ class _ComputeAPIUnitTestMixIn(object): mock_get_hm.assert_not_called() +# TODO(stephenfin): The separation of the mixin is a hangover from cells v1 +# days and should be removed class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): def setUp(self): super(ComputeAPIUnitTestCase, self).setUp() diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 4ca83e6edb6e..d604a67516b1 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -9056,7 +9056,6 @@ class ComputeAPITestCase(BaseTestCase): def test_rebuild_in_error_not_launched(self): instance = self._create_fake_instance_obj(params={'image_ref': ''}) - flavor = instance.flavor self.stub_out('nova.tests.unit.image.fake._FakeImageService.show', self.fake_show) self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, @@ -9066,8 +9065,7 @@ class ComputeAPITestCase(BaseTestCase): {"vm_state": vm_states.ERROR, "launched_at": None}) - instance = db.instance_get_by_uuid(self.context, instance['uuid']) - instance['flavor'] = flavor + instance = objects.Instance.get_by_uuid(self.context, instance.uuid) self.assertRaises(exception.InstanceInvalidState, self.compute_api.rebuild, diff --git a/nova/tests/unit/policies/test_servers.py b/nova/tests/unit/policies/test_servers.py index 32ee253384cb..0728992e7f41 100644 --- a/nova/tests/unit/policies/test_servers.py +++ b/nova/tests/unit/policies/test_servers.py @@ -1218,6 +1218,9 @@ class ServersPolicyTest(base.BasePolicyTest): self.controller.create, req, body=body) + @mock.patch( + 'nova.objects.Instance.image_meta', + new=objects.ImageMeta.from_dict({})) @mock.patch('nova.compute.api.API._check_requested_networks') @mock.patch('nova.compute.api.API._allow_resize_to_same_host') @mock.patch('nova.objects.RequestSpec.get_by_instance_uuid') diff --git a/nova/utils.py b/nova/utils.py index 84c28224c87d..0a40fa6ffc26 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -537,6 +537,8 @@ def instance_meta(instance): return metadata_to_dict(instance['metadata']) +# TODO(stephenfin): Instance.system_metadata is always a dict now (thanks, +# o.vo) so this check (and the function as a whole) can be removed def instance_sys_meta(instance): if not instance.get('system_metadata'): return {}