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.

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

Queue reporting work for any zone #18041

Merged
merged 3 commits into from
Oct 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion app/models/miq_queue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,16 @@ def self.submit_job(options)
# TODO: can we transition to zone = nil
when "notifier"
options[:role] = service
options[:zone] = nil # any zone
when "reporting"
options[:queue_name] = "generic"
options[:role] = service
when "smartproxy"
options[:queue_name] = "smartproxy"
options[:role] = "smartproxy"
end

# Note, options[:zone] is set in 'put' via 'determine_queue_zone' and handles setting
# a nil (any) zone for regional roles. Therefore, regional roles don't need to set zone here.
put(options)
end

Expand Down
9 changes: 4 additions & 5 deletions app/models/miq_report/generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ def queue_generate_table(options = {})
if sync
_async_generate_table(task.id, options)
else
MiqQueue.put(
:queue_name => "reporting",
Copy link
Member Author

Choose a reason for hiding this comment

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

This was most likely the cause of the reporting bug where reporting work would be enqueued for a zone without a reporting role. The absence of the :role => "reporting" and :zone => nil causes this message to be queued for :zone => Zone.my_zone (the current server's zone)

MiqQueue.submit_job(
:service => "reporting",
Copy link
Member Author

Choose a reason for hiding this comment

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

To be clear, submit_job will set the role as reporting and because of this, determine_queue_zone, from put, will set the zone to nil for "any" zone.

:class_name => self.class.name,
:instance_id => id,
:method_name => "_async_generate_table",
Expand Down Expand Up @@ -792,9 +792,8 @@ def queue_report_result(options, res_opts)
_log.info("Adding generate report task to the message queue...")
task = MiqTask.create(:name => "Generate Report: '#{name}'", :userid => options[:userid])

MiqQueue.put(
:queue_name => "reporting",
:role => "reporting",
Copy link
Member Author

Choose a reason for hiding this comment

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

Note, this was probably not causing the issue seen above because it specified the role as reporting, which should set the zone to nil. To simplify things, I moved this code to use the submit_job method to be more consistent.

MiqQueue.submit_job(
:service => "reporting",
:class_name => self.class.name,
:instance_id => id,
:method_name => "build_report_result",
Expand Down
1 change: 0 additions & 1 deletion app/models/miq_report/generator/async.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ def async_generate_table(options = {})
if self.new_record?
MiqQueue.submit_job(
:service => "reporting",
:role => "reporting",
:class_name => self.class.to_s,
:method_name => "_async_generate_table",
:args => [task.id, self, options],
Expand Down
1 change: 1 addition & 0 deletions app/models/miq_widget.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ def queue_generate_content_for_users_or_group(*args)
MiqQueue.create_with(callback).put_unless_exists(
:queue_name => "reporting",
:role => "reporting",
:zone => nil, # any zone
Copy link
Member Author

Choose a reason for hiding this comment

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

Note, I didn't change this to use submit_job because that method doesn't support put_unless_exists. Therefore, I have to set the zone to nil.

:class_name => self.class.to_s,
:instance_id => id,
:msg_timeout => 3600,
Expand Down
33 changes: 33 additions & 0 deletions spec/models/miq_report/async_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,38 @@
describe MiqReport do
context "Generator::Async" do
context ".async_generate_tables" do
let(:report) do
MiqReport.new(
:name => "Custom VM report",
:title => "Custom VM report",
:rpt_group => "Custom",
:rpt_type => "Custom",
:db => "ManageIQ::Providers::InfraManager::Vm",
)
end

it "creates task, queue, audit event" do
User.seed
EvmSpecHelper.local_miq_server
ServerRole.seed
expect(AuditEvent).to receive(:success)

described_class.async_generate_tables(:reports => [report])
task = MiqTask.first
expect(task.name).to eq("Generate Reports: [\"#{report.name}\"]")

message = MiqQueue.find_by(:method_name => "_async_generate_tables")
expect(message).to have_attributes(
:role => "reporting",
:zone => nil,
:class_name => report.class.name,
:method_name => "_async_generate_tables"
)

expect(message.args.first).to eq(task.id)
end
end

context "._async_generate_tables" do
it "unknown taskid" do
expect { MiqReport._async_generate_tables(111111, :reports => []) }.to raise_error(MiqException::Error)
Expand Down
57 changes: 57 additions & 0 deletions spec/models/miq_report/generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,63 @@ def create_rollup(host, profile, used_mem)
end
end

describe "creates task, queue, audit event" do
let(:report) do
MiqReport.new(
:name => "Custom VM report",
:title => "Custom VM report",
:rpt_group => "Custom",
:rpt_type => "Custom",
:db => "ManageIQ::Providers::InfraManager::Vm",
)
end

before do
User.seed
EvmSpecHelper.local_miq_server
ServerRole.seed
expect(AuditEvent).to receive(:success)
end

it "#queue_generate_table" do
report.queue_generate_table(:userid => "admin")
task = MiqTask.first
expect(task).to have_attributes(
:name => "Generate Report: '#{report.name}'",
:userid => "admin"
)

message = MiqQueue.find_by(:method_name => "_async_generate_table")
expect(message).to have_attributes(
:role => "reporting",
:zone => nil,
:class_name => report.class.name,
:method_name => "_async_generate_table"
)

expect(message.args.first).to eq(task.id)
end

it "#queue_report_result" do
task_id = report.queue_report_result({:userid => "admin"}, {})
task = MiqTask.find(task_id)
expect(task).to have_attributes(
:name => "Generate Report: '#{report.name}'",
:userid => "admin"
)

message = MiqQueue.find_by(:method_name => "build_report_result")
expect(message).to have_attributes(
:role => "reporting",
:zone => nil,
:class_name => report.class.name,
:method_name => "build_report_result"
)

expect(message.args.first).to eq(task_id)
end
end

describe "#cols_for_report" do
it "uses cols" do
rpt = MiqReport.new(:db => "VmOrTemplate", :cols => %w(vendor version name))
Expand Down
1 change: 1 addition & 0 deletions spec/models/miq_widget_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def add_dashboard_for_user(db_name, userid, group)

@q_options = {:queue_name => "reporting",
:role => "reporting",
:zone => nil,
:class_name => @widget.class.name,
:instance_id => @widget.id,
:msg_timeout => 3600
Expand Down