From 1f7d77ecffd6a5ce85c75187087bbd9b11f0f01d Mon Sep 17 00:00:00 2001 From: tamarrow Date: Wed, 31 Aug 2016 17:36:04 -0700 Subject: [PATCH] add Subproc class to handle env change needed for subprocess (#747) Due to bugs in pyinstaller and setuptools we need to modify the environment when forking a command. --- cli/dcoscli/help/main.py | 5 ++- cli/dcoscli/node/main.py | 5 ++- cli/dcoscli/service/main.py | 6 ++-- dcos/subcommand.py | 48 +++++----------------------- dcos/subprocess.py | 64 +++++++++++++++++++++++++++++++++++++ dcos/util.py | 25 --------------- 6 files changed, 78 insertions(+), 75 deletions(-) create mode 100644 dcos/subprocess.py diff --git a/cli/dcoscli/help/main.py b/cli/dcoscli/help/main.py index 6b78bb5..3476020 100644 --- a/cli/dcoscli/help/main.py +++ b/cli/dcoscli/help/main.py @@ -1,9 +1,8 @@ -import subprocess from concurrent.futures import ThreadPoolExecutor import dcoscli import docopt -from dcos import cmds, emitting, options, subcommand, util +from dcos import cmds, emitting, options, subcommand, subprocess, util from dcos.errors import DCOSException from dcoscli.subcommand import (default_command_documentation, default_command_info, default_doc) @@ -107,4 +106,4 @@ def _help_command(command): return 0 else: executable = subcommand.command_executables(command) - return subprocess.call([executable, command, '--help']) + return subprocess.Subproc().call([executable, command, '--help']) diff --git a/cli/dcoscli/node/main.py b/cli/dcoscli/node/main.py index 27c8dd7..81a6d02 100644 --- a/cli/dcoscli/node/main.py +++ b/cli/dcoscli/node/main.py @@ -1,12 +1,11 @@ import functools import os -import subprocess import dcoscli import docopt import six from dcos import (cmds, config, cosmospackage, emitting, errors, http, mesos, - util) + subprocess, util) from dcos.errors import DCOSException, DefaultError from dcoscli import log, tables from dcoscli.package.main import confirm, get_cosmos_url @@ -587,4 +586,4 @@ def _ssh(leader, slave, option, config_file, user, master_proxy, command): "network than DC/OS, consider using " "`--master-proxy`")) - return subprocess.call(cmd, shell=True) + return subprocess.Subproc().call(cmd, shell=True) diff --git a/cli/dcoscli/service/main.py b/cli/dcoscli/service/main.py index 740f723..ec81bf2 100644 --- a/cli/dcoscli/service/main.py +++ b/cli/dcoscli/service/main.py @@ -1,9 +1,7 @@ -import subprocess - import dcoscli import docopt import six -from dcos import cmds, emitting, marathon, mesos, util +from dcos import cmds, emitting, marathon, mesos, subprocess, util from dcos.errors import DCOSException, DefaultError from dcoscli import log, tables from dcoscli.subcommand import default_command_info, default_doc @@ -277,4 +275,4 @@ def _log_marathon(follow, lines, ssh_config_file): emitter.publish(DefaultError("Running `{}`".format(cmd))) - return subprocess.call(cmd, shell=True) + return subprocess.Subproc().call(cmd, shell=True) diff --git a/dcos/subcommand.py b/dcos/subcommand.py index 46c2e33..3805071 100644 --- a/dcos/subcommand.py +++ b/dcos/subcommand.py @@ -1,6 +1,5 @@ from __future__ import print_function -import functools import hashlib import json import os @@ -11,11 +10,12 @@ import subprocess import sys import zipfile from distutils.version import LooseVersion -from subprocess import PIPE, Popen +from subprocess import PIPE import requests from dcos import constants, util from dcos.errors import DCOSException +from dcos.subprocess import Subproc logger = util.get_logger(__name__) @@ -159,36 +159,6 @@ def documentation(executable_path): return (path_noun, info(executable_path, path_noun)) -def executable_env(fn): - """Decorator for environment fork/execs should run under - - Setuptools overrides path to executable from virtualenv, - modify this so we can specify a different path - - :param fn: function that fork/execs - :type fn: function - :rtype: Response - :returns: Response - """ - - @functools.wraps(fn) - def update_running_env(*args, **kwargs): - """Run fork/exec under modified environment - - :param response: response from cosmos - :type response: Response - :returns: Response or raises Exception - :rtype: valid response - """ - - with util.set_env('__PYVENV_LAUNCHER__', None): - cmd = fn(*args, **kwargs) - return cmd - - return update_running_env - - -@executable_env def info(executable_path, path_noun): """Collects subcommand information @@ -200,13 +170,12 @@ def info(executable_path, path_noun): :rtype: str """ - out = subprocess.check_output( + out = Subproc().check_output( [executable_path, path_noun, '--info']) return out.decode('utf-8').strip() -@executable_env def config_schema(executable_path, noun=None): """Collects subcommand config schema @@ -220,7 +189,7 @@ def config_schema(executable_path, noun=None): if noun is None: noun = noun(executable_path) - out = subprocess.check_output( + out = Subproc().check_output( [executable_path, noun, '--config-schema']) return json.loads(out.decode('utf-8')) @@ -597,7 +566,6 @@ def _install_with_pip( return None -@executable_env def _execute_command(command): """ :param command: a command to execute @@ -608,7 +576,7 @@ def _execute_command(command): logger.info('Calling: %r', command) - process = subprocess.Popen( + process = Subproc().Popen( command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) @@ -703,7 +671,6 @@ class SubcommandProcess(): self._command = command self._args = args - @executable_env def run_and_capture(self): """ Run a command and capture exceptions. This is a blocking call @@ -711,8 +678,9 @@ class SubcommandProcess(): :rtype: int, str | None """ - subproc = Popen([self._executable, self._command] + self._args, - stderr=PIPE) + subproc = subprocess.Subproc().Popen( + [self._executable, self._command] + self._args, + stderr=PIPE) err = '' while subproc.poll() is None: diff --git a/dcos/subprocess.py b/dcos/subprocess.py new file mode 100644 index 0000000..945affe --- /dev/null +++ b/dcos/subprocess.py @@ -0,0 +1,64 @@ +import os +import subprocess + + +class Subproc(): + """Represents a wrapper for subprocess + """ + + def __init__(self): + self._env = os.environ.copy() + lib_path = 'LD_LIBRARY_PATH' + lib_path_value = self._env.get(lib_path + '_ORIG') + # remove pyinstaller overriding `LD_LIBRARY_PATH` + # remove with stable release of fix: + # https://github.com/pyinstaller/pyinstaller/pull/2148 + if lib_path_value is not None: + self._env[lib_path] = lib_path_value + elif self._env.get(lib_path): + del self._env[lib_path] + + # Setuptools overrides path to executable from virtualenv, + # modify this so we can specify a different path + launcher = '__PYVENV_LAUNCHER__' + pyvenv_launcher = self._env.get(launcher) + if pyvenv_launcher is not None: + del self._env[launcher] + + def check_output(self, args, stdin=None, stderr=None, shell=False): + """ + call subprocess.check_ouput with modified environment + """ + + return subprocess.check_output( + args, + stdin=stdin, + stderr=stderr, + shell=shell, + env=self._env) + + def Popen(self, args, stdin=None, stdout=None, stderr=None, shell=False): + """ + call subprocess.Popen with modified environment + """ + + return subprocess.Popen( + args, + stdin=stdin, + stdout=stdout, + stderr=stderr, + shell=shell, + env=self._env) + + def call(self, args, stdin=None, stdout=None, stderr=None, shell=False): + """ + call subprocess.call with modified environment + """ + + return subprocess.call( + args, + stdin=stdin, + stdout=stdout, + stderr=stderr, + shell=shell, + env=self._env) diff --git a/dcos/util.py b/dcos/util.py index 536b3fa..3ee1458 100644 --- a/dcos/util.py +++ b/dcos/util.py @@ -32,31 +32,6 @@ def get_logger(name): return logging.getLogger(name) -@contextlib.contextmanager -def set_env(env_var, new_value): - """A context manager for temporary updating env vars - - :param env_var: name of environment variable to alter, None to remove - :type env_var: str - :param new_val: new value to change env_var to - :type new_var: str | None - :rtype: str | None - """ - - old_value = os.environ.get(env_var) - if new_value is not None: - os.environ[env_var] = new_value - elif new_value is None and old_value is not None: - del os.environ[env_var] - try: - yield - finally: - if old_value is not None: - os.environ[env_var] = old_value - elif old_value is None and new_value is not None: - del os.environ[env_var] - - @contextlib.contextmanager def tempdir(): """A context manager for temporary directories.