From 44e21f7062977b2cf6bd0c9fdab8b89a1ef896c4 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 19 Apr 2016 20:08:01 -0700 Subject: [PATCH 1/6] Allow AnsibleModules to be instantiated more than once in a module Fix SELINUX monkeypatch in test_basic --- lib/ansible/module_utils/basic.py | 41 ++++++++++++------- .../module_utils/basic/test_exit_json.py | 3 ++ test/units/module_utils/basic/test_log.py | 4 ++ .../module_utils/basic/test_run_command.py | 1 + .../module_utils/basic/test_safe_eval.py | 7 ++++ test/units/module_utils/test_basic.py | 27 +++++++++++- 6 files changed, 66 insertions(+), 17 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index 59e808afebe..7ae2cb941cc 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -223,6 +223,12 @@ from ansible import __version__ # Backwards compat. New code should just import and use __version__ ANSIBLE_VERSION = __version__ +# Internal global holding passed in params and constants. This is consulted +# in case multiple AnsibleModules are created. Otherwise each AnsibleModule +# would attempt to read from stdin. Other code should not use this directly +# as it is an internal implementation detail +_ANSIBLE_ARGS = None + FILE_COMMON_ARGUMENTS=dict( src = dict(), mode = dict(type='raw'), @@ -1457,23 +1463,28 @@ class AnsibleModule(object): ''' read the input and set the params attribute. Sets the constants as well.''' # debug overrides to read args from file or cmdline - # Avoid tracebacks when locale is non-utf8 - # We control the args and we pass them as utf8 - if len(sys.argv) > 1: - if os.path.isfile(sys.argv[1]): - fd = open(sys.argv[1], 'rb') - buffer = fd.read() - fd.close() - else: - buffer = sys.argv[1] - if sys.version_info >= (3,): - buffer = buffer.encode('utf-8', errors='surrogateescape') - # default case, read from stdin + global _ANSIBLE_ARGS + if _ANSIBLE_ARGS is not None: + buffer = _ANSIBLE_ARGS else: - if sys.version_info < (3,): - buffer = sys.stdin.read() + # Avoid tracebacks when locale is non-utf8 + # We control the args and we pass them as utf8 + if len(sys.argv) > 1: + if os.path.isfile(sys.argv[1]): + fd = open(sys.argv[1], 'rb') + buffer = fd.read() + fd.close() + else: + buffer = sys.argv[1] + if sys.version_info >= (3,): + buffer = buffer.encode('utf-8', errors='surrogateescape') + # default case, read from stdin else: - buffer = sys.stdin.buffer.read() + if sys.version_info < (3,): + buffer = sys.stdin.read() + else: + buffer = sys.stdin.buffer.read() + _ANSIBLE_ARGS = buffer try: params = json.loads(buffer.decode('utf-8')) diff --git a/test/units/module_utils/basic/test_exit_json.py b/test/units/module_utils/basic/test_exit_json.py index 1bd25002d4b..73a97ed6873 100644 --- a/test/units/module_utils/basic/test_exit_json.py +++ b/test/units/module_utils/basic/test_exit_json.py @@ -45,6 +45,7 @@ class TestAnsibleModuleExitJson(unittest.TestCase): self.stdout_swap_ctx = swap_stdout() self.fake_stream = self.stdout_swap_ctx.__enter__() + reload(basic) self.module = basic.AnsibleModule(argument_spec=dict()) def tearDown(self): @@ -125,6 +126,7 @@ class TestAnsibleModuleExitValuesRemoved(unittest.TestCase): params = json.dumps(params) with swap_stdin_and_argv(stdin_data=params): + reload(basic) with swap_stdout(): module = basic.AnsibleModule( argument_spec = dict( @@ -146,6 +148,7 @@ class TestAnsibleModuleExitValuesRemoved(unittest.TestCase): params = dict(ANSIBLE_MODULE_ARGS=args, ANSIBLE_MODULE_CONSTANTS={}) params = json.dumps(params) with swap_stdin_and_argv(stdin_data=params): + reload(basic) with swap_stdout(): module = basic.AnsibleModule( argument_spec = dict( diff --git a/test/units/module_utils/basic/test_log.py b/test/units/module_utils/basic/test_log.py index e0258999db8..f9a00bb55e4 100644 --- a/test/units/module_utils/basic/test_log.py +++ b/test/units/module_utils/basic/test_log.py @@ -49,6 +49,7 @@ class TestAnsibleModuleSysLogSmokeTest(unittest.TestCase): self.stdin_swap = swap_stdin_and_argv(stdin_data=args) self.stdin_swap.__enter__() + reload(basic) self.am = basic.AnsibleModule( argument_spec = dict(), ) @@ -85,6 +86,7 @@ class TestAnsibleModuleJournaldSmokeTest(unittest.TestCase): self.stdin_swap = swap_stdin_and_argv(stdin_data=args) self.stdin_swap.__enter__() + reload(basic) self.am = basic.AnsibleModule( argument_spec = dict(), ) @@ -132,6 +134,7 @@ class TestAnsibleModuleLogSyslog(unittest.TestCase): self.stdin_swap = swap_stdin_and_argv(stdin_data=args) self.stdin_swap.__enter__() + reload(basic) self.am = basic.AnsibleModule( argument_spec = dict(), ) @@ -192,6 +195,7 @@ class TestAnsibleModuleLogJournal(unittest.TestCase): self.stdin_swap = swap_stdin_and_argv(stdin_data=args) self.stdin_swap.__enter__() + reload(basic) self.am = basic.AnsibleModule( argument_spec = dict(), ) diff --git a/test/units/module_utils/basic/test_run_command.py b/test/units/module_utils/basic/test_run_command.py index 781dfa286eb..aaf9b0e1344 100644 --- a/test/units/module_utils/basic/test_run_command.py +++ b/test/units/module_utils/basic/test_run_command.py @@ -67,6 +67,7 @@ class TestAnsibleModuleRunCommand(unittest.TestCase): self.stdin_swap = swap_stdin_and_argv(stdin_data=args) self.stdin_swap.__enter__() + reload(basic) self.module = AnsibleModule(argument_spec=dict()) self.module.fail_json = MagicMock(side_effect=SystemExit) diff --git a/test/units/module_utils/basic/test_safe_eval.py b/test/units/module_utils/basic/test_safe_eval.py index 912dee17e3f..e38ed9b46eb 100644 --- a/test/units/module_utils/basic/test_safe_eval.py +++ b/test/units/module_utils/basic/test_safe_eval.py @@ -26,6 +26,12 @@ import json from ansible.compat.tests import unittest from units.mock.procenv import swap_stdin_and_argv +try: + from importlib import reload +except: + # Py2 has reload as a builtin + pass + class TestAnsibleModuleExitJson(unittest.TestCase): def test_module_utils_basic_safe_eval(self): @@ -34,6 +40,7 @@ class TestAnsibleModuleExitJson(unittest.TestCase): args = json.dumps(dict(ANSIBLE_MODULE_ARGS={}, ANSIBLE_MODULE_CONSTANTS={})) with swap_stdin_and_argv(stdin_data=args): + reload(basic) am = basic.AnsibleModule( argument_spec=dict(), ) diff --git a/test/units/module_utils/test_basic.py b/test/units/module_utils/test_basic.py index 8beefed6e88..662d3fe37c0 100644 --- a/test/units/module_utils/test_basic.py +++ b/test/units/module_utils/test_basic.py @@ -31,6 +31,12 @@ try: except ImportError: import __builtin__ as builtins +try: + from importlib import reload +except: + # Py2 has reload as a builtin + pass + from units.mock.procenv import swap_stdin_and_argv from ansible.compat.tests import unittest @@ -291,6 +297,7 @@ class TestModuleUtilsBasic(unittest.TestCase): args = json.dumps(dict(ANSIBLE_MODULE_ARGS={"foo": "hello"}, ANSIBLE_MODULE_CONSTANTS={})) with swap_stdin_and_argv(stdin_data=args): + reload(basic) am = basic.AnsibleModule( argument_spec = arg_spec, mutually_exclusive = mut_ex, @@ -307,6 +314,7 @@ class TestModuleUtilsBasic(unittest.TestCase): args = json.dumps(dict(ANSIBLE_MODULE_ARGS={}, ANSIBLE_MODULE_CONSTANTS={})) with swap_stdin_and_argv(stdin_data=args): + reload(basic) self.assertRaises( SystemExit, basic.AnsibleModule, @@ -353,6 +361,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_load_file_common_arguments(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -401,6 +410,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_selinux_mls_enabled(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -420,6 +430,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_selinux_initial_context(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -433,6 +444,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_selinux_enabled(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -464,6 +476,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_selinux_default_context(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -499,6 +512,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_selinux_context(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -540,6 +554,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_is_special_selinux_path(self): from ansible.module_utils import basic + reload(basic) args = json.dumps(dict(ANSIBLE_MODULE_ARGS={}, ANSIBLE_MODULE_CONSTANTS={"SELINUX_SPECIAL_FS": "nfs,nfsd,foos"})) @@ -584,6 +599,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_to_filesystem_str(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -608,6 +624,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_find_mount_point(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -631,18 +648,19 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_set_context_if_different(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), ) - basic.HAS_SELINUX = False + basic.HAVE_SELINUX = False am.selinux_enabled = MagicMock(return_value=False) self.assertEqual(am.set_context_if_different('/path/to/file', ['foo_u', 'foo_r', 'foo_t', 's0'], True), True) self.assertEqual(am.set_context_if_different('/path/to/file', ['foo_u', 'foo_r', 'foo_t', 's0'], False), False) - basic.HAS_SELINUX = True + basic.HAVE_SELINUX = True am.selinux_enabled = MagicMock(return_value=True) am.selinux_context = MagicMock(return_value=['bar_u', 'bar_r', None, None]) @@ -675,6 +693,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_set_owner_if_different(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -713,6 +732,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_set_group_if_different(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -751,6 +771,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module_set_mode_if_different(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -838,6 +859,7 @@ class TestModuleUtilsBasic(unittest.TestCase): ): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), @@ -1015,6 +1037,7 @@ class TestModuleUtilsBasic(unittest.TestCase): def test_module_utils_basic_ansible_module__symbolic_mode_to_octal(self): from ansible.module_utils import basic + reload(basic) am = basic.AnsibleModule( argument_spec = dict(), From e9553c975fda4df3b0bac4a8612c96d97d1c553c Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 20 Apr 2016 10:34:57 -0700 Subject: [PATCH 2/6] Add debug usage to comments in the ziploader wrapper and don't strip comments if ANSIBLE_KEEP_REMOTE_FILES=1 --- lib/ansible/executor/module_common.py | 74 +++++++++++++++++++-------- 1 file changed, 54 insertions(+), 20 deletions(-) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 459e3ef8473..e043a1997a3 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -134,12 +134,36 @@ def invoke_module(module, modlib_path, json_params): def debug(command, zipped_mod, json_params): # The code here normally doesn't run. It's only used for debugging on the - # remote machine. Run with ANSIBLE_KEEP_REMOTE_FILES=1 envvar and -vvv - # to save the module file remotely. Login to the remote machine and use - # /path/to/module explode to extract the ZIPDATA payload into source - # files. Edit the source files to instrument the code or experiment with - # different values. Then use /path/to/module execute to run the extracted - # files you've edited instead of the actual zipped module. + # remote machine. + # + # The subcommands in this function make it easier to debug ziploader + # modules. Here's the basic steps: + # + # Run ansible with the environment variable: ANSIBLE_KEEP_REMOTE_FILES=1 and -vvv + # to save the module file remotely:: + # $ ANSIBLE_KEEP_REMOTE_FILES=1 ansible host1 -m ping -a 'data=october' -vvv + # + # Part of the verbose output will tell you where on the remote machine the + # module was written to:: + # [...] + # SSH: EXEC ssh -C -q -o ControlMaster=auto -o ControlPersist=60s -o KbdInteractiveAuthentication=no -o + # PreferredAuthentications=gssapi-with-mic,gssapi-keyex,hostbased,publickey -o PasswordAuthentication=no -o ConnectTimeout=10 -o + # ControlPath=/home/badger/.ansible/cp/ansible-ssh-%%h-%%p-%%r -tt rhel7 '/bin/sh -c '"'"'LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 + # LC_MESSAGES=en_US.UTF-8 /usr/bin/python /home/badger/.ansible/tmp/ansible-tmp-1461173013.93-9076457629738/ping'"'"'' + # [...] + # + # Login to the remote machine and run the module file via from the previous + # step with the explode subcommand to extract the module payload into + # source files:: + # $ ssh host1 + # $ /usr/bin/python /home/badger/.ansible/tmp/ansible-tmp-1461173013.93-9076457629738/ping explode + # Module expanded into: + # /home/badger/.ansible/tmp/ansible-tmp-1461173408.08-279692652635227/ansible + # + # You can now edit the source files to instrument the code or experiment with + # different parameter values. When you're ready to run the code you've modified + # (instead of the code from the actual zipped module), use the execute subcommand like this:: + # $ /usr/bin/python /home/badger/.ansible/tmp/ansible-tmp-1461173013.93-9076457629738/ping execute # Okay to use __file__ here because we're running from a kept file basedir = os.path.abspath(os.path.dirname(__file__)) @@ -225,6 +249,10 @@ def debug(command, zipped_mod, json_params): return exitcode if __name__ == '__main__': + # + # See comments in the debug() method for information on debugging + # + ZIPLOADER_PARAMS = %(params)s if PY3: ZIPLOADER_PARAMS = ZIPLOADER_PARAMS.encode('utf-8') @@ -252,6 +280,24 @@ if __name__ == '__main__': sys.exit(exitcode) ''' +def _strip_comments(source): + # Strip comments and blank lines from the wrapper + buf = [] + for line in source.splitlines(): + l = line.strip() + if not l or l.startswith(u'#'): + continue + buf.append(line) + return u'\n'.join(buf) + +if C.DEFAULT_KEEP_REMOTE_FILES: + # Keep comments when KEEP_REMOTE_FILES is set. That way users will see + # the comments with some nice usage instructions + ACTIVE_ZIPLOADER_TEMPLATE = ZIPLOADER_TEMPLATE +else: + # ZIPLOADER_TEMPLATE stripped of comments for smaller over the wire size + ACTIVE_ZIPLOADER_TEMPLATE = _strip_comments(ZIPLOADER_TEMPLATE) + class ModuleDepFinder(ast.NodeVisitor): # Caveats: # This code currently does not handle: @@ -301,19 +347,6 @@ class ModuleDepFinder(ast.NodeVisitor): self.generic_visit(node) -def _strip_comments(source): - # Strip comments and blank lines from the wrapper - buf = [] - for line in source.splitlines(): - l = line.strip() - if not l or l.startswith(u'#'): - continue - buf.append(line) - return u'\n'.join(buf) - -# ZIPLOADER_TEMPLATE stripped of comments for smaller over the wire size -STRIPPED_ZIPLOADER_TEMPLATE = _strip_comments(ZIPLOADER_TEMPLATE) - def _slurp(path): if not os.path.exists(path): raise AnsibleError("imported module support code does not exist at %s" % os.path.abspath(path)) @@ -555,10 +588,11 @@ def _find_snippet_imports(module_name, module_data, module_path, module_args, ta raise AnsibleError('A different worker process failed to create module file. Look at traceback for that process for debugging information.') # Fool the check later... I think we should just remove the check py_module_names.add(('basic',)) + shebang, interpreter = _get_shebang(u'/usr/bin/python', task_vars) if shebang is None: shebang = u'#!/usr/bin/python' - output.write(to_bytes(STRIPPED_ZIPLOADER_TEMPLATE % dict( + output.write(to_bytes(ACTIVE_ZIPLOADER_TEMPLATE % dict( zipdata=zipdata, ansible_module=module_name, params=python_repred_params, From 3c135ef3f2b7234e1ba20cc30822ebaca45a2bac Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 22 Apr 2016 15:55:13 -0700 Subject: [PATCH 3/6] Push debug files into a subdirectory to keep things cleaner. We now have the ansible module directory, ansible_module_*.py script file and the args file. Makes sense to push them all into a separate subdir. --- lib/ansible/executor/module_common.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index e043a1997a3..5bbdcc7dc24 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -166,8 +166,10 @@ def debug(command, zipped_mod, json_params): # $ /usr/bin/python /home/badger/.ansible/tmp/ansible-tmp-1461173013.93-9076457629738/ping execute # Okay to use __file__ here because we're running from a kept file - basedir = os.path.abspath(os.path.dirname(__file__)) + basedir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'debug_dir') args_path = os.path.join(basedir, 'args') + script_path = os.path.join(basedir, 'ansible_module_%(ansible_module)s.py') + if command == 'explode': # transform the ZIPDATA into an exploded directory of code and then # print the path to the code. This is an easy way for people to look @@ -205,13 +207,14 @@ def debug(command, zipped_mod, json_params): # This differs slightly from default Ansible execution of Python modules # as it passes the arguments to the module via a file instead of stdin. + # Set pythonpath to the debug dir pythonpath = os.environ.get('PYTHONPATH') if pythonpath: os.environ['PYTHONPATH'] = ':'.join((basedir, pythonpath)) else: os.environ['PYTHONPATH'] = basedir - p = subprocess.Popen(['%(interpreter)s', 'ansible_module_%(ansible_module)s.py', args_path], env=os.environ, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE) + p = subprocess.Popen(['%(interpreter)s', script_path, args_path], env=os.environ, shell=False, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE) (stdout, stderr) = p.communicate() if not isinstance(stderr, (bytes, unicode)): @@ -236,8 +239,10 @@ def debug(command, zipped_mod, json_params): # not actual bugs (as they don't affect the real way that we invoke # ansible modules) - # stub the + # stub the args and python path sys.argv = ['%(ansible_module)s', args_path] + sys.path.insert(0, basedir) + from ansible_module_%(ansible_module)s import main main() print('WARNING: Module returned to wrapper instead of exiting') @@ -257,6 +262,9 @@ if __name__ == '__main__': if PY3: ZIPLOADER_PARAMS = ZIPLOADER_PARAMS.encode('utf-8') try: + # There's a race condition with the controller removing the + # remote_tmpdir and this module executing under async. So we cannot + # store this in remote_tmpdir (use system tempdir instead) temp_path = tempfile.mkdtemp(prefix='ansible_') zipped_mod = os.path.join(temp_path, 'ansible_modlib.zip') modlib = open(zipped_mod, 'wb') From 3ffd55ce7f64f2d2d72ffea9b910150c7b5f6764 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Sat, 23 Apr 2016 11:14:47 -0700 Subject: [PATCH 4/6] Don't include "ansible" in the module output as there are a few files in the upper directory (args and the module) --- lib/ansible/executor/module_common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index 5bbdcc7dc24..ec50ab40316 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -197,7 +197,7 @@ def debug(command, zipped_mod, json_params): f.close() print('Module expanded into:') - print('%%s' %% os.path.join(basedir, 'ansible')) + print('%%s' %% basedir) exitcode = 0 elif command == 'execute': From bdd73e31dc62bfdeadc250ad5cb1fdecf866a10e Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 22 Apr 2016 16:04:04 -0700 Subject: [PATCH 5/6] Have test-module clean up the local temp dir when it exits Get test-module's debugger switch to do something useful with ziploader modules --- hacking/test-module | 56 +++++++++++++++++++++------ lib/ansible/executor/module_common.py | 1 + 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/hacking/test-module b/hacking/test-module index 6db1a12fe39..9f05ec30c0e 100755 --- a/hacking/test-module +++ b/hacking/test-module @@ -35,6 +35,7 @@ import os import subprocess import sys import traceback +import shutil import ansible.utils.vars as utils_vars from ansible.parsing.dataloader import DataLoader @@ -141,18 +142,43 @@ def boilerplate_module(modfile, args, interpreter, check, destfile): task_vars=task_vars ) + if module_style == 'new' and 'ZIPLOADER_WRAPPER = True' in module_data: + module_style = 'ziploader' + modfile2_path = os.path.expanduser(destfile) print("* including generated source, if any, saving to: %s" % modfile2_path) - print("* this may offset any line numbers in tracebacks/debuggers!") + if module_style not in ('ziploader', 'old'): + print("* this may offset any line numbers in tracebacks/debuggers!") modfile2 = open(modfile2_path, 'w') modfile2.write(module_data) modfile2.close() modfile = modfile2_path - return (modfile2_path, module_style) + return (modfile2_path, modname, module_style) -def runtest( modfile, argspath): +def ziploader_setup(modfile, modname): + os.system("chmod +x %s" % modfile) + + cmd = subprocess.Popen([modfile, 'explode'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + out, err = cmd.communicate() + lines = out.splitlines() + if len(lines) != 2 or 'Module expanded into' not in lines[0]: + print("*" * 35) + print("INVALID OUTPUT FROM ZIPLOADER MODULE WRAPPER") + print(out) + sys.exit(1) + debug_dir = lines[1].strip() + + argsfile = os.path.join(debug_dir, 'args') + modfile = os.path.join(debug_dir, 'ansible_module_%s.py' % modname) + + print("* ziploader module detected; extracted module source to: %s" % debug_dir) + return modfile, argsfile + +def runtest(modstyle, modfile, argspath, modname, module_style): """Test run a module, piping it's output for reporting.""" + if module_style == 'ziploader': + modfile, argspath = ziploader_setup(modfile, modname) os.system("chmod +x %s" % modfile) @@ -164,25 +190,28 @@ def runtest( modfile, argspath): (out, err) = cmd.communicate() try: - print("***********************************") + print("*" * 35) print("RAW OUTPUT") print(out) print(err) results = json.loads(out) except: - print("***********************************") + print("*" * 35) print("INVALID OUTPUT FORMAT") print(out) traceback.print_exc() sys.exit(1) - print("***********************************") + print("*" * 35) print("PARSED OUTPUT") print(jsonify(results,format=True)) -def rundebug(debugger, modfile, argspath): +def rundebug(debugger, modfile, argspath, modname, module_style): """Run interactively with console debugger.""" + if module_style == 'ziploader': + modfile, argspath = ziploader_setup(modfile, modname) + if argspath is not None: subprocess.call("%s %s %s" % (debugger, modfile, argspath), shell=True) else: @@ -191,10 +220,10 @@ def rundebug(debugger, modfile, argspath): def main(): options, args = parse() - (modfile, module_style) = boilerplate_module(options.module_path, options.module_args, options.interpreter, options.check, options.filename) + (modfile, modname, module_style) = boilerplate_module(options.module_path, options.module_args, options.interpreter, options.check, options.filename) argspath = None - if module_style != 'new': + if module_style not in ('new', 'ziploader'): if module_style == 'non_native_want_json': argspath = write_argsfile(options.module_args, json=True) elif module_style == 'old': @@ -203,10 +232,13 @@ def main(): raise Exception("internal error, unexpected module style: %s" % module_style) if options.execute: if options.debugger: - rundebug(options.debugger, modfile, argspath) + rundebug(options.debugger, modfile, argspath, modname, module_style) else: - runtest(modfile, argspath) + runtest(modfile, argspath, modname, module_style) if __name__ == "__main__": - main() + try: + main() + finally: + shutil.rmtree(C.DEFAULT_LOCAL_TMP, True) diff --git a/lib/ansible/executor/module_common.py b/lib/ansible/executor/module_common.py index ec50ab40316..2d10bd8ac78 100644 --- a/lib/ansible/executor/module_common.py +++ b/lib/ansible/executor/module_common.py @@ -60,6 +60,7 @@ _SNIPPET_PATH = os.path.join(os.path.dirname(__file__), '..', 'module_utils') ZIPLOADER_TEMPLATE = u'''%(shebang)s %(coding)s +ZIPLOADER_WRAPPER = True # For test-module script to tell this is a ZIPLOADER_WRAPPER # This code is part of Ansible, but is an independent component. # The code in this particular templatable string, and this templatable string # only, is BSD licensed. Modules which end up using this snippet, which is From 2e86260e1712c412c2376f3d8e79cb66bdba3e56 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Tue, 19 Apr 2016 20:08:23 -0700 Subject: [PATCH 6/6] Update Program flow documentation for new way that ziploader works Add documentation on how to debug ziploader modules --- docsite/rst/developing_modules.rst | 212 ++++++++++++++---- .../rst/developing_program_flow_modules.rst | 67 +++--- 2 files changed, 203 insertions(+), 76 deletions(-) diff --git a/docsite/rst/developing_modules.rst b/docsite/rst/developing_modules.rst index 4b26e5716cd..89ca9b978e9 100644 --- a/docsite/rst/developing_modules.rst +++ b/docsite/rst/developing_modules.rst @@ -3,25 +3,28 @@ Developing Modules .. contents:: Topics -Ansible modules are reusable units of magic that can be used by the Ansible API, -or by the `ansible` or `ansible-playbook` programs. +Ansible modules are reusable, standalone scripts that can be used by the Ansible API, +or by the :command:`ansible` or :command:`ansible-playbook` programs. They +return information to ansible by printing a JSON string to stdout before +exiting. They take arguments in in one of several ways which we'll go into +as we work through this tutorial. See :doc:`modules` for a list of various ones developed in core. Modules can be written in any language and are found in the path specified -by `ANSIBLE_LIBRARY` or the ``--module-path`` command line option. +by :envvar:`ANSIBLE_LIBRARY` or the ``--module-path`` command line option. -By default, everything that ships with ansible is pulled from its source tree, but +By default, everything that ships with Ansible is pulled from its source tree, but additional paths can be added. -The directory "./library", alongside your top level playbooks, is also automatically +The directory i:file:`./library`, alongside your top level :term:`playbooks`, is also automatically added as a search directory. Should you develop an interesting Ansible module, consider sending a pull request to the `modules-extras project `_. There's also a core repo for more established and widely used modules. "Extras" modules may be promoted to core periodically, -but there's no fundamental difference in the end - both ship with ansible, all in one package, regardless -of how you acquire ansible. +but there's no fundamental difference in the end - both ship with Ansible, all in one package, regardless +of how you acquire Ansible. .. _module_dev_tutorial: @@ -41,14 +44,14 @@ written in any language OTHER than Python are going to have to do exactly this. way later. So, here's an example. You would never really need to build a module to set the system time, -the 'command' module could already be used to do this. Though we're going to make one. +the 'command' module could already be used to do this. -Reading the modules that come with ansible (linked above) is a great way to learn how to write -modules. Keep in mind, though, that some modules in ansible's source tree are internalisms, -so look at `service` or `yum`, and don't stare too close into things like `async_wrapper` or -you'll turn to stone. Nobody ever executes async_wrapper directly. +Reading the modules that come with Ansible (linked above) is a great way to learn how to write +modules. Keep in mind, though, that some modules in Ansible's source tree are internalisms, +so look at :ref:`service` or :ref:`yum`, and don't stare too close into things like :ref:`async_wrapper` or +you'll turn to stone. Nobody ever executes :ref:`async_wrapper` directly. -Ok, let's get going with an example. We'll use Python. For starters, save this as a file named `timetest.py`:: +Ok, let's get going with an example. We'll use Python. For starters, save this as a file named :file:`timetest.py`:: #!/usr/bin/python @@ -65,13 +68,12 @@ Ok, let's get going with an example. We'll use Python. For starters, save this Testing Modules ```````````````` -There's a useful test script in the source checkout for ansible:: +There's a useful test script in the source checkout for Ansible:: git clone git://github.com/ansible/ansible.git --recursive source ansible/hacking/env-setup - chmod +x ansible/hacking/test-module -For instructions on setting up ansible from source, please see +For instructions on setting up Ansible from source, please see :doc:`intro_installation`. Let's run the script you just wrote with that:: @@ -80,7 +82,7 @@ Let's run the script you just wrote with that:: You should see output that looks something like this:: - {u'time': u'2012-03-14 22:13:48.539183'} + {'time': '2012-03-14 22:13:48.539183'} If you did not, you might have a typo in your module, so recheck it and try again. @@ -105,7 +107,7 @@ If no time parameter is set, we'll just leave the time as is and return the curr .. note:: This is obviously an unrealistic idea for a module. You'd most likely just - use the shell module. However, it probably makes a decent tutorial. + use the command module. However, it makes for a decent tutorial. Let's look at the code. Read the comments as we'll explain as we go. Note that this is highly verbose because it's intended as an educational example. You can write modules @@ -126,10 +128,12 @@ a lot shorter than this:: args_file = sys.argv[1] args_data = file(args_file).read() - # for this module, we're going to do key=value style arguments - # this is up to each module to decide what it wants, but all - # core modules besides 'command' and 'shell' take key=value - # so this is highly recommended + # For this module, we're going to do key=value style arguments. + # Modules can choose to receive json instead by adding the string: + # WANT_JSON + # Somewhere in the file. + # Modules can also take free-form arguments instead of key-value or json + # but this is not recommended. arguments = shlex.split(args_data) for arg in arguments: @@ -205,7 +209,7 @@ This should return something like:: Module Provided 'Facts' ```````````````````````` -The 'setup' module that ships with Ansible provides many variables about a system that can be used in playbooks +The :ref:`setup` module that ships with Ansible provides many variables about a system that can be used in playbooks and templates. However, it's possible to also add your own facts without modifying the system module. To do this, just have the module return a `ansible_facts` key, like so, along with other return data:: @@ -238,43 +242,52 @@ Rather than mention these here, the best way to learn is to read some of the `so The 'group' and 'user' modules are reasonably non-trivial and showcase what this looks like. -Key parts include always ending the module file with:: +Key parts include always importing the boilerplate code from +:mod:`ansible.module_utils.basic` like this:: - from ansible.module_utils.basic import * + from ansible.module_utils.basic import AnsibleModule if __name__ == '__main__': main() +.. note:: + Prior to Ansible-2.1.0, importing only what you used from + :mod:`ansible.module_utils.basic` did not work. You needed to use + a wildcard import like this:: + + from ansible.module_utils.basic import * + And instantiating the module class like:: - module = AnsibleModule( - argument_spec = dict( - state = dict(default='present', choices=['present', 'absent']), - name = dict(required=True), - enabled = dict(required=True, type='bool'), - something = dict(aliases=['whatever']) + def main(): + module = AnsibleModule( + argument_spec = dict( + state = dict(default='present', choices=['present', 'absent']), + name = dict(required=True), + enabled = dict(required=True, type='bool'), + something = dict(aliases=['whatever']) + ) ) - ) -The AnsibleModule provides lots of common code for handling returns, parses your arguments +The :class:`AnsibleModule` provides lots of common code for handling returns, parses your arguments for you, and allows you to check inputs. Successful returns are made like this:: module.exit_json(changed=True, something_else=12345) -And failures are just as simple (where 'msg' is a required parameter to explain the error):: +And failures are just as simple (where `msg` is a required parameter to explain the error):: module.fail_json(msg="Something fatal happened") -There are also other useful functions in the module class, such as module.sha1(path). See -lib/ansible/module_utils/basic.py in the source checkout for implementation details. +There are also other useful functions in the module class, such as :func:`module.sha1(path)`. See +:file:`lib/ansible/module_utils/basic.py` in the source checkout for implementation details. -Again, modules developed this way are best tested with the hacking/test-module script in the git +Again, modules developed this way are best tested with the :file:`hacking/test-module` script in the git source checkout. Because of the magic involved, this is really the only way the scripts can function outside of Ansible. -If submitting a module to ansible's core code, which we encourage, use of the AnsibleModule -class is required. +If submitting a module to Ansible's core code, which we encourage, use of +:class:`AnsibleModule` is required. .. _developing_for_check_mode: @@ -449,13 +462,126 @@ built and appear in the 'docsite/' directory. You can set the environment variable ANSIBLE_KEEP_REMOTE_FILES=1 on the controlling host to prevent ansible from deleting the remote files so you can debug your module. -.. _module_contribution: +.. _debugging_ansiblemodule_based_modules: + +Debugging AnsibleModule-based modules +````````````````````````````````````` + +.. tip:: + + If you're using the :file:`hacking/test-module` script then most of this + is taken care of for you. If you need to do some debugging of the module + on the remote machine that the module will actually run on or when the + module is used in a playbook then you may need to use this information + instead of relying on test-module. + +Starting with Ansible-2.1.0, AnsibleModule-based modules are put together as +a zip file consisting of the module file and the various python module +boilerplate inside of a wrapper script instead of as a single file with all of +the code concatenated together. Without some help, this can be harder to +debug as the file needs to be extracted from the wrapper in order to see +what's actually going on in the module. Luckily the wrapper script provides +some helper methods to do just that. + +If you are using Ansible with the :envvar:`ANSIBLE_KEEP_REMOTE_FILES` +environment variables to keep the remote module file, here's a sample of how +your debugging session will start:: + + $ ANSIBLE_KEEP_REMOTE_FILES=1 ansible localhost -m ping -a 'data=debugging_session' -vvv + <127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: badger + <127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo $HOME/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595 `" && echo "` echo $HOME/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595 `" )' + <127.0.0.1> PUT /var/tmp/tmpjdbJ1w TO /home/badger/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595/ping + <127.0.0.1> EXEC /bin/sh -c 'LANG=en_US.UTF-8 LC_ALL=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8 /usr/bin/python /home/badger/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595/ping' + localhost | SUCCESS => { + "changed": false, + "invocation": { + "module_args": { + "data": "debugging_session" + }, + "module_name": "ping" + }, + "ping": "debugging_session" + } + +Setting :envvar:`ANSIBLE_KEEP_REMOTE_FILE` to ``1`` tells Ansible to keep the +remote module files instead of deleting them after the module finishes +executing. Giving Ansible the ``-vvv`` optin makes Ansible more verbose. +That way it prints the file name of the temporary module file for you to see. + +If you want to examine the wrapper file you can. It will show a small python +script with a large, base64 encoded string. The string contains the module +that is going to be executed. Run the wrapper's explode command to turn the +string into some python files that you can work with:: + + $ python /home/badger/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595/ping explode + Module expanded into: + /home/badger/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595/debug_dir + +When you look into the debug_dir you'll see a directory structure like this:: + + ├── ansible_module_ping.py + ├── args + └── ansible + ├── __init__.py + └── module_utils + ├── basic.py + └── __init__.py + +* :file:`ansible_module_ping.py` is the code for the module itself. The name + is based on the name of the module with a prefix so that we don't clash with + any other python module names. You can modify this code to see what effect + it would have on your module. + +* The :file:`args` file contains a JSON string. The string is a dictionary + containing the module arguments and other variables that Ansible passes into + the module to change it's behaviour. If you want to modify the parameters + that are passed to the module, this is the file to do it in. + +* The :file:`ansible` directory contains code from + :module:`ansible.module_utils` that is used by the module. Ansible includes + files for any :`module:`ansible.module_utils` imports in the module but not + no files from any other module. So if your module uses + :module:`ansible.module_utils.url` Ansible will include it for you, but if + your module includes :module:`requests` then you'll have to make sure that + the python requests library is installed on the system before running the + module. You can modify files in this directory if you suspect that the + module is having a problem in some of this boilerplate code rather than in + the module code you have written. + +Once you edit the code or arguments in the exploded tree you need some way to +run it. There's a separate wrapper subcommand for this:: + + $ python /home/badger/.ansible/tmp/ansible-tmp-1461434734.35-235318071810595/ping execute + {"invocation": {"module_args": {"data": "debugging_session"}}, "changed": false, "ping": "debugging_session"} + +This subcommand takes care of setting the PYTHONPATH to use the exploded +:file:`debug_dir/ansible/module_utils` directory and invoking the script using +the arguments in the :file:`args` file. You can continue to run it like this +until you understand the problem. Then you can copy it back into your real +module file and test that the real module works via :command:`ansible` or +:command:`ansible-playbook`. + +.. note:: + + The wrapper provides one more subcommand, ``excommunicate``. This + subcommand is very similar to ``execute`` in that it invokes the exploded + module on the arguments in the :file:`args`. The way it does this is + different, however. ``excommunicate`` imports the :function:`main` + function from the module and then calls that. This makes excommunicate + execute the module in the wrapper's process. This may be useful for + running the module under some graphical debuggers but it is very different + from the way the module is executed by Ansible itself. Some modules may + not work with ``excommunicate`` or may behave differently than when used + with Ansible normally. Those are not bugs in the module; they're + limitations of ``excommunicate``. Use at your own risk. + +.. _module_paths Module Paths ```````````` If you are having trouble getting your module "found" by ansible, be -sure it is in the ``ANSIBLE_LIBRARY`` environment variable. +sure it is in the :envvar:`ANSIBLE_LIBRARY` environment variable. If you have a fork of one of the ansible module projects, do something like this:: @@ -468,6 +594,8 @@ To be safe, if you're working on a variant on something in Ansible's normal dist a bad idea to give it a new name while you are working on it, to be sure you know you're pulling your version. +.. _module_contribution: + Getting Your Module Into Ansible ```````````````````````````````` @@ -548,7 +676,7 @@ The following checklist items are important guidelines for people who want to c * Are module actions idempotent? If not document in the descriptions or the notes. * Import module snippets `from ansible.module_utils.basic import *` at the bottom, conserves line numbers for debugging. * Call your :func:`main` from a conditional so that it would be possible to - test them in the future example:: + import them into unittests in the future example:: if __name__ == '__main__': main() diff --git a/docsite/rst/developing_program_flow_modules.rst b/docsite/rst/developing_program_flow_modules.rst index 554bbc28521..c4f821231e4 100644 --- a/docsite/rst/developing_program_flow_modules.rst +++ b/docsite/rst/developing_program_flow_modules.rst @@ -286,9 +286,20 @@ imports of things in module_utils instead of merely preprocessing the module. It does this by constructing a zipfile--which includes the module file, files in :file:`ansible/module_utils` that are imported by the module, and some boilerplate to pass in the constants. The zipfile is then Base64 encoded and -wrapped in a small Python script which unzips the file on the managed node and -then invokes Python on the file. (Ansible wraps the zipfile in the Python -script so that pipelining will work.) +wrapped in a small Python script which decodes the Base64 encoding and places +the zipfile into a temp direcrtory on the managed node. It then extracts just +the ansible module script from the zip file and places that in the temporary +directory as well. Then it sets the PYTHONPATH to find python modules inside +of the zip file and invokes :command:`python` on the extracted ansible module. + +.. note:: + Ansible wraps the zipfile in the Python script for two reasons: + + * for compatibility with Python-2.4 and Python-2.6 which have less + featureful versions of Python's ``-m`` command line switch. + * so that pipelining will function properly. Pipelining needs to pipe the + Python module into the Python interpreter on the remote node. Python + understands scripts on stdin but does not understand zip files. In ziploader, any imports of Python modules from the ``ansible.module_utils`` package trigger inclusion of that Python file into the zipfile. Instances of @@ -299,16 +310,10 @@ that are included from module_utils are themselves scanned for imports of other Python modules from module_utils to be included in the zipfile as well. .. warning:: - At present, there are two caveats to how ziploader determines other files - to import: - - * Ziploader cannot determine whether an import should be included if it is - a relative import. Always use an absolute import that has - ``ansible.module_utils`` in it to allow ziploader to determine that the - file should be included. - * Ziploader does not include Python packages (directories with - :file:`__init__.py`` in them). Ziploader only works on :file:`*.py` - files that are directly in the :file:`ansible/module_utils` directory. + At present, Ziploader cannot determine whether an import should be + included if it is a relative import. Always use an absolute import that + has ``ansible.module_utils`` in it to allow ziploader to determine that + the file should be included. .. _flow_passing_module_args: @@ -317,13 +322,11 @@ Passing args In :ref:`module_replacer`, module arguments are turned into a JSON-ified string and substituted into the combined module file. In :ref:`ziploader`, -the JSON-ified string is placed in the the :envvar:`ANSIBLE_MODULE_ARGS` -environment variable. When :code:`ansible.module_utils.basic` is imported, -it places this string in the global variable -``ansible.module_utils.basic.MODULE_COMPLEX_ARGS`` and removes it from the -environment. Modules should not access this variable directly. Instead, they -should instantiate an :class:`AnsibleModule()` and use -:meth:`AnsibleModule.params` to access the parsed version of the arguments. +the JSON-ified string is passed into the module via stdin. When +a :class:`ansible.module_utils.basic.AnsibleModule` is instantiated, +it parses this string and places the args into +:attribute:`AnsibleModule.params` where it can be accessed by the module's +other code. .. _flow_passing_module_constants: @@ -351,21 +354,17 @@ For now, :code:`ANSIBLE_VERSION` is also available at its old location inside of ``ansible.module_utils.basic``, but that will eventually be removed. ``SELINUX_SPECIAL_FS`` and ``SYSLOG_FACILITY`` have changed much more. -:ref:`ziploader` passes these as another JSON-ified string inside of the -:envvar:`ANSIBLE_MODULE_CONSTANTS` environment variable. When -``ansible.module_utils.basic`` is imported, it places this string in the global -variable :code:`ansible.module_utils.basic.MODULE_CONSTANTS` and removes it from -the environment. The constants are parsed when an :class:`AnsibleModule` is -instantiated. Modules shouldn't access any of those directly. Instead, they -should instantiate an :class:`AnsibleModule` and use -:attr:`AnsibleModule.constants` to access the parsed version of these values. +:ref:`ziploader` passes these as part of the JSON-ified argument string via stdin. +When +:class:`ansible.module_utils.basic.AnsibleModule` is instantiated, it parses this +string and places the constants into :attribute:`AnsibleModule.constants` +where other code can access it. -Unlike the ``ANSIBLE_ARGS`` and ``ANSIBLE_VERSION``, where some efforts were -made to keep the old backwards compatible globals available, these two -constants are not available at their old names. This is a combination of the -degree to which these are internal to the needs of ``module_utils.basic`` and, -in the case of ``SYSLOG_FACILITY``, how hacky and unsafe the previous -implementation was. +Unlike the ``ANSIBLE_VERSION``, where some efforts were made to keep the old +backwards compatible globals available, these two constants are not available +at their old names. This is a combination of the degree to which these are +internal to the needs of ``module_utils.basic`` and, in the case of +``SYSLOG_FACILITY``, how hacky and unsafe the previous implementation was. Porting code from the :ref:`module_replacer` method of getting ``SYSLOG_FACILITY`` to the new one is a little more tricky than the other