From 4d7148709c5de098141fbee12ad2e78c61e3b174 Mon Sep 17 00:00:00 2001 From: lixipeng Date: Wed, 22 Nov 2017 12:03:58 +0800 Subject: [PATCH] Fix bug case by none token context When set reclaim_instance_interval > 0, and then delete an instance which booted from volume with `delete_on_termination` set as true. After reclaim_instance_interval time pass, all volumes boot instance will with state: attached and in-use, but attached instances was deleted. This bug case as admin context from `nova.compute.manager._reclaim_queued_deletes` did not have any token info, then call cinder api would be failed. So add user/project CONF with admin role at cinder group, and when determine context is_admin and without token, do authenticaion with user/project info to call cinder api. Conflicts: nova/volume/cinder.py nova/tests/unit/test_cinder.py NOTE(huanhongda): The conflict is due to not having change Ifc01dbf98545104c998ab96f65ff8623a6db0f28 in Pike. Change-Id: I3c35bba43fee81baebe8261f546c1424ce3a3383 Closes-Bug: #1733736 Closes-Bug: #1734025 Partial-Bug: #1736773 (cherry picked from commit ca6daf148debb9c9646fcf6db9660c830da5a594) --- nova/conf/cinder.py | 7 ++++- nova/tests/unit/test_cinder.py | 2 +- nova/tests/unit/volume/test_cinder.py | 19 ++++++++++++ nova/volume/cinder.py | 29 ++++++++++++++++++- ...g-cinder-admin-creds-b86038a3e87a1021.yaml | 19 ++++++++++++ 5 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/config-cinder-admin-creds-b86038a3e87a1021.yaml diff --git a/nova/conf/cinder.py b/nova/conf/cinder.py index 133e9de039ff..ef85c49a2b22 100644 --- a/nova/conf/cinder.py +++ b/nova/conf/cinder.py @@ -102,11 +102,16 @@ def register_opts(conf): conf.register_opts(cinder_opts, group=cinder_group) ks_loading.register_session_conf_options(conf, cinder_group.name) + ks_loading.register_auth_conf_options(conf, cinder_group.name) def list_opts(): return { cinder_group.name: ( cinder_opts + - ks_loading.get_session_conf_options()) + ks_loading.get_session_conf_options() + + ks_loading.get_auth_common_conf_options() + + ks_loading.get_auth_plugin_conf_options('password') + + ks_loading.get_auth_plugin_conf_options('v2password') + + ks_loading.get_auth_plugin_conf_options('v3password')) } diff --git a/nova/tests/unit/test_cinder.py b/nova/tests/unit/test_cinder.py index 8b08d8b0fce1..9bb379f019a2 100644 --- a/nova/tests/unit/test_cinder.py +++ b/nova/tests/unit/test_cinder.py @@ -127,7 +127,7 @@ class CinderV1TestCase(test.NoDBTestCase): get_endpoint = mock.Mock( return_value='http://localhost:8776/v1/%(project_id)s') fake_session = mock.Mock(get_endpoint=get_endpoint) - ctxt = context.get_admin_context() + ctxt = context.get_context() with mock.patch.object(cinder, '_SESSION', fake_session): self.assertRaises(exception.UnsupportedCinderAPIVersion, cinder.cinderclient, ctxt) diff --git a/nova/tests/unit/volume/test_cinder.py b/nova/tests/unit/volume/test_cinder.py index 914d853b7d19..c388e36eb3a0 100644 --- a/nova/tests/unit/volume/test_cinder.py +++ b/nova/tests/unit/volume/test_cinder.py @@ -15,6 +15,7 @@ from cinderclient import api_versions as cinder_api_versions from cinderclient import exceptions as cinder_exception +from keystoneauth1 import loading as ks_loading from keystoneclient import exceptions as keystone_exception import mock from oslo_utils import timeutils @@ -857,3 +858,21 @@ class CinderClientTestCase(test.NoDBTestCase): client.api_version) get_volume_api.assert_called_once_with( self.mock_session.get_endpoint.return_value) + + @mock.patch.object(ks_loading, 'load_auth_from_conf_options') + def test_load_auth_plugin_failed(self, mock_load_from_conf): + mock_load_from_conf.return_value = None + self.assertRaises(cinder_exception.Unauthorized, + cinder._load_auth_plugin, CONF) + + @mock.patch('cinderclient.client.get_volume_api_from_url') + @mock.patch('nova.volume.cinder._ADMIN_AUTH') + def test_admin_context_without_token(self, + mock_admin_auth, + mock_get_volume_api): + + mock_admin_auth.return_value = '_FAKE_ADMIN_AUTH' + mock_get_volume_api.return_value = '2' + admin_ctx = context.get_admin_context() + client = cinder.cinderclient(admin_ctx) + self.assertEqual(client.client.auth, mock_admin_auth) diff --git a/nova/volume/cinder.py b/nova/volume/cinder.py index e7a8ae47b592..6d461275fbb5 100644 --- a/nova/volume/cinder.py +++ b/nova/volume/cinder.py @@ -47,16 +47,31 @@ CONF = nova.conf.CONF LOG = logging.getLogger(__name__) +_ADMIN_AUTH = None _SESSION = None def reset_globals(): """Testing method to reset globals. """ + global _ADMIN_AUTH global _SESSION + + _ADMIN_AUTH = None _SESSION = None +def _load_auth_plugin(conf): + auth_plugin = ks_loading.load_auth_from_conf_options(conf, + nova.conf.cinder.cinder_group.name) + + if auth_plugin: + return auth_plugin + + err_msg = _('Unknown auth type: %s') % conf.cinder.auth_type + raise cinder_exception.Unauthorized(401, message=err_msg) + + def _check_microversion(url, microversion): """Checks to see if the requested microversion is supported by the current version of python-cinderclient and the volume API endpoint. @@ -92,16 +107,28 @@ def cinderclient(context, microversion=None, skip_version_check=False): is used directly. This should only be used if a previous check for the same microversion was successful. """ + global _ADMIN_AUTH global _SESSION if not _SESSION: _SESSION = ks_loading.load_session_from_conf_options( CONF, nova.conf.cinder.cinder_group.name) + # NOTE(lixipeng): Auth token is none when call + # cinder API from compute periodic tasks, context + # from them generated from 'context.get_admin_context' + # which only set is_admin=True but is without token. + # So add load_auth_plugin when this condition appear. + if context.is_admin and not context.auth_token: + if not _ADMIN_AUTH: + _ADMIN_AUTH = _load_auth_plugin(CONF) + auth = _ADMIN_AUTH + else: + auth = service_auth.get_auth_plugin(context) + url = None endpoint_override = None - auth = service_auth.get_auth_plugin(context) service_type, service_name, interface = CONF.cinder.catalog_info.split(':') service_parameters = {'service_type': service_type, diff --git a/releasenotes/notes/config-cinder-admin-creds-b86038a3e87a1021.yaml b/releasenotes/notes/config-cinder-admin-creds-b86038a3e87a1021.yaml new file mode 100644 index 000000000000..fabc9cd992ea --- /dev/null +++ b/releasenotes/notes/config-cinder-admin-creds-b86038a3e87a1021.yaml @@ -0,0 +1,19 @@ +--- +fixes: + - | + It is now possible to configure the ``[cinder]`` section of nova.conf to + allow setting admin-role credentials for scenarios where a user token is + not available to perform actions on a volume. For example, when + ``reclaim_instance_interval`` is a positive integer, instances are + soft deleted until the nova-compute service periodic task removes them. + If a soft deleted instance has volumes attached, the compute service needs + to be able to detach and possibly delete the associated volumes, otherwise + they will be orphaned in the block storage service. Similarly, if + ``running_deleted_instance_poll_interval`` is set and + ``running_deleted_instance_action = reap``, then the compute service will + need to be able to detach and possibly delete volumes attached to + instances that are reaped. See `bug 1733736`_ and `bug 1734025`_ for more + details. + + .. _bug 1733736: https://bugs.launchpad.net/nova/+bug/1733736 + .. _bug 1734025: https://bugs.launchpad.net/nova/+bug/1734025