From 4b8cb6582b6717e01ee5512333ea0fb029f626e0 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Thu, 12 Nov 2020 12:22:57 -0500 Subject: [PATCH] pause - do not hang if run in the background (#72065) * Consolidate logic for determining whether or not session is interactive into a single function, is_interactive() * Increase test coverage I wasn't able to find a good way of simulating running a backgrounded test with CI since the whole test is essentially run not in a TTY, which is similar enough to cause the new is_interactive() function to always return false. --- .../32143-pause-background-hangs.yml | 4 + lib/ansible/plugins/action/pause.py | 123 ++++++++++-------- .../targets/pause/test-pause-background.yml | 10 ++ test/integration/targets/pause/test-pause.yml | 30 +++++ 4 files changed, 113 insertions(+), 54 deletions(-) create mode 100644 changelogs/fragments/32143-pause-background-hangs.yml create mode 100644 test/integration/targets/pause/test-pause-background.yml diff --git a/changelogs/fragments/32143-pause-background-hangs.yml b/changelogs/fragments/32143-pause-background-hangs.yml new file mode 100644 index 00000000000..b125e542f1c --- /dev/null +++ b/changelogs/fragments/32143-pause-background-hangs.yml @@ -0,0 +1,4 @@ +bugfixes: + - > + pause - Fix indefinite hang when using a pause task on a background + process (https://github.com/ansible/ansible/issues/32142) diff --git a/lib/ansible/plugins/action/pause.py b/lib/ansible/plugins/action/pause.py index 86bd7e1ed94..5dbaa02082b 100644 --- a/lib/ansible/plugins/action/pause.py +++ b/lib/ansible/plugins/action/pause.py @@ -24,7 +24,11 @@ import termios import time import tty -from os import isatty +from os import ( + getpgrp, + isatty, + tcgetpgrp, +) from ansible.errors import AnsibleError from ansible.module_utils._text import to_text, to_native from ansible.module_utils.parsing.convert_bool import boolean @@ -67,6 +71,19 @@ def clear_line(stdout): stdout.write(b'\x1b[%s' % CLEAR_TO_EOL) +def is_interactive(fd=None): + if fd is None: + return False + + if isatty(fd): + # Compare the current process group to the process group associated + # with terminal of the given file descriptor to determine if the process + # is running in the background. + return getpgrp() == tcgetpgrp(fd) + else: + return False + + class ActionModule(ActionBase): ''' pauses execution for a length or time, or until input is received ''' @@ -177,71 +194,69 @@ class ActionModule(ActionBase): stdout_fd = stdout.fileno() except (ValueError, AttributeError): # ValueError: someone is using a closed file descriptor as stdin - # AttributeError: someone is using a null file descriptor as stdin on windoez + # AttributeError: someone is using a null file descriptor as stdin on windoze stdin = None + interactive = is_interactive(stdin_fd) + if interactive: + # grab actual Ctrl+C sequence + try: + intr = termios.tcgetattr(stdin_fd)[6][termios.VINTR] + except Exception: + # unsupported/not present, use default + intr = b'\x03' # value for Ctrl+C - if stdin_fd is not None: - if isatty(stdin_fd): - # grab actual Ctrl+C sequence - try: - intr = termios.tcgetattr(stdin_fd)[6][termios.VINTR] - except Exception: - # unsupported/not present, use default - intr = b'\x03' # value for Ctrl+C + # get backspace sequences + try: + backspace = termios.tcgetattr(stdin_fd)[6][termios.VERASE] + except Exception: + backspace = [b'\x7f', b'\x08'] - # get backspace sequences - try: - backspace = termios.tcgetattr(stdin_fd)[6][termios.VERASE] - except Exception: - backspace = [b'\x7f', b'\x08'] + old_settings = termios.tcgetattr(stdin_fd) + tty.setraw(stdin_fd) - old_settings = termios.tcgetattr(stdin_fd) - tty.setraw(stdin_fd) + # Only set stdout to raw mode if it is a TTY. This is needed when redirecting + # stdout to a file since a file cannot be set to raw mode. + if isatty(stdout_fd): + tty.setraw(stdout_fd) - # Only set stdout to raw mode if it is a TTY. This is needed when redirecting - # stdout to a file since a file cannot be set to raw mode. - if isatty(stdout_fd): - tty.setraw(stdout_fd) + # Only echo input if no timeout is specified + if not seconds and echo: + new_settings = termios.tcgetattr(stdin_fd) + new_settings[3] = new_settings[3] | termios.ECHO + termios.tcsetattr(stdin_fd, termios.TCSANOW, new_settings) - # Only echo input if no timeout is specified - if not seconds and echo: - new_settings = termios.tcgetattr(stdin_fd) - new_settings[3] = new_settings[3] | termios.ECHO - termios.tcsetattr(stdin_fd, termios.TCSANOW, new_settings) - - # flush the buffer to make sure no previous key presses - # are read in below - termios.tcflush(stdin, termios.TCIFLUSH) + # flush the buffer to make sure no previous key presses + # are read in below + termios.tcflush(stdin, termios.TCIFLUSH) while True: + if not interactive: + display.warning("Not waiting for response to prompt as stdin is not interactive") + if seconds is not None: + # Give the signal handler enough time to timeout + time.sleep(seconds + 1) + break try: - if stdin_fd is not None: + key_pressed = stdin.read(1) - key_pressed = stdin.read(1) + if key_pressed == intr: # value for Ctrl+C + clear_line(stdout) + raise KeyboardInterrupt - if key_pressed == intr: # value for Ctrl+C - clear_line(stdout) - raise KeyboardInterrupt - - if not seconds: - if stdin_fd is None or not isatty(stdin_fd): - display.warning("Not waiting for response to prompt as stdin is not interactive") - break - - # read key presses and act accordingly - if key_pressed in (b'\r', b'\n'): - clear_line(stdout) - break - elif key_pressed in backspace: - # delete a character if backspace is pressed - result['user_input'] = result['user_input'][:-1] - clear_line(stdout) - if echo: - stdout.write(result['user_input']) - stdout.flush() - else: - result['user_input'] += key_pressed + # read key presses and act accordingly + if key_pressed in (b'\r', b'\n'): + clear_line(stdout) + break + elif key_pressed in backspace: + # delete a character if backspace is pressed + result['user_input'] = result['user_input'][:-1] + clear_line(stdout) + if echo: + stdout.write(result['user_input']) + stdout.flush() + else: + result['user_input'] += key_pressed except KeyboardInterrupt: signal.alarm(0) diff --git a/test/integration/targets/pause/test-pause-background.yml b/test/integration/targets/pause/test-pause-background.yml new file mode 100644 index 00000000000..e480a77400f --- /dev/null +++ b/test/integration/targets/pause/test-pause-background.yml @@ -0,0 +1,10 @@ +- name: Test pause in a background task + hosts: localhost + gather_facts: no + become: no + + tasks: + - pause: + + - pause: + seconds: 1 diff --git a/test/integration/targets/pause/test-pause.yml b/test/integration/targets/pause/test-pause.yml index 2703eed434b..6fefbaa1f0a 100644 --- a/test/integration/targets/pause/test-pause.yml +++ b/test/integration/targets/pause/test-pause.yml @@ -4,6 +4,36 @@ become: no tasks: + - name: non-integer for duraction (EXPECTED FAILURE) + pause: + seconds: hello + register: result + ignore_errors: yes + + - assert: + that: + - result is failed + - "'non-integer' in result.msg" + + - name: non-boolean for echo (EXPECTED FAILURE) + pause: + echo: hello + register: result + ignore_errors: yes + + - assert: + that: + - result is failed + - "'not a valid boolean' in result.msg" + + - pause: + seconds: 0.1 + register: results + + - assert: + that: + - results.stdout is search('Paused for \d+\.\d+ seconds') + - pause: seconds: 1 register: results