From 2d3cbcf2ea0c1c1c3381fee2a258e472dbcf6e81 Mon Sep 17 00:00:00 2001 From: James Wong Date: Wed, 6 Dec 2017 22:36:27 -0500 Subject: [PATCH] orchestrate_destroy for provider https://bugzilla.redhat.com/show_bug.cgi?id=1491704 --- app/models/ext_management_system.rb | 44 +++++++---------------- app/models/miq_worker.rb | 4 +++ app/models/mixins/async_delete_mixin.rb | 14 +++++--- app/models/provider.rb | 32 +++++++++++++++++ spec/models/async_delete_mixin_spec.rb | 5 ++- spec/models/ext_management_system_spec.rb | 24 ++++++------- spec/models/provider_spec.rb | 32 ++++++++++++++++- 7 files changed, 100 insertions(+), 55 deletions(-) diff --git a/app/models/ext_management_system.rb b/app/models/ext_management_system.rb index a3e1b9ffd344..6abb6256db94 100644 --- a/app/models/ext_management_system.rb +++ b/app/models/ext_management_system.rb @@ -456,48 +456,28 @@ def destroy_queue :status => MiqTask::STATUS_OK, :message => msg, ) - - child_managers.each(&:destroy_queue) - self.class.schedule_destroy_queue(id, task.id) - + self.class._queue_task('destroy', id, task.id) task.id end - def self.schedule_destroy_queue(id, task_id, deliver_on = nil) - MiqQueue.put( - :class_name => name, - :instance_id => id, - :method_name => "orchestrate_destroy", - :deliver_on => deliver_on, - :args => [task_id], - ) - end - - # Wait until all associated workers are dead to destroy this ems - def orchestrate_destroy(task_id) + def destroy(task_id = nil) disable! if enabled? - if self.destroy == false - msg = "Cannot destroy #{self.class.name} with id: #{id}, workers still in progress. Requeuing destroy..." - MiqTask.update_status(task_id, MiqTask::STATE_ACTIVE, MiqTask::STATUS_OK, msg) + _log.info("Destroying #{child_managers.count} child_managers") + child_managers.destroy_all - _log.info(msg) + # kill workers + MiqWorker.find_alive.where(:queue_name => queue_name).each(&:kill) - self.class.schedule_destroy_queue(id, task_id, 15.seconds.from_now) - else - msg = "#{self.class.name} with id: #{id} destroyed" - MiqTask.update_status(task_id, MiqTask::STATE_FINISHED, MiqTask::STATUS_OK, msg) - - _log.info(msg) + super().tap do + if task_id + msg = "#{self.class.name} with id: #{id} destroyed" + MiqTask.update_status(task_id, MiqTask::STATE_FINISHED, MiqTask::STATUS_OK, msg) + _log.info(msg) + end 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) } diff --git a/app/models/miq_worker.rb b/app/models/miq_worker.rb index be23a3ba7194..9ce35f0d7ed3 100644 --- a/app/models/miq_worker.rb +++ b/app/models/miq_worker.rb @@ -415,6 +415,10 @@ def kill begin _log.info("Killing worker: ID [#{id}], PID [#{pid}], GUID [#{guid}], status [#{status}]") Process.kill(9, pid) + loop do + break unless alive? + sleep(0.01) + end rescue Errno::ESRCH _log.warn("Worker ID [#{id}] PID [#{pid}] GUID [#{guid}] has been killed") rescue => err diff --git a/app/models/mixins/async_delete_mixin.rb b/app/models/mixins/async_delete_mixin.rb index 5932aa0d33fa..77737f15581d 100644 --- a/app/models/mixins/async_delete_mixin.rb +++ b/app/models/mixins/async_delete_mixin.rb @@ -1,14 +1,18 @@ module AsyncDeleteMixin extend ActiveSupport::Concern included do - def self._queue_task(task, ids) - ids.each do |id| - MiqQueue.put( + def self._queue_task(task, ids, task_id = nil) + Array.wrap(ids).each do |id| + ops = { :class_name => name, :instance_id => id, :msg_timeout => 3600, - :method_name => task.to_s - ) + :method_name => task.to_s, + } + if task_id + ops[:args] = [task_id] + end + MiqQueue.put(ops) end end diff --git a/app/models/provider.rb b/app/models/provider.rb index 0715e4470181..b359992e7c34 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -63,4 +63,36 @@ def refresh_ems(opts = {}) end managers.flat_map { |manager| EmsRefresh.queue_refresh(manager, nil, opts) } end + + def self.destroy_queue(ids) + find(ids).each(&:destroy_queue) + end + + def destroy_queue + msg = "Destroying #{self.class.name} with id: #{id}" + + _log.info(msg) + task = MiqTask.create( + :name => "Destroying #{self.class.name} with id: #{id}", + :state => MiqTask::STATE_QUEUED, + :status => MiqTask::STATUS_OK, + :message => msg, + ) + self.class._queue_task('destroy', id, task.id) + task.id + end + + def destroy(task_id = nil) + _log.info("To destroy managers of provider: #{self.class.name} with id: #{id}") + managers.each(&:destroy) + + _log.info("To destroy provider: #{self.class.name} with id: #{id}") + super().tap do + if task_id + msg = "#{self.class.name} with id: #{id} destroyed" + MiqTask.update_status(task_id, MiqTask::STATE_FINISHED, MiqTask::STATUS_OK, msg) + _log.info(msg) + end + end + end end diff --git a/spec/models/async_delete_mixin_spec.rb b/spec/models/async_delete_mixin_spec.rb index f794652970f2..329079b62b59 100644 --- a/spec/models/async_delete_mixin_spec.rb +++ b/spec/models/async_delete_mixin_spec.rb @@ -36,7 +36,6 @@ def self.should_define_delete_queue_class_method 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, queue_method] - expect { @obj.destroy_queue }.not_to raise_error expect(MiqQueue.where(cond).count).to eq(1) expect_any_instance_of(@obj.class).to receive(:destroy).once @@ -129,8 +128,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("orchestrate_destroy") - should_queue_destroy_on_class_with_many_ids("orchestrate_destroy") + should_queue_destroy_on_instance + should_queue_destroy_on_class_with_many_ids should_define_delete_queue_instance_method should_define_delete_queue_class_method diff --git a/spec/models/ext_management_system_spec.rb b/spec/models/ext_management_system_spec.rb index 9e8cddd5ddaf..46730ec5a408 100644 --- a/spec/models/ext_management_system_spec.rb +++ b/spec/models/ext_management_system_spec.rb @@ -404,11 +404,12 @@ expect(ExtManagementSystem.count).to eq(0) end - it "does not destroy an ems with active workers" do + it "destroys 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") + worker = FactoryGirl.create(:miq_ems_refresh_worker, :queue_name => ems.queue_name, :status => "started") ems.destroy - expect(ExtManagementSystem.count).to eq(1) + expect(ExtManagementSystem.count).to eq(0) + expect(worker.class.exists?(worker.id)).to be_falsy end end @@ -439,22 +440,17 @@ deliver_queue_message - expect(MiqQueue.count).to eq(1) - expect(MiqQueue.last.deliver_on).to_not be_nil + expect(MiqQueue.count).to eq(0) + expect(ExtManagementSystem.count).to eq(0) end it "destroys the ems when the active worker shuts down" do - refresh_worker = FactoryGirl.create(:miq_ems_refresh_worker, :queue_name => ems.queue_name, :status => "started", :miq_server => server) + FactoryGirl.create(:miq_ems_refresh_worker, :queue_name => ems.queue_name, :status => "started", :miq_server => server) ems.destroy_queue deliver_queue_message - expect(ExtManagementSystem.count).to eq(1) - - refresh_worker.destroy - - deliver_queue_message - + expect(MiqQueue.count).to eq(0) expect(ExtManagementSystem.count).to eq(0) end end @@ -466,8 +462,8 @@ it "queues up destroy for child_managers" do described_class.destroy_queue(ems.id) - expect(MiqQueue.count).to eq(2) - expect(MiqQueue.pluck(:instance_id)).to include(ems.id, child_manager.id) + expect(MiqQueue.count).to eq(1) + expect(MiqQueue.first.instance_id).to eq(ems.id) end end diff --git a/spec/models/provider_spec.rb b/spec/models/provider_spec.rb index c73c708d4416..e57567d9a91e 100644 --- a/spec/models/provider_spec.rb +++ b/spec/models/provider_spec.rb @@ -1,5 +1,5 @@ describe Provider do - let(:provider) { described_class.new } + let(:provider) { FactoryGirl.create(:provider) } describe "#verify_ssl" do context "when non set" do @@ -61,4 +61,34 @@ expect(tenant.providers).to include(provider) end end + + context "#destroy_queue" do + let!(:miq_server) { EvmSpecHelper.local_miq_server } + it "queues destroy" do + provider.destroy_queue + expect(MiqQueue.find_by(:instance_id => provider.id)).to have_attributes( + 'method_name' => 'destroy', + 'class_name' => provider.class.name, + ) + end + end + + context "#destroy" do + it "destroys its managers and itself" do + manager = FactoryGirl.create(:ext_management_system) + provider.managers = [manager] + expect(manager).to receive(:destroy) + task = MiqTask.create( + :name => "Destroying #{self.class.name} with id: #{provider.id}", + :state => MiqTask::STATE_QUEUED, + :status => MiqTask::STATUS_OK, + ) + provider.destroy(task.id) + task.reload + expect(task).to have_attributes( + 'state' => MiqTask::STATE_FINISHED, + 'status' => MiqTask::STATUS_OK + ) + end + end end