Adopt Ironic's own context
Adopt Ironic's own context in Ironic and add tests. Refactor ensure_thread_contain_context to Ironic's own context class, this will be more generical and not bind to TaskManager anymore. Explicitly call ensure_thread_contain_context() in Inspector driver for inspect hardware action. Change-Id: Ic2bb16a2deb02054b4fca795d431c965e30a246f Closes-Bug: #1560264
This commit is contained in:
parent
0ad5b13b5a
commit
0607226fc4
@ -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
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
@ -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.
|
||||
|
||||
|
@ -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:
|
||||
|
@ -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
|
||||
|
117
ironic/tests/unit/common/test_context.py
Normal file
117
ironic/tests/unit/common/test_context.py
Normal file
@ -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)
|
@ -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
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
@ -0,0 +1,4 @@
|
||||
---
|
||||
fixes:
|
||||
- Fixes a bug where Ironic won't log the request-id during
|
||||
hardware inspecting.
|
Loading…
Reference in New Issue
Block a user