From d088030fa68ea58c5448597b555daf9208601d45 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 20 Apr 2017 10:51:36 -0700 Subject: [PATCH] Facts Timeout was not settable via ansible.cfg The timeout for gathering facts needs to be settable from three places (highest precedence to lowest): * programmatically * ansible.cfg (equivalent to the user specifying it explicitly when calling setup) * from the default value The code was changed in b4bd6c80deccc64655e158cf4799edcf604c76d1 to allow programmatically and the default value to work correctly but setting via ansible.cfg/parameter was broken. This change should fix setting via ansible.cfg and adds unittests for all three cases Fixes #23753 --- lib/ansible/module_utils/facts.py | 14 ++- test/units/module_utils/facts/__init__.py | 0 test/units/module_utils/facts/test_timeout.py | 112 ++++++++++++++++++ 3 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 test/units/module_utils/facts/__init__.py create mode 100644 test/units/module_utils/facts/test_timeout.py diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py index cef0b05270..46b0654171 100644 --- a/lib/ansible/module_utils/facts.py +++ b/lib/ansible/module_utils/facts.py @@ -77,22 +77,24 @@ if platform.system() != 'SunOS': # steps do not exceed a time limit GATHER_TIMEOUT=None +DEFAULT_GATHER_TIMEOUT = 10 class TimeoutError(Exception): pass def timeout(seconds=None, error_message="Timer expired"): - if seconds is None: - seconds = globals().get('GATHER_TIMEOUT') or 10 - def decorator(func): def _handle_timeout(signum, frame): raise TimeoutError(error_message) def wrapper(*args, **kwargs): + local_seconds = seconds # Make local var as we modify this every time it's invoked + if local_seconds is None: + local_seconds = globals().get('GATHER_TIMEOUT') or DEFAULT_GATHER_TIMEOUT + signal.signal(signal.SIGALRM, _handle_timeout) - signal.alarm(seconds) + signal.alarm(local_seconds) try: result = func(*args, **kwargs) finally: @@ -103,11 +105,11 @@ def timeout(seconds=None, error_message="Timer expired"): # If we were called as @timeout, then the first parameter will be the # function we are to wrap instead of the number of seconds. Detect this - # and correct it by setting seconds to our default value and return the + # and correct it by setting seconds to our sentinel value and return the # inner decorator function manually wrapped around the function if callable(seconds): func = seconds - seconds = 10 + seconds = None return decorator(func) # If we were called as @timeout([...]) then python itself will take diff --git a/test/units/module_utils/facts/__init__.py b/test/units/module_utils/facts/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/units/module_utils/facts/test_timeout.py b/test/units/module_utils/facts/test_timeout.py new file mode 100644 index 0000000000..9c452c4397 --- /dev/null +++ b/test/units/module_utils/facts/test_timeout.py @@ -0,0 +1,112 @@ +# -*- coding: utf-8 -*- +# (c) 2017, Toshio Kuratomi +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division) +__metaclass__ = type + +import time + +import pytest + +from ansible.compat.tests import unittest +from ansible.compat.tests.mock import patch, MagicMock + +from ansible.module_utils import facts + + +@pytest.fixture +def set_gather_timeout_higher(): + default_timeout = facts.GATHER_TIMEOUT + facts.GATHER_TIMEOUT = facts.DEFAULT_GATHER_TIMEOUT + 5 + yield + facts.GATHER_TIMEOUT = default_timeout + + +@pytest.fixture +def set_gather_timeout_lower(): + default_timeout = facts.GATHER_TIMEOUT + facts.GATHER_TIMEOUT = 2 + yield + facts.GATHER_TIMEOUT = default_timeout + + +@facts.timeout +def sleep_amount_implicit(amount): + # implicit refers to the lack of argument to the decorator + time.sleep(amount) + return 'Succeeded after {0} sec'.format(amount) + + +@facts.timeout(facts.DEFAULT_GATHER_TIMEOUT + 5) +def sleep_amount_explicit_higher(amount): + # explicit refers to the argument to the decorator + time.sleep(amount) + return 'Succeeded after {0} sec'.format(amount) + + +@facts.timeout(2) +def sleep_amount_explicit_lower(amount): + # explicit refers to the argument to the decorator + time.sleep(amount) + return 'Succeeded after {0} sec'.format(amount) + + +def test_defaults_still_within_bounds(): + # If the default changes outside of these bounds, some of the tests will + # no longer test the right thing. Need to review and update the timeouts + # in the other tests if this fails + assert facts.DEFAULT_GATHER_TIMEOUT >= 4 + + +def test_implicit_file_default_succeeds(): + # amount checked must be less than DEFAULT_GATHER_TIMEOUT + assert sleep_amount_implicit(1) == 'Succeeded after 1 sec' + + +def test_implicit_file_default_timesout(): + # sleep_time is greater than the default + sleep_time = facts.DEFAULT_GATHER_TIMEOUT + 1 + with pytest.raises(facts.TimeoutError): + assert sleep_amount_implicit(sleep_time) == '(Not expected to succeed)' + + +def test_implicit_file_overridden_succeeds(set_gather_timeout_higher): + # Set sleep_time greater than the default timeout and less than our new timeout + sleep_time = facts.DEFAULT_GATHER_TIMEOUT + 1 + assert sleep_amount_implicit(sleep_time) == 'Succeeded after {0} sec'.format(sleep_time) + + +def test_implicit_file_overridden_timesout(set_gather_timeout_lower): + # Set sleep_time greater than our new timeout but less than the default + sleep_time = 3 + with pytest.raises(facts.TimeoutError): + assert sleep_amount_implicit(sleep_time) == '(Not expected to Succeed)' + + +def test_explicit_succeeds(): + # Set sleep_time greater than the default timeout and less than our new timeout + sleep_time = facts.DEFAULT_GATHER_TIMEOUT + 1 + assert sleep_amount_explicit_higher(sleep_time) == 'Succeeded after {0} sec'.format(sleep_time) + + +def test_explicit_timeout(): + # Set sleep_time greater than our new timeout but less than the default + sleep_time = 3 + with pytest.raises(facts.TimeoutError): + assert sleep_amount_explicit_lower(sleep_time) == '(Not expected to succeed)'