Skip to content

Commit

Permalink
Fix sporadic ansible-runner macOS bug and remove extra listener thread
Browse files Browse the repository at this point in the history
It was found that on macOS a sporadic failure would occur where the internal listener,
while started, wouldn't actually start listening right away, causing the listener to
miss the file we were waiting for.

The listen gem [creates an internal thread, `@run_thread`][1], which on most target systems
is where the actually listening is done. However, [on macOS, `@run_thread` creates a
second thread, `@worker_thread`][2], which does the actual listening. It's possible that
although the listener is started, the @worker_thread hasn't actually started yet.
This leaves a window where the target_path we are waiting on can actually be created
before the `@worker_thread` is started and we "miss" the creation of the target_path.
This commit ensures that we won't move on until that thread is ready, further ensuring
we can't miss the creation of the target_path.

Ansible::Runner#wait_for was also creating an extra thread when starting the listener,
but this thread is unnecessary as the listen gem creates its own internal thread under
the covers.

[1]: https://github.com/guard/listen/blob/f186b2fa159a2458f3ff7e8680c3a4fcbdc636d1/lib/listen/adapter/base.rb#L75
[2]: https://github.com/guard/listen/blob/f186b2fa159a2458f3ff7e8680c3a4fcbdc636d1/lib/listen/adapter/darwin.rb#L49
  • Loading branch information
Fryguy committed Oct 11, 2024
1 parent 786f149 commit 1834baa
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 10 deletions.
25 changes: 15 additions & 10 deletions lib/ansible/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -378,23 +378,28 @@ def wait_for(base_dir, target_path, timeout: 10.seconds)
listener = Listen.to(base_dir, :only => %r{\A#{target_path}\z}) do |modified, added, _removed|
path_created.set if added.include?(base_dir.join(target_path).to_s) || modified.include?(base_dir.join(target_path).to_s)
end

thread = Thread.new do
listener.start
rescue ArgumentError => err
# If the main thread raises an exception immediately it is possible
# for the ensure block to call `listener.stop` before this thread
# begins its execution resulting in an ArgumentError due to the state
# being `:stopped`
raise unless err.message.include?("cannot start from state :stopped")
listener.start

# The listen gem creates an internal thread, @run_thread, which on most target systems
# is where the actually listening is done. However, on macOS, @run_thread creates a
# second thread, @worker_thread, which does the actual listening. It's possible that
# although the listener is started, the @worker_thread hasn't actually started yet.
# This leaves a window where the target_path we are waiting on can actually be created
# before the @worker_thread is started and we "miss" the creation of the target_path.
# This line ensures that we won't move on until that thread is ready, further ensuring
# we can't miss the creation of the target_path.
if RbConfig::CONFIG['host_os'].include?("darwin")
listener_adapter = listener.instance_variable_get(:@backend).instance_variable_get(:@adapter)
until listener_adapter.instance_variable_get(:@worker_thread)&.alive?
sleep(0.01) # yield to other threads to allow them to start
end
end

begin
res = yield
raise "Timed out waiting for #{target_path}" unless path_created.wait(timeout)
ensure
listener.stop
thread.join
end

res
Expand Down
8 changes: 8 additions & 0 deletions spec/lib/ansible/runner_execution_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ def expect_ansible_runner_success(response)
expect(response.human_stdout).to include('"msg": "Hello World! example_var=\'example var value\'"')
end
end

it "with a payload that fails before running even starts" do
playbook = data_directory.join("hello_world.yml")

expect(AwesomeSpawn).to receive(:run).and_raise(RuntimeError.new("Some failure"))

expect { Ansible::Runner.public_send(method_under_test, env_vars, extra_vars, playbook) }.to raise_error(RuntimeError, "Some failure")
end
end

describe ".run" do
Expand Down

0 comments on commit 1834baa

Please sign in to comment.