Merge "Get admin auth token for Glance client in image_service"

8 files changed, 53 insertions, 57 deletions

diff --git a/ironic/common/image_service.py b/ironic/common/image_service.py
index 428f971f1..9086beb83 100644
--- a/ironic/common/image_service.py
+++ b/ironic/common/image_service.py
@@ -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)
 
 
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 31b031c72..f437991cb 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -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
diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py
index d5f905ce0..b97a02949 100644
--- a/ironic/drivers/modules/ilo/deploy.py
+++ b/ironic/drivers/modules/ilo/deploy.py
@@ -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
diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py
index 381079385..c003aea2c 100644
--- a/ironic/tests/conductor/test_manager.py
+++ b/ironic/tests/conductor/test_manager.py
@@ -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,
diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py
index 053a76228..bf86419bc 100644
--- a/ironic/tests/drivers/ilo/test_deploy.py
+++ b/ironic/tests/drivers/ilo/test_deploy.py
@@ -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)
diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py
index 65a303be3..fc26cd223 100644
--- a/ironic/tests/drivers/test_pxe.py
+++ b/ironic/tests/drivers/test_pxe.py
@@ -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')
diff --git a/ironic/tests/test_glance_service.py b/ironic/tests/test_glance_service.py
index 3bde77d1d..498409d04 100644
--- a/ironic/tests/test_glance_service.py
+++ b/ironic/tests/test_glance_service.py
@@ -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')
diff --git a/ironic/tests/test_image_service.py b/ironic/tests/test_image_service.py
index dac077341..b887cbc55 100644
--- a/ironic/tests/test_image_service.py
+++ b/ironic/tests/test_image_service.py
@@ -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)