Fix #31694: running with closed stdin on python 3

[adelton]

SUMMARY

Fixes #31694.

When stdin is /dev/null, self._connection._new_stdin.buffer fails with AttributeError.

This amends #26495.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

./lib/ansible/plugins/action/pause.py

ANSIBLE VERSION

ansible-playbook 2.4.0.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/root/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.6/site-packages/ansible
  executable location = /usr/bin/ansible-playbook
  python version = 3.6.2 (default, Oct  2 2017, 16:51:32) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]

ADDITIONAL INFORMATION

Before:

# python3 /usr/bin/ansible-playbook -i localhost, /tmp/pause.yaml < /dev/null

PLAY [localhost] ***************************************************************

TASK [pause] *******************************************************************
Pausing for 5 seconds
(ctrl+C then 'C' = continue early, ctrl+C then 'A' = abort)
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: UnboundLocalError: local variable 'stdin' referenced before assignment
fatal: [localhost]: FAILED! => {"failed": true, "msg": "Unexpected failure during module execution.", "stdout": ""}

NO MORE HOSTS LEFT *************************************************************
	to retry, use: --limit @/tmp/pause.retry

PLAY RECAP *********************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1   

After:

# python3 /usr/bin/ansible-playbook -i localhost, /tmp/pause.yaml < /dev/null

PLAY [localhost] ***************************************************************

TASK [pause] *******************************************************************
Pausing for 5 seconds
(ctrl+C then 'C' = continue early, ctrl+C then 'A' = abort)
ok: [localhost]

PLAY RECAP *********************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0   

[adelton]

Alternative to #31586.

[adelton]

Could this change have caused the https://app.shippable.com/github/ansible/ansible/runs/40761/24/console failure or is that merely a fluke?

[mkrizek]

@adelton I just re-triggered the shippable job, we'll see...

[adelton]

@mkrizek, what is the best course of action to get this PR reviewed and merged?

[abadger]

Overall change looks good, but it seems a little clunky to check for closed file descriptor for stdin twice. Perhaps structure the change like this:

[...]
‎‎fd = None
‎try:
‎‎    if PY3:
‎‎        fd = self._connection._new_stdin.buffer.fileno()
‎‎    else:
‎        fd = self._connection._new_stdin.fileno()
‎‎except (ValueError, AttributeError):
‎‎    # ValueError: someone is using a closed file descriptor as stdin
‎‎    [...]

[adelton]

The problem is, that stdin (result of either stdin = self._connection._new_stdin.buffer or stdin = stdin = self._connection._new_stdin) is used further down the code: termios.tcflush(stdin, termios.TCIFLUSH), key_pressed = stdin.read(1), ...

So we cannot just skip the assignment to stdin and go directly with the .fileno().

[abadger]

Okay. I think we should still combine the try excepts, then but assign to stdin inside of the try: except block as well.

[adelton]

@abadger, thanks. Please consider rebased / modified patch 5707ad8.