From d0f540742efc1004b4d2c64814dcc6f7b9f0ccf6 Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Wed, 30 Jan 2019 15:10:25 +0000 Subject: [PATCH] Eventlet monkey patching should be as early as possible We were seeing infinite recursion opening an ssl socket when running various combinations of python3, eventlet, and urllib3. It is not clear exactly what combination of versions are affected, but for background there is an example of this issue documented here: https://github.com/eventlet/eventlet/issues/371 The immediate cause in nova's case was that we were calling eventlet.monkey_patch() after importing urllib3. Specifically, change Ie7bf5d012e2ccbcd63c262ddaf739782afcdaf56 introduced the nova.utils.monkey_patch() method to make monkey patching common between WSGI and non-WSGI services. Unfortunately, before executing this method you must first import nova.utils, which imports a large number of modules itself. Anything imported (transitively) by nova.utils would therefore be imported before monkey patching, which included urllib3. This triggers the infinite recursion problem described above if you have an affected combination of library versions. While this specific issue may eventually be worked around or fixed in eventlet or urllib3, it remains true that eventlet best practises are to monkey patch as early as possible, which we were not doing. To avoid this and hopefully future similar issues, this change ensures that monkey patching happens as early as possible, and only a minimum number of modules are imported first. This change fixes monkey patching for both non-wsgi and wsgi callers: * Non-WSGI services (nova/cmd) This is fixed by using the new monkey_patch module, which has minimal dependencies. * WSGI services (nova/api/openstack) This is fixed both by using the new monkey_patch module, and by moving the patching point up one level so that it is done before importing anything in nova/api/openstack/__init__.py. This move causes issues for some external tools which load this path from nova and now monkey patch where they previously did not. However, it is unfortunately unavoidable to enable monkey patching for the wsgi entry point without major restructuring. This change includes a workaround for sphinx to avoid this issue. This change has been through several iterations. I started with what seemed like the simplest and most obvious change, and moved on as I discovered more interactions which broke. It is clear that eventlet monkey patching is extremely fragile, especially when done implicitly at module load time as we do. I would advocate a code restructure to improve this situation, but I think the time would be better spent removing the eventlet dependency entirely. Co-authored-by: Lee Yarwood Closes-Bug: #1808975 Closes-Bug: #1808951 Change-Id: Id46e76666b553a10ec4654d4418a9884975b5b95 (cherry picked from commit 3c5e2b0e9fac985294a949852bb8c83d4ed77e04) --- doc/source/conf.py | 8 +++ nova/__init__.py | 11 ---- nova/api/openstack/__init__.py | 1 + nova/api/openstack/wsgi_app.py | 2 - nova/cmd/__init__.py | 6 +- nova/monkey_patch.py | 97 +++++++++++++++++++++++++++++++ nova/test.py | 3 +- nova/tests/functional/__init__.py | 4 +- nova/utils.py | 21 ------- 9 files changed, 110 insertions(+), 43 deletions(-) create mode 100644 nova/monkey_patch.py diff --git a/doc/source/conf.py b/doc/source/conf.py index 628365d55b2c..1220e0f5cbf1 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -160,6 +160,14 @@ openstack_projects = [ ] # -- Custom extensions -------------------------------------------------------- +# NOTE(mdbooth): (2019-03-20) Sphinx loads policies defined in setup.cfg, which +# includes the placement policy at nova/api/openstack/placement/policies.py. +# Loading this imports nova/api/openstack/__init__.py, which imports +# nova.monkey_patch, which will do eventlet monkey patching to the sphinx +# process. As well as being unnecessary and a bad idea, this breaks on +# python3.6 (but not python3.7), so don't do that. +os.environ['OS_NOVA_DISABLE_EVENTLET_PATCHING'] = '1' + def monkey_patch_blockdiag(): """Monkey patch the blockdiag library. diff --git a/nova/__init__.py b/nova/__init__.py index 228c89d33dce..b8044faecee0 100644 --- a/nova/__init__.py +++ b/nova/__init__.py @@ -22,14 +22,3 @@ :platform: Unix :synopsis: Infrastructure-as-a-Service Cloud platform. """ - -import os - -os.environ['EVENTLET_NO_GREENDNS'] = 'yes' - -# NOTE(rpodolyaka): import oslo_service first, so that it makes eventlet hub -# use a monotonic clock to avoid issues with drifts of system time (see -# LP 1510234 for details) -import oslo_service # noqa - -import eventlet # noqa diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py index 13467c180236..087c447b2116 100644 --- a/nova/api/openstack/__init__.py +++ b/nova/api/openstack/__init__.py @@ -17,6 +17,7 @@ """ WSGI middleware for OpenStack API controllers. """ +import nova.monkey_patch # noqa from oslo_log import log as logging import routes diff --git a/nova/api/openstack/wsgi_app.py b/nova/api/openstack/wsgi_app.py index d09d28c47a0b..c789da51bb52 100644 --- a/nova/api/openstack/wsgi_app.py +++ b/nova/api/openstack/wsgi_app.py @@ -23,13 +23,11 @@ from nova import context from nova import exception from nova import objects from nova import service -from nova import utils CONF = cfg.CONF CONFIG_FILES = ['api-paste.ini', 'nova.conf'] -utils.monkey_patch() objects.register_all() diff --git a/nova/cmd/__init__.py b/nova/cmd/__init__.py index 8b0ed86cb74f..726e0c3da541 100644 --- a/nova/cmd/__init__.py +++ b/nova/cmd/__init__.py @@ -1,4 +1,4 @@ -# Copyright (c) 2013 Hewlett-Packard Development Company, L.P. +# Copyright (c) 2019 Red Hat, Inc. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -13,6 +13,4 @@ # License for the specific language governing permissions and limitations # under the License. -from nova import utils - -utils.monkey_patch() +import nova.monkey_patch # noqa diff --git a/nova/monkey_patch.py b/nova/monkey_patch.py new file mode 100644 index 000000000000..6d0cacd6c7d4 --- /dev/null +++ b/nova/monkey_patch.py @@ -0,0 +1,97 @@ +# Copyright 2010 United States Government as represented by the +# Administrator of the National Aeronautics and Space Administration. +# Copyright 2011 Justin Santa Barbara +# Copyright 2019 Red Hat, Inc. +# All Rights Reserved. +# +# 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. + +"""Enable eventlet monkey patching.""" + +import os + + +def _monkey_patch(): + # NOTE(mdbooth): Anything imported here will not be monkey patched. It is + # important to take care not to import anything here which requires monkey + # patching. + import eventlet + import sys + + # NOTE(mdbooth): Imports only sys (2019-01-30). Other modules imported at + # runtime on execution of debugger.init(). + from nova import debugger + + # Note any modules with known monkey-patching issues which have been + # imported before monkey patching. + # urllib3: https://bugs.launchpad.net/nova/+bug/1808951 + # oslo_context.context: https://bugs.launchpad.net/nova/+bug/1773102 + problems = (set(['urllib3', 'oslo_context.context']) & + set(sys.modules.keys())) + + # See https://bugs.launchpad.net/nova/+bug/1164822 + # TODO(mdbooth): This feature was deprecated and removed in eventlet at + # some point but brought back in version 0.21.0, presumably because some + # users still required it to work round issues. However, there have been a + # number of greendns fixes in eventlet since then. Specifically, it looks + # as though the originally reported IPv6 issue may have been fixed in + # version 0.24.0. We should remove this when we can confirm that the + # original issue is fixed. + os.environ['EVENTLET_NO_GREENDNS'] = 'yes' + + if debugger.enabled(): + # turn off thread patching to enable the remote debugger + eventlet.monkey_patch(thread=False) + elif os.name == 'nt': + # for nova-compute running on Windows(Hyper-v) + # pipes don't support non-blocking I/O + eventlet.monkey_patch(os=False) + else: + eventlet.monkey_patch() + + # NOTE(rpodolyaka): import oslo_service first, so that it makes eventlet + # hub use a monotonic clock to avoid issues with drifts of system time (see + # LP 1510234 for details) + # NOTE(mdbooth): This was fixed in eventlet 0.21.0. Remove when bumping + # eventlet version. + import oslo_service # noqa + eventlet.hubs.use_hub("oslo_service:service_hub") + + # NOTE(mdbooth): Log here instead of earlier to avoid loading oslo logging + # before monkey patching. + # NOTE(mdbooth): Ideally we would raise an exception here, as this is + # likely to cause problems when executing nova code. However, some non-nova + # tools load nova only to extract metadata and do not execute it. Two + # examples are oslopolicy-policy-generator and sphinx, both of which can + # fail if we assert here. It is not ideal that these utilities are monkey + # patching at all, but we should not break them. + # TODO(mdbooth): If there is any way to reliably determine if we are being + # loaded in that kind of context without breaking existing callers, we + # should do it and bypass monkey patching here entirely. + if problems: + from oslo_log import log as logging + + LOG = logging.getLogger(__name__) + LOG.warning("Modules with known eventlet monkey patching issues were " + "imported prior to eventlet monkey patching: %s. This " + "warning can usually be ignored if the caller is only " + "importing and not executing nova code.", + ', '.join(problems)) + +# NOTE(mdbooth): This workaround is required to avoid breaking sphinx. See +# separate comment in doc/source/conf.py. It may also be useful for other +# non-nova utilities. Ideally the requirement for this workaround will be +# removed as soon as possible, so do not rely on, or extend it. +if (os.environ.get('OS_NOVA_DISABLE_EVENTLET_PATCHING', '').lower() + not in ('1', 'true', 'yes')): + _monkey_patch() diff --git a/nova/test.py b/nova/test.py index 43bcb1a40275..d32cee482632 100644 --- a/nova/test.py +++ b/nova/test.py @@ -21,8 +21,7 @@ inline callbacks. """ -import eventlet # noqa -eventlet.monkey_patch() +import nova.monkey_patch # noqa import abc import copy diff --git a/nova/tests/functional/__init__.py b/nova/tests/functional/__init__.py index 71f41e30f33e..4ced04433b1c 100644 --- a/nova/tests/functional/__init__.py +++ b/nova/tests/functional/__init__.py @@ -20,6 +20,4 @@ :platform: Unix """ -import eventlet - -eventlet.monkey_patch() +import nova.monkey_patch # noqa diff --git a/nova/utils.py b/nova/utils.py index 1245f80fbe66..7f453cfca837 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -49,10 +49,8 @@ from oslo_utils import timeutils from oslo_utils import units import six from six.moves import range -from six.moves import reload_module import nova.conf -from nova import debugger from nova import exception from nova.i18n import _, _LE, _LI, _LW import nova.network @@ -1293,25 +1291,6 @@ def generate_hostid(host, project_id): return "" -def monkey_patch(): - if debugger.enabled(): - # turn off thread patching to enable the remote debugger - eventlet.monkey_patch(thread=False) - elif os.name == 'nt': - # for nova-compute running on Windows(Hyper-v) - # pipes don't support non-blocking I/O - eventlet.monkey_patch(os=False) - else: - eventlet.monkey_patch() - - # NOTE(rgerganov): oslo.context is storing a global thread-local variable - # which keeps the request context for the current thread. If oslo.context - # is imported before calling monkey_patch(), then this thread-local won't - # be green. To workaround this, reload the module after calling - # monkey_patch() - reload_module(importutils.import_module('oslo_context.context')) - - if six.PY2: nested_contexts = contextlib.nested else: