-
Notifications
You must be signed in to change notification settings - Fork 898
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
Don't create retirement tasks for things that are already retired #18895
Don't create retirement tasks for things that are already retired #18895
Conversation
@miq-bot add_label bug |
c2309eb
to
2a6b8f3
Compare
Hi @d-m-u , here's the BZ for this issue: https://bugzilla.redhat.com/show_bug.cgi?id=1720338 |
@mkanoor Please review. |
Thanks @d-m-u for making the changes I requested. |
@bdunne any chance I could con you into merging this please? |
2a6b8f3
to
c6822d3
Compare
Checked commit d-m-u@c6822d3 with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
@miq-bot add_label hammer/yes |
…service_retire_task_create Don't create retirement tasks for things that are already retired (cherry picked from commit 6bbf5f4) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1728889
Hammer backport details:
|
We shouldn't create tasks for already retired VMs.
bz is https://bugzilla.redhat.com/show_bug.cgi?id=1720338
This is ugly but it'll work for now. The refactor ideally involves using the general status check that Lucy's working on, but until that's baked in and finished and well-tested, we have to do something kinda hacky like this. This check doesn't belong inside the retireable? mixin check because that was intended as a class check for, can this class of thing actually be retired? So since this is an instance method check, it has to be here for now.