-
Notifications
You must be signed in to change notification settings - Fork 897
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
Create a task when destroying an ems #16669
Create a task when destroying an ems #16669
Conversation
cc @blomquisg |
app/models/ext_management_system.rb
Outdated
) | ||
end | ||
|
||
# Wait until all associated workers are dead to destroy this ems | ||
def orchestrate_destroy | ||
def orchestrate_destroy(task_id) | ||
disable! if enabled? | ||
|
||
if self.destroy == false | ||
_log.info("Cannot destroy #{self.class.name} with id: #{id}, workers still in progress. Requeuing destroy...") | ||
self.class.schedule_destroy_queue(id, 15.seconds.from_now) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this have to change since you changed the signature of schedule_destroy_queue ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, done
expect(described_class).to receive(:schedule_destroy_queue).with(ems.id) | ||
expect(described_class).to receive(:schedule_destroy_queue).with(child_manager.id) | ||
expect(described_class).to receive(:schedule_destroy_queue).with(ems.id, Integer) | ||
expect(described_class).to receive(:schedule_destroy_queue).with(child_manager.id, Integer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you also need a test for rescheduling a destroy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
a3b6ed2
to
1e0a9e2
Compare
1e0a9e2
to
da3d5f4
Compare
When queueing a destroy on an EMS create a task and return the ID to be used by the caller to track when the destroy is complete. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1525498
da3d5f4
to
d74ee55
Compare
@blomquisg @Fryguy PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna let @Fryguy merge since he already had other comments. Wanna give him the chance to double-verify.
expect(described_class).to receive(:schedule_destroy_queue).with(ems.id) | ||
expect(described_class).to receive(:schedule_destroy_queue).with(child_manager.id) | ||
described_class.destroy_queue(ems.id) | ||
def deliver_queue_message(queue_message = MiqQueue.first) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can lead to sproadic test failures if there are multiple queue items, because .first is not guranteed to return in a particular order...prefer MiqQueue.order(:id).first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 done
Checked commits agrare/manageiq@b9309e7~...fbd94fe with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
…eting_provider Create a task when destroying an ems (cherry picked from commit 2b27e62) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1530646
Gaprindashvili backport details:
|
When queueing a destroy on an EMS create a task and return the ID to be
used by the caller to track when the destroy is complete.
Task page while destroy is running:
Task after destroy completes:
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1525498