Skip to content

Commit

Permalink
We only use guid/ems_id, so pass those, not a hash
Browse files Browse the repository at this point in the history
Don't break an interface for yet another option hash.

Change the tests to not care about the order of some of the CLI
arguments since awesome spawn builds them in hash insertion order and we
don't really care of the order of some of them anyway.
  • Loading branch information
jrafanie committed Oct 24, 2017
1 parent 7edeff0 commit 54462a1
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
14 changes: 8 additions & 6 deletions app/models/miq_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,14 +359,16 @@ def start_runner_via_fork
pid
end

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)
def self.build_command_line(guid, ems_id = nil)
raise ArgumentError, "No guid provided" unless guid

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

options = {:guid => guid, :heartbeat => nil}
options[:ems_id] = ems_id if ems_id
"#{AwesomeSpawn::CommandLineBuilder.new.build(cmd, options)} #{name}"
end

def self.runner_script
Expand All @@ -376,7 +378,7 @@ def self.runner_script
end

def command_line
self.class.build_command_line(worker_options)
self.class.build_command_line(*worker_options.values_at(:guid, :ems_id))
end

def start_runner_via_spawn
Expand Down
10 changes: 4 additions & 6 deletions spec/models/miq_worker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,6 @@ def check_has_required_role(worker_role_names, expected_result)
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)
Expand All @@ -369,7 +364,10 @@ def check_has_required_role(worker_role_names, expected_result)
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")
expect(cmd).to include("--ems-id 1234")
expect(cmd).to include("--guid #{@worker.guid}")
expect(cmd).to include("--heartbeat")
expect(cmd).to end_with("MiqWorker")
ensure
# ENV['x'] = nil deletes the key because ENV accepts only string values
ENV['APPLIANCE'] = old_env
Expand Down

0 comments on commit 54462a1

Please sign in to comment.