From: Tomas Sedovic <tsedovic(a)redhat.com>
This fixes Redmine #1098
Pools can be deleted only if they either don't have any instances associated
with them or if all those instances are stopped and stateless.
This also provides a dummy check to see whether an instance is stateful or
stateless. Currently, we only support stateless instances, but when we modify
the instance.restartable? method later on, nothing should break.
---
src/app/controllers/resources/pools_controller.rb | 20 ++++++++++++++++++--
src/app/models/instance.rb | 7 +++++++
src/app/models/pool.rb | 6 ++++++
src/config/locales/en.yml | 10 +++++++++-
src/config/navigation.rb | 2 +-
5 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/app/controllers/resources/pools_controller.rb
b/src/app/controllers/resources/pools_controller.rb
index 337a1b8..04fdf41 100644
--- a/src/app/controllers/resources/pools_controller.rb
+++ b/src/app/controllers/resources/pools_controller.rb
@@ -73,16 +73,32 @@ class Resources::PoolsController < ApplicationController
end
def multi_destroy
+ destroyed = []
+ failed = []
+ error_messages = []
Pool.find(params[:pools_selected]).each do |pool|
# FIXME: remove this check when pools can be assigned to new users
# default_pool cannot be deleted because metadata object has it tied
# to id of 1 and deleting it prevents new users from being created
if pool.id == MetadataObject.lookup("self_service_default_pool").id
- flash[:notice] = "The default pool cannot be deleted"
+ error_messages << "The default pool cannot be deleted"
+ elsif check_privilege(Privilege::MODIFY, pool) && pool.destroyable?
+ pool.destroy
+ destroyed << pool.name
else
- pool.destroy if check_privilege(Privilege::MODIFY, pool)
+ failed << pool.name
end
end
+
+ unless destroyed.empty?
+ flash[:notice] = t('pools.index.pool_deleted', :count =>
destroyed.length, :list => destroyed.join(', '))
+ end
+ unless failed.empty?
+ error_messages << t('pools.index.pool_not_deleted', :count =>
failed.length, :list => failed.join(', '))
+ end
+ unless error_messages.empty?
+ flash[:error] = error_messages.join('<br />')
+ end
redirect_to resources_pools_url
end
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 1841ecc..cd7d400 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -267,6 +267,13 @@ class Instance < ActiveRecord::Base
return stats
end
+ def restartable?
+ # TODO: we don't support stateful instances yet, so it's `false` for the time
being.
+ # In the meantime, we can use this method to write validation code for cases
+ # where does matter whether an instance is stateful or stateless.
+ false
+ end
+
named_scope :with_hardware_profile, lambda {
{:include => :hardware_profile}
}
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index ebee420..c4081ec 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -60,6 +60,8 @@ class Pool < ActiveRecord::Base
:include => [:role],
:order => "permissions.id ASC"
+ before_destroy :destroyable?
+
def cloud_accounts
accounts = []
instances.each do |instance|
@@ -73,4 +75,8 @@ class Pool < ActiveRecord::Base
HardwareProfile.find(:all, :conditions => {:provider_id => nil})
end
+ def destroyable?
+ instances.all? {|i| i.state == Instance::STATE_STOPPED and not i.restartable? }
+ end
+
end
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 7510a05..677af3b 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -69,7 +69,6 @@ en:
choose_treatment: Choose Treatment
apply: Apply
resource_management: Resource Management
- pools: Pools
deployments: Deployments
instances: Instances
searches: Searches
@@ -83,6 +82,15 @@ en:
provider_accounts_item: Provider Account
cloud_engine_hardware_profiles: Hardware Profiles
cloud_engine_realms: Realms
+ pools:
+ index:
+ pools: Pools
+ pool_deleted:
+ one: "Pool %{list} was deleted."
+ other: "Pools %{list} were deleted."
+ pool_not_deleted:
+ one: "Pool %{list} was not deleted. There are instances associated with
it."
+ other: "Pools %{list} were not deleted. They have instances associated with
them."
pool_families:
pool_families: Pool Families
index:
diff --git a/src/config/navigation.rb b/src/config/navigation.rb
index b6944a5..78a7862 100644
--- a/src/config/navigation.rb
+++ b/src/config/navigation.rb
@@ -2,7 +2,7 @@ SimpleNavigation::Configuration.run do |navigation|
navigation.autogenerate_item_ids = false
navigation.items do |first_level|
first_level.item :resource_management, t(:resource_management), resources_pools_path,
:highlights_on => /^\/$/ do |second_level|
- second_level.item :pools, t(:pools), resources_pools_path
+ second_level.item :pools, t('pools.index.pools'), resources_pools_path
second_level.item :deployments, t(:deployments),resources_deployments_path,
:highlights_on => /^\/$|\/deployments/
second_level.item :instances, t(:instances), resources_instances_path
end
--
1.7.4.2
Show replies by date
From: Tomas Sedovic <tsedovic(a)redhat.com>
---
src/features/pool.feature | 3 ++-
src/spec/models/pool_spec.rb | 26 ++++++++++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletions(-)
diff --git a/src/features/pool.feature b/src/features/pool.feature
index 7f1a00b..6a12f19 100644
--- a/src/features/pool.feature
+++ b/src/features/pool.feature
@@ -51,7 +51,8 @@ Feature: Manage Pools
And I press "Destroy"
Then there should only be 1 pools
And I should be on the resources pools page
- And I should not see "Redhat Voyager Pool"
+ When I go to the resources pools page
+ Then I should not see "Redhat Voyager Pool"
And I should not see "Amazon Startrek Pool"
Scenario: Create multiple pools
diff --git a/src/spec/models/pool_spec.rb b/src/spec/models/pool_spec.rb
index ca754fa..58ddec4 100644
--- a/src/spec/models/pool_spec.rb
+++ b/src/spec/models/pool_spec.rb
@@ -22,4 +22,30 @@ describe Pool do
u.errors[:name].should =~ /^is too long.*/
end
+ it "should not be destroyable when it has running instances" do
+ pool = Factory.create(:pool)
+ Pool.find(pool.id).should be_destroyable
+
+ instance = Factory.create(:instance, :pool_id => pool.id)
+ Pool.find(pool.id).should_not be_destroyable
+
+ instance.state = Instance::STATE_STOPPED
+ instance.save!
+ Pool.find(pool.id).should be_destroyable
+ end
+
+ it "should not be destroyable when it has stopped stateful instances" do
+ pool = Factory.build(:pool)
+ pool.should be_destroyable
+
+ instance = Factory.build(:instance, :pool_id => pool.id)
+ instance.stub!(:restartable?).and_return(true)
+ pool.instances << instance
+ pool.should_not be_destroyable
+
+ instance.state = Instance::STATE_STOPPED
+ instance.stub!(:restartable?).and_return(true)
+ pool.should_not be_destroyable
+ end
+
end
--
1.7.4.2