Hi, this patch fixes provider deletion. Disables deletion of provider when
provider's cloud accounts are not deleted because of existing instances.
Show replies by date
From: Jan Provaznik <jprovazn(a)redhat.com>
A provider was deleted even if there was deletion of cloud account
failed because of existing instances associated with the cloud account.
---
src/app/controllers/provider_controller.rb | 13 ++++++++++---
src/app/models/provider.rb | 17 +++++++++++++++++
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/src/app/controllers/provider_controller.rb
b/src/app/controllers/provider_controller.rb
index 9058c3e..1605eaf 100644
--- a/src/app/controllers/provider_controller.rb
+++ b/src/app/controllers/provider_controller.rb
@@ -52,11 +52,18 @@ class ProviderController < ApplicationController
def destroy
if request.post?
- p =Provider.find(params[:provider][:id])
+ @provider = Provider.find(params[:provider][:id])
require_privilege(Privilege::PROVIDER_MODIFY, p)
- p.destroy
+ if @provider.destroy and @provider.destroyed?
+ redirect_to :action => "index"
+ else
+ flash[:error] = {
+ :summary => "Failed to delete Provider",
+ :failures => @provider.errors.full_messages,
+ }
+ render :action => 'show'
+ end
end
- redirect_to :action => "index"
end
def hardware_profiles
diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
index ae8e98f..ed8fc70 100644
--- a/src/app/models/provider.rb
+++ b/src/app/models/provider.rb
@@ -38,6 +38,23 @@ class Provider < ActiveRecord::Base
:include => [:role],
:order => "permissions.id ASC"
+ before_destroy :destroyable?
+
+ # there is a destroy dependency for a cloud accounts association,
+ # but a cloud account is silently not destroyed when there is
+ # an instance for the cloud account
+ def destroyable?
+ unless self.cloud_accounts.empty?
+ self.cloud_accounts.each do |c|
+ unless c.instances.empty?
+ inst_list = c.instances.map {|i| i.name}.join(', ')
+ self.errors.add_to_base "there are instances for cloud account
'#{c.name}': #{inst_list}"
+ end
+ end
+ end
+ return self.errors.empty?
+ end
+
def set_cloud_type
self.cloud_type = connect.driver_name
end
--
1.7.2.1
From: Jan Provaznik <jprovazn(a)redhat.com>
---
src/spec/models/provider_spec.rb | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/src/spec/models/provider_spec.rb b/src/spec/models/provider_spec.rb
index 7cabea7..fd555e9 100644
--- a/src/spec/models/provider_spec.rb
+++ b/src/spec/models/provider_spec.rb
@@ -66,6 +66,20 @@ describe Provider do
@provider.set_cloud_type
@provider.should be_valid
end
+
+ it "should not destroy provider if deletion of associated cloud account
fails" do
+ # TODO: front end HW profiles are not deleted with provider, which
+ # involves "External name is already used" error.
+ # This should be solved when implementing "Scripted import of Hardware
Profiles
+ # from EC2" scenario, then it's possible to delete this line
+ # note: same situation will be with images
+ HardwareProfile.destroy_all
+
+ instance = Factory(:instance)
+ provider = instance.cloud_account.provider
+ provider.destroy
+ provider.destroyed?.should be_false
+ end
end
context "(using original connect method)" do
--
1.7.2.1