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

Wait for ems workers to finish before destroying the ems #14848

Merged
merged 2 commits into from
Jun 7, 2017
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
44 changes: 44 additions & 0 deletions app/models/ext_management_system.rb
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,46 @@ def enable!
update!(:enabled => true)
end

def self.destroy_queue(ids)
ids = Array.wrap(ids)
_log.info("Queuing destroy of #{name} with the following ids: #{ids.inspect}")
ids.each do |id|
schedule_destroy_queue(id)
end
end

# override destroy_queue from AsyncDeleteMixin
def destroy_queue
self.class.schedule_destroy_queue(id)
end

def self.schedule_destroy_queue(id, deliver_on = nil)
MiqQueue.put(
:class_name => name,
:instance_id => id,
:method_name => "orchestrate_destroy",
:deliver_on => deliver_on,
)
end

# Wait until all associated workers are dead to destroy this ems
def orchestrate_destroy
disable! if enabled?

if self.destroy == false
Copy link
Member

Choose a reason for hiding this comment

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

remove self to make rubocop happy

_log.info("Cant #{self.class.name} with id: #{id}, workers still in progress. Requeuing destroy...")
schedule_destroy_queue(id, :deliver_on => 15.seconds.from_now)
else
_log.info("#{self.class.name} with id: #{id} destroyed")
end
end

before_destroy :assert_no_queues_present

def assert_no_queues_present
throw(:abort) if MiqWorker.find_alive.where(:queue_name => queue_name).any?
end

def disconnect_inv
hosts.each { |h| h.disconnect_ems(self) }
vms.each { |v| v.disconnect_ems(self) }
Expand All @@ -431,6 +471,10 @@ def disconnect_inv
resource_pools.destroy_all
end

def queue_name
"ems_#{id}"
end

def enforce_policy(target, event)
inputs = {:ext_management_system => self}
inputs[:vm] = target if target.kind_of?(Vm)
Expand Down
2 changes: 1 addition & 1 deletion app/models/mixins/per_ems_worker_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def queue_name_for_ems(ems)
return "generic" if ems.kind_of?(Host) && ems.acts_as_ems?

return ems unless ems.kind_of?(ExtManagementSystem)
"ems_#{ems.id}"
ems.queue_name
Copy link
Member

Choose a reason for hiding this comment

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

thanks. good change

Copy link
Member

Choose a reason for hiding this comment

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

its also in #14864

end

def ems_id_from_queue_name(queue_name)
Expand Down
12 changes: 6 additions & 6 deletions spec/models/async_delete_mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def self.should_define_delete_queue_class_method
end
end

def self.should_queue_destroy_on_instance
def self.should_queue_destroy_on_instance(queue_method = "destroy")
it "should queue up destroy on instance" do
cond = ["class_name = ? AND instance_id = ? AND method_name = ?", @obj.class.name, @obj.id, "destroy"]
cond = ["class_name = ? AND instance_id = ? AND method_name = ?", @obj.class.name, @obj.id, queue_method]

expect { @obj.destroy_queue }.not_to raise_error
expect(MiqQueue.where(cond).count).to eq(1)
Expand All @@ -48,10 +48,10 @@ def self.should_queue_destroy_on_instance
end
end

def self.should_queue_destroy_on_class_with_many_ids
def self.should_queue_destroy_on_class_with_many_ids(queue_method = "destroy")
it "should queue up destroy on class method with many ids" do
ids = @objects.collect(&:id)
cond = ["class_name = ? AND instance_id in (?) AND method_name = ?", @obj.class.name, ids, "destroy"]
cond = ["class_name = ? AND instance_id in (?) AND method_name = ?", @obj.class.name, ids, queue_method]

expect { @obj.class.destroy_queue(ids) }.not_to raise_error
expect(MiqQueue.where(cond).count).to eq(ids.length)
Expand Down Expand Up @@ -129,8 +129,8 @@ def self.should_queue_delete_on_class_with_many_ids

should_define_destroy_queue_instance_method
should_define_destroy_queue_class_method
should_queue_destroy_on_instance
should_queue_destroy_on_class_with_many_ids
should_queue_destroy_on_instance("orchestrate_destroy")
should_queue_destroy_on_class_with_many_ids("orchestrate_destroy")

should_define_delete_queue_instance_method
should_define_delete_queue_class_method
Expand Down
15 changes: 15 additions & 0 deletions spec/models/ext_management_system_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,19 @@
expect(tenant.ext_management_systems).to include(ems)
end
end

context "destroy" do
it "destroys an ems with no active workers" do
ems = FactoryGirl.create(:ext_management_system)
ems.destroy
expect(ExtManagementSystem.count).to eq(0)
end

it "does not destroy an ems with active workers" do
ems = FactoryGirl.create(:ext_management_system)
FactoryGirl.create(:miq_ems_refresh_worker, :queue_name => ems.queue_name, :status => "started")
ems.destroy
expect(ExtManagementSystem.count).to eq(1)
end
end
end