From: Tomas Sedovic tsedovic@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 | 13 +++++++++++++ src/app/models/pool.rb | 6 ++++++ src/config/locales/en.yml | 10 +++++++++- src/config/navigation.rb | 2 +- 5 files changed, 47 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..2f5776d 100644 --- a/src/app/models/instance.rb +++ b/src/app/models/instance.rb @@ -121,6 +121,8 @@ class Instance < ActiveRecord::Base validates_inclusion_of :state, :in => STATES
+ before_destroy :destroyable? + def validate if assembly and template errors.add(:assembly, "Please specify either template or assembly, but not both") @@ -267,6 +269,17 @@ 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 + + def destroyable? + (state == STATE_CREATE_FAILED) or (state == STATE_STOPPED and not restartable?) + 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..2e00655 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.destroyable? } + 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