From 2f1df7f82d1d04e46d22f1d6be60e707027f1535 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 22 Apr 2018 14:27:41 +0100 Subject: [PATCH 01/10] ansible: FileService wasn't sleeping properly. "_schedule_pending" is a function, "_pending_by_stream" is the map we want to test. --- ansible_mitogen/services.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index fee50934..d2cb9355 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -420,7 +420,7 @@ class FileService(mitogen.service.Service): #: Time spent by the scheduler thread asleep when it has no more data to #: pump, but while at least one transfer remains active. With #: max_queue_size=1MiB and a sleep of 10ms, maximum throughput on any - #: single stream is 100MiB/sec, which is 5x what SSH can handle on my + #: single stream is 112MiB/sec, which is >5x what SSH can handle on my #: laptop. sleep_delay_ms = 0.01 @@ -500,7 +500,7 @@ class FileService(mitogen.service.Service): :meth:`on_shutdown` hasn't been called yet, otherwise :data:`False`. """ - if self._schedule_pending: + if self._pending_by_stream: timeout = self.sleep_delay_ms else: timeout = None @@ -523,7 +523,7 @@ class FileService(mitogen.service.Service): """ Scheduler thread's main function. Sleep until :meth:`_sleep_on_queue` indicates the queue has been shut down, - pending pending file chunks each time we wake. + pumping pending file chunks each time we wake. """ while self._sleep_on_queue(): for stream, pending in list(self._pending_by_stream.items()): From 7c6ce726aa99a99c553b94631ad590d007cdaad1 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 22 Apr 2018 15:14:47 +0100 Subject: [PATCH 02/10] ansible: rename variable to reflect correct time unit --- ansible_mitogen/services.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index d2cb9355..79c8de46 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -422,7 +422,7 @@ class FileService(mitogen.service.Service): #: max_queue_size=1MiB and a sleep of 10ms, maximum throughput on any #: single stream is 112MiB/sec, which is >5x what SSH can handle on my #: laptop. - sleep_delay_ms = 0.01 + sleep_delay_secs = 0.01 def __init__(self, router): super(FileService, self).__init__(router) @@ -430,7 +430,7 @@ class FileService(mitogen.service.Service): self._size_by_path = {} #: Queue used to communicate from service to scheduler thread. self._queue = mitogen.core.Latch() - #: Mapping of Stream->[(sender, fp)]. + #: Mapping of Stream->[(Sender, file object)]. self._pending_by_stream = {} self._thread = threading.Thread(target=self._scheduler_main) self._thread.start() @@ -488,9 +488,9 @@ class FileService(mitogen.service.Service): def _sleep_on_queue(self): """ - Sleep indefinitely (no active transfers) or for :attr:`sleep_delay_ms` - (active transfers) waiting for a new transfer request to arrive from - the :meth:`fetch` method. + Sleep indefinitely (no active transfers) or for + :attr:`sleep_delay_secs` (active transfers) waiting for a new transfer + request to arrive from the :meth:`fetch` method. If a new request arrives, add it to the appropriate list in :attr:`_pending_by_stream`. @@ -501,7 +501,7 @@ class FileService(mitogen.service.Service): :data:`False`. """ if self._pending_by_stream: - timeout = self.sleep_delay_ms + timeout = self.sleep_delay_secs else: timeout = None From 962ba862e9686799c1de44d54f5fa04277feb62f Mon Sep 17 00:00:00 2001 From: David Wilson Date: Sun, 22 Apr 2018 19:40:58 +0100 Subject: [PATCH 03/10] tests: use test-targets group, not all group --- tests/ansible/bench/file_transfer.yml | 2 +- tests/ansible/hosts | 2 ++ .../ansible/integration/action/low_level_execute_command.yml | 2 +- tests/ansible/integration/action/make_tmp_path.yml | 2 +- tests/ansible/integration/action/remote_file_exists.yml | 2 +- tests/ansible/integration/action/transfer_data.yml | 2 +- .../integration/async/result_binary_producing_json.yml | 2 +- .../integration/async/result_binary_producing_junk.yml | 2 +- tests/ansible/integration/async/result_shell_echo_hi.yml | 2 +- tests/ansible/integration/async/runner_job_timeout.yml | 2 +- tests/ansible/integration/async/runner_one_job.yml | 2 +- .../integration/async/runner_two_simultaneous_jobs.yml | 2 +- tests/ansible/integration/become/sudo_flags_failure.yml | 2 +- tests/ansible/integration/become/sudo_nonexistent.yml | 2 +- tests/ansible/integration/become/sudo_nopassword.yml | 2 +- tests/ansible/integration/become/sudo_password.yml | 2 +- tests/ansible/integration/become/sudo_requiretty.yml | 2 +- .../ansible/integration/connection_loader/local_blemished.yml | 2 +- .../integration/connection_loader/paramiko_unblemished.yml | 2 +- tests/ansible/integration/connection_loader/ssh_blemished.yml | 2 +- tests/ansible/integration/context_service/lru_one_target.yml | 2 +- tests/ansible/integration/context_service/reconnection.yml | 2 +- tests/ansible/integration/playbook_semantics/become_flags.yml | 4 ++-- tests/ansible/integration/playbook_semantics/delegate_to.yml | 2 +- tests/ansible/integration/playbook_semantics/environment.yml | 2 +- tests/ansible/integration/playbook_semantics/with_items.yml | 2 +- tests/ansible/integration/runner/builtin_command_module.yml | 2 +- .../integration/runner/custom_bash_old_style_module.yml | 2 +- .../integration/runner/custom_bash_want_json_module.yml | 2 +- .../integration/runner/custom_binary_producing_json.yml | 2 +- .../integration/runner/custom_binary_producing_junk.yml | 4 ++-- .../ansible/integration/runner/custom_binary_single_null.yml | 4 ++-- .../integration/runner/custom_perl_json_args_module.yml | 2 +- .../integration/runner/custom_perl_want_json_module.yml | 2 +- .../integration/runner/custom_python_json_args_module.yml | 2 +- .../runner/custom_python_new_style_missing_interpreter.yml | 2 +- .../integration/runner/custom_python_new_style_module.yml | 2 +- .../integration/runner/custom_python_want_json_module.yml | 2 +- .../ansible/integration/runner/custom_script_interpreter.yml | 2 +- tests/ansible/integration/runner/forking_behaviour.yml | 2 +- tests/ansible/integration/runner/remote_tmp.yml | 2 +- tests/ansible/osx_setup.yml | 2 +- .../issue_109__target_has_old_ansible_installed.yml | 2 +- .../regression/issue_113__duplicate_module_imports.yml | 2 +- .../ansible/regression/issue_118__script_not_marked_exec.yml | 2 +- .../ansible/regression/issue_122__environment_difference.yml | 2 +- tests/ansible/regression/issue_140__thread_pileup.yml | 2 +- .../regression/issue_152__local_action_wrong_interpreter.yml | 2 +- .../ansible/regression/issue_152__virtualenv_python_fails.yml | 2 +- tests/ansible/regression/issue_154__module_state_leaks.yml | 2 +- tests/ansible/regression/issue_177__copy_module_failing.yml | 2 +- 51 files changed, 55 insertions(+), 53 deletions(-) diff --git a/tests/ansible/bench/file_transfer.yml b/tests/ansible/bench/file_transfer.yml index 99666b84..d0ac727d 100644 --- a/tests/ansible/bench/file_transfer.yml +++ b/tests/ansible/bench/file_transfer.yml @@ -1,6 +1,6 @@ - name: bench/file_transfer.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/hosts b/tests/ansible/hosts index 2fbb50c4..3c85d1eb 100644 --- a/tests/ansible/hosts +++ b/tests/ansible/hosts @@ -1 +1,3 @@ + +[test-targets] localhost diff --git a/tests/ansible/integration/action/low_level_execute_command.yml b/tests/ansible/integration/action/low_level_execute_command.yml index d900f143..842d99d2 100644 --- a/tests/ansible/integration/action/low_level_execute_command.yml +++ b/tests/ansible/integration/action/low_level_execute_command.yml @@ -1,7 +1,7 @@ # Verify the behaviour of _low_level_execute_command(). - name: integration/action__low_level_execute_command.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/action/make_tmp_path.yml b/tests/ansible/integration/action/make_tmp_path.yml index 4384aa86..603ebd09 100644 --- a/tests/ansible/integration/action/make_tmp_path.yml +++ b/tests/ansible/integration/action/make_tmp_path.yml @@ -1,6 +1,6 @@ - name: integration/action/make_tmp_path.yml - hosts: all + hosts: test-targets any_errors_fatal: true gather_facts: true tasks: diff --git a/tests/ansible/integration/action/remote_file_exists.yml b/tests/ansible/integration/action/remote_file_exists.yml index bfda9656..20a825c8 100644 --- a/tests/ansible/integration/action/remote_file_exists.yml +++ b/tests/ansible/integration/action/remote_file_exists.yml @@ -1,6 +1,6 @@ - name: integration/action/remote_file_exists.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/action/transfer_data.yml b/tests/ansible/integration/action/transfer_data.yml index 8f7381e3..2a5a7b56 100644 --- a/tests/ansible/integration/action/transfer_data.yml +++ b/tests/ansible/integration/action/transfer_data.yml @@ -1,6 +1,6 @@ - name: integration/action/transfer_data.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/async/result_binary_producing_json.yml b/tests/ansible/integration/async/result_binary_producing_json.yml index 71bf4351..8b6d59b9 100644 --- a/tests/ansible/integration/async/result_binary_producing_json.yml +++ b/tests/ansible/integration/async/result_binary_producing_json.yml @@ -1,7 +1,7 @@ - name: integration/async/result_binary_producing_json.yml gather_facts: true - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/async/result_binary_producing_junk.yml b/tests/ansible/integration/async/result_binary_producing_junk.yml index 11de5d31..37f31704 100644 --- a/tests/ansible/integration/async/result_binary_producing_junk.yml +++ b/tests/ansible/integration/async/result_binary_producing_junk.yml @@ -1,7 +1,7 @@ - name: integration/async/result_binary_producing_junk.yml gather_facts: true - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/async/result_shell_echo_hi.yml b/tests/ansible/integration/async/result_shell_echo_hi.yml index ce439743..77678318 100644 --- a/tests/ansible/integration/async/result_shell_echo_hi.yml +++ b/tests/ansible/integration/async/result_shell_echo_hi.yml @@ -1,7 +1,7 @@ - name: integration/async/result_shell_echo_hi.yml gather_facts: true - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/async/runner_job_timeout.yml b/tests/ansible/integration/async/runner_job_timeout.yml index b15fc2dd..e279c5cb 100644 --- a/tests/ansible/integration/async/runner_job_timeout.yml +++ b/tests/ansible/integration/async/runner_job_timeout.yml @@ -1,7 +1,7 @@ # Verify 'async: ' functions as desired. - name: integration/async/runner_job_timeout.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/async/runner_one_job.yml b/tests/ansible/integration/async/runner_one_job.yml index 846b173c..989a7cda 100644 --- a/tests/ansible/integration/async/runner_one_job.yml +++ b/tests/ansible/integration/async/runner_one_job.yml @@ -2,7 +2,7 @@ # fields. - name: integration/async/runner_one_job.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml b/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml index 5c20fc5d..51ceef2f 100644 --- a/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml +++ b/tests/ansible/integration/async/runner_two_simultaneous_jobs.yml @@ -1,6 +1,6 @@ - name: integration/async/runner_two_simultaneous_jobs.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/become/sudo_flags_failure.yml b/tests/ansible/integration/become/sudo_flags_failure.yml index 0ced9ddb..484134c5 100644 --- a/tests/ansible/integration/become/sudo_flags_failure.yml +++ b/tests/ansible/integration/become/sudo_flags_failure.yml @@ -1,5 +1,5 @@ - name: integration/become/sudo_flags_failure.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/become/sudo_nonexistent.yml b/tests/ansible/integration/become/sudo_nonexistent.yml index 25d2dc47..ccb94e34 100644 --- a/tests/ansible/integration/become/sudo_nonexistent.yml +++ b/tests/ansible/integration/become/sudo_nonexistent.yml @@ -1,5 +1,5 @@ - name: integration/become/sudo_nonexistent.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/become/sudo_nopassword.yml b/tests/ansible/integration/become/sudo_nopassword.yml index 1d50715c..03644423 100644 --- a/tests/ansible/integration/become/sudo_nopassword.yml +++ b/tests/ansible/integration/become/sudo_nopassword.yml @@ -1,7 +1,7 @@ # Verify passwordless sudo behaviour in various cases. - name: integration/become/sudo_basic.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/become/sudo_password.yml b/tests/ansible/integration/become/sudo_password.yml index 2f6f00e6..128d8aee 100644 --- a/tests/ansible/integration/become/sudo_password.yml +++ b/tests/ansible/integration/become/sudo_password.yml @@ -1,7 +1,7 @@ # Verify passwordful sudo behaviour - name: integration/become/sudo_password.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/become/sudo_requiretty.yml b/tests/ansible/integration/become/sudo_requiretty.yml index a31d5712..59b8b823 100644 --- a/tests/ansible/integration/become/sudo_requiretty.yml +++ b/tests/ansible/integration/become/sudo_requiretty.yml @@ -1,7 +1,7 @@ # Verify requiretty support - name: integration/become/sudo_requiretty.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/connection_loader/local_blemished.yml b/tests/ansible/integration/connection_loader/local_blemished.yml index 72e28ec2..f0a6d4de 100644 --- a/tests/ansible/integration/connection_loader/local_blemished.yml +++ b/tests/ansible/integration/connection_loader/local_blemished.yml @@ -1,7 +1,7 @@ # Ensure 'local' connections are grabbed. - name: integration/connection_loader__local_blemished.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - determine_strategy: diff --git a/tests/ansible/integration/connection_loader/paramiko_unblemished.yml b/tests/ansible/integration/connection_loader/paramiko_unblemished.yml index c29c25d2..a71af868 100644 --- a/tests/ansible/integration/connection_loader/paramiko_unblemished.yml +++ b/tests/ansible/integration/connection_loader/paramiko_unblemished.yml @@ -1,7 +1,7 @@ # Ensure paramiko connections aren't grabbed. - name: integration/connection_loader__paramiko_unblemished.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_python_detect_environment: diff --git a/tests/ansible/integration/connection_loader/ssh_blemished.yml b/tests/ansible/integration/connection_loader/ssh_blemished.yml index 4f47a72f..04ada1e3 100644 --- a/tests/ansible/integration/connection_loader/ssh_blemished.yml +++ b/tests/ansible/integration/connection_loader/ssh_blemished.yml @@ -1,7 +1,7 @@ # Ensure 'ssh' connections are grabbed. - name: integration/connection_loader__ssh_blemished.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - determine_strategy: diff --git a/tests/ansible/integration/context_service/lru_one_target.yml b/tests/ansible/integration/context_service/lru_one_target.yml index 54264598..01a9e0dd 100644 --- a/tests/ansible/integration/context_service/lru_one_target.yml +++ b/tests/ansible/integration/context_service/lru_one_target.yml @@ -1,7 +1,7 @@ # Verify a maximum number of contexts are possible on one machine. - name: integration/context_service/lru_one_target.yml - hosts: all + hosts: test-targets any_errors_fatal: true vars: max_interps: "{{lookup('env', 'MITOGEN_MAX_INTERPRETERS')}}" diff --git a/tests/ansible/integration/context_service/reconnection.yml b/tests/ansible/integration/context_service/reconnection.yml index 8d7c7e60..f56719d8 100644 --- a/tests/ansible/integration/context_service/reconnection.yml +++ b/tests/ansible/integration/context_service/reconnection.yml @@ -2,7 +2,7 @@ # cleanup of dependent (via=) contexts. - name: integration/context_service/reconnection.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/playbook_semantics/become_flags.yml b/tests/ansible/integration/playbook_semantics/become_flags.yml index 1f1cb15c..f2ab0b5d 100644 --- a/tests/ansible/integration/playbook_semantics/become_flags.yml +++ b/tests/ansible/integration/playbook_semantics/become_flags.yml @@ -3,7 +3,7 @@ # - name: integration/playbook_semantics/become_flags.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: @@ -15,7 +15,7 @@ - assert: that: "out.stdout == ''" -- hosts: all +- hosts: test-targets any_errors_fatal: true become_flags: -E tasks: diff --git a/tests/ansible/integration/playbook_semantics/delegate_to.yml b/tests/ansible/integration/playbook_semantics/delegate_to.yml index 16c4b5a0..4d4da028 100644 --- a/tests/ansible/integration/playbook_semantics/delegate_to.yml +++ b/tests/ansible/integration/playbook_semantics/delegate_to.yml @@ -1,5 +1,5 @@ - name: integration/playbook_semantics/delegate_to.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: # diff --git a/tests/ansible/integration/playbook_semantics/environment.yml b/tests/ansible/integration/playbook_semantics/environment.yml index 47437d9b..1c183a5a 100644 --- a/tests/ansible/integration/playbook_semantics/environment.yml +++ b/tests/ansible/integration/playbook_semantics/environment.yml @@ -1,7 +1,7 @@ # Ensure environment: is preserved during call. - name: integration/playbook_semantics/environment.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - shell: echo $SOME_ENV diff --git a/tests/ansible/integration/playbook_semantics/with_items.yml b/tests/ansible/integration/playbook_semantics/with_items.yml index 876f7e03..db94cb19 100644 --- a/tests/ansible/integration/playbook_semantics/with_items.yml +++ b/tests/ansible/integration/playbook_semantics/with_items.yml @@ -2,7 +2,7 @@ # the correct context. - name: integration/playbook_semantics/with_items.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/runner/builtin_command_module.yml b/tests/ansible/integration/runner/builtin_command_module.yml index 8cfcc25c..afa34902 100644 --- a/tests/ansible/integration/runner/builtin_command_module.yml +++ b/tests/ansible/integration/runner/builtin_command_module.yml @@ -1,6 +1,6 @@ - name: integration/runner__builtin_command_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true gather_facts: true tasks: diff --git a/tests/ansible/integration/runner/custom_bash_old_style_module.yml b/tests/ansible/integration/runner/custom_bash_old_style_module.yml index 030049ca..96bc9297 100644 --- a/tests/ansible/integration/runner/custom_bash_old_style_module.yml +++ b/tests/ansible/integration/runner/custom_bash_old_style_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_bash_old_style_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/runner/custom_bash_want_json_module.yml b/tests/ansible/integration/runner/custom_bash_want_json_module.yml index 4700aac5..f46a32c7 100644 --- a/tests/ansible/integration/runner/custom_bash_want_json_module.yml +++ b/tests/ansible/integration/runner/custom_bash_want_json_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_bash_want_json_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_bash_want_json_module: diff --git a/tests/ansible/integration/runner/custom_binary_producing_json.yml b/tests/ansible/integration/runner/custom_binary_producing_json.yml index 951a203b..ed3da57a 100644 --- a/tests/ansible/integration/runner/custom_binary_producing_json.yml +++ b/tests/ansible/integration/runner/custom_binary_producing_json.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_binary_producing_json.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_binary_producing_json: diff --git a/tests/ansible/integration/runner/custom_binary_producing_junk.yml b/tests/ansible/integration/runner/custom_binary_producing_junk.yml index 1d526947..32797b55 100644 --- a/tests/ansible/integration/runner/custom_binary_producing_junk.yml +++ b/tests/ansible/integration/runner/custom_binary_producing_junk.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_binary_producing_junk.yml - hosts: all + hosts: test-targets tasks: - custom_binary_producing_junk: foo: true @@ -8,7 +8,7 @@ register: out -- hosts: all +- hosts: test-targets any_errors_fatal: true tasks: - debug: msg={{out}} diff --git a/tests/ansible/integration/runner/custom_binary_single_null.yml b/tests/ansible/integration/runner/custom_binary_single_null.yml index fa5a86ca..f23ec1c2 100644 --- a/tests/ansible/integration/runner/custom_binary_single_null.yml +++ b/tests/ansible/integration/runner/custom_binary_single_null.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_binary_single_null.yml - hosts: all + hosts: test-targets tasks: - custom_binary_single_null: foo: true @@ -7,7 +7,7 @@ ignore_errors: true register: out -- hosts: all +- hosts: test-targets any_errors_fatal: true tasks: - assert: diff --git a/tests/ansible/integration/runner/custom_perl_json_args_module.yml b/tests/ansible/integration/runner/custom_perl_json_args_module.yml index 7606ef3d..a59ab441 100644 --- a/tests/ansible/integration/runner/custom_perl_json_args_module.yml +++ b/tests/ansible/integration/runner/custom_perl_json_args_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_perl_json_args_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_perl_json_args_module: diff --git a/tests/ansible/integration/runner/custom_perl_want_json_module.yml b/tests/ansible/integration/runner/custom_perl_want_json_module.yml index 7bc4e75f..c1dc1a2d 100644 --- a/tests/ansible/integration/runner/custom_perl_want_json_module.yml +++ b/tests/ansible/integration/runner/custom_perl_want_json_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_perl_want_json_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_perl_want_json_module: diff --git a/tests/ansible/integration/runner/custom_python_json_args_module.yml b/tests/ansible/integration/runner/custom_python_json_args_module.yml index b72de0a2..a7d6937d 100644 --- a/tests/ansible/integration/runner/custom_python_json_args_module.yml +++ b/tests/ansible/integration/runner/custom_python_json_args_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_python_json_args_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_python_json_args_module: diff --git a/tests/ansible/integration/runner/custom_python_new_style_missing_interpreter.yml b/tests/ansible/integration/runner/custom_python_new_style_missing_interpreter.yml index 6ffc90d5..a095018b 100644 --- a/tests/ansible/integration/runner/custom_python_new_style_missing_interpreter.yml +++ b/tests/ansible/integration/runner/custom_python_new_style_missing_interpreter.yml @@ -1,6 +1,6 @@ - name: integration/runner__custom_python_new_style_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_python_new_style_missing_interpreter: diff --git a/tests/ansible/integration/runner/custom_python_new_style_module.yml b/tests/ansible/integration/runner/custom_python_new_style_module.yml index 0630a844..5a2a98a8 100644 --- a/tests/ansible/integration/runner/custom_python_new_style_module.yml +++ b/tests/ansible/integration/runner/custom_python_new_style_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_python_new_style_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_python_new_style_module: diff --git a/tests/ansible/integration/runner/custom_python_want_json_module.yml b/tests/ansible/integration/runner/custom_python_want_json_module.yml index c1009fb7..c3ff2614 100644 --- a/tests/ansible/integration/runner/custom_python_want_json_module.yml +++ b/tests/ansible/integration/runner/custom_python_want_json_module.yml @@ -1,5 +1,5 @@ - name: integration/runner__custom_python_want_json_module.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: - custom_python_want_json_module: diff --git a/tests/ansible/integration/runner/custom_script_interpreter.yml b/tests/ansible/integration/runner/custom_script_interpreter.yml index 9ad350bb..4c6b3ef5 100644 --- a/tests/ansible/integration/runner/custom_script_interpreter.yml +++ b/tests/ansible/integration/runner/custom_script_interpreter.yml @@ -1,5 +1,5 @@ - name: integration/runner/custom_script_interpreter.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/runner/forking_behaviour.yml b/tests/ansible/integration/runner/forking_behaviour.yml index 35ec3bfa..7268fce0 100644 --- a/tests/ansible/integration/runner/forking_behaviour.yml +++ b/tests/ansible/integration/runner/forking_behaviour.yml @@ -1,6 +1,6 @@ - name: integration/runner/forking_behaviour.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/integration/runner/remote_tmp.yml b/tests/ansible/integration/runner/remote_tmp.yml index 19b33f0a..a0b05d50 100644 --- a/tests/ansible/integration/runner/remote_tmp.yml +++ b/tests/ansible/integration/runner/remote_tmp.yml @@ -4,7 +4,7 @@ # remotely. # - name: integration/runner__remote_tmp.yml - hosts: all + hosts: test-targets any_errors_fatal: true gather_facts: true tasks: diff --git a/tests/ansible/osx_setup.yml b/tests/ansible/osx_setup.yml index 9e29dabb..e47e87fe 100644 --- a/tests/ansible/osx_setup.yml +++ b/tests/ansible/osx_setup.yml @@ -6,7 +6,7 @@ # WARNING: this creates non-privilged accounts with pre-set passwords! # -- hosts: all +- hosts: test-targets gather_facts: true become: true tasks: diff --git a/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml b/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml index d13f25bb..01d20b3b 100644 --- a/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml +++ b/tests/ansible/regression/issue_109__target_has_old_ansible_installed.yml @@ -2,7 +2,7 @@ # does not conflict with operation. - name: regression/issue_109__target_has_old_ansible_installed.yml - hosts: all + hosts: test-targets any_errors_fatal: true gather_facts: true tasks: diff --git a/tests/ansible/regression/issue_113__duplicate_module_imports.yml b/tests/ansible/regression/issue_113__duplicate_module_imports.yml index 0d3a7e18..2b9e3ea8 100644 --- a/tests/ansible/regression/issue_113__duplicate_module_imports.yml +++ b/tests/ansible/regression/issue_113__duplicate_module_imports.yml @@ -3,7 +3,7 @@ - name: regression/issue_113__duplicate_module_imports.yml any_errors_fatal: true - hosts: all + hosts: test-targets tasks: - name: Get auth token diff --git a/tests/ansible/regression/issue_118__script_not_marked_exec.yml b/tests/ansible/regression/issue_118__script_not_marked_exec.yml index 376addf0..724644f9 100644 --- a/tests/ansible/regression/issue_118__script_not_marked_exec.yml +++ b/tests/ansible/regression/issue_118__script_not_marked_exec.yml @@ -1,7 +1,7 @@ # issue #118 repro: chmod +x not happening during script upload - name: regression/issue_118__script_not_marked_exec.yml - hosts: all + hosts: test-targets become: True tasks: diff --git a/tests/ansible/regression/issue_122__environment_difference.yml b/tests/ansible/regression/issue_122__environment_difference.yml index a7e791c7..bf9df861 100644 --- a/tests/ansible/regression/issue_122__environment_difference.yml +++ b/tests/ansible/regression/issue_122__environment_difference.yml @@ -6,7 +6,7 @@ # - name: regression/issue_122__environment_difference.yml - hosts: all + hosts: test-targets tasks: - script: scripts/print_env.sh diff --git a/tests/ansible/regression/issue_140__thread_pileup.yml b/tests/ansible/regression/issue_140__thread_pileup.yml index f5fcd804..0c86b237 100644 --- a/tests/ansible/regression/issue_140__thread_pileup.yml +++ b/tests/ansible/regression/issue_140__thread_pileup.yml @@ -3,7 +3,7 @@ # with_items should crash for other reasons (RAM, file descriptor count, ..) - name: regression/issue_140__thread_pileup.yml - hosts: all + hosts: test-targets any_errors_fatal: true tasks: diff --git a/tests/ansible/regression/issue_152__local_action_wrong_interpreter.yml b/tests/ansible/regression/issue_152__local_action_wrong_interpreter.yml index df200469..5de67ab9 100644 --- a/tests/ansible/regression/issue_152__local_action_wrong_interpreter.yml +++ b/tests/ansible/regression/issue_152__local_action_wrong_interpreter.yml @@ -4,7 +4,7 @@ # can test for. - name: regression/issue_152__local_action_wrong_interpreter.yml - hosts: all + hosts: test-targets connection: local any_errors_fatal: true tasks: diff --git a/tests/ansible/regression/issue_152__virtualenv_python_fails.yml b/tests/ansible/regression/issue_152__virtualenv_python_fails.yml index 9e9a9beb..c17b3dd5 100644 --- a/tests/ansible/regression/issue_152__virtualenv_python_fails.yml +++ b/tests/ansible/regression/issue_152__virtualenv_python_fails.yml @@ -1,6 +1,6 @@ - name: regression/issue_152__virtualenv_python_fails.yml any_errors_fatal: true - hosts: all + hosts: test-targets tasks: # Can't use pip module because you can't fricking just create a virtualenv, diff --git a/tests/ansible/regression/issue_154__module_state_leaks.yml b/tests/ansible/regression/issue_154__module_state_leaks.yml index 471d92de..faef6e63 100644 --- a/tests/ansible/regression/issue_154__module_state_leaks.yml +++ b/tests/ansible/regression/issue_154__module_state_leaks.yml @@ -3,7 +3,7 @@ - name: regression/issue_154__module_state_leaks.yml any_errors_fatal: true - hosts: all + hosts: test-targets tasks: - custom_python_leaky_class_vars: diff --git a/tests/ansible/regression/issue_177__copy_module_failing.yml b/tests/ansible/regression/issue_177__copy_module_failing.yml index 2b192d91..593a0ae7 100644 --- a/tests/ansible/regression/issue_177__copy_module_failing.yml +++ b/tests/ansible/regression/issue_177__copy_module_failing.yml @@ -1,6 +1,6 @@ - name: regression/issue_177__copy_module_failing.yml any_errors_fatal: true - hosts: all + hosts: test-targets tasks: - copy: From 3a07ed3d653eb4cc18a2c0e688ab3f6c3cd27748 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 24 Apr 2018 22:41:47 +0100 Subject: [PATCH 04/10] Add mitogen.service to preamble_size.py --- preamble_size.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/preamble_size.py b/preamble_size.py index 19e0e988..6e3c7924 100644 --- a/preamble_size.py +++ b/preamble_size.py @@ -9,6 +9,7 @@ import zlib import mitogen.fakessh import mitogen.master import mitogen.parent +import mitogen.service import mitogen.ssh import mitogen.sudo @@ -35,6 +36,7 @@ print( for mod in ( mitogen.master, mitogen.parent, + mitogen.service, mitogen.ssh, mitogen.sudo, mitogen.fakessh, From 3fab8a3af5724d75054134d8fac64dc848bfa529 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Tue, 24 Apr 2018 22:42:02 +0100 Subject: [PATCH 05/10] ansible: connection delegation v1 This implements the first edition of Connection Delegation, where delegating connection establishment is initially single-threaded. ansible_mitogen/strategy.py: ansible_mitogen/plugins/connection/*: Begin splitting connection.Connection into subclasses, exposing them directly as "mitogen_ssh", "mitogen_local", etc. connection types. This is far from removing strategy.py, but it's a tiny start. ansible_mitogen/connection.py: * config_from_play_context() and config_from_host_vars() build up a huge dictionary containing either more or less PlayContext contents, or our best attempt at reconstructing a host's connection config from its hostvars, where that config is not the current WorkerProcess target. They both produce the same format with the same keys, allowing remaining code to have a single input format. These dicts contain fields named after how Ansible refers to them, e.g. "sudo_exe". * _config_from_via() parses a basic connection specification like "username@inventory_name" into one of the aforementioned dicts. * _stack_from_config() produces a list of dicts describing the order in which (Mitogen) connections should be established, such that each element is proxied via= the previous element. The dicts produced by this function use Mitogen keyword arguments, the former di. These dicts contain fields named after how Mitogen refers to them, e.g. "sudo_path". * Pass the stack to ContextService, which is responsible for actual setup of the full chain. ansible_mitogen/services.py: Teach get() to walk the supplied stack, establishing each connection in turn, creating refounts for it before continuing. TODO: refcounting is broken in a variety of cases. --- ansible_mitogen/connection.py | 394 +++++++++++------- .../plugins/connection/mitogen_docker.py | 56 +++ .../plugins/connection/mitogen_local.py | 56 +++ .../connection/{mitogen.py => mitogen_ssh.py} | 2 +- ansible_mitogen/services.py | 151 ++++--- ansible_mitogen/strategy.py | 3 +- 6 files changed, 425 insertions(+), 237 deletions(-) create mode 100644 ansible_mitogen/plugins/connection/mitogen_docker.py create mode 100644 ansible_mitogen/plugins/connection/mitogen_local.py rename ansible_mitogen/plugins/connection/{mitogen.py => mitogen_ssh.py} (97%) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 69dec732..1554cf9d 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -33,6 +33,8 @@ import shlex import sys import time +import jinja2.runtime +import ansible.constants as C import ansible.errors import ansible.plugins.connection @@ -47,6 +49,135 @@ import ansible_mitogen.services LOG = logging.getLogger(__name__) +def _connect_local(spec): + return { + 'method': 'local', + 'kwargs': { + 'python_path': spec['python_path'], + } + } + + +def _connect_ssh(spec): + return { + 'method': 'ssh', + 'kwargs': { + 'check_host_keys': False, # TODO + 'hostname': spec['remote_addr'], + 'username': spec['remote_user'], + 'password': spec['password'], + 'port': spec['port'], + 'python_path': spec['python_path'], + 'identity_file': spec['private_key_file'], + 'ssh_path': spec['ssh_executable'], + 'connect_timeout': spec['ansible_ssh_timeout'], + 'ssh_args': spec['ssh_args'], + } + } + + +def _connect_docker(spec): + return { + 'method': 'docker', + 'kwargs': { + 'username': spec['remote_user'], + 'container': spec['remote_addr'], + 'python_path': spec['python_path'], + 'connect_timeout': spec['ansible_ssh_timeout'] or spec['timeout'], + } + } + + +def _connect_sudo(spec): + return { + 'method': 'sudo', + 'kwargs': { + 'username': spec['become_user'], + 'password': spec['become_pass'], + 'python_path': spec['python_path'], + 'sudo_path': spec['sudo_exe'], + 'connect_timeout': spec['timeout'], + 'sudo_args': spec['sudo_args'], + } + } + + +CONNECTION_METHOD = { + 'sudo': _connect_sudo, + 'ssh': _connect_ssh, + 'local': _connect_local, + 'docker': _connect_docker, +} + + +def config_from_play_context(transport, inventory_name, connection): + """ + Return a dict representing all important connection configuration, allowing + the same functions to work regardless of whether configuration came from + play_context (direct connection) or host vars (mitogen_via=). + """ + return { + 'transport': transport, + 'inventory_name': inventory_name, + 'remote_addr': connection._play_context.remote_addr, + 'remote_user': connection._play_context.remote_user, + 'become': connection._play_context.become, + 'become_method': connection._play_context.become_method, + 'become_user': connection._play_context.become_user, + 'become_pass': connection._play_context.become_pass, + 'password': connection._play_context.password, + 'port': connection._play_context.port, + 'python_path': connection.python_path, + 'private_key_file': connection._play_context.private_key_file, + 'ssh_executable': connection._play_context.ssh_executable, + 'timeout': connection._play_context.timeout, + 'ansible_ssh_timeout': connection.ansible_ssh_timeout, + 'ssh_args': [ + term + for s in ( + getattr(connection._play_context, 'ssh_args', ''), + getattr(connection._play_context, 'ssh_common_args', ''), + getattr(connection._play_context, 'ssh_extra_args', '') + ) + for term in shlex.split(s or '') + ], + 'sudo_exe': connection._play_context.sudo_exe, + 'sudo_args': [ + term + for s in ( + connection._play_context.sudo_flags, + connection._play_context.become_flags + ) + for term in shlex.split(s or '') + ], + 'mitogen_via': connection.mitogen_via, + } + + +def config_from_hostvars(transport, inventory_name, connection, + hostvars, become_user): + """ + Override config_from_play_context() to take equivalent information from + host vars. + """ + config = config_from_play_context(transport, inventory_name, connection) + hostvars = dict(hostvars) + return dict(config, **{ + 'remote_addr': hostvars.get('ansible_hostname', inventory_name), + 'become': bool(become_user), + 'become_user': become_user, + 'become_pass': None, + 'remote_user': hostvars.get('ansible_user'), # TODO + 'password': (hostvars.get('ansible_ssh_pass') or + hostvars.get('ansible_password')), + 'port': hostvars.get('ansible_port'), + 'python_path': hostvars.get('ansible_python_interpreter'), + 'private_key_file': (hostvars.get('ansible_ssh_private_key_file') or + hostvars.get('ansible_private_key_file')), + 'mitogen_via': hostvars.get('mitogen_via'), + }) + + class Connection(ansible.plugins.connection.ConnectionBase): #: mitogen.master.Broker for this worker. broker = None @@ -58,45 +189,39 @@ class Connection(ansible.plugins.connection.ConnectionBase): #: presently always the master process. parent = None - #: mitogen.master.Context connected to the target machine's initial SSH - #: account. - host = None - #: mitogen.master.Context connected to the target user account on the - #: target machine (i.e. via sudo), or simply a copy of :attr:`host` if - #: become is not in use. + #: target machine (i.e. via sudo). context = None #: Only sudo is supported for now. become_methods = ['sudo'] - #: Set by the constructor according to whichever connection type this - #: connection should emulate. We emulate the original connection type to - #: work around artificial limitations in e.g. the synchronize action, which - #: hard-codes 'local' and 'ssh' as the only allowable connection types. - transport = None - #: Set to 'ansible_python_interpreter' by on_action_run(). python_path = None #: Set to 'ansible_sudo_exe' by on_action_run(). - sudo_path = None + sudo_exe = None #: Set to 'ansible_ssh_timeout' by on_action_run(). ansible_ssh_timeout = None + #: Set to 'mitogen_via' by on_action_run(). + mitogen_via = None + + #: Set to 'inventory_hostname' by on_action_run(). + inventory_hostname = None + + #: Set to 'hostvars' by on_action_run() + host_vars = None + #: Set after connection to the target context's home directory. _homedir = None - def __init__(self, play_context, new_stdin, original_transport, **kwargs): + def __init__(self, play_context, new_stdin, **kwargs): assert ansible_mitogen.process.MuxProcess.unix_listener_path, ( - 'The "mitogen" connection plug-in may only be instantiated ' - 'by the "mitogen" strategy plug-in.' + 'Mitogen connection types may only be instantiated ' + 'while the "mitogen" strategy is active.' ) - - self.original_transport = original_transport - self.transport = original_transport - self.kwargs = kwargs super(Connection, self).__init__(play_context, new_stdin) def __del__(self): @@ -114,19 +239,13 @@ class Connection(ansible.plugins.connection.ConnectionBase): executing. We use the opportunity to grab relevant bits from the task-specific data. """ - self.ansible_ssh_timeout = task_vars.get( - 'ansible_ssh_timeout', - None - ) - self.python_path = task_vars.get( - 'ansible_python_interpreter', - '/usr/bin/python' - ) - self.sudo_path = task_vars.get( - 'ansible_sudo_exe', - 'sudo' - ) - + self.ansible_ssh_timeout = task_vars.get('ansible_ssh_timeout') + self.python_path = task_vars.get('ansible_python_interpreter', + '/usr/bin/python') + self.sudo_exe = task_vars.get('ansible_sudo_exe', C.DEFAULT_SUDO_EXE) + self.mitogen_via = task_vars.get('mitogen_via') + self.inventory_hostname = task_vars['inventory_hostname'] + self.host_vars = task_vars['hostvars'] self.close(new_task=True) @property @@ -138,109 +257,52 @@ class Connection(ansible.plugins.connection.ConnectionBase): def connected(self): return self.context is not None - def _on_connection_error(self, msg): - raise ansible.errors.AnsibleConnectionFailure(msg) - - def _on_become_error(self, msg): - # TODO: vanilla become failures yield this: - # { - # "changed": false, - # "module_stderr": "sudo: sorry, you must have a tty to run sudo\n", - # "module_stdout": "", - # "msg": "MODULE FAILURE", - # "rc": 1 - # } - # - # Currently we yield this: - # { - # "msg": "EOF on stream; last 300 bytes received: 'sudo: ....\n'" - # } - raise ansible.errors.AnsibleModuleError(msg) - - def _wrap_connect(self, on_error, kwargs): - dct = mitogen.service.call( - context=self.parent, - handle=ansible_mitogen.services.ContextService.handle, - method='get', - kwargs=mitogen.utils.cast(kwargs), - ) + def _config_from_via(self, via_spec): + become_user, _, inventory_name = via_spec.rpartition('@') + via_vars = self.host_vars[inventory_name] + if isinstance(via_vars, jinja2.runtime.Undefined): + raise ansible.errors.AnsibleConnectionFailure( + self.unknown_via_msg % ( + self.mitogen_via, + config['inventory_name'], + ) + ) - if dct['msg']: - on_error(dct['msg']) + return config_from_hostvars( + transport=via_vars.get('ansible_connection', 'ssh'), + inventory_name=inventory_name, + connection=self, + hostvars=via_vars, + become_user=become_user or None, + ) - return dct['context'], dct['home_dir'] + unknown_via_msg = 'mitogen_via=%s of %s specifies an unknown hostname' + via_cycle_msg = 'mitogen_via=%s of %s creates a cycle (%s)' + + def _stack_from_config(self, config, stack=(), seen_names=()): + if config['inventory_name'] in seen_names: + raise ansible.errors.AnsibleConnectionFailure( + self.via_cycle_msg % ( + config['mitogen_via'], + config['inventory_name'], + ' -> '.join(reversed( + seen_names + (config['inventory_name'],) + )), + ) + ) - def _connect_local(self): - """ - Fetch a reference to the local() Context from ContextService in the - master process. - """ - return self._wrap_connect(self._on_connection_error, { - 'method_name': 'local', - 'python_path': self.python_path, - }) + if config['mitogen_via']: + stack, seen_names = self._stack_from_config( + self._config_from_via(config['mitogen_via']), + stack=stack, + seen_names=seen_names + (config['inventory_name'],) + ) - def _connect_ssh(self): - """ - Fetch a reference to an SSH Context matching the play context from - ContextService in the master process. - """ - return self._wrap_connect(self._on_connection_error, { - 'method_name': 'ssh', - 'check_host_keys': False, # TODO - 'hostname': self._play_context.remote_addr, - 'username': self._play_context.remote_user, - 'password': self._play_context.password, - 'port': self._play_context.port, - 'python_path': self.python_path, - 'identity_file': self._play_context.private_key_file, - 'ssh_path': self._play_context.ssh_executable, - 'connect_timeout': self.ansible_ssh_timeout, - 'ssh_args': [ - term - for s in ( - getattr(self._play_context, 'ssh_args', ''), - getattr(self._play_context, 'ssh_common_args', ''), - getattr(self._play_context, 'ssh_extra_args', '') - ) - for term in shlex.split(s or '') - ] - }) - - def _connect_docker(self): - return self._wrap_connect(self._on_connection_error, { - 'method_name': 'docker', - 'container': self._play_context.remote_addr, - 'python_path': self.python_path, - 'connect_timeout': self._play_context.timeout, - }) - - def _connect_sudo(self, via=None, python_path=None): - """ - Fetch a reference to a sudo Context matching the play context from - ContextService in the master process. + stack += (CONNECTION_METHOD[config['transport']](config),) + if config['become']: + stack += (CONNECTION_METHOD[config['become_method']](config),) - :param via: - Parent Context of the sudo Context. For Ansible, this should always - be a Context returned by _connect_ssh(). - """ - return self._wrap_connect(self._on_become_error, { - 'method_name': 'sudo', - 'username': self._play_context.become_user, - 'password': self._play_context.become_pass, - 'python_path': python_path or self.python_path, - 'sudo_path': self.sudo_path, - 'connect_timeout': self._play_context.timeout, - 'via': via, - 'sudo_args': [ - term - for s in ( - self._play_context.sudo_flags, - self._play_context.become_flags - ) - for term in shlex.split(s or '') - ], - }) + return stack, seen_names def _connect(self): """ @@ -263,24 +325,30 @@ class Connection(ansible.plugins.connection.ConnectionBase): broker=self.broker, ) - if self.original_transport == 'local': - if self._play_context.become: - self.context, self._homedir = self._connect_sudo( - python_path=sys.executable - ) - else: - self.context, self._homedir = self._connect_local() - return + stack, _ = self._stack_from_config( + config_from_play_context( + transport=self.transport, + inventory_name=self.inventory_hostname, + connection=self + ) + ) + + dct = mitogen.service.call( + context=self.parent, + handle=ansible_mitogen.services.ContextService.handle, + method='get', + kwargs=mitogen.utils.cast({ + 'stack': stack, + }) + ) - if self.original_transport == 'docker': - self.host, self._homedir = self._connect_docker() - elif self.original_transport == 'ssh': - self.host, self._homedir = self._connect_ssh() + if dct['msg']: + if dct['method_name'] in self.become_methods: + raise ansible.errors.AnsibleModuleError(dct['msg']) + raise ansible.errors.AnsibleConnectionFailure(dct['msg']) - if self._play_context.become: - self.context, self._homedir = self._connect_sudo(via=self.host) - else: - self.context = self.host + self.context = dct['context'] + self._homedir = dct['home_dir'] def get_context_name(self): """ @@ -296,18 +364,16 @@ class Connection(ansible.plugins.connection.ConnectionBase): gracefully shut down, and wait for shutdown to complete. Safe to call multiple times. """ - for context in set([self.host, self.context]): - if context: - mitogen.service.call( - context=self.parent, - handle=ansible_mitogen.services.ContextService.handle, - method='put', - kwargs={ - 'context': context - } - ) + if self.context: + mitogen.service.call( + context=self.parent, + handle=ansible_mitogen.services.ContextService.handle, + method='put', + kwargs={ + 'context': self.context + } + ) - self.host = None self.context = None if self.broker and not new_task: self.broker.shutdown() @@ -420,3 +486,15 @@ class Connection(ansible.plugins.connection.ConnectionBase): in_path=in_path, out_path=out_path ) + + +class SshConnection(Connection): + transport = 'ssh' + + +class LocalConnection(Connection): + transport = 'local' + + +class DockerConnection(Connection): + transport = 'docker' diff --git a/ansible_mitogen/plugins/connection/mitogen_docker.py b/ansible_mitogen/plugins/connection/mitogen_docker.py new file mode 100644 index 00000000..70a90147 --- /dev/null +++ b/ansible_mitogen/plugins/connection/mitogen_docker.py @@ -0,0 +1,56 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import os.path +import sys + +# +# This is not the real Connection implementation module, it simply exists as a +# proxy to the real module, which is loaded using Python's regular import +# mechanism, to prevent Ansible's PluginLoader from making up a fake name that +# results in ansible_mitogen plugin modules being loaded twice: once by +# PluginLoader with a name like "ansible.plugins.connection.mitogen", which is +# stuffed into sys.modules even though attempting to import it will trigger an +# ImportError, and once under its canonical name, "ansible_mitogen.connection". +# +# Therefore we have a proxy module that imports it under the real name, and +# sets up the duff PluginLoader-imported module to just contain objects from +# the real module, so duplicate types don't exist in memory, and things like +# debuggers and isinstance() work predictably. +# + +try: + import ansible_mitogen +except ImportError: + base_dir = os.path.dirname(__file__) + sys.path.insert(0, os.path.abspath(os.path.join(base_dir, '../../..'))) + del base_dir + +from ansible_mitogen.connection import DockerConnection as Connection +del os +del sys diff --git a/ansible_mitogen/plugins/connection/mitogen_local.py b/ansible_mitogen/plugins/connection/mitogen_local.py new file mode 100644 index 00000000..fc1a8565 --- /dev/null +++ b/ansible_mitogen/plugins/connection/mitogen_local.py @@ -0,0 +1,56 @@ +# Copyright 2017, David Wilson +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF +# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import os.path +import sys + +# +# This is not the real Connection implementation module, it simply exists as a +# proxy to the real module, which is loaded using Python's regular import +# mechanism, to prevent Ansible's PluginLoader from making up a fake name that +# results in ansible_mitogen plugin modules being loaded twice: once by +# PluginLoader with a name like "ansible.plugins.connection.mitogen", which is +# stuffed into sys.modules even though attempting to import it will trigger an +# ImportError, and once under its canonical name, "ansible_mitogen.connection". +# +# Therefore we have a proxy module that imports it under the real name, and +# sets up the duff PluginLoader-imported module to just contain objects from +# the real module, so duplicate types don't exist in memory, and things like +# debuggers and isinstance() work predictably. +# + +try: + import ansible_mitogen +except ImportError: + base_dir = os.path.dirname(__file__) + sys.path.insert(0, os.path.abspath(os.path.join(base_dir, '../../..'))) + del base_dir + +from ansible_mitogen.connection import LocalConnection as Connection +del os +del sys diff --git a/ansible_mitogen/plugins/connection/mitogen.py b/ansible_mitogen/plugins/connection/mitogen_ssh.py similarity index 97% rename from ansible_mitogen/plugins/connection/mitogen.py rename to ansible_mitogen/plugins/connection/mitogen_ssh.py index 5ae51ab1..e0a30672 100644 --- a/ansible_mitogen/plugins/connection/mitogen.py +++ b/ansible_mitogen/plugins/connection/mitogen_ssh.py @@ -51,6 +51,6 @@ except ImportError: sys.path.insert(0, os.path.abspath(os.path.join(base_dir, '../../..'))) del base_dir -from ansible_mitogen.connection import Connection +from ansible_mitogen.connection import SshConnection as Connection del os del sys diff --git a/ansible_mitogen/services.py b/ansible_mitogen/services.py index 79c8de46..ae6e8ed7 100644 --- a/ansible_mitogen/services.py +++ b/ansible_mitogen/services.py @@ -82,9 +82,9 @@ class ContextService(mitogen.service.Service): #: Records the :meth:`get` result dict for successful calls, returned #: for identical subsequent calls. Keyed by :meth:`key_from_kwargs`. self._response_by_key = {} - #: List of :class:`mitogen.core.Message` waiting for the result dict - #: for a particular connection config. Keyed as sbove. - self._waiters_by_key = {} + #: List of :class:`mitogen.core.Latch` awaiting the result for a + #: particular key. + self._latches_by_key = {} #: Mapping of :class:`mitogen.core.Context` -> reference count. Each #: call to :meth:`get` increases this by one. Calls to :meth:`put` #: decrease it by one. @@ -134,10 +134,10 @@ class ContextService(mitogen.service.Service): """ self._lock.acquire() try: - waiters = self._waiters_by_key.pop(key) - count = len(waiters) - for msg in waiters: - msg.reply(response) + latches = self._latches_by_key.pop(key) + count = len(latches) + for latch in latches: + latch.put(response) finally: self._lock.release() return count @@ -164,17 +164,12 @@ class ContextService(mitogen.service.Service): finally: self._lock.release() - def _update_lru(self, new_context, **kwargs): + def _update_lru(self, new_context, spec, via): """ Update the LRU ("MRU"?) list associated with the connection described by `kwargs`, destroying the most recently created context if the list is full. Finally add `new_context` to the list. """ - via = kwargs.get('via') - if via is None: - # We don't have a limit on the number of directly connections. - return - lru = self._lru_by_via.setdefault(via, []) if len(lru) < self.max_interpreters: lru.append(new_context) @@ -207,7 +202,7 @@ class ContextService(mitogen.service.Service): """ # TODO: there is a race between creation of a context and disconnection # of its related stream. An error reply should be sent to any message - # in _waiters_by_key below. + # in _latches_by_key below. self._lock.acquire() try: for context, key in list(self._key_by_context.items()): @@ -215,14 +210,14 @@ class ContextService(mitogen.service.Service): LOG.info('Dropping %r due to disconnect of %r', context, stream) self._response_by_key.pop(key, None) - self._waiters_by_key.pop(key, None) + self._latches_by_key.pop(key, None) self._refs_by_context.pop(context, None) self._lru_by_via.pop(context, None) self._refs_by_context.pop(context, None) finally: self._lock.release() - def _connect(self, key, method_name, **kwargs): + def _connect(self, key, spec, via=None): """ Actual connect implementation. Arranges for the Mitogen connection to be created and enqueues an asynchronous call to start the forked task @@ -230,11 +225,8 @@ class ContextService(mitogen.service.Service): :param key: Deduplication key representing the connection configuration. - :param method_name: - :class:`mitogen.parent.Router` method implementing the connection - type. - :param kwargs: - Keyword arguments passed to the router method. + :param spec: + Connection specification. :returns: Dict like:: @@ -248,21 +240,14 @@ class ContextService(mitogen.service.Service): :data:`None`, or `msg` is :data:`None` and the remaining fields are set. """ - method = getattr(self.router, method_name, None) - if method is None: - raise Error('no such Router method: %s' % (method_name,)) - try: - context = method(**kwargs) - except mitogen.core.StreamError as e: - return { - 'context': None, - 'home_dir': None, - 'msg': str(e), - } - - if kwargs.get('via'): - self._update_lru(context, method_name=method_name, **kwargs) + method = getattr(self.router, spec['method']) + except AttributeError: + raise Error('unsupported method: %(transport)s' % spec) + + context = method(via=via, **spec['kwargs']) + if via: + self._update_lru(context, spec, via) else: # For directly connected contexts, listen to the associated # Stream's disconnect event and use it to invalidate dependent @@ -289,60 +274,74 @@ class ContextService(mitogen.service.Service): 'msg': None, } - @mitogen.service.expose(mitogen.service.AllowParents()) - @mitogen.service.arg_spec({ - 'method_name': str - }) - def get(self, msg, **kwargs): - """ - Return a Context referring to an established connection with the given - configuration, establishing a new connection as necessary. - - :param str method_name: - The :class:`mitogen.parent.Router` connection method to use. - :param dict kwargs: - Keyword arguments passed to `mitogen.master.Router.[method_name]()`. - - :returns tuple: - Tuple of `(context, home_dir)`, where: - * `context` is the mitogen.master.Context referring to the - target context. - * `home_dir` is a cached copy of the remote directory. - """ - key = self.key_from_kwargs(**kwargs) + def _wait_or_start(self, spec, via=None): + latch = mitogen.core.Latch() + key = self.key_from_kwargs(via=via, **spec) self._lock.acquire() try: response = self._response_by_key.get(key) if response is not None: self._refs_by_context[response['context']] += 1 - return response - - waiters = self._waiters_by_key.get(key) - if waiters is not None: - waiters.append(msg) - return self.NO_REPLY + latch.put(response) + return latch - self._waiters_by_key[key] = [msg] + latches = self._latches_by_key.setdefault(key, []) + first = len(latches) == 0 + latches.append(latch) finally: self._lock.release() - # I'm the first thread to wait, so I will create the connection. - try: - response = self._connect(key, **kwargs) - count = self._produce_response(key, response) - if response['msg'] is None: + if first: + # I'm the first requestee, so I will create the connection. + try: + response = self._connect(key, spec, via=via) + count = self._produce_response(key, response) # Only record the response for non-error results. self._response_by_key[key] = response # Set the reference count to the number of waiters. self._refs_by_context[response['context']] += count - except mitogen.core.CallError: - e = sys.exc_info()[1] - self._produce_response(key, e) - except Exception: - e = sys.exc_info()[1] - self._produce_response(key, mitogen.core.CallError(e)) - - return self.NO_REPLY + except Exception: + self._produce_response(key, sys.exc_info()) + + return latch + + @mitogen.service.expose(mitogen.service.AllowParents()) + @mitogen.service.arg_spec({ + 'stack': list + }) + def get(self, msg, stack): + """ + Return a Context referring to an established connection with the given + configuration, establishing new connections as necessary. + + :param list stack: + Connection descriptions. Each element is a dict containing 'method' + and 'kwargs' keys describing the Router method and arguments. + Subsequent elements are proxied via the previous. + + :returns dict: + * context: mitogen.master.Context or None. + * homedir: Context's home directory or None. + * msg: StreamError exception text or None. + * method_name: string failing method name. + """ + via = None + for spec in stack: + try: + result = self._wait_or_start(spec, via=via).get() + if isinstance(result, tuple): # exc_info() + e1, e2, e3 = result + raise e1, e2, e3 + via = result['context'] + except mitogen.core.StreamError as e: + return { + 'context': None, + 'home_dir': None, + 'method_name': spec['method'], + 'msg': str(e), + } + + return result class FileService(mitogen.service.Service): diff --git a/ansible_mitogen/strategy.py b/ansible_mitogen/strategy.py index 430ea74b..ef6a37ac 100644 --- a/ansible_mitogen/strategy.py +++ b/ansible_mitogen/strategy.py @@ -69,8 +69,7 @@ def wrap_connection_loader__get(name, play_context, new_stdin, **kwargs): an argument, so that it can emulate the original type. """ if name in ('ssh', 'local', 'docker'): - kwargs['original_transport'] = name - name = 'mitogen' + name = 'mitogen_' + name return connection_loader__get(name, play_context, new_stdin, **kwargs) From fa4746f656718ffe00f5fdee80d2b76e9c81d833 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 27 Apr 2018 01:32:21 +0100 Subject: [PATCH 06/10] ansible: add connection delegation test targets. For command line use only, no integration tests yet. --- tests/ansible/hosts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/ansible/hosts b/tests/ansible/hosts index 3c85d1eb..45bfb9ef 100644 --- a/tests/ansible/hosts +++ b/tests/ansible/hosts @@ -1,3 +1,16 @@ [test-targets] localhost + +[connection-delegation-test] +cd-bastion +cd-rack11 mitogen_via=ssh-user@cd-bastion +cd-rack11a mitogen_via=root@cd-rack11 +cd-rack11a-docker mitogen_via=docker-admin@cd-rack11a ansible_connection=docker + +[connection-delegation-cycle] +# Create cycle with Docker container. +cdc-bastion mitogen_via=cdc-rack11a-docker +cdc-rack11 mitogen_via=ssh-user@cdc-bastion +cdc-rack11a mitogen_via=root@cdc-rack11 +cdc-rack11a-docker mitogen_via=docker-admin@cdc-rack11a ansible_connection=docker From 3ce6b3693236935c09de156f3a9c8bc319c5c5d3 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 27 Apr 2018 01:32:51 +0100 Subject: [PATCH 07/10] docs: major Ansible page update. --- docs/ansible.rst | 612 +++++++++++++++++++++++++++-------------------- 1 file changed, 350 insertions(+), 262 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index f1b6a6e1..18ee53ea 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -1,64 +1,57 @@ -Ansible Extension -================= - .. image:: images/ansible/cell_division.png :align: right -An extension to `Ansible`_ is included that implements host connections over -Mitogen, replacing embedded shell invocations with pure-Python equivalents -invoked via highly efficient remote procedure calls tunnelled over SSH. No -changes are required to the target hosts. +Mitogen for Ansible +=================== -The extension is approaching a generally dependable state, and works well for -many real-world playbooks. `Bug reports`_ in this area are very welcome – -Ansible is a huge beast, and only significant testing will prove the -extension's soundness. -Divergence from Ansible's normal behaviour is considered a bug, so please -report anything you notice, regardless of how inconsequential it may seem. +An extension to `Ansible`_ is included that implements connections over +Mitogen, replacing embedded shell invocations with pure-Python equivalents +invoked via highly efficient remote procedure calls to persistent interpreters +tunnelled over SSH. No changes are required to target hosts. + +The extension is approaching stability and real-world testing is now +encouraged. `Bug reports`_ are welcome: Ansible is huge, and only wide testing +will ensure soundness. .. _Ansible: https://www.ansible.com/ .. _Bug reports: https://goo.gl/yLKZiJ - Overview -------- -You should **expect a 1.25x - 7x speedup** and a **CPU usage reduction of at -least 2x**, depending on network conditions, the specific modules executed, and -time spent by the target host already doing useful work. Mitogen cannot speed -up a module once it is executing, it can only ensure the module executes as -quickly as possible. - -* **A single SSH connection is used for each target host**, in addition to one - sudo invocation per distinct user account. Subsequent playbook steps always - reuse the same connection. This is much better than SSH multiplexing combined - with pipelining, as significant state can be maintained in RAM between steps, - and the system logs aren't filled with spam from repeat SSH and sudo - invocations. - -* **A single Python interpreter is used** per host and sudo account combination - for the duration of the run, avoiding the repeat cost of invoking multiple - interpreters and recompiling imports, saving 300-800 ms for every playbook - step. - -* Remote interpreters reuse Mitogen's module import mechanism, caching uploaded - dependencies between steps at the host and user account level. As a - consequence, **bandwidth usage is consistently an order of magnitude lower** - compared to SSH pipelining, and around 5x fewer frames are required to - traverse the wire for a run to complete successfully. - -* **No writes to the target host's filesystem occur**, unless explicitly - triggered by a playbook step. In all typical configurations, Ansible - repeatedly rewrites and extracts ZIP files to multiple temporary directories - on the target host. Since no temporary files are used, security issues - relating to those files in cross-account scenarios are entirely avoided. +**Expect a 1.25x - 7x speedup** and a **CPU usage reduction of at least 2x**, +depending on network conditions, modules executed, and time already spent by +targets on useful work. Mitogen cannot improve a module once it is executing, +it can only ensure the module executes as quickly as possible. + +* **One connection is used per target**, in addition to one sudo invocation per + user account. This is much better than SSH multiplexing combined with + pipelining, as significant state can be maintained in RAM between steps, and + system logs aren't spammed with repeat authentication events. + +* **A single network roundtrip is used** to execute a step whose code already + exists in RAM on the target. Eliminating multiplexed SSH channel creation + saves 5 ms runtime per 1 ms of network latency for every playbook step. + +* **Processes are aggressively reused**, avoiding the cost of invoking Python + and recompiling imports, saving 300-800 ms for every playbook step. + +* Code is ephemerally cached in RAM, **reducing bandwidth usage by an order + of magnitude** compared to SSH pipelining, with around 5x fewer frames + traversing the network in a typical run. + +* **No writes to the target's filesystem occur**, unless explicitly triggered + by a playbook step. In all typical configurations, Ansible repeatedly + rewrites and extracts ZIP files to multiple temporary directories on the + target. Since no temporary files are used, security issues relating to those + files in cross-account scenarios are entirely avoided. Demo ----- +~~~~ This demonstrates Ansible running a subset of the Mitogen integration tests concurrent to an equivalent run using the extension. @@ -71,7 +64,7 @@ concurrent to an equivalent run using the extension. Testimonials ------------- +~~~~~~~~~~~~ * "With mitogen **my playbook runtime went from 45 minutes to just under 3 minutes**. Awesome work!" @@ -96,14 +89,11 @@ Testimonials Installation ------------ -.. caution:: - - Please review the behavioural differences documented below prior to use. - -1. Verify Ansible 2.4 and Python 2.7 are listed in the output of ``ansible - --version`` -2. Download and extract https://github.com/dw/mitogen/archive/master.zip -3. Modify ``ansible.cfg``: +1. Thoroughly review the documented behavioural differences. +2. Verify Ansible 2.4/2.5 and Python 2.7 are listed in ``ansible --version`` + output. +3. Download and extract https://github.com/dw/mitogen/archive/master.zip +4. Modify ``ansible.cfg``: .. code-block:: dosini @@ -111,70 +101,256 @@ Installation strategy_plugins = /path/to/mitogen-master/ansible_mitogen/plugins/strategy strategy = mitogen_linear - The ``strategy`` key is optional. If omitted, you can set the - ``ANSIBLE_STRATEGY=mitogen_linear`` environment variable on a per-run basis. - Like ``mitogen_linear``, the ``mitogen_free`` strategy also exists to mimic - the built-in ``free`` strategy. + The ``strategy`` key is optional. If omitted, the + ``ANSIBLE_STRATEGY=mitogen_linear`` environment variable can be set on a + per-run basis. Like ``mitogen_linear``, the ``mitogen_free`` strategy exists + to mimic the ``free`` strategy. + + +Noteworthy Differences +---------------------- + +* Ansible 2.4 and 2.5 are supported. File bugs to register interest in older + releases. -4. Cross your fingers and try it. +* The ``sudo`` become method is available and ``su`` is planned. File bugs to + register interest in additional methods. +* The ``ssh``, ``local`` and ``docker`` connection types are available, with + more planned. File bugs to register interest. -Limitations ------------ +* Local commands execute in a reuseable interpreter created identically to + interpreters on targets. Presently one interpreter per ``become_user`` + exists, and so only one local action may execute simultaneously. -* Only Ansible 2.4 is being used for development, with occasional tests under - 2.5, 2.3 and 2.2. It should be more than possible to fully support at least - 2.3, if not also 2.2. + Ansible usually permits up to ``forks`` simultaneous local actions. Any + long-running local actions that execute for every target will experience + artificial serialization, causing slowdown equivalent to `task_duration * + num_targets`. This will be fixed soon. -* Only the ``sudo`` become method is available, however adding new methods is - straightforward, and eventually at least ``su`` will be included. +* Asynchronous jobs presently exist only for the duration of a run, and time + limits are not implemented. -* The extension's performance benefits do not scale perfectly linearly with the - number of targets. This is a subject of ongoing investigation and - improvements will appear in time. +* Due to use of :func:`select.select` the IO multiplexer breaks down around 100 + targets, expect performance degradation as this number is approached and + errant behaviour as it is exceeded. A replacement will appear soon. -* "Module Replacer" style modules are not yet supported. These rarely appear in - practice, and light Github code searches failed to reveal many examples of - them. +* The undocumented ability to extend :mod:`ansible.module_utils` by supplying a + ``module_utils`` directory alongside a custom new-style module is not yet + supported. +* "Module Replacer" style modules are not supported. These rarely appear in + practice, and light web searches failed to reveal many examples of them. -Behavioural Differences ------------------------ +* Ansible permits up to ``forks`` connections to be setup in parallel, whereas + in Mitogen this is handled by a fixed-size thread pool. Up to 16 connections + may be established in parallel by default, this can be modified by setting + the ``MITOGEN_POOL_SIZE`` environment variable. -* Ansible permits up to ``forks`` SSH connections to be setup simultaneously, - whereas in Mitogen this is handled by a thread pool. Eventually this pool - will become per-CPU, but meanwhile, a maximum of 16 SSH connections may be - established simultaneously by default. This can be increased or decreased - setting the ``MITOGEN_POOL_SIZE`` environment variable. +* Performance does not scale perfectly linearly with target count. This will + improve over time. -* Mitogen treats connection timeouts for the SSH and become steps of a task - invocation separately, meaning that in some circumstances the configured - timeout may appear to be doubled. This is since Mitogen internally treats the - creation of an SSH account context separately to the creation of a sudo - account context proxied via that SSH account. +* Timeouts normally apply to the combined runtime of the SSH and become steps + of a task. As Mitogen treats SSH and sudo distincly, during a failure the + effective timeout may appear to double. - A future revision may detect a sudo account context created immediately - following its parent SSH account, and try to emulate Ansible's existing - timeout semantics. -* Local commands are executed in a reuseable Python interpreter created - identically to interpreters used on remote hosts. At present only one such - interpreter per ``become_user`` exists, and so only one local action may be - executed simultaneously per local user account. +New Features & Notes +-------------------- + + +Connection Delegation +~~~~~~~~~~~~~~~~~~~~~ + +.. image:: images/jumpbox.png + :align: right + +Included is a preview of **Connection Delegation**, a Mitogen-specific +implementation of `stackable connection plug-ins`_. This enables multi-hop +connections via a bastion, or Docker connections delegated via their host +machine, where reaching the host may itself entail recursive delegation. + +.. _Stackable connection plug-ins: https://github.com/ansible/proposals/issues/25 + +Unlike with SSH forwarding Ansible has complete visibility of the final +topology, declarative configuration via static/dynamic inventory is possible, +and data can be cached and re-served, and code executed on every intermediary. + +For example when targeting Docker containers on a remote machine, each module +need only be uploaded once for the first task and container that requires it, +then cached and served from the SSH account for every future task in any +container. + +.. raw:: html + +
+ + +.. caution:: + + Connection delegation is a work in progress, bug reports are welcome. - Ansible usually permits up to ``ansible.cfg:forks`` simultaneous local - actions. Any long-running local actions that execute for every target will - experience artificial serialization, causing slowdown equivalent to - `task_duration * num_targets`. This will be fixed soon. + * While imports are cached on intermediaries, module scripts are needlessly + reuploaded for each target. Fixing this is equivalent to implementing + **Topology-Aware File Synchronization**, so it may remain unfixed until + that feature is started. + + * Delegated connection setup is single-threaded; only one connection can be + constructed in parallel per intermediary. + + * Unbounded queue RAM growth may occur in an intermediary during large file + transfers if the link between any two hops is slower than the link + between the controller and the first hop. + + * Inferring the configuration of intermediaries may be buggy, manifesting + as duplicate connections between hops, due to not perfectly replicating + the configuration Ansible would normally use for the intermediary. + + * The extension does not understand the difference between a delegated + connection and a ``become_user``. If interpreter recycling kicks in, a + delegated connection could be prematurely recycled. + +To enable connection delegation, set ``mitogen_via=`` on the +command line, or as host and group variables. + +.. code-block:: ini + + # Docker container on web1.dc1 is reachable via web1.dc1. + [app-containers.web1.dc1] + app1.web1.dc1 ansible_host=app1 ansible_connection=docker mitogen_via=web1.dc1 + + # Web servers in DC1 are reachable via bastion.dc1 + [dc1] + web1.dc1 + web2.dc1 + web3.dc1 + + [dc1:vars] + mitogen_via = bastion.dc1 + + # Web servers in DC2 are reachable via bastion.dc2 + [dc2] + web1.dc2 + web2.dc2 + web3.dc2 + + [dc2:vars] + mitogen_via = bastion.dc2 + + # Prod bastions are reachable via a corporate network gateway. + [bastions] + bastion.dc1 mitogen_via=corp-gateway.internal + bastion.dc2 mitogen_via=corp-gateway.internal + + [corp-gateway] + corp-gateway.internal + + +File Transfer +~~~~~~~~~~~~~ + +Normally a tool like ``scp`` is used to copy a file with the ``copy`` or +``template`` actions, or when uploading modules with pipelining disabled. With +Mitogen copies are implemented natively using the same interpreters, connection +tree, and routed message bus that carries RPCs. + +This permits streaming directly between endpoints regardless of execution +environment, without necessitating temporary copies in intermediary accounts or +machines, for example when ``become`` is active, or in the presence of +connection delegation. It also neatly avoids the problem of securely sharing +temporary files between accounts and machines. + +One roundtrip is required to initiate a transfer. For any tool that operates +via SSH multiplexing, 5 are required to configure the associated IO channel, in +addition to the time needed to start the local and remote processes. A complete +localhost invocation of ``scp`` requires around 15 ms. + +As the implementation is self-contained, it is simple to make future +improvements like prioritizing transfers, supporting resume, or displaying +progress bars. + + +Interpreter Reuse +~~~~~~~~~~~~~~~~~ + +Python interpreters are aggressively reused to execute modules. While this +works well, it violates an unwritten assumption, and so it is possible an +earlier module execution could cause a subsequent module to fail, or for +unrelated modules to interact poorly due to bad hygiene, such as +monkey-patching that becomes stacked over repeat invocations. + +Before reporting a bug relating to a misbehaving module, please re-run with +``-e mitogen_task_isolation=fork`` to see if the problem abates. This may be +set per-task, paying attention to the possibility an earlier task may be the +true cause of a failure. + +.. code-block:: yaml + + - name: My task. + broken_module: + some_option: true + vars: + mitogen_task_isolation: fork -* Asynchronous jobs exist only for the duration of a run, and cannot be - queried by subsequent ansible-playbook invocations. Since the ability to - query job IDs across runs relied on an implementation detail, it is not - expected this will break any real-world playbooks. +If forking solves your problem, **please report a bug regardless**, as an +internal list can be updated to prevent others bumping into the same problem. + + +Interpreter Recycling +~~~~~~~~~~~~~~~~~~~~~ + +There is a per-target limit on the number of interpreters. Once 20 exist, the +youngest is terminated before starting any new interpreter, preventing +situations like below from triggering memory exhaustion. + +.. code-block:: yaml + + - hosts: corp_boxes + vars: + user_directory: [ + # 10,000 corporate user accounts + ] + tasks: + - name: Create user bashrc + become: true + vars: + ansible_become_user: "{{item}}" + copy: + src: bashrc + dest: "~{{item}}/.bashrc" + with_items: "{{user_directory}}" + +The youngest is chosen to preserve useful accounts like ``root`` and +``postgresql`` that often appear early in a run, however it is simple to +construct a playbook that defeats this strategy. A future version will key +interpreters on the identity of their creating task, avoiding useful account +recycling in every scenario. + +To modify the limit, set the ``MITOGEN_MAX_INTERPRETERS`` environment variable. + + +Standard IO +~~~~~~~~~~~ + +Ansible uses pseudo TTYs for most invocations to allow it to type interactive +passwords, however pseudo TTYs are disabled where standard input is required or +``sudo`` is not in use. Additionally when SSH multiplexing is enabled, a string +like ``Shared connection to localhost closed\r\n`` appears in ``stderr`` of +every invocation. + +Mitogen does not naturally require either of these, as command output is always +embedded within framed messages, and it can simply call :py:func:`pty.openpty` +in any location an interactive password must be typed. + +A major downside to Ansible's behaviour is that ``stdout`` and ``stderr`` are +merged together into a single ``stdout`` variable, with carriage returns +inserted in the output by the TTY layer. However ugly, the extension emulates +this precisely, to avoid breaking playbooks that expect text to appear in +specific variables with a particular linefeed style. How Modules Execute -------------------- +~~~~~~~~~~~~~~~~~~~ Ansible usually modifies, recompresses and reuploads modules every time they run on a target, work that must be repeated by the controller for every @@ -218,47 +394,51 @@ cached in RAM for the remainder of the run. key2=repr(value2)[ ..]] "``. -Sample Profiles ---------------- - -Local VM connection -~~~~~~~~~~~~~~~~~~~ +Runtime Patches +~~~~~~~~~~~~~~~ -This demonstrates Mitogen vs. connection pipelining to a local VM, executing -the 100 simple repeated steps of ``run_hostname_100_times.yml`` from the -examples directory. Mitogen requires **43x less bandwidth and 4.25x less -time**. +Three small runtime patches are employed in ``strategy.py`` to hook into +desirable locations, in order to override uses of shell, the module executor, +and the mechanism for selecting a connection plug-in. While it is hoped the +patches can be avoided in future, for interesting versions of Ansible deployed +today this simply is not possible, and so they continue to be required. -.. image:: images/ansible/run_hostname_100_times.png +The patches are concise and behave conservatively, including by disabling +themselves when non-Mitogen connections are in use. Additional third party +plug-ins are unlikely to attempt similar patches, so the risk to an established +configuration should be minimal. -Kathmandu to Paris -~~~~~~~~~~~~~~~~~~ +Flag Emulation +~~~~~~~~~~~~~~ -This is a full Django application playbook over a ~180ms link between Kathmandu -and Paris. Aside from large pauses where the host performs useful work, the -high latency of this link means Mitogen only manages a 1.7x speedup. +Mitogen re-parses ``sudo_flags``, ``become_flags``, and ``ssh_flags`` using +option parsers extracted from `sudo(1)` and `ssh(1)` in order to emulate their +equivalent semantics. This allows: -Many early roundtrips are due to inefficiencies in Mitogen's importer that will -be fixed over time, however the majority, comprising at least 10 seconds, are -due to idling while the host's previous result and next command are in-flight -on the network. +* robust support for common ``ansible.cfg`` tricks without reconfiguration, + such as forwarding SSH agents across ``sudo`` invocations, +* reporting on conflicting flag combinations, +* reporting on unsupported flag combinations, +* internally special-casing certain behaviour (like recursive agent forwarding) + without boring the user with the details, +* avoiding opening the extension up to untestable scenarios where users can + insert arbitrary garbage between Mitogen and the components it integrates + with, +* precise emulation by an alternative implementation, for example if Mitogen + grew support for Paramiko. -The initial extension lays groundwork for exciting structural changes to the -execution model: a future version will tackle latency head-on by delegating -some control flow to the target host, melding the performance and scalability -benefits of pull-based operation with the management simplicity of push-based -operation. -.. image:: images/ansible/costapp.png +Supported Variables +------------------- +Matching Ansible's model, variables are treated on a per-task basis, causing +establishment of additional reuseable interpreters as necessary to match the +configuration of each task. -SSH Variables -------------- -Matching Ansible's existing model, these variables are treated on a per-task -basis, causing establishment of additional reuseable interpreters as necessary -to match the configuration of each task. +SSH +~~~ This list will grow as more missing pieces are discovered. @@ -272,8 +452,8 @@ This list will grow as more missing pieces are discovered. * ``ssh_args``, ``ssh_common_args``, ``ssh_extra_args`` -Sudo Variables --------------- +Sudo +~~~~ * ``ansible_python_interpreter`` * ``ansible_sudo_exe``, ``ansible_become_exe`` @@ -283,31 +463,23 @@ Sudo Variables * ansible.cfg: ``timeout`` -Docker Variables ----------------- - -Note: Docker support is only intended for developer testing, it might disappear -entirely prior to a stable release. +Docker +~~~~~~ -* ansible_host +Docker support has received relatively little testing, expect increased +probability of surprises for the time being. - -Chat on IRC ------------ - -Some users and developers hang out on the -`#mitogen `_ channel on the -FreeNode IRC network. +* ``ansible_host`` Debugging --------- -Normally with Ansible, diagnostics and use of the :py:mod:`logging` package -output on the target machine are discarded. With Mitogen, all of this is -captured and returned to the host machine, where it can be viewed as desired -with ``-vvv``. Basic high level logs are produced with ``-vvv``, with logging -of all IO on the controller with ``-vvvv`` or higher. +Diagnostics and use of the :py:mod:`logging` package output on the target +machine are usually discarded. With Mitogen, all of this is captured and +returned to the controller, where it can be viewed as desired with ``-vvv``. +Basic high level logs are produced with ``-vvv``, with logging of all IO on the +controller with ``-vvvv`` or higher. Although use of standard IO and the logging package on the target is forwarded to the controller, it is not possible to receive IO activity logs, as the @@ -320,129 +492,45 @@ When file-based logging is enabled, one file per context will be created on the local machine and every target machine, as ``/tmp/mitogen..log``. -Implementation Notes --------------------- - -Interpreter Reuse -~~~~~~~~~~~~~~~~~ - -The extension aggressively reuses the single target Python interpreter to -execute every module. While this generally works well, it violates an unwritten -assumption regarding Ansible modules, and so it is possible a buggy module -could cause a run to fail, or for unrelated modules to interact with each other -due to bad hygiene. - -Before reporting a bug relating to a module behaving incorrectly, please re-run -your playbook with ``-e mitogen_task_isolation=fork`` to see if the problem -abates. This may also be set on a per-task basis: - -:: - - - name: My task. - broken_module: - some_option: true - vars: - mitogen_task_isolation: fork - -If forking fixes your problem, **please report a bug regardless**, as an -internal list can be updated to prevent users bumping into the same problem in -future. - - -Interpreter Recycling -~~~~~~~~~~~~~~~~~~~~~ - -The extension limits the number of persistent interpreters in use. When the -limit is reached, the youngest interpreter is terminated before starting a new -interpreter, preventing situations like below from triggering memory -exhaustion. - -.. code-block:: yaml - - - hosts: corp_boxes - vars: - user_directory: [ - # 10,000 corporate user accounts - ] - tasks: - - name: Create user bashrc - become: true - vars: - ansible_become_user: "{{item}}" - copy: - src: bashrc - dest: "~{{item}}/.bashrc" - with_items: "{{user_directory}}" - -This recycling does not occur for direct connections from the controller, and -it is keyed on a per-target basis, i.e. up to 20 interpreters may exist for -each directly connected target. - -The youngest interpreter is chosen to preserve useful accounts, like "root" or -"postgresql" that tend to appear early in a run, however it is simple to -construct a playbook that defeats this strategy. A future version will key -interpreters on the identity of their creating task, file and/or playbook, -avoiding useful account recycling in every scenario. - -To raise or lower the limit from 20, set the ``MITOGEN_MAX_INTERPRETERS`` -environment variable to a new value. - - -Runtime Patches -~~~~~~~~~~~~~~~ +Getting Help +~~~~~~~~~~~~ +Some users and developers hang out on the +`#mitogen `_ channel on the +FreeNode IRC network. -Three small runtime patches are employed in ``strategy.py`` to hook into -desirable locations, in order to override uses of shell, the module executor, -and the mechanism for selecting a connection plug-in. While it is hoped the -patches can be avoided in future, for interesting versions of Ansible deployed -today this simply is not possible, and so they continue to be required. - -The patches are concise and behave conservatively, including by disabling -themselves when non-Mitogen connections are in use. Additional third party -plug-ins are unlikely to attempt similar patches, so the risk to an established -configuration should be minimal. +Sample Profiles +--------------- -Standard IO -~~~~~~~~~~~ +Local VM connection +~~~~~~~~~~~~~~~~~~~ -Ansible uses pseudo TTYs for most invocations, to allow it to handle typing -passwords interactively, however it disables pseudo TTYs for certain commands -where standard input is required or ``sudo`` is not in use. Additionally when -SSH multiplexing is enabled, a string like ``Shared connection to localhost -closed\r\n`` appears in ``stderr`` of every invocation. +This demonstrates Mitogen vs. connection pipelining to a local VM, executing +the 100 simple repeated steps of ``run_hostname_100_times.yml`` from the +examples directory. Mitogen requires **43x less bandwidth and 4.25x less +time**. -Mitogen does not naturally require either of these, as command output is -embedded within the SSH stream, and it can simply call :py:func:`pty.openpty` -in every location an interactive password must be typed. +.. image:: images/ansible/run_hostname_100_times.png -A major downside to Ansible's behaviour is that ``stdout`` and ``stderr`` are -merged together into a single ``stdout`` variable, with carriage returns -inserted in the output by the TTY layer. However ugly, the extension emulates -all of this behaviour precisely, to avoid breaking playbooks that expect -certain text to appear in certain variables with certain linefeed characters. -See `Ansible#14377`_ for related discussion. +Kathmandu to Paris +~~~~~~~~~~~~~~~~~~ -.. _Ansible#14377: https://github.com/ansible/ansible/issues/14377 +This is a full Django application playbook over a ~180ms link between Kathmandu +and Paris. Aside from large pauses where the host performs useful work, the +high latency of this link means Mitogen only manages a 1.7x speedup. +Many early roundtrips are due to inefficiencies in Mitogen's importer that will +be fixed over time, however the majority, comprising at least 10 seconds, are +due to idling while the host's previous result and next command are in-flight +on the network. -Flag Emulation -~~~~~~~~~~~~~~ +The initial extension lays groundwork for exciting structural changes to the +execution model: a future version will tackle latency head-on by delegating +some control flow to the target host, melding the performance and scalability +benefits of pull-based operation with the management simplicity of push-based +operation. -Mitogen re-parses ``sudo_flags``, ``become_flags``, and ``ssh_flags`` using -option parsers extracted from `sudo(1)` and `ssh(1)` in order to emulate their -equivalent semantics. This allows: +.. image:: images/ansible/costapp.png -* robust support for common ``ansible.cfg`` tricks without reconfiguration, - such as forwarding SSH agents across ``sudo`` invocations, -* reporting on conflicting flag combinations, -* reporting on unsupported flag combinations, -* internally special-casing certain behaviour (like recursive agent forwarding) - without boring the user with the details, -* avoiding opening the extension up to untestable scenarios where users can - insert arbitrary garbage between Mitogen and the components it integrates - with, -* precise emulation by an alternative implementation, for example if Mitogen - grew support for Paramiko. From 39f5aa76ae2c90b5338c108525f6b8c889d48a53 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 27 Apr 2018 01:33:07 +0100 Subject: [PATCH 08/10] docs: add initial ChangeLog. --- ChangeLog | 5 +++++ docs/changelog.rst | 6 ++++++ 2 files changed, 11 insertions(+) create mode 100644 ChangeLog create mode 100644 docs/changelog.rst diff --git a/ChangeLog b/ChangeLog new file mode 100644 index 00000000..82cf7ac0 --- /dev/null +++ b/ChangeLog @@ -0,0 +1,5 @@ + +2018-04-30 v0.0.1 + +* Initial release to support the Mitogen extension for Ansible. + diff --git a/docs/changelog.rst b/docs/changelog.rst new file mode 100644 index 00000000..cbd8c2cd --- /dev/null +++ b/docs/changelog.rst @@ -0,0 +1,6 @@ + +Change Log +---------- + +.. literalinclude:: ../ChangeLog + :language: none From 02ce332b265c466a10ca6ce11c9a06e9c3919620 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 27 Apr 2018 01:40:17 +0100 Subject: [PATCH 09/10] docs: show become_user example for connection delegation. --- docs/ansible.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/ansible.rst b/docs/ansible.rst index 18ee53ea..6ac03ca0 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -237,10 +237,11 @@ command line, or as host and group variables. [dc2:vars] mitogen_via = bastion.dc2 - # Prod bastions are reachable via a corporate network gateway. + # Prod bastions are reachable via a magic account on a + # corporate network gateway. [bastions] - bastion.dc1 mitogen_via=corp-gateway.internal - bastion.dc2 mitogen_via=corp-gateway.internal + bastion.dc1 mitogen_via=prod-ssh-access@corp-gateway.internal + bastion.dc2 mitogen_via=prod-ssh-access@corp-gateway.internal [corp-gateway] corp-gateway.internal From 4893889a880fe50bf475e4f253e442812d00bca7 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Fri, 27 Apr 2018 01:42:20 +0100 Subject: [PATCH 10/10] ansible: remove vestiges of old/wrong sudo_exe source. --- ansible_mitogen/connection.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index 1554cf9d..fcb5851d 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -95,7 +95,7 @@ def _connect_sudo(spec): 'username': spec['become_user'], 'password': spec['become_pass'], 'python_path': spec['python_path'], - 'sudo_path': spec['sudo_exe'], + 'sudo_path': spec['become_exe'], 'connect_timeout': spec['timeout'], 'sudo_args': spec['sudo_args'], } @@ -141,7 +141,7 @@ def config_from_play_context(transport, inventory_name, connection): ) for term in shlex.split(s or '') ], - 'sudo_exe': connection._play_context.sudo_exe, + 'become_exe': connection._play_context.become_exe, 'sudo_args': [ term for s in ( @@ -199,9 +199,6 @@ class Connection(ansible.plugins.connection.ConnectionBase): #: Set to 'ansible_python_interpreter' by on_action_run(). python_path = None - #: Set to 'ansible_sudo_exe' by on_action_run(). - sudo_exe = None - #: Set to 'ansible_ssh_timeout' by on_action_run(). ansible_ssh_timeout = None @@ -242,7 +239,6 @@ class Connection(ansible.plugins.connection.ConnectionBase): self.ansible_ssh_timeout = task_vars.get('ansible_ssh_timeout') self.python_path = task_vars.get('ansible_python_interpreter', '/usr/bin/python') - self.sudo_exe = task_vars.get('ansible_sudo_exe', C.DEFAULT_SUDO_EXE) self.mitogen_via = task_vars.get('mitogen_via') self.inventory_hostname = task_vars['inventory_hostname'] self.host_vars = task_vars['hostvars']