Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move embedded ansible worker thread up to start_runner #14256

Merged
merged 9 commits into from
Mar 10, 2017
Merged

Move embedded ansible worker thread up to start_runner #14256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
18ffca9
Move the thread knowledge to where we start the runner
jrafanie Mar 9, 2017
31394bb
You know, there's a way to ensure super is called :trollface:
jrafanie Mar 9, 2017
172639d
The default of 10 minutes may not be enough.
jrafanie Mar 9, 2017
9538cbd
Put the do_before_work_loop before the do_work_loop.
jrafanie Mar 9, 2017
c4434df
Exit with the message and set the exit status
jrafanie Mar 9, 2017
d9a6425
Use the before_exit hook instead of overriding do_exit.
jrafanie Mar 9, 2017
6906682
Simplify do_work by overriding heartbeat.
jrafanie Mar 9, 2017
ffac44c
Move setup_ansible.
jrafanie Mar 9, 2017
bba7fbf
Add test for do_before_work_loop
jrafanie Mar 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions app/models/embedded_ansible_worker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,18 @@ class EmbeddedAnsibleWorker < MiqWorker
self.required_roles = ['embedded_ansible']

def start_runner
self.class::Runner.start_worker(worker_options)
# TODO: return supervisord pid
Thread.new do
begin
self.class::Runner.start_worker(worker_options)
# TODO: return supervisord pid
rescue SystemExit
# Because we're running in a thread on the Server
# we need to intercept SystemExit and exit our thread,
# not the main server thread!
log.info("#{log_prefix} SystemExit received, exiting monitoring Thread")
Thread.exit
end
end
end

def kill
Expand Down
58 changes: 20 additions & 38 deletions app/models/embedded_ansible_worker/runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,31 @@ class EmbeddedAnsibleWorker::Runner < MiqWorker::Runner
self.wait_for_worker_monitor = false

def prepare
# Override prepare so we don't get set as started
ObjectSpace.garbage_collect
# Overriding prepare so we can set started when we're ready
do_before_work_loop
started_worker_record
self
end

# This thread runs forever until a stop request is received, which with send us to do_exit to exit our thread
def do_work_loop
Thread.new do
begin
setup_ansible
started_worker_record
def do_before_work_loop
setup_ansible
update_embedded_ansible_provider
rescue => err
_log.log_backtrace(err)
do_exit(err.message, 1)
end

def heartbeat
super if EmbeddedAnsible.alive?
end

update_embedded_ansible_provider
def do_work
EmbeddedAnsible.start if !EmbeddedAnsible.alive? && !EmbeddedAnsible.running?
end

_log.info("entering ansible monitor loop")
loop do
do_work
send(poll_method)
end
rescue => err
_log.log_backtrace(err)
do_exit
end
end
def before_exit(*_)
EmbeddedAnsible.disable
end

def setup_ansible
Expand All @@ -36,26 +38,6 @@ def setup_ansible
_log.info("calling EmbeddedAnsible.start finished")
end

def do_work
if EmbeddedAnsible.alive?
heartbeat
else
EmbeddedAnsible.start unless EmbeddedAnsible.running?
end
end

# Because we're running in a thread on the Server
# we need to intercept SystemExit and exit our thread,
# not the main server thread!
def do_exit(*args)
# ensure this doesn't fail or that we can still get to the super call
EmbeddedAnsible.disable
super
rescue SystemExit
_log.info("#{log_prefix} SystemExit received, exiting monitoring Thread")
Thread.exit
end

def update_embedded_ansible_provider
server = MiqServer.my_server(true)
provider = ManageIQ::Providers::EmbeddedAnsible::Provider.first_or_initialize
Expand Down
1 change: 1 addition & 0 deletions config/settings.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1204,6 +1204,7 @@
:starting_timeout: 10.minutes
:stopping_timeout: 10.minutes
:embedded_ansible_worker:
:starting_timeout: 20.minutes
Copy link
Member

@carbonin carbonin Mar 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my tests on a VM running in workstation with 6GB of RAM and no other workers running the initial configuration took about 7 minutes and the subsequent starts took about 1 minute.

I'm okay with this value, but we could probably make it smaller if we have a good reason to.
Thoughts?

Copy link
Member Author

@jrafanie jrafanie Mar 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't know. As long as this timeout is used for the initial setup, a task that has a variable number of things it does, I'd be concerned with having ansible workers that can never start. I'd rather be cautious here.

:poll: 10.seconds
:memory_threshold: 0.megabytes
:ems_refresh_core_worker:
Expand Down
7 changes: 7 additions & 0 deletions spec/models/embedded_ansible_worker/runner_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
described_class.new(:guid => worker_guid)
}

it "#do_before_work_loop exits on exceptions" do
expect(runner).to receive(:setup_ansible)
expect(runner).to receive(:update_embedded_ansible_provider).and_raise(StandardError)
expect(runner).to receive(:do_exit)
runner.do_before_work_loop
end

context "#update_embedded_ansible_provider" do
before do
EvmSpecHelper.local_guid_miq_server_zone
Expand Down