Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acl fix performance #197

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/197-acl-fix-performance.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- "acl - Fix acl module performance (200~https://github.com/ansible-collections/ansible.posix/pull/197)."
alfonso-escribano marked this conversation as resolved.
Show resolved Hide resolved
89 changes: 78 additions & 11 deletions plugins/modules/acl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -220,19 +224,71 @@ 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':
return True

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

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


Expand Down Expand Up @@ -353,7 +409,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)
Expand All @@ -365,15 +424,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)

Expand Down