From 8e645bb9ce959d43bf816e75e98df26ea14d239a Mon Sep 17 00:00:00 2001 From: Alfonso Escribano Date: Fri, 28 May 2021 13:42:34 +0200 Subject: [PATCH 1/5] Fix acl performance --- plugins/modules/acl.py | 113 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 102 insertions(+), 11 deletions(-) diff --git a/plugins/modules/acl.py b/plugins/modules/acl.py index c4a55422fd..eeecb2e7ec 100644 --- a/plugins/modules/acl.py +++ b/plugins/modules/acl.py @@ -138,9 +138,13 @@ import os import platform +import fcntl from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_native +from ansible.module_utils.compat import selectors +from ansible.module_utils.common.text.converters import to_native, to_text, to_bytes +from ansible.module_utils.six import b def split_entry(entry): @@ -220,7 +224,7 @@ def build_command(module, mode, path, follow, default, recursive, recalculate_ma return cmd -def acl_changed(module, cmd): +def acl_changed(module, cmd, check_rc=True): '''Returns true if the provided command affects the existing ACLs, false otherwise.''' # FreeBSD do not have a --test flag, so by default, it is safer to always say "true" if platform.system().lower() == 'freebsd': @@ -228,11 +232,87 @@ def acl_changed(module, cmd): cmd = cmd[:] # lists are mutables so cmd would be overwritten without this cmd.insert(1, '--test') - lines = run_acl(module, cmd) + module._acl_changed = False + + def _process_stdout_from_pipe(proc, _acl_module=module): + stdout = b'' + try: + selector = selectors.DefaultSelector() + except (IOError, OSError): + # Failed to detect default selector for the given platform + # Select PollSelector which is supported by major platforms + selector = selectors.PollSelector() + + selector.register(proc.stdout, selectors.EVENT_READ) + if os.name == 'posix': + fcntl.fcntl(proc.stdout.fileno(), fcntl.F_SETFL, fcntl.fcntl(proc.stdout.fileno(), fcntl.F_GETFL) | os.O_NONBLOCK) + + while True: + events = selector.select(1) + for key, event in events: + b_chunk = key.fileobj.read() + if b_chunk == b(''): + selector.unregister(key.fileobj) + if key.fileobj == proc.stdout: + stdout = b_chunk + if _acl_module._acl_changed: + continue + lines = [] + for l in stdout.splitlines(): + lines.append(l.strip()) + for line in lines: + if not line.endswith(b'*,*'): + proc.terminate() + _acl_module._acl_changed = True + proc.returncode = 0 + + # only break out if no pipes are left to read or + # the pipes are completely read and + # the process is terminated + if (not events or not selector.get_map()) and proc.poll() is not None: + break + # No pipes are left to read but process is not yet terminated + # Only then it is safe to wait for the process to be finished + # NOTE: Actually proc.poll() is always None here if no selectors are left + elif not selector.get_map() and proc.poll() is None: + proc.wait() + # The process is terminated. Since no pipes to read from are + # left, there is no need to call select() again. + break + +# proc.stdout.close() +# proc.stderr.close() +# selector.close() + + +# while True: +# output = proc.stdout.readline() +# if output == '' and proc.poll() is not None: +# break +# if _acl_module._acl_changed: +# continue +# lines = [] +# for l in output.splitlines(): +# lines.append(l.strip()) +# for line in lines: +# if not line.endswith(b'*,*'): +# proc.terminate() +# while True: +# output = proc.stdout.readline() +# if output == '' and proc.poll() is not None: +# break +# proc.returncode=0 +# _acl_module._acl_changed=True - for line in lines: - if not line.endswith('*,*'): - return True + try: + (rc, out, err) = module.run_command( + cmd, check_rc=check_rc, + before_communicate_callback=_process_stdout_from_pipe) + except Exception as e: + module.fail_json(msg=to_native(e)) + + if module._acl_changed: + return True return False @@ -353,7 +433,10 @@ def main(): if changed and not module.check_mode: run_acl(module, command) - msg = "%s is present" % entry + if recursive: + msg = "%s is present recursively" % entry + else: + msg = "%s is present" % entry elif state == 'absent': entry = build_entry(etype, entity, use_nfsv4_acls) @@ -365,15 +448,23 @@ def main(): if changed and not module.check_mode: run_acl(module, command, False) - msg = "%s is absent" % entry + if recursive: + msg = "%s is absent recursively" % entry + else: + msg = "%s is absent" % entry elif state == 'query': msg = "current acl" - acl = run_acl( - module, - build_command(module, 'get', path, follow, default, recursive, recalculate_mask) - ) + if recursive: + acl = [] + warn = "Not showing resulting acls in the recursive mode" + module.exit_json(changed=changed, msg=msg, acl=acl, warnings=warn) + else: + acl = run_acl( + module, + build_command(module, 'get', path, follow, default, recursive, recalculate_mask) + ) module.exit_json(changed=changed, msg=msg, acl=acl) From b1db0b8276f3f3ddaab4df0780ef7938f2458e74 Mon Sep 17 00:00:00 2001 From: Alfonso Escribano Date: Fri, 28 May 2021 13:48:07 +0200 Subject: [PATCH 2/5] Correct identation --- plugins/modules/acl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/acl.py b/plugins/modules/acl.py index eeecb2e7ec..89f029cf2f 100644 --- a/plugins/modules/acl.py +++ b/plugins/modules/acl.py @@ -255,8 +255,8 @@ def _process_stdout_from_pipe(proc, _acl_module=module): selector.unregister(key.fileobj) if key.fileobj == proc.stdout: stdout = b_chunk - if _acl_module._acl_changed: - continue + if _acl_module._acl_changed: + continue lines = [] for l in stdout.splitlines(): lines.append(l.strip()) From d0e01dd77f9f2a18b4a7a808104da7de8d34c113 Mon Sep 17 00:00:00 2001 From: alfonso-escribano Date: Mon, 31 May 2021 10:57:36 +0200 Subject: [PATCH 3/5] Clean old ansible versions commented code --- plugins/modules/acl.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/plugins/modules/acl.py b/plugins/modules/acl.py index 89f029cf2f..6b359bb47d 100644 --- a/plugins/modules/acl.py +++ b/plugins/modules/acl.py @@ -280,30 +280,6 @@ def _process_stdout_from_pipe(proc, _acl_module=module): # left, there is no need to call select() again. break -# proc.stdout.close() -# proc.stderr.close() -# selector.close() - - -# while True: -# output = proc.stdout.readline() -# if output == '' and proc.poll() is not None: -# break -# if _acl_module._acl_changed: -# continue -# lines = [] -# for l in output.splitlines(): -# lines.append(l.strip()) -# for line in lines: -# if not line.endswith(b'*,*'): -# proc.terminate() -# while True: -# output = proc.stdout.readline() -# if output == '' and proc.poll() is not None: -# break -# proc.returncode=0 -# _acl_module._acl_changed=True - try: (rc, out, err) = module.run_command( cmd, check_rc=check_rc, From 119bba68a662d197df24929af329c943c8861702 Mon Sep 17 00:00:00 2001 From: alfonso-escribano Date: Mon, 31 May 2021 12:31:16 +0200 Subject: [PATCH 4/5] Add Changelog fragment for PR#197 --- changelogs/fragments/197-acl-fix-performance.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/197-acl-fix-performance.yml diff --git a/changelogs/fragments/197-acl-fix-performance.yml b/changelogs/fragments/197-acl-fix-performance.yml new file mode 100644 index 0000000000..f9aa0211cf --- /dev/null +++ b/changelogs/fragments/197-acl-fix-performance.yml @@ -0,0 +1,2 @@ +bugfixes: + - "acl - Fix acl module performance (200~https://github.com/ansible-collections/ansible.posix/pull/197)." From 6782f88e390feb39f8e08e7f5d72c994495e3492 Mon Sep 17 00:00:00 2001 From: Alfonso Escribano Merino Date: Mon, 31 May 2021 13:44:03 +0200 Subject: [PATCH 5/5] Update changelogs/fragments/197-acl-fix-performance.yml Co-authored-by: Abhijeet Kasurde --- changelogs/fragments/197-acl-fix-performance.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/197-acl-fix-performance.yml b/changelogs/fragments/197-acl-fix-performance.yml index f9aa0211cf..cf96ac6c48 100644 --- a/changelogs/fragments/197-acl-fix-performance.yml +++ b/changelogs/fragments/197-acl-fix-performance.yml @@ -1,2 +1,2 @@ bugfixes: - - "acl - Fix acl module performance (200~https://github.com/ansible-collections/ansible.posix/pull/197)." + - acl - Fix module performance (https://github.com/ansible-collections/ansible.posix/pull/197).