From f64237d89056d75300aa9cafaed679b728407261 Mon Sep 17 00:00:00 2001 From: Diana Clarke Date: Fri, 8 Apr 2016 20:55:59 -0400 Subject: [PATCH] Fix database poison warnings The following warning appears in the unit test logs a number of times. This patch fixes some of those warnings by mocking the get instance calls higher up (Instance.get_by_uuid vs. db.instance_get_by_uuid). "UserWarning: This test uses methods that set internal oslo_db state, but it does not claim to use the database. This will conflict with the setup of tests that do use the database and cause failures later. Note that this warning is only emitted once per unit test worker, so new offenders will show up in the logs each time you fix a test until they are all gone. Related-Bug: #1568414 Change-Id: I76422d8c13559322bba0be0c24882330fb947af1 --- .../compute/test_instance_actions.py | 42 ++++---------- .../unit/api/openstack/compute/test_shelve.py | 57 ++++++++++--------- 2 files changed, 41 insertions(+), 58 deletions(-) diff --git a/nova/tests/unit/api/openstack/compute/test_instance_actions.py b/nova/tests/unit/api/openstack/compute/test_instance_actions.py index 220de4cec61d..b5f4c71a9cde 100644 --- a/nova/tests/unit/api/openstack/compute/test_instance_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_instance_actions.py @@ -16,6 +16,7 @@ import copy import uuid +import mock from oslo_policy import policy as oslo_policy import six from webob import exc @@ -31,7 +32,6 @@ from nova import objects from nova import policy from nova import test from nova.tests.unit.api.openstack import fakes -from nova.tests.unit import fake_instance from nova.tests.unit import fake_server_actions FAKE_UUID = fake_server_actions.FAKE_UUID @@ -80,41 +80,30 @@ class InstanceActionsPolicyTestV21(test.NoDBTestCase): fake_url = '/123/servers/12/%s' % action return fakes.HTTPRequest.blank(fake_url) + def _get_instance_other_project(self, req): + context = req.environ['nova.context'] + project_id = '%s_unequal' % context.project_id + return objects.Instance(project_id=project_id) + def _set_policy_rules(self): rules = {'compute:get': '', 'os_compute_api:os-instance-actions': 'project_id:%(project_id)s'} policy.set_rules(oslo_policy.Rules.from_dict(rules)) - def test_list_actions_restricted_by_project(self): + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_list_actions_restricted_by_project(self, mock_instance_get): self._set_policy_rules() - - def fake_instance_get_by_uuid(context, instance_id, - columns_to_join=None, - use_slave=False): - return fake_instance.fake_db_instance( - **{'name': 'fake', 'project_id': '%s_unequal' % - context.project_id}) - - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) req = self._get_http_req('os-instance-actions') + mock_instance_get.return_value = self._get_instance_other_project(req) self.assertRaises(exception.Forbidden, self.controller.index, req, str(uuid.uuid4())) - def test_get_action_restricted_by_project(self): + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_get_action_restricted_by_project(self, mock_instance_get): self._set_policy_rules() - - def fake_instance_get_by_uuid(context, instance_id, - columns_to_join=None, - use_slave=False): - return fake_instance.fake_db_instance( - **{'name': 'fake', 'project_id': '%s_unequal' % - context.project_id}) - - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) req = self._get_http_req('os-instance-actions/1') + mock_instance_get.return_value = self._get_instance_other_project(req) self.assertRaises(exception.Forbidden, self.controller.show, req, str(uuid.uuid4()), '1') @@ -142,14 +131,7 @@ class InstanceActionsTestV21(test.NoDBTestCase): self.controller = self.instance_actions.InstanceActionsController() self.fake_actions = copy.deepcopy(fake_server_actions.FAKE_ACTIONS) self.fake_events = copy.deepcopy(fake_server_actions.FAKE_EVENTS) - - def fake_instance_get_by_uuid(context, instance_id, use_slave=False): - return fake_instance.fake_instance_obj(None, - **{'name': 'fake', 'project_id': context.project_id}) - self.stubs.Set(compute_api.API, 'get', self.fake_get) - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) def _get_http_req(self, action, use_admin_context=False): fake_url = '/123/servers/12/%s' % action diff --git a/nova/tests/unit/api/openstack/compute/test_shelve.py b/nova/tests/unit/api/openstack/compute/test_shelve.py index c25c64ff2cde..9b9317fbd527 100644 --- a/nova/tests/unit/api/openstack/compute/test_shelve.py +++ b/nova/tests/unit/api/openstack/compute/test_shelve.py @@ -14,6 +14,7 @@ import uuid +import mock from oslo_policy import policy as oslo_policy import webob @@ -21,16 +22,11 @@ from nova.api.openstack.compute.legacy_v2.contrib import shelve as shelve_v2 from nova.api.openstack.compute import shelve as shelve_v21 from nova.compute import api as compute_api from nova import exception +from nova import objects from nova import policy from nova import test from nova.tests.unit.api.openstack import fakes -from nova.tests.unit import fake_instance - - -def fake_instance_get_by_uuid(context, instance_id, - columns_to_join=None, use_slave=False): - return fake_instance.fake_db_instance( - **{'name': 'fake', 'project_id': '%s_unequal' % context.project_id}) +from nova.tests import uuidsentinel class ShelvePolicyTestV21(test.NoDBTestCase): @@ -50,9 +46,10 @@ class ShelvePolicyTestV21(test.NoDBTestCase): self.assertRaises(exception.Forbidden, self.controller._shelve, self.req, str(uuid.uuid4()), {}) - def test_shelve_locked_server(self): - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_shelve_locked_server(self, mock_instance_get): + instance = objects.Instance(uuid=uuidsentinel.instance1) + mock_instance_get.return_value = instance self.stubs.Set(compute_api.API, 'shelve', fakes.fake_actions_to_locked_server) self.assertRaises(webob.exc.HTTPConflict, self.controller._shelve, @@ -65,9 +62,10 @@ class ShelvePolicyTestV21(test.NoDBTestCase): self.assertRaises(exception.Forbidden, self.controller._unshelve, self.req, str(uuid.uuid4()), {}) - def test_unshelve_locked_server(self): - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_unshelve_locked_server(self, mock_instance_get): + instance = objects.Instance(uuid=uuidsentinel.instance1) + mock_instance_get.return_value = instance self.stubs.Set(compute_api.API, 'unshelve', fakes.fake_actions_to_locked_server) self.assertRaises(webob.exc.HTTPConflict, self.controller._unshelve, @@ -82,9 +80,10 @@ class ShelvePolicyTestV21(test.NoDBTestCase): self.controller._shelve_offload, self.req, str(uuid.uuid4()), {}) - def test_shelve_offload_locked_server(self): - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_shelve_offload_locked_server(self, mock_instance_get): + instance = objects.Instance(uuid=uuidsentinel.instance1) + mock_instance_get.return_value = instance self.stubs.Set(compute_api.API, 'shelve_offload', fakes.fake_actions_to_locked_server) self.assertRaises(webob.exc.HTTPConflict, @@ -97,33 +96,35 @@ class ShelvePolicyTestV2(ShelvePolicyTestV21): prefix = '' offload = 'shelveOffload' - # These 3 cases are covered in ShelvePolicyEnforcementV21 - def test_shelve_allowed(self): + def _get_instance_other_project(self): + context = self.req.environ['nova.context'] + project_id = '%s_unequal' % context.project_id + return objects.Instance(project_id=project_id) + + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_shelve_allowed(self, mock_instance_get): + mock_instance_get.return_value = self._get_instance_other_project() rules = {'compute:get': '', 'compute_extension:%sshelve' % self.prefix: ''} policy.set_rules(oslo_policy.Rules.from_dict(rules)) - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) self.assertRaises(exception.Forbidden, self.controller._shelve, self.req, str(uuid.uuid4()), {}) - def test_unshelve_allowed(self): + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_unshelve_allowed(self, mock_instance_get): + mock_instance_get.return_value = self._get_instance_other_project() rules = {'compute:get': '', 'compute_extension:%sunshelve' % self.prefix: ''} policy.set_rules(oslo_policy.Rules.from_dict(rules)) - - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) self.assertRaises(exception.Forbidden, self.controller._unshelve, self.req, str(uuid.uuid4()), {}) - def test_shelve_offload_allowed(self): + @mock.patch('nova.objects.instance.Instance.get_by_uuid') + def test_shelve_offload_allowed(self, mock_instance_get): + mock_instance_get.return_value = self._get_instance_other_project() rules = {'compute:get': '', 'compute_extension:%s%s' % (self.prefix, self.offload): ''} policy.set_rules(oslo_policy.Rules.from_dict(rules)) - - self.stub_out('nova.db.instance_get_by_uuid', - fake_instance_get_by_uuid) self.assertRaises(exception.Forbidden, self.controller._shelve_offload, self.req,