Skip to content

Commit

Permalink
Pass worker_options as command arguments
Browse files Browse the repository at this point in the history
Fixes a bug first discovered after ManageIQ#16130 was merged.
That PR was subsequently reverted.  We'll remove fork support once spawn
has complete parity without bugs.
  • Loading branch information
jrafanie committed Oct 25, 2017
1 parent 1d9af07 commit 26b5ac9
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 36 deletions.
6 changes: 3 additions & 3 deletions Procfile.example
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@
# These workers subscribe to a queue named based on the particular EMS they connect to
# The "<id>" tag in these lines should be replaced by the id of the manager instance these workers apply to

# vmware_refresh_<id>: env QUEUE=ems_<id> ruby lib/workers/bin/run_single_worker.rb ManageIQ::Providers::Vmware::InfraManager::RefreshWorker
# vmware_event_<id>: env QUEUE=ems_<id> ruby lib/workers/bin/run_single_worker.rb ManageIQ::Providers::Vmware::InfraManager::EventCatcher
# vmware_refresh_core_<id>: env QUEUE=ems_<id> ruby lib/workers/bin/run_single_worker.rb MiqEmsRefreshCoreWorker
# vmware_refresh_<id>: ruby lib/workers/bin/run_single_worker.rb --ems-id <id> ManageIQ::Providers::Vmware::InfraManager::RefreshWorker
# vmware_event_<id>: ruby lib/workers/bin/run_single_worker.rb --ems-id <id> ManageIQ::Providers::Vmware::InfraManager::EventCatcher
# vmware_refresh_core_<id>: ruby lib/workers/bin/run_single_worker.rb --ems-id <id> MiqEmsRefreshCoreWorker

# Logs
#
Expand Down
17 changes: 13 additions & 4 deletions app/models/miq_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,9 +359,14 @@ def start_runner_via_fork
pid
end

def self.build_command_line(guid)
command_line = "#{Gem.ruby} #{runner_script} --heartbeat --guid=#{guid} #{name}"
ENV['APPLIANCE'] ? "nice #{nice_increment} #{command_line}" : command_line
def self.build_command_line(options)
require 'awesome_spawn'
raise ArgumentError, "expected options hash, received: #{options.class}" unless options.kind_of?(Hash)
raise ArgumentError, "missing :guid options key" unless options.key?(:guid)

cmd = "#{Gem.ruby} #{runner_script}"
cmd = "nice #{nice_increment} #{cmd}" if ENV["APPLIANCE"]
"#{AwesomeSpawn::CommandLineBuilder.new.build(cmd, options.merge(:heartbeat => nil))} #{name}"
end

def self.runner_script
Expand All @@ -370,8 +375,12 @@ def self.runner_script
script
end

def command_line
self.class.build_command_line(worker_options)
end

def start_runner_via_spawn
pid = Kernel.spawn(self.class.build_command_line(guid), [:out, :err] => [Rails.root.join("log", "evm.log"), "a"])
pid = Kernel.spawn(command_line, [:out, :err] => [Rails.root.join("log", "evm.log"), "a"])
Process.detach(pid)
pid
end
Expand Down
10 changes: 7 additions & 3 deletions lib/workers/bin/run_single_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
options[:guid] = val
end

opts.on("-e=ems_id", "--ems-id=ems_id", "Provide an ems id to a provider worker.") do |val|
options[:ems_id] = val
end

opts.on("-h", "--help", "Displays this help") do
puts opts
exit
Expand Down Expand Up @@ -60,9 +64,9 @@
create_options = {:pid => Process.pid}
runner_options = {}

if ENV["QUEUE"]
create_options[:queue_name] = ENV["QUEUE"]
runner_options[:ems_id] = worker_class.ems_id_from_queue_name(ENV["QUEUE"]) if worker_class.respond_to?(:ems_id_from_queue_name)
if options[:ems_id]
create_options[:queue_name] = "ems_#{options[:ems_id]}"
runner_options[:ems_id] = options[:ems_id]
end

worker = if options[:guid]
Expand Down
58 changes: 32 additions & 26 deletions spec/models/miq_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,6 @@ def all_workers
expect(result.command_line).to eq "renice -n 5 -p 123"
end

context ".build_command_line" do
before do
allow(MiqGenericWorker).to receive(:nice_increment).and_return("+10")
end

it "with ENV['APPLIANCE']" do
begin
old_env = ENV.delete('DATABASE_URL')
ENV['APPLIANCE'] = 'true'
w = FactoryGirl.build(:miq_generic_worker)
cmd = w.class.build_command_line(123)
expect(cmd).to start_with("nice +10")
expect(cmd).to end_with("MiqGenericWorker")
ensure
# ENV['x'] = nil deletes the key because ENV accepts only string values
ENV['APPLIANCE'] = old_env
end
end

it "without ENV['APPLIANCE']" do
w = FactoryGirl.build(:miq_generic_worker)
cmd = w.class.build_command_line(123)
expect(cmd).to_not start_with("nice +10")
end
end

context ".has_required_role?" do
def check_has_required_role(worker_role_names, expected_result)
allow(described_class).to receive(:required_roles).and_return(worker_role_names)
Expand Down Expand Up @@ -371,6 +345,38 @@ def check_has_required_role(worker_role_names, expected_result)
expect(@worker.worker_options).to eq(:guid => @worker.guid)
end

context "#command_line" do
it "with nil worker_options" do
allow(@worker).to receive(:worker_options).and_return(nil)
expect { @worker.command_line }.to raise_error(ArgumentError)
end

it "without guid in worker_options" do
allow(@worker).to receive(:worker_options).and_return({})
expect { @worker.command_line }.to raise_error(ArgumentError)
end

it "without ENV['APPLIANCE']" do
allow(@worker).to receive(:worker_options).and_return(:ems_id => 1234, :guid => @worker.guid)
expect(@worker.command_line).to_not include("nice")
end

it "with ENV['APPLIANCE']" do
begin
allow(MiqWorker).to receive(:nice_increment).and_return("+10")
allow(@worker).to receive(:worker_options).and_return(:ems_id => 1234, :guid => @worker.guid)
old_env = ENV.delete('APPLIANCE')
ENV['APPLIANCE'] = 'true'
cmd = @worker.command_line
expect(cmd).to start_with("nice +10")
expect(cmd).to end_with("--ems-id 1234 --guid #{@worker.guid} --heartbeat MiqWorker")
ensure
# ENV['x'] = nil deletes the key because ENV accepts only string values
ENV['APPLIANCE'] = old_env
end
end
end

describe "#stopping_for_too_long?" do
subject { @worker.stopping_for_too_long? }

Expand Down

0 comments on commit 26b5ac9

Please sign in to comment.