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

[WIP] Change MiqRequest to use MiqSchedules for scheduled requests #17542

Closed
wants to merge 7 commits into from
Closed
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
26 changes: 23 additions & 3 deletions app/models/miq_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -417,14 +417,35 @@ def task_check_on_execute
raise _("approval is required for %{task}") % {:task => self.class::TASK_DESCRIPTION} unless approved?
end

def miq_schedule
# HACK: this should be a real relation, but for now it's using a reserve_attribute for backport reasons
Reserve.where(:resource_type => "MiqSchedule").detect { |r| r.reserved == {:resource_id => id} }&.resource
end

def execute
task_check_on_execute

deliver_on = nil
if get_option(:schedule_type) == "schedule"
deliver_on = get_option(:schedule_time).utc rescue nil
start_time = get_option(:schedule_time).utc rescue nil
Copy link
Member

Choose a reason for hiding this comment

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

D'oh I meant to put this here

Copying @gmcculloug 's comment (#17520 (comment))

I would recommend that we refactor this line. The classic UI sets get_option(:schedule_time) to a ActiveSupport::TimeWithZone object which supports the utc call.

I suspect that if this is set from the API or some other means it would set the time as a string in which case the code will silently error and use nil. It is also a good opportunity to get rid of the inline rescue as well.

Copy link
Member

Choose a reason for hiding this comment

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

Fixed by #17547


MiqSchedule.create!(
:name => "Request scheduled",
:description => "Request scheduled",
:sched_action => {:method => "queue_create_request_tasks"},
:resource_id => id,
:towhat => "MiqRequest",
:run_at => {
:interval => {:unit => "once"},
:start_time => start_time,
:tz => "UTC",
},
)
else
queue_create_request_tasks
end
end

def queue_create_request_tasks
# self.create_request_tasks
MiqQueue.put(
:class_name => self.class.name,
Expand All @@ -434,7 +455,6 @@ def execute
:role => my_role(:create_request_tasks),
:tracking_label => tracking_label_id,
:msg_timeout => 3600,
:deliver_on => deliver_on
)
end

Expand Down
36 changes: 24 additions & 12 deletions app/models/miq_schedule.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
class MiqSchedule < ApplicationRecord
include ReservedMixin
reserve_attribute :resource_id, :big_integer

validates :name, :uniqueness => {:scope => [:userid, :towhat]}
validates :name, :description, :towhat, :run_at, :presence => true
validate :validate_run_at, :validate_file_depot
Expand Down Expand Up @@ -39,6 +42,11 @@ class MiqSchedule < ApplicationRecord
default_value_for :enabled, true
default_value_for(:zone_id) { MiqServer.my_server.zone_id }

def resource
# HACK: this should be a real relation, but for now it's using a reserve_attribute for backport reasons
Object.const_get(towhat).find_by(:id => resource_id)
end

def set_start_time_and_prod_default
run_at # Internally this will correct :start_time to UTC
self.prod_default = "system" if SYSTEM_SCHEDULE_CLASSES.include?(towhat.to_s)
Expand Down Expand Up @@ -72,21 +80,25 @@ def self.queue_scheduled_work(id, _rufus_job_id, at, _params)
_log.info("Queueing start of schedule id: [#{id}] [#{sched.name}] [#{sched.towhat}] [#{method}]")

action = "action_" + method
unless sched.respond_to?(action)

if sched.respond_to?(action)
msg = MiqQueue.submit_job(
:class_name => name,
:instance_id => sched.id,
:method_name => "invoke_actions",
:args => [action, at],
:msg_timeout => 1200
)

_log.info("Queueing start of schedule id: [#{id}] [#{sched.name}] [#{sched.towhat}] [#{method}]...complete")
msg
elsif sched.resource.respond_to?(method)
sched.resource.send(method)
sched.update_attributes(:last_run_on => Time.now.utc)
else
_log.warn("[#{sched.name}] no such action: [#{method}], aborting schedule")
return
end

msg = MiqQueue.submit_job(
:class_name => name,
:instance_id => sched.id,
:method_name => "invoke_actions",
:args => [action, at],
:msg_timeout => 1200
)

_log.info("Queueing start of schedule id: [#{id}] [#{sched.name}] [#{sched.towhat}] [#{method}]...complete")
msg
end

def invoke_actions(action, at)
Expand Down
30 changes: 30 additions & 0 deletions spec/models/miq_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,36 @@ def approvals
it_behaves_like "#calls create_request_tasks with the proper role"
end
end

it "scheduled - creates an associated schedule" do
EvmSpecHelper.local_miq_server
request = FactoryGirl.create(:automation_request, :options => {:schedule_type => "schedule", :schedule_time => Time.now.utc})
request.miq_approvals.all.each { |a| a.update_attributes!(:state => "approved") }

request.reload.execute

expect(MiqQueue.count).to eq(0)
expect(MiqSchedule.count).to eq(1)

# HACK: This should be a real relation
expect(Reserve.count).to eq(1)
expect(request.miq_schedule).to be_kind_of(MiqSchedule)
end

it "non_scheduled - is queued directly" do
EvmSpecHelper.local_miq_server
request = FactoryGirl.create(:automation_request)
request.miq_approvals.all.each { |a| a.update_attributes!(:state => "approved") }

request.reload.execute

expect(MiqQueue.count).to eq(1)
expect(MiqSchedule.count).to eq(0)

# HACK: This should be a real relation
expect(Reserve.count).to eq(0)
expect(request.miq_schedule).to be_nil
end
end

context '#post_create_request_tasks' do
Expand Down
34 changes: 34 additions & 0 deletions spec/models/miq_schedule_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -695,4 +695,38 @@
expect(MiqSchedule.updated_since(1.month.ago)).to eq([s])
end
end

context ".queue_scheduled_work" do
it "When action exists" do
s = FactoryGirl.create(:miq_schedule, :sched_action => {:method => "scan"})
MiqSchedule.queue_scheduled_work(s.id, nil, "abc", nil)

expect(MiqQueue.first).to have_attributes(
:class_name => "MiqSchedule",
:instance_id => s.id,
:method_name => "invoke_actions",
:args => ["action_scan", "abc"],
:msg_timeout => 1200
)
end

it "with no action method, but resource exists and responds to the method" do
resource = FactoryGirl.create(:host)
s = FactoryGirl.create(:miq_schedule, :resource_id => resource.id, :towhat => resource.class.name, :sched_action => {:method => "test_method"})

expect_any_instance_of(Host).to receive("test_method").once

MiqSchedule.queue_scheduled_work(s.id, nil, "abc", nil)
end

it "with no action or resource" do
s = FactoryGirl.create(:miq_schedule, :sched_action => {:method => "test_method"})

expect($log).to receive(:warn) do |message|
expect(message).to include("no such action: [test_method], aborting schedule")
end

MiqSchedule.queue_scheduled_work(s.id, nil, "abc", nil)
end
end
end