openstack/ironic
Code Issues Proposed changes

Get admin auth token for Glance client in image_service

Browse Source 
In some cases we do not have auth_token in context, but Glance client
requires it for authentication. This patch moves usage of Keystone
to image_service and adds check for Glance auth strategy. This
changes fix problems with Ironic standalone mode and reduces quantity
of Keystone API calls.

Change-Id: I5c625db6f30a76da10d72c4d429d3f3cbfd6f432
This commit is contained in:
Yuriy Zveryanskyy 2015-05-27 20:42:45 +03:00
parent 8b677de0e8
commit b0840884a1
8 changed files with 53 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
ironic/common/image_service.py
View File

@ -29,6 +29,7 @@ import six.moves.urllib.parse as urlparse
from ironic.common import exception
from ironic.common.i18n import _
from ironic.common import keystone
LOG = logging.getLogger(__name__)
@ -83,6 +84,9 @@ def import_versioned_module(version, submodule=None):
def GlanceImageService(client=None, version=1, context=None):
module = import_versioned_module(version, 'image_service')
service_class = getattr(module, 'GlanceImageService')
if (context is not None and CONF.glance.auth_strategy == 'keystone'
and not context.auth_token):
context.auth_token = keystone.get_admin_auth_token()
return service_class(client, version, context)

12
ironic/conductor/manager.py
View File

@ -51,7 +51,6 @@ import eventlet
from eventlet import greenpool
from oslo_concurrency import lockutils
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
import oslo_messaging as messaging
@ -69,7 +68,6 @@ from ironic.common.i18n import _LE
from ironic.common.i18n import _LI
from ironic.common.i18n import _LW
from ironic.common import images
from ironic.common import keystone
from ironic.common import rpc
from ironic.common import states
from ironic.common import swift
@ -1150,22 +1148,14 @@ class ConductorManager(periodic_task.PeriodicTasks):
node_iter = self.iter_nodes(fields=['id', 'conductor_affinity'],
filters=filters)
admin_context = None
workers_count = 0
for node_uuid, driver, node_id, conductor_affinity in node_iter:
if conductor_affinity == self.conductor.id:
continue
# NOTE(lucasagomes): The context provided by the periodic task
# will make the glance client to fail with an 401 (Unauthorized)
# so we have to use the admin_context with an admin auth_token
if not admin_context:
admin_context = ironic_context.get_admin_context()
admin_context.auth_token = keystone.get_admin_auth_token()
# Node is mapped here, but not updated by this conductor last
try:
with task_manager.acquire(admin_context, node_uuid) as task:
with task_manager.acquire(context, node_uuid) as task:
# NOTE(deva): now that we have the lock, check again to
# avoid racing with deletes and other state changes
node = task.node

4
ironic/drivers/modules/ilo/deploy.py
View File

@ -31,7 +31,6 @@ from ironic.common.i18n import _LI
from ironic.common.i18n import _LW
from ironic.common import image_service
from ironic.common import images
from ironic.common import keystone
from ironic.common import states
from ironic.common import swift
from ironic.conductor import task_manager
@ -870,9 +869,6 @@ class VendorPassthru(agent_base_vendor.BaseAgentVendor):
task, root_uuid=root_uuid,
efi_system_part_uuid=efi_system_part_uuid)
else:
# Agent vendorpassthru are made without auth token.
# We require auth_token to talk to glance while building boot iso.
task.context.auth_token = keystone.get_admin_auth_token()
self._configure_vmedia_boot(task, root_uuid)
# Set boot mode

41
ironic/tests/conductor/test_manager.py
View File

