From 31cfdbd4076bb6556cf9612171ba43fa44475d71 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 13 Oct 2015 21:26:39 +0200 Subject: [PATCH] Fix Python 3 support for eventlet monkey-patching Use eventlet.green.subprocess if eventlet is used and enable eventlet tests on Python 3. This change adds oslo_rootwrap.subprocess which is eventlet.green.subprocess if eventlet monkey-patching is enabled or if the TEST_EVENTLET environment variable is set, or subprocess of the Python standard library otherwise. When eventlet is used (with monkey-patching or not), it's more reliable to use eventlet.green.subprocess instead of using directly subprocess from the Python standard library. On Python 2, it "works" to use directly subprocess: subprocess.Popen calls os.pipe() and os.fdopen(fd) which are both monkey-patched. On Python 3, it doesn't work because subprocess uses os.pipe() and io.open(fd), and the io module is *not* monkey-patched at all. Change-Id: Ib859bebe52612b35f0f1f53aedf76222683795e7 --- oslo_rootwrap/__init__.py | 30 ++++++++++++++++++++++++++ oslo_rootwrap/client.py | 15 +++---------- oslo_rootwrap/daemon.py | 2 +- oslo_rootwrap/tests/run_daemon.py | 2 +- oslo_rootwrap/tests/test_functional.py | 4 ++-- oslo_rootwrap/tests/test_rootwrap.py | 2 +- oslo_rootwrap/wrapper.py | 2 +- tox.ini | 6 ------ 8 files changed, 39 insertions(+), 24 deletions(-) diff --git a/oslo_rootwrap/__init__.py b/oslo_rootwrap/__init__.py index e69de29..483f60c 100644 --- a/oslo_rootwrap/__init__.py +++ b/oslo_rootwrap/__init__.py @@ -0,0 +1,30 @@ +# Copyright (c) 2015 Red Hat. +# 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. + +import os + +try: + import eventlet.patcher +except ImportError: + _patched_socket = False +else: + # In tests patching happens later, so we'll rely on environment variable + _patched_socket = (eventlet.patcher.is_monkey_patched('socket') or + os.environ.get('TEST_EVENTLET', False)) + +if not _patched_socket: + import subprocess +else: + from eventlet.green import subprocess # noqa diff --git a/oslo_rootwrap/client.py b/oslo_rootwrap/client.py index 6f7fd37..dec161e 100644 --- a/oslo_rootwrap/client.py +++ b/oslo_rootwrap/client.py @@ -16,24 +16,15 @@ import logging from multiprocessing import managers from multiprocessing import util as mp_util -import os -import subprocess import threading import weakref -try: - import eventlet.patcher -except ImportError: - patched_socket = False -else: - # In tests patching happens later, so we'll rely on environment variable - patched_socket = (eventlet.patcher.is_monkey_patched('socket') or - os.environ.get('TEST_EVENTLET', False)) - +import oslo_rootwrap from oslo_rootwrap import daemon from oslo_rootwrap import jsonrpc +from oslo_rootwrap import subprocess -if patched_socket: +if oslo_rootwrap._patched_socket: # We have to use slow version of recvall with eventlet because of a bug in # GreenSocket.recv_into: # https://bitbucket.org/eventlet/eventlet/pull-request/41 diff --git a/oslo_rootwrap/daemon.py b/oslo_rootwrap/daemon.py index aa3612b..20de185 100644 --- a/oslo_rootwrap/daemon.py +++ b/oslo_rootwrap/daemon.py @@ -22,12 +22,12 @@ import os import shutil import signal import stat -import subprocess import sys import tempfile import threading from oslo_rootwrap import jsonrpc +from oslo_rootwrap import subprocess from oslo_rootwrap import wrapper LOG = logging.getLogger(__name__) diff --git a/oslo_rootwrap/tests/run_daemon.py b/oslo_rootwrap/tests/run_daemon.py index 669b298..8483ff1 100644 --- a/oslo_rootwrap/tests/run_daemon.py +++ b/oslo_rootwrap/tests/run_daemon.py @@ -14,11 +14,11 @@ # under the License. import logging -import subprocess import sys import threading from oslo_rootwrap import cmd +from oslo_rootwrap import subprocess def forward_stream(fr, to): diff --git a/oslo_rootwrap/tests/test_functional.py b/oslo_rootwrap/tests/test_functional.py index a491f79..7013d85 100644 --- a/oslo_rootwrap/tests/test_functional.py +++ b/oslo_rootwrap/tests/test_functional.py @@ -19,7 +19,6 @@ import logging import os import pwd import signal -import subprocess import sys import threading @@ -35,6 +34,7 @@ import testtools from testtools import content from oslo_rootwrap import client +from oslo_rootwrap import subprocess from oslo_rootwrap.tests import run_daemon from oslo_rootwrap import wrapper @@ -121,7 +121,7 @@ class RootwrapDaemonTest(_FunctionalBase, testtools.TestCase): # Collect daemon logs daemon_log = io.BytesIO() - p = mock.patch('subprocess.Popen', + p = mock.patch('oslo_rootwrap.subprocess.Popen', run_daemon.forwarding_popen(daemon_log)) p.start() self.addCleanup(p.stop) diff --git a/oslo_rootwrap/tests/test_rootwrap.py b/oslo_rootwrap/tests/test_rootwrap.py index 1d8ef04..22c23b5 100644 --- a/oslo_rootwrap/tests/test_rootwrap.py +++ b/oslo_rootwrap/tests/test_rootwrap.py @@ -15,7 +15,6 @@ import logging import logging.handlers import os -import subprocess import uuid import fixtures @@ -26,6 +25,7 @@ import testtools from oslo_rootwrap import cmd from oslo_rootwrap import daemon from oslo_rootwrap import filters +from oslo_rootwrap import subprocess from oslo_rootwrap import wrapper diff --git a/oslo_rootwrap/wrapper.py b/oslo_rootwrap/wrapper.py index 98f0729..36a5287 100644 --- a/oslo_rootwrap/wrapper.py +++ b/oslo_rootwrap/wrapper.py @@ -18,11 +18,11 @@ import logging.handlers import os import pwd import signal -import subprocess from six import moves from oslo_rootwrap import filters +from oslo_rootwrap import subprocess class NoFilterMatched(Exception): diff --git a/tox.ini b/tox.ini index 107bdb6..f5b7eb1 100644 --- a/tox.ini +++ b/tox.ini @@ -16,12 +16,6 @@ commands = python setup.py testr --slowest --testr-args='(?!tests.test_functional_eventlet)tests {posargs}' env TEST_EVENTLET=1 python setup.py testr --slowest --testr-args='tests.test_functional_eventlet' -[testenv:py34] -commands = - python setup.py testr --slowest --testr-args='{posargs}' -# FIXME: Eventlet tests does not work yet here -# env TEST_EVENTLET=1 python setup.py testr --slowest --testr-args='tests.test_functional_eventlet' - [testenv:pep8] commands = flake8