Fix Vm#supports_terminate? checking EMS#vm_destroy #20222
Conversation
|
I think this PR needs a cross-repo test with all the providers. |
|
This is going to take some rework because since the We need to move this closer to the rest of the supports feature calls where the supports :terminate block is defined on the provider model class not in core. |
agrare
changed the title
agrare
force-pushed
the
fix_vm_supports_terminate
branch
from
44abd09
to
d33b685
Compare
| context "#supports_terminate?" do | ||
| let(:ems_does_vm_destroy) { FactoryBot.create(:ems_vmware) } | ||
| let(:ems_doesnot_vm_destroy) { FactoryBot.create(:ems_storage) } | ||
| context "#supports_control?" do |
There was a problem hiding this comment.
Most of these were actually testing supports_control?
There was a problem hiding this comment.
Was supports_control? supposed to stick around?
https://github.com/ManageIQ/manageiq/blob/master/app/models/mixins/supports_feature_mixin.rb#L66-L67
There was a problem hiding this comment.
It is still an internal helper method used all over the place, e.g.: https://github.com/ManageIQ/manageiq-providers-vmware/blob/master/app/models/manageiq/providers/vmware/infra_manager/vm/operations/guest.rb#L6
The purpose of this PR isn't to refactor supports_control
agrare
changed the title
Testing: ManageIQ/manageiq#20222 ManageIQ/manageiq-providers-amazon#626 ManageIQ/manageiq-providers-azure#397 ManageIQ/manageiq-providers-google#137 ManageIQ/manageiq-providers-kubevirt#164 ManageIQ/manageiq-providers-openstack#598 ManageIQ/manageiq-providers-ovirt#497 ManageIQ/manageiq-providers-vmware#584
agrare
force-pushed
the
fix_vm_supports_terminate
branch
from
d33b685
to
3b7dd5d
Compare
|
Cross repo tests are green except for an unrelated error: https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/jobs/341417657#L2392-L2423 This occurs if all plugins that bring extra |
|
unrelated error is fixed in #20225 |
We removed the redundant EMS#vm_destroy methods which just called VM#raw_destroy for all providers except VMware. The supports_feature block however was still checking if the EMS respond_to?(:vm_destroy) rather than checking if the vm object responds to it.
agrare
force-pushed
the
fix_vm_supports_terminate
branch
from
3b7dd5d
to
6a2602e
Compare
|
Checked commit agrare@6a2602e with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
Fix Vm#supports_terminate? checking EMS#vm_destroy (cherry picked from commit 74d620c)
|
Jansa backport details: |
We removed the redundant EMS#vm_destroy methods which just called VM#raw_destroy for all providers except VMware. The supports_feature block however was still checking if the EMS respond_to?(:vm_destroy) so no vm was supporting terminate.
Related to: #19452
Depends:
Change seeding specs to look for a specific plugin path #20225
Add supports terminate feature manageiq-providers-amazon#626
Add supports terminate feature manageiq-providers-azure#397
Add supports terminate feature manageiq-providers-google#137
Add supports terminate feature manageiq-providers-kubevirt#164
Add supports terminate feature manageiq-providers-openstack#598
Add supports terminate feature manageiq-providers-ovirt#497
Add supports terminate feature manageiq-providers-vmware#584
Cross repo tests: ManageIQ/manageiq-cross_repo-tests#135