@ -23,7 +23,6 @@ import datetime
import eventlet
import mock
from oslo_config import cfg
from oslo_context import context
from oslo_db import exception as db_exception
import oslo_messaging as messaging
from oslo_utils import strutils
@ -33,7 +32,6 @@ from ironic.common import boot_devices
from ironic.common import driver_factory
from ironic.common import exception
from ironic.common import images
from ironic.common import keystone
from ironic.common import states
from ironic.common import swift
from ironic.conductor import manager
@ -3395,7 +3393,6 @@ class ManagerTestProperties(tests_db_base.DbTestCase):
self.assertEqual(exception.DriverNotFound, exc.exc_info[0])
@mock.patch.object(keystone, 'get_admin_auth_token')
@mock.patch.object(task_manager, 'acquire')
@mock.patch.object(manager.ConductorManager, '_mapped_to_this_conductor')
@mock.patch.object(dbapi.IMPL, 'get_nodeinfo_list')
@ -3423,8 +3420,7 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
get_nodeinfo_mock.assert_called_once_with(
columns=self.columns, filters=self.filters)
def test_not_mapped(self, get_nodeinfo_mock, mapped_mock, acquire_mock,
get_authtoken_mock):
def test_not_mapped(self, get_nodeinfo_mock, mapped_mock, acquire_mock):
get_nodeinfo_mock.return_value = self._get_nodeinfo_list_response()
mapped_mock.return_value = False
@ -3433,11 +3429,10 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
self._assert_get_nodeinfo_args(get_nodeinfo_mock)
mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
self.assertFalse(acquire_mock.called)
self.assertFalse(get_authtoken_mock.called)
self.service.ring_manager.reset.assert_called_once_with()
def test_already_mapped(self, get_nodeinfo_mock, mapped_mock,
acquire_mock, get_authtoken_mock):
acquire_mock):
# Node is already mapped to the conductor running the periodic task
self.node.conductor_affinity = 123
self.service.conductor.id = 123
@ -3450,13 +3445,9 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
self._assert_get_nodeinfo_args(get_nodeinfo_mock)
mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
self.assertFalse(acquire_mock.called)
self.assertFalse(get_authtoken_mock.called)
self.service.ring_manager.reset.assert_called_once_with()
@mock.patch.object(context, 'get_admin_context')
def test_good(self, get_ctx_mock, get_nodeinfo_mock, mapped_mock,
acquire_mock, get_authtoken_mock):
get_ctx_mock.return_value = self.context
def test_good(self, get_nodeinfo_mock, mapped_mock, acquire_mock):
get_nodeinfo_mock.return_value = self._get_nodeinfo_list_response()
mapped_mock.return_value = True
acquire_mock.side_effect = self._get_acquire_side_effect(self.task)
@ -3465,17 +3456,14 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
self._assert_get_nodeinfo_args(get_nodeinfo_mock)
mapped_mock.assert_called_once_with(self.node.uuid, self.node.driver)
get_authtoken_mock.assert_called_once_with()
acquire_mock.assert_called_once_with(self.context, self.node.uuid)
# assert spawn_after has been called
self.task.spawn_after.assert_called_once_with(
self.service._spawn_worker,
self.service._do_takeover, self.task)
@mock.patch.object(context, 'get_admin_context')
def test_no_free_worker(self, get_ctx_mock, get_nodeinfo_mock, mapped_mock,
acquire_mock, get_authtoken_mock):
get_ctx_mock.return_value = self.context
def test_no_free_worker(self, get_nodeinfo_mock, mapped_mock,
acquire_mock):
mapped_mock.return_value = True
acquire_mock.side_effect = self._get_acquire_side_effect(
[self.task] * 3)
@ -3503,18 +3491,12 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
expected = [mock.call(self.context, self.node.uuid)] * 2
self.assertEqual(expected, acquire_mock.call_args_list)
# Only one auth token needed for all runs
get_authtoken_mock.assert_called_once_with()
# assert spawn_after has been called twice
expected = [mock.call(self.service._spawn_worker,
self.service._do_takeover, self.task)] * 2
self.assertEqual(expected, self.task.spawn_after.call_args_list)
@mock.patch.object(context, 'get_admin_context')
def test_node_locked(self, get_ctx_mock, get_nodeinfo_mock, mapped_mock,
acquire_mock, get_authtoken_mock):
get_ctx_mock.return_value = self.context
def test_node_locked(self, get_nodeinfo_mock, mapped_mock, acquire_mock,):
mapped_mock.return_value = True
acquire_mock.side_effect = self._get_acquire_side_effect(
[self.task, exception.NodeLocked('error'), self.task])
@ -3536,20 +3518,14 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
expected = [mock.call(self.context, self.node.uuid)] * 3
self.assertEqual(expected, acquire_mock.call_args_list)
# Only one auth token needed for all runs
get_authtoken_mock.assert_called_once_with()
# assert spawn_after has been called only 2 times
expected = [mock.call(self.service._spawn_worker,
self.service._do_takeover, self.task)] * 2
self.assertEqual(expected, self.task.spawn_after.call_args_list)
@mock.patch.object(context, 'get_admin_context')
def test_worker_limit(self, get_ctx_mock, get_nodeinfo_mock, mapped_mock,
acquire_mock, get_authtoken_mock):
def test_worker_limit(self, get_nodeinfo_mock, mapped_mock, acquire_mock):
# Limit to only 1 worker
self.config(periodic_max_workers=1, group='conductor')
get_ctx_mock.return_value = self.context
mapped_mock.return_value = True
acquire_mock.side_effect = self._get_acquire_side_effect(
[self.task] * 3)
@ -3570,9 +3546,6 @@ class ManagerSyncLocalStateTestCase(_CommonMixIn, tests_db_base.DbTestCase):
# assert acquire() gets called only once because of the worker limit
acquire_mock.assert_called_once_with(self.context, self.node.uuid)
# Only one auth token needed for all runs
get_authtoken_mock.assert_called_once_with()
# assert spawn_after has been called
self.task.spawn_after.assert_called_once_with(
self.service._spawn_worker,

6
ironic/tests/drivers/ilo/test_deploy.py
View File

@ -26,7 +26,6 @@ from ironic.common import exception
from ironic.common.glance_service import service_utils
from ironic.common import image_service
from ironic.common import images
from ironic.common import keystone
from ironic.common import states
from ironic.common import swift
from ironic.conductor import task_manager
@ -990,7 +989,6 @@ class VendorPassthruTestCase(db_base.DbTestCase):
@mock.patch.object(ilo_deploy, '_update_secure_boot_mode', autospec=True)
@mock.patch.object(ilo_common, 'update_boot_mode', autospec=True)
@mock.patch.object(keystone, 'get_admin_auth_token', autospec=True)
@mock.patch.object(agent_base_vendor.BaseAgentVendor,
'reboot_and_finish_deploy', autospec=True)
@mock.patch.object(ilo_deploy.VendorPassthru, '_configure_vmedia_boot',
@ -1001,7 +999,6 @@ class VendorPassthruTestCase(db_base.DbTestCase):
do_agent_iscsi_deploy_mock,
configure_vmedia_boot_mock,
reboot_and_finish_deploy_mock,
keystone_mock,
boot_mode_cap_mock,
update_secure_boot_mock):
self.node.provision_state = states.DEPLOYWAIT
@ -1009,7 +1006,6 @@ class VendorPassthruTestCase(db_base.DbTestCase):
self.node.save()
do_agent_iscsi_deploy_mock.return_value = {
'root uuid': 'some-root-uuid'}
keystone_mock.return_value = 'admin-token'
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.vendor.continue_deploy(task)
@ -1022,8 +1018,6 @@ class VendorPassthruTestCase(db_base.DbTestCase):
update_secure_boot_mock.assert_called_once_with(task, True)
reboot_and_finish_deploy_mock.assert_called_once_with(
mock.ANY, task)
# Ensure that admin token is populated in task
self.assertEqual('admin-token', task.context.auth_token)
@mock.patch.object(ilo_deploy, '_update_secure_boot_mode', autospec=True)
@mock.patch.object(ilo_common, 'update_boot_mode', autospec=True)

1
ironic/tests/drivers/test_pxe.py
View File

@ -151,6 +151,7 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase):
self.node.uuid,
'deploy_kernel'))}
show_mock.return_value = properties
self.context.auth_token = 'fake'
image_info = pxe._get_image_info(self.node, self.context)
show_mock.assert_called_once_with(mock.ANY, 'glance://image_uuid',
method='get')

