From f5c92f6bc1659d1e95657c1b8872a56a13d6ecff Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Fri, 15 Feb 2019 17:52:35 -0800 Subject: [PATCH] Allow setting resource.RLIMIT_NOFILE in modules (#51989) --- lib/ansible/config/base.yml | 14 +++++ lib/ansible/executor/module_common.py | 31 ++++++++++ .../targets/ansiballz_python/aliases | 1 + .../library/check_rlimit_and_maxfd.py | 31 ++++++++++ .../targets/ansiballz_python/tasks/main.yml | 56 +++++++++++++++++++ test/runner/lib/ansible_util.py | 1 + test/units/plugins/action/test_action.py | 3 +- 7 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 test/integration/targets/ansiballz_python/aliases create mode 100644 test/integration/targets/ansiballz_python/library/check_rlimit_and_maxfd.py create mode 100644 test/integration/targets/ansiballz_python/tasks/main.yml diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index d8fb002ccc..ce7ec70b0b 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -1590,6 +1590,20 @@ PLUGIN_FILTERS_CFG: - key: plugin_filters_cfg section: defaults type: path +PYTHON_MODULE_RLIMIT_NOFILE: + name: Adjust maximum file descriptor soft limit during Python module execution + description: + - Attempts to set RLIMIT_NOFILE soft limit to the specified value when executing Python modules (can speed up subprocess usage on + Python 2.x. See https://bugs.python.org/issue11284). The value will be limited by the existing hard limit. Default + value of 0 does not attempt to adjust existing system-defined limits. + default: 0 + env: + - {name: ANSIBLE_PYTHON_MODULE_RLIMIT_NOFILE} + ini: + - {key: python_module_rlimit_nofile, section: defaults} + vars: + - {name: ansible_python_module_rlimit_nofile} + version_added: '2.8' RETRY_FILES_ENABLED: name: Retry files default: True diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 8402f21217..d3a9c9fee8 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -94,6 +94,7 @@ _ANSIBALLZ_WRAPPER = True # For test-module script to tell this is a ANSIBALLZ_W # LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE # USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. def _ansiballz_main(): +%(rlimit)s import os import os.path import sys @@ -350,6 +351,22 @@ ANSIBALLZ_COVERAGE_TEMPLATE = ''' cov.start() ''' +ANSIBALLZ_RLIMIT_TEMPLATE = ''' + import resource + + existing_soft, existing_hard = resource.getrlimit(resource.RLIMIT_NOFILE) + + # adjust soft limit subject to existing hard limit + requested_soft = min(existing_hard, %(rlimit_nofile)d) + + if requested_soft != existing_soft: + try: + resource.setrlimit(resource.RLIMIT_NOFILE, (requested_soft, existing_hard)) + except ValueError: + # some platforms (eg macOS) lie about their hard limit + pass +''' + def _strip_comments(source): # Strip comments and blank lines from the wrapper @@ -764,6 +781,19 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas interpreter_parts = interpreter.split(u' ') interpreter = u"'{0}'".format(u"', '".join(interpreter_parts)) + # FUTURE: the module cache entry should be invalidated if we got this value from a host-dependent source + rlimit_nofile = C.config.get_config_value('PYTHON_MODULE_RLIMIT_NOFILE', variables=task_vars) + + if not isinstance(rlimit_nofile, int): + rlimit_nofile = int(templar.template(rlimit_nofile)) + + if rlimit_nofile: + rlimit = ANSIBALLZ_RLIMIT_TEMPLATE % dict( + rlimit_nofile=rlimit_nofile, + ) + else: + rlimit = '' + coverage_config = os.environ.get('_ANSIBLE_COVERAGE_CONFIG') if coverage_config: @@ -791,6 +821,7 @@ def _find_module_utils(module_name, b_module_data, module_path, module_args, tas minute=now.minute, second=now.second, coverage=coverage, + rlimit=rlimit, ))) b_module_data = output.getvalue() diff --git a/test/integration/targets/ansiballz_python/aliases b/test/integration/targets/ansiballz_python/aliases new file mode 100644 index 0000000000..a6dafcf8cd --- /dev/null +++ b/test/integration/targets/ansiballz_python/aliases @@ -0,0 +1 @@ +shippable/posix/group1 diff --git a/test/integration/targets/ansiballz_python/library/check_rlimit_and_maxfd.py b/test/integration/targets/ansiballz_python/library/check_rlimit_and_maxfd.py new file mode 100644 index 0000000000..a01ee99763 --- /dev/null +++ b/test/integration/targets/ansiballz_python/library/check_rlimit_and_maxfd.py @@ -0,0 +1,31 @@ +#!/usr/bin/python +# +# Copyright 2018 Red Hat | Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import resource +import subprocess + +from ansible.module_utils.basic import AnsibleModule + + +def main(): + module = AnsibleModule( + argument_spec=dict() + ) + + rlimit_nofile = resource.getrlimit(resource.RLIMIT_NOFILE) + + try: + maxfd = subprocess.MAXFD + except AttributeError: + maxfd = -1 + + module.exit_json(rlimit_nofile=rlimit_nofile, maxfd=maxfd, infinity=resource.RLIM_INFINITY) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/ansiballz_python/tasks/main.yml b/test/integration/targets/ansiballz_python/tasks/main.yml new file mode 100644 index 0000000000..f9ca58898a --- /dev/null +++ b/test/integration/targets/ansiballz_python/tasks/main.yml @@ -0,0 +1,56 @@ +- name: get the ansible-test imposed file descriptor limit + check_rlimit_and_maxfd: + register: rlimit_limited_return + +- name: get existing file descriptor limit + check_rlimit_and_maxfd: + register: rlimit_original_return + vars: + ansible_python_module_rlimit_nofile: 0 # ignore limit set by ansible-test + +- name: attempt to set a value lower than existing soft limit + check_rlimit_and_maxfd: + vars: + ansible_python_module_rlimit_nofile: '{{ rlimit_original_return.rlimit_nofile[0] - 1 }}' + register: rlimit_below_soft_return + +- name: attempt to set a value higher than existing soft limit + check_rlimit_and_maxfd: + vars: + ansible_python_module_rlimit_nofile: '{{ rlimit_original_return.rlimit_nofile[0] + 1 }}' + register: rlimit_above_soft_return + +- name: attempt to set a value lower than existing hard limit + check_rlimit_and_maxfd: + vars: + ansible_python_module_rlimit_nofile: '{{ rlimit_original_return.rlimit_nofile[1] - 1 }}' + register: rlimit_below_hard_return + +- name: attempt to set a value higher than existing hard limit + check_rlimit_and_maxfd: + vars: + ansible_python_module_rlimit_nofile: '{{ rlimit_original_return.rlimit_nofile[1] + 1 }}' + register: rlimit_above_hard_return + +- assert: + that: + # make sure ansible-test was able to set the limit unless it exceeds the hard limit or the value is lower on macOS + - rlimit_limited_return.rlimit_nofile[0] == 1024 or rlimit_original_return.rlimit_nofile[1] < 1024 or (rlimit_limited_return.rlimit_nofile[0] < 1024 and ansible_distribution == 'MacOSX') + # make sure that maxfd matches the soft limit on Python 2.x (-1 on Python 3.x) + - rlimit_limited_return.maxfd == rlimit_limited_return.rlimit_nofile[0] or rlimit_limited_return.maxfd == -1 + + # we should always be able to set the limit lower than the existing soft limit + - rlimit_below_soft_return.rlimit_nofile[0] == rlimit_original_return.rlimit_nofile[0] - 1 + # the hard limit should not have changed + - rlimit_below_soft_return.rlimit_nofile[1] == rlimit_original_return.rlimit_nofile[1] + # lowering the limit should also lower the max file descriptors reported by Python 2.x (-1 on Python 3.x) + - rlimit_below_soft_return.maxfd == rlimit_original_return.rlimit_nofile[0] - 1 or rlimit_below_soft_return.maxfd == -1 + + # we should be able to set the limit higher than the existing soft limit if it does not exceed the hard limit (except on macOS) + - rlimit_above_soft_return.rlimit_nofile[0] == rlimit_original_return.rlimit_nofile[0] + 1 or rlimit_original_return.rlimit_nofile[0] == rlimit_original_return.rlimit_nofile[1] or ansible_distribution == 'MacOSX' + + # we should be able to set the limit lower than the existing hard limit (except on macOS) + - rlimit_below_hard_return.rlimit_nofile[0] == rlimit_original_return.rlimit_nofile[1] - 1 or ansible_distribution == 'MacOSX' + + # setting the limit higher than the existing hard limit should use the hard limit (except on macOS) + - rlimit_above_hard_return.rlimit_nofile[0] == rlimit_original_return.rlimit_nofile[1] or ansible_distribution == 'MacOSX' diff --git a/test/runner/lib/ansible_util.py b/test/runner/lib/ansible_util.py index bef1e99bee..512be0c024 100644 --- a/test/runner/lib/ansible_util.py +++ b/test/runner/lib/ansible_util.py @@ -40,6 +40,7 @@ def ansible_environment(args, color=True, ansible_config=None): raise ApplicationError('Configuration not found: %s' % ansible_config) ansible = dict( + ANSIBLE_PYTHON_MODULE_RLIMIT_NOFILE='1024', ANSIBLE_FORCE_COLOR='%s' % 'true' if args.color and color else 'false', ANSIBLE_DEPRECATION_WARNINGS='false', ANSIBLE_HOST_KEY_CHECKING='false', diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 79e4456a27..b01b0e703a 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -133,6 +133,7 @@ class TestActionBase(unittest.TestCase): mock_module_loader.find_plugin.side_effect = mock_find_plugin mock_shared_obj_loader = MagicMock() mock_shared_obj_loader.module_loader = mock_module_loader + mock_templar = MagicMock() # we're using a real play context here play_context = PlayContext() @@ -143,7 +144,7 @@ class TestActionBase(unittest.TestCase): connection=mock_connection, play_context=play_context, loader=fake_loader, - templar=None, + templar=mock_templar, shared_loader_obj=mock_shared_obj_loader, )