Do not get stuck on destroy

[yaacov]

Can not destroy a none-valid EMS.

destroy! calls disable! before destroying the object, disable check for object validity, and do not let us destroy the object.

When destroying the object, we do not care for object validity, we want to remove it from the DB even if it is not valid.

Example

ems = ManageIQ::Providers::Openshift::ContainerManager.first

ems.hostname = nil
ems.default_endpoint.save!

ems:send(disable!)
=> NoMethodError: undefined method `ipaddress?' for nil:NilClass
=> from /home/yzamir/go/src/github.com/ManageIQ/manageiq/app/models/ext_management_system.rb:130:in `hostname_format_valid?'

ems.send(:disable!, :validate => false)
=> true

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1733587

[yaacov]

@cben @agrare please review

[miq-bot]

Checked commit yaacov@6db56dd with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

[agrare]

👍 makes sense thanks @yaacov

[cben]

LGTM (we paired on this).

I guess the disable! if enabled? involving a save and thus validation was a regression (of this rare aspect) introduced by orchestrated destroy effort (#14848, #16755 and more).

We tried to also test this with evm:start and actual workers, to see whether other issues from invalid EMS might block the deletion.
But since it had bad auth status, no EMS-specific workers were running, MiqWorker.find_alive... returned zero workers, so "kill workers" stage passed vacuously. This will also probably be the case for users who somehow have an invalid EMS 🤷‍♂️...
Anyway, issues could happen inside workers, but that shouldn't matter as MiqWorker#kill just does kill -9 🔫.

The rest of the associated destroy chains worked fine.
[P.S. unrelated: deleting associated event_streams seems to do inefficient 1+N 🐌]

[simaishi]

@yaacov ivanchuk/yes?

[yaacov]

https://bugzilla.redhat.com/show_bug.cgi?id=1733587

BZ target is 5.10.z

[simaishi]

Which means it should be ivanchuk/yes and hammer/yes if this PR can be backported as is.

[yaacov]

IMHO It can be safely backported

[agrare]

ACK safe to backport

[simaishi]

Thank you both 😄

[simaishi]

Ivanchuk backport details:

$ git log -1
commit 1dd63780d70590671376547848420d53999e80d6
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Jul 30 09:20:52 2019 -0400

    Merge pull request #19077 from yaacov/do-not-get-stuck-on-destroy
    
    Do not get stuck on destroy
    
    (cherry picked from commit 23750927bd53c88c249dd0c7f0abc0bc2155afaf)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1733587

[simaishi]

Hammer backport details:

$ git log -1
commit 1b32f43f4b464772d4abfb4b089f4c36466bc41b
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Jul 30 09:20:52 2019 -0400

    Merge pull request #19077 from yaacov/do-not-get-stuck-on-destroy
    
    Do not get stuck on destroy
    
    (cherry picked from commit 23750927bd53c88c249dd0c7f0abc0bc2155afaf)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1763857