1
ironic/tests/test_glance_service.py
View File

@ -646,6 +646,7 @@ class TestGlanceSwiftTempURL(base.TestCase):
super(TestGlanceSwiftTempURL, self).setUp()
client = stubs.StubGlanceClient()
self.context = context.RequestContext()
self.context.auth_token = 'fake'
self.service = service.GlanceImageService(client, 2, self.context)
self.config(swift_temp_url_key='correcthorsebatterystaple',
group='glance')

41
ironic/tests/test_image_service.py
View File

@ -22,6 +22,7 @@ import six.moves.builtins as __builtin__
from ironic.common import exception
from ironic.common.glance_service.v1 import image_service as glance_v1_service
from ironic.common import image_service
from ironic.common import keystone
from ironic.tests import base
if six.PY3:
@ -227,21 +228,57 @@ class FileImageServiceTestCase(base.TestCase):
class ServiceGetterTestCase(base.TestCase):
@mock.patch.object(keystone, 'get_admin_auth_token', autospec=True)
@mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
return_value=None, autospec=True)
def test_get_glance_image_service(self, glance_service_mock):
def test_get_glance_image_service(self, glance_service_mock, token_mock):
image_href = 'image-uuid'
self.context.auth_token = 'fake'
image_service.get_image_service(image_href, context=self.context)
glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
self.context)
self.assertFalse(token_mock.called)
@mock.patch.object(keystone, 'get_admin_auth_token', autospec=True)
@mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
return_value=None, autospec=True)
def test_get_glance_image_service_url(self, glance_service_mock):
def test_get_glance_image_service_url(self, glance_service_mock,
token_mock):
image_href = 'glance://image-uuid'
self.context.auth_token = 'fake'
image_service.get_image_service(image_href, context=self.context)
glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
self.context)
self.assertFalse(token_mock.called)
@mock.patch.object(keystone, 'get_admin_auth_token', autospec=True)
@mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
return_value=None, autospec=True)
def test_get_glance_image_service_no_token(self, glance_service_mock,
token_mock):
image_href = 'image-uuid'
self.context.auth_token = None
token_mock.return_value = 'admin-token'
image_service.get_image_service(image_href, context=self.context)
glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
self.context)
token_mock.assert_called_once_with()
self.assertEqual('admin-token', self.context.auth_token)
@mock.patch.object(keystone, 'get_admin_auth_token', autospec=True)
@mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
return_value=None, autospec=True)
def test_get_glance_image_service_token_not_needed(self,
glance_service_mock,
token_mock):
image_href = 'image-uuid'
self.context.auth_token = None
self.config(auth_strategy='noauth', group='glance')
image_service.get_image_service(image_href, context=self.context)
glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
self.context)
self.assertFalse(token_mock.called)
self.assertIsNone(self.context.auth_token)
@mock.patch.object(image_service.HttpImageService, '__init__',
return_value=None, autospec=True)