Skip to content

Commit

Permalink
Merge pull request #209 from Fryguy/fix_provider_destroy
Browse files Browse the repository at this point in the history
Bypass the superclass orchestrated destroy for this Provider.
(cherry picked from commit 158b93f)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1538240
  • Loading branch information
aufi authored and simaishi committed Jan 25, 2018
1 parent a0736e1 commit 53194e3
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
14 changes: 14 additions & 0 deletions app/models/manageiq/providers/openstack/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,21 @@ class ManageIQ::Providers::Openstack::Provider < ::Provider
has_many :network_managers,
:foreign_key => "provider_id",
:class_name => "ManageIQ::Providers::Openstack::NetworkManager",
:dependent => :nullify,
:autosave => true

validates :name, :presence => true, :uniqueness => true

def destroy
# Bypass the superclass orchestrated destroy for this Provider.
# In the OpenStack provider, the Provider instance is only tightly coupled
# to the InfraManager. That is, when the InfraManager is created, then
# the Provider is created and vice-versa, when the InfraManager is
# destroyed, we need to destroy the Provider. The CloudManager objects
# associated to that Provider should *not* be destroyed, and only need to
# be nullified. For other Providers, the Provider instance is what is
# destroyed, so by default the orchestrated destroy destroys all of its
# managers, but we here don't want that to happen, so we are bypassing.
self.class.instance_method(:destroy).super_method.super_method.bind(self).call
end
end
18 changes: 18 additions & 0 deletions spec/models/manageiq/providers/openstack/cloud_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,24 @@
end
end

context "provider hooks" do
it "related EmsOpenstack and ProviderOpenstack are left around on EmsOpenstackCloud destroy" do
@ems = FactoryGirl.create(:ems_openstack_infra_with_authentication)
@ems_cloud = FactoryGirl.create(:ems_openstack_with_authentication)
@ems.provider.cloud_ems << @ems_cloud

# compare they both use the same provider
expect(@ems_cloud.provider).to eq(@ems.provider)

@ems_cloud.destroy
expect(ManageIQ::Providers::Openstack::CloudManager.count).to eq 0

# Ensure the ems infra and provider still stays around
expect(ManageIQ::Providers::Openstack::Provider.count).to eq 1
expect(ManageIQ::Providers::Openstack::InfraManager.count).to eq 1
end
end

it "event_monitor_options" do
allow(ManageIQ::Providers::Openstack::CloudManager::EventCatcher).to receive_messages(:worker_settings => {:amqp_port => 1234})
@ems = FactoryGirl.build(:ems_openstack, :hostname => "host", :ipaddress => "::1")
Expand Down
13 changes: 8 additions & 5 deletions spec/models/manageiq/providers/openstack/infra_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@
end
end

context "provider_hooks" do
before :each do
context "provider hooks" do
before do
@ems = FactoryGirl.create(:ems_openstack_infra_with_authentication)
end

Expand All @@ -103,10 +103,13 @@
# compare they both use the same provider
expect(@ems_cloud.provider).to eq(@ems.provider)

# destroy ems and see the ems_cloud removed as well
@ems.destroy
expect(ManageIQ::Providers::Openstack::Provider.count).to eq 0
expect(ManageIQ::Providers::Openstack::CloudManager.count).to eq 0
expect(ManageIQ::Providers::Openstack::InfraManager.count).to eq 0
expect(ManageIQ::Providers::Openstack::Provider.count).to eq 0

# Ensure the ems_cloud still stays around
expect(ManageIQ::Providers::Openstack::CloudManager.count).to eq 1
expect(@ems_cloud.reload.provider).to be_nil
end
end

Expand Down

0 comments on commit 53194e3

Please sign in to comment.