A tool and python library that helps when interfacing with Ansible directly or as part of another system whether that be through a container image interface, as a standalone tool, or as a Python module that can be imported. The goal is to provide a stable and consistent interface abstraction to Ansible.
Other
969
stars
357
forks
source link
Restore original signal handlers in finished_callback #1402
The change in #862 added signal handlers for SIGINT and SIGTERM that may replace any existing SIGINT or SIGTERM handlers set by the process. Those existing handlers are not restored when the ansible_runner.run call finishes.
The solution here uses the finished_callback to restore the handlers. We're relying on the finished_callback actually getting called, which I'm not sure is a real guarantee. I'd be open to a solution that uses context managers or try-finally if we can figure out the right refactoring / where to put the try-finally semantics.
We are solving this problem today by using an ExitStack and a custom cancel_callback (that basically implements the same SIGINT/SIGTERM cancellation behavior as #862) that guarantees to restore the original handlers after the call completes.
TODOS
[ ] I wasn't sure where or how to write a test for the end-to-end signal restoring functionality. I found unit tests in test/unit/test_interface.py and test/unit/utils/test_utils.py that seem related, but wouldn't test the full "set and reset" behavior implemented here. Any pointers appreciated.
[ ] There is a TODO noted in the code that saving and restoring signal handlers set from outside Python, i.e. a C extension, is not possible. signal.getsignal returns None for these handlers. How does ansible-runner want to handle such a case? Options seem to be: (1) silently fail to restore (same behavior as today), (2) raise an exception (breaking change), or (3) allow the behavior to be configured.
The change in #862 added signal handlers for SIGINT and SIGTERM that may replace any existing SIGINT or SIGTERM handlers set by the process. Those existing handlers are not restored when the
ansible_runner.runcall finishes.The solution here uses the
finished_callbackto restore the handlers. We're relying on thefinished_callbackactually getting called, which I'm not sure is a real guarantee. I'd be open to a solution that uses context managers ortry-finallyif we can figure out the right refactoring / where to put thetry-finallysemantics.We are solving this problem today by using an
ExitStackand a customcancel_callback(that basically implements the same SIGINT/SIGTERM cancellation behavior as #862) that guarantees to restore the original handlers after the call completes.TODOS
test/unit/test_interface.pyandtest/unit/utils/test_utils.pythat seem related, but wouldn't test the full "set and reset" behavior implemented here. Any pointers appreciated.signal.getsignalreturnsNonefor these handlers. How doesansible-runnerwant to handle such a case? Options seem to be: (1) silently fail to restore (same behavior as today), (2)raisean exception (breaking change), or (3) allow the behavior to be configured.