diff --git a/cinder/context.py b/cinder/context.py index ca26768c85d..5fec65974d5 100644 --- a/cinder/context.py +++ b/cinder/context.py @@ -21,6 +21,7 @@ import copy from oslo_config import cfg from oslo_context import context +from oslo_db.sqlalchemy import enginefacade from oslo_log import log as logging from oslo_utils import timeutils import six @@ -45,6 +46,7 @@ CONF.register_opts(context_opts) LOG = logging.getLogger(__name__) +@enginefacade.transaction_context_provider class RequestContext(context.RequestContext): """Security context and request information. diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index c464c2080ca..6cb2d32b6df 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -25,7 +25,6 @@ import functools import itertools import re import sys -import threading import time import uuid @@ -33,7 +32,6 @@ from oslo_config import cfg from oslo_db import exception as db_exc from oslo_db import options from oslo_db.sqlalchemy import enginefacade -from oslo_db.sqlalchemy import session as db_session from oslo_log import log as logging from oslo_utils import importutils from oslo_utils import timeutils @@ -69,42 +67,30 @@ LOG = logging.getLogger(__name__) options.set_defaults(CONF, connection='sqlite:///$state_path/cinder.sqlite') -_LOCK = threading.Lock() -_FACADE = None +main_context_manager = enginefacade.transaction_context() -def _create_facade_lazily(): - global _LOCK - with _LOCK: - global _FACADE - if _FACADE is None: - _FACADE = db_session.EngineFacade( - CONF.database.connection, - **dict(CONF.database) - ) - - # NOTE(geguileo): To avoid a cyclical dependency we import the - # group here. Dependency cycle is objects.base requires db.api, - # which requires db.sqlalchemy.api, which requires service which - # requires objects.base - CONF.import_group("profiler", "cinder.service") - if CONF.profiler.enabled: - if CONF.profiler.trace_sqlalchemy: - osprofiler_sqlalchemy.add_tracing(sqlalchemy, - _FACADE.get_engine(), - "db") - - return _FACADE +def configure(conf): + main_context_manager.configure(**dict(conf.database)) + # NOTE(geguileo): To avoid a cyclical dependency we import the + # group here. Dependency cycle is objects.base requires db.api, + # which requires db.sqlalchemy.api, which requires service which + # requires objects.base + CONF.import_group("profiler", "cinder.service") + if CONF.profiler.enabled: + if CONF.profiler.trace_sqlalchemy: + lambda eng: osprofiler_sqlalchemy.add_tracing(sqlalchemy, + eng, "db") -def get_engine(): - facade = _create_facade_lazily() - return facade.get_engine() +def get_engine(use_slave=False): + return main_context_manager._factory.get_legacy_facade().get_engine( + use_slave=use_slave) -def get_session(**kwargs): - facade = _create_facade_lazily() - return facade.get_session(**kwargs) +def get_session(use_slave=False, **kwargs): + return main_context_manager._factory.get_legacy_facade().get_session( + use_slave=use_slave, **kwargs) def dispose_engine(): @@ -275,11 +261,12 @@ def handle_db_data_error(f): def model_query(context, model, *args, **kwargs): """Query helper that accounts for context's `read_deleted` field. - :param context: context to query under - :param session: if present, the session to use + :param context: context to query under + :param model: Model to query. Must be a subclass of ModelBase. + :param args: Arguments to query. If None - model is used. :param read_deleted: if present, overrides context's read_deleted field. :param project_only: if present and context is user-type, then restrict - query to match the context's project_id. + query to match the context's project_id. """ session = kwargs.get('session') or get_session() read_deleted = kwargs.get('read_deleted') or context.read_deleted diff --git a/cinder/test.py b/cinder/test.py index ea27736cb60..f1f1befdca6 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -57,6 +57,7 @@ from cinder.volume import utils CONF = cfg.CONF _DB_CACHE = None +SESSION_CONFIGURED = False class TestingException(Exception): @@ -66,6 +67,12 @@ class TestingException(Exception): class Database(fixtures.Fixture): def __init__(self, db_api, db_migrate, sql_connection): + # NOTE(lhx_): oslo_db.enginefacade is configured in tests the same + # way as it's done for any other services that uses the db + global SESSION_CONFIGURED + if not SESSION_CONFIGURED: + sqla_api.configure(CONF) + SESSION_CONFIGURED = True self.sql_connection = sql_connection # Suppress logging for test runs diff --git a/cinder/tests/unit/objects/test_base.py b/cinder/tests/unit/objects/test_base.py index 2b7bdc9cf29..c5d6957f735 100644 --- a/cinder/tests/unit/objects/test_base.py +++ b/cinder/tests/unit/objects/test_base.py @@ -719,15 +719,6 @@ class TestCinderObjectConditionalUpdate(test.TestCase): {objects.Backup.model.status: 'available', objects.Snapshot.model.status: 'available'}) - def test_conditional_update_not_multitable(self): - volume = self._create_volume() - with mock.patch('cinder.db.sqlalchemy.api._create_facade_lazily') as m: - res = volume.conditional_update( - {objects.Volume.model.status: 'deleting', - objects.Volume.model.size: 12}, reflect_changes=False) - self.assertTrue(res) - self.assertTrue(m.called) - @ddt.data(('available', 'error', None), ('error', 'rolling_back', [{'fake_filter': 'faked'}])) @ddt.unpack diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 2f5315cfd06..bd23b0ae8db 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -3133,6 +3133,56 @@ class DBAPIGenericTestCase(BaseTest): self.assertTrue(res, msg="Admin cannot find the Snapshot") +class EngineFacadeTestCase(BaseTest): + + """Tests for message operations""" + def setUp(self): + super(EngineFacadeTestCase, self).setUp() + self.user_id = fake.USER_ID + self.project_id = fake.PROJECT_ID + self.context = context.RequestContext(self.user_id, self.project_id) + + @mock.patch.object(sqlalchemy_api, 'get_session') + def test_use_single_context_session_writer(self, mock_get_session): + # Checks that session in context would not be overwritten by + # annotation @sqlalchemy_api.main_context_manager.writer if annotation + # is used twice. + + @sqlalchemy_api.main_context_manager.writer + def fake_parent_method(context): + session = context.session + return fake_child_method(context), session + + @sqlalchemy_api.main_context_manager.writer + def fake_child_method(context): + session = context.session + sqlalchemy_api.model_query(context, models.Volume) + return session + + parent_session, child_session = fake_parent_method(self.context) + self.assertEqual(parent_session, child_session) + + @mock.patch.object(sqlalchemy_api, 'get_session') + def test_use_single_context_session_reader(self, mock_get_session): + # Checks that session in context would not be overwritten by + # annotation @sqlalchemy_api.main_context_manager.reader if annotation + # is used twice. + + @sqlalchemy_api.main_context_manager.reader + def fake_parent_method(context): + session = context.session + return fake_child_method(context), session + + @sqlalchemy_api.main_context_manager.reader + def fake_child_method(context): + session = context.session + sqlalchemy_api.model_query(context, models.Volume) + return session + + parent_session, child_session = fake_parent_method(self.context) + self.assertEqual(parent_session, child_session) + + @ddt.ddt class DBAPIBackendTestCase(BaseTest): @ddt.data((True, True), (True, False), (False, True), (False, False))