diff --git a/ironic/common/context.py b/ironic/common/context.py index ccd2222ea4..8601dc1c54 100644 --- a/ironic/common/context.py +++ b/ironic/common/context.py @@ -21,24 +21,33 @@ class RequestContext(context.RequestContext): def __init__(self, auth_token=None, domain_id=None, domain_name=None, user=None, tenant=None, is_admin=False, is_public_api=False, read_only=False, show_deleted=False, request_id=None, - roles=None, show_password=True): - """Stores several additional request parameters: + roles=None, show_password=True, overwrite=True): + """Initialize the RequestContext + :param auth_token: The authentication token of the current request. :param domain_id: The ID of the domain. :param domain_name: The name of the domain. + :param user: The name of the user. + :param tenant: The name of the tenant. + :param is_admin: Indicates if the request context is an administrator. :param is_public_api: Specifies whether the request should be processed without authentication. + :param read_only: unused flag for Ironic. + :param show_deleted: unused flag for Ironic. + :param request_id: The UUID of the request. :param roles: List of user's roles if any. :param show_password: Specifies whether passwords should be masked before sending back to API call. - + :param overwrite: Set to False to ensure that the greenthread local + copy of the index is not overwritten. """ super(RequestContext, self).__init__(auth_token=auth_token, user=user, tenant=tenant, is_admin=is_admin, read_only=read_only, show_deleted=show_deleted, - request_id=request_id) + request_id=request_id, + overwrite=overwrite) self.is_public_api = is_public_api self.domain_id = domain_id self.domain_name = domain_name @@ -67,3 +76,26 @@ class RequestContext(context.RequestContext): values.pop('user', None) values.pop('tenant', None) return cls(**values) + + def ensure_thread_contain_context(self): + """Ensure threading contains context + + For async/periodic tasks, the context of local thread is missing. + Set it with request context and this is useful to log the request_id + in log messages. + + """ + if context.get_current(): + return + self.update_store() + + +def get_admin_context(): + """Create an administrator context. + + """ + context = RequestContext(None, + tenant=None, + is_admin=True, + overwrite=False) + return context diff --git a/ironic/common/service.py b/ironic/common/service.py index cfd8a04543..2a82b7bced 100644 --- a/ironic/common/service.py +++ b/ironic/common/service.py @@ -19,7 +19,6 @@ import socket from oslo_concurrency import processutils from oslo_config import cfg -from oslo_context import context from oslo_log import log import oslo_messaging as messaging from oslo_service import service @@ -28,6 +27,7 @@ from oslo_utils import importutils from ironic.api import app from ironic.common import config +from ironic.common import context from ironic.common import exception from ironic.common.i18n import _ from ironic.common.i18n import _LE diff --git a/ironic/conductor/base_manager.py b/ironic/conductor/base_manager.py index 4452fa1be2..361cf0904a 100644 --- a/ironic/conductor/base_manager.py +++ b/ironic/conductor/base_manager.py @@ -19,11 +19,11 @@ import futurist from futurist import periodics from futurist import rejection from oslo_config import cfg -from oslo_context import context as ironic_context from oslo_db import exception as db_exception from oslo_log import log from oslo_utils import excutils +from ironic.common import context as ironic_context from ironic.common import driver_factory from ironic.common import exception from ironic.common import hash_ring as hash diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index 3aed594ae0..fa63b9c12c 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -96,7 +96,6 @@ raised in the background thread.): import futurist from oslo_config import cfg -from oslo_context import context as oslo_context from oslo_log import log as logging from oslo_utils import excutils from oslo_utils import timeutils @@ -137,7 +136,7 @@ def require_exclusive_lock(f): raise exception.ExclusiveLockRequired() # NOTE(lintan): This is a workaround to set the context of async tasks, # which should contain an exclusive lock. - ensure_thread_contain_context(task.context) + task.context.ensure_thread_contain_context() return f(*args, **kwargs) return wrapper @@ -156,25 +155,11 @@ def acquire(context, node_id, shared=False, driver_name=None, """ # NOTE(lintan): This is a workaround to set the context of periodic tasks. - ensure_thread_contain_context(context) + context.ensure_thread_contain_context() return TaskManager(context, node_id, shared=shared, driver_name=driver_name, purpose=purpose) -def ensure_thread_contain_context(context): - """Ensure threading contains context - - For async/periodic tasks, the context of local thread is missing. - Set it with request context and this is useful to log the request_id - in log messages. - - :param context: Request context - """ - if oslo_context.get_current(): - return - context.update_store() - - class TaskManager(object): """Context manager for tasks. diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index e89083aad1..294bbf2b43 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -151,6 +151,7 @@ def _call_inspector(func, uuid, context): def _start_inspection(node_uuid, context): """Call to inspector to start inspection.""" + context.ensure_thread_contain_context() try: _call_inspector(client.introspect, node_uuid, context) except Exception as exc: diff --git a/ironic/tests/base.py b/ironic/tests/base.py index f6a89a5fe7..cb25178c3c 100644 --- a/ironic/tests/base.py +++ b/ironic/tests/base.py @@ -31,11 +31,11 @@ eventlet.monkey_patch(os=False) import fixtures from oslo_config import cfg from oslo_config import fixture as config_fixture -from oslo_context import context as ironic_context from oslo_log import log as logging import testtools from ironic.common import config as ironic_config +from ironic.common import context as ironic_context from ironic.common import hash_ring from ironic.objects import base as objects_base from ironic.tests.unit import policy_fixture diff --git a/ironic/tests/unit/common/test_context.py b/ironic/tests/unit/common/test_context.py new file mode 100644 index 0000000000..a20e01ada3 --- /dev/null +++ b/ironic/tests/unit/common/test_context.py @@ -0,0 +1,117 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock +from oslo_context import context as oslo_context + +from ironic.common import context +from ironic.tests import base as tests_base + + +class RequestContextTestCase(tests_base.TestCase): + def setUp(self): + super(RequestContextTestCase, self).setUp() + + @mock.patch.object(oslo_context.RequestContext, "__init__") + def test_create_context(self, context_mock): + test_context = context.RequestContext() + context_mock.assert_called_once_with( + auth_token=None, user=None, tenant=None, is_admin=False, + read_only=False, show_deleted=False, request_id=None, + overwrite=True) + self.assertFalse(test_context.is_public_api) + self.assertIsNone(test_context.domain_id) + self.assertIsNone(test_context.domain_name) + self.assertTrue(test_context.show_password) + self.assertEqual([], test_context.roles) + + def test_from_dict(self): + dict = { + "user": "user1", + "tenant": "tenant1", + "is_public_api": True, + "domain_id": "domain_id1", + "domain_name": "domain_name1", + "show_password": False, + "roles": None + } + ctx = context.RequestContext.from_dict(dict) + self.assertIsNone(ctx.user) + self.assertIsNone(ctx.tenant) + self.assertTrue(ctx.is_public_api) + self.assertEqual("domain_id1", ctx.domain_id) + self.assertEqual("domain_name1", ctx.domain_name) + self.assertFalse(ctx.show_password) + self.assertEqual([], ctx.roles) + + def test_to_dict(self): + values = { + 'auth_token': 'auth_token1', + "user": "user1", + "tenant": "tenant1", + 'is_admin': True, + 'read_only': True, + 'show_deleted': True, + 'request_id': 'id1', + "is_public_api": True, + "domain_id": "domain_id1", + "domain_name": "domain_name1", + "show_password": False, + "roles": None, + "overwrite": True + } + ctx = context.RequestContext(**values) + ctx_dict = ctx.to_dict() + self.assertIn('auth_token', ctx_dict) + self.assertIn('user', ctx_dict) + self.assertIn('tenant', ctx_dict) + self.assertIn('is_admin', ctx_dict) + self.assertIn('read_only', ctx_dict) + self.assertIn('show_deleted', ctx_dict) + self.assertIn('request_id', ctx_dict) + self.assertIn('domain_id', ctx_dict) + self.assertIn('roles', ctx_dict) + self.assertIn('domain_name', ctx_dict) + self.assertIn('show_password', ctx_dict) + self.assertIn('is_public_api', ctx_dict) + self.assertNotIn('overwrite', ctx_dict) + + self.assertEqual('auth_token1', ctx_dict['auth_token']) + self.assertEqual('user1', ctx_dict['user']) + self.assertEqual('tenant1', ctx_dict['tenant']) + self.assertTrue(ctx_dict['is_admin']) + self.assertTrue(ctx_dict['read_only']) + self.assertTrue(ctx_dict['show_deleted']) + self.assertEqual('id1', ctx_dict['request_id']) + self.assertTrue(ctx_dict['is_public_api']) + self.assertEqual('domain_id1', ctx_dict['domain_id']) + self.assertEqual('domain_name1', ctx_dict['domain_name']) + self.assertFalse(ctx_dict['show_password']) + self.assertEqual([], ctx_dict['roles']) + + def test_get_admin_context(self): + admin_context = context.get_admin_context() + self.assertTrue(admin_context.is_admin) + + @mock.patch.object(oslo_context, 'get_current') + def test_thread_without_context(self, context_get_mock): + self.context.update_store = mock.Mock() + context_get_mock.return_value = None + self.context.ensure_thread_contain_context() + self.context.update_store.assert_called_once_with() + + @mock.patch.object(oslo_context, 'get_current') + def test_thread_with_context(self, context_get_mock): + self.context.update_store = mock.Mock() + context_get_mock.return_value = self.context + self.context.ensure_thread_contain_context() + self.assertFalse(self.context.update_store.called) diff --git a/ironic/tests/unit/common/test_glance_service.py b/ironic/tests/unit/common/test_glance_service.py index 30f6cfb658..0490c05e26 100644 --- a/ironic/tests/unit/common/test_glance_service.py +++ b/ironic/tests/unit/common/test_glance_service.py @@ -21,12 +21,12 @@ from glanceclient import client as glance_client from glanceclient import exc as glance_exc import mock from oslo_config import cfg -from oslo_context import context from oslo_serialization import jsonutils from oslo_utils import uuidutils from six.moves.urllib import parse as urlparse import testtools +from ironic.common import context from ironic.common import exception from ironic.common.glance_service import base_image_service from ironic.common.glance_service import service_utils diff --git a/ironic/tests/unit/conductor/test_task_manager.py b/ironic/tests/unit/conductor/test_task_manager.py index 3f42e4f8d7..529249affe 100644 --- a/ironic/tests/unit/conductor/test_task_manager.py +++ b/ironic/tests/unit/conductor/test_task_manager.py @@ -19,10 +19,8 @@ import futurist import mock -from oslo_context import context as oslo_context from oslo_utils import uuidutils -from ironic.common import context from ironic.common import driver_factory from ironic.common import exception from ironic.common import fsm @@ -739,20 +737,3 @@ class ThreadExceptionTestCase(tests_base.TestCase): self.future_mock.exception.assert_called_once_with() self.assertIsNone(self.node.last_error) self.assertTrue(log_mock.called) - - -@mock.patch.object(oslo_context, 'get_current') -class TaskManagerContextTestCase(tests_base.TestCase): - def setUp(self): - super(TaskManagerContextTestCase, self).setUp() - self.context = mock.Mock(spec=context.RequestContext) - - def test_thread_without_context(self, context_get_mock): - context_get_mock.return_value = False - task_manager.ensure_thread_contain_context(self.context) - self.assertTrue(self.context.update_store.called) - - def test_thread_with_context(self, context_get_mock): - context_get_mock.return_value = True - task_manager.ensure_thread_contain_context(self.context) - self.assertFalse(self.context.update_store.called) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 81f4a5aa9c..574492e5e5 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -294,7 +294,7 @@ class CleanupAfterTimeoutTestCase(tests_base.TestCase): def setUp(self): super(CleanupAfterTimeoutTestCase, self).setUp() self.task = mock.Mock(spec=task_manager.TaskManager) - self.task.context = mock.sentinel.context + self.task.context = self.context self.task.driver = mock.Mock(spec_set=['deploy']) self.task.shared = False self.task.node = mock.Mock(spec_set=objects.Node) diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index 9f5d5e6786..8504fa73fe 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -67,7 +67,7 @@ class BaseTestCase(db_base.DbTestCase): self.driver = driver_factory.get_driver("fake_inspector") self.node = obj_utils.get_test_node(self.context) self.task = mock.MagicMock(spec=task_manager.TaskManager) - self.task.context = mock.MagicMock(spec_set=['auth_token']) + self.task.context = self.context self.task.shared = False self.task.node = self.node self.task.driver = self.driver diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 391f6ca40a..991f94244e 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -18,12 +18,12 @@ import gettext import iso8601 import mock -from oslo_context import context from oslo_versionedobjects import base as object_base from oslo_versionedobjects import exception as object_exception from oslo_versionedobjects import fixture as object_fixture import six +from ironic.common import context from ironic.objects import base from ironic.objects import fields from ironic.tests import base as test_base diff --git a/releasenotes/notes/adopt-ironic-context-5e75540dc2b2f009.yaml b/releasenotes/notes/adopt-ironic-context-5e75540dc2b2f009.yaml new file mode 100644 index 0000000000..cde8e70fca --- /dev/null +++ b/releasenotes/notes/adopt-ironic-context-5e75540dc2b2f009.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixes a bug where Ironic won't log the request-id during + hardware inspecting.