From: Jan Provaznik <[email protected]>
A reason why deletion failed is now displayed. This required more changes
(cleanups):
- fixes formatting of flash error
- moves destroy_deployment_config call into before_destroy callback
- fixes setting of flash message on multi_destroy action
- adds new exception to catch (most common) stop&destroy error - unstoppable
instance
---
src/app/controllers/deployments_controller.rb | 87 +++++++++++++++--------
src/app/models/deployment.rb | 25 +++----
src/app/views/layouts/_new_notification.html.haml | 7 +-
src/config/locales/en.yml | 7 +-
src/lib/exceptions.rb | 1 +
src/spec/models/deployment_spec.rb | 10 +++
6 files changed, 88 insertions(+), 49 deletions(-)
diff --git a/src/app/controllers/deployments_controller.rb
b/src/app/controllers/deployments_controller.rb
index d9ec800..7df640b 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -256,53 +256,84 @@ class DeploymentsController < ApplicationController
def destroy
deployment = Deployment.find(params[:id])
- if check_privilege(Privilege::MODIFY, deployment)
- begin
- deployment.stop_instances_and_destroy!
- flash[:success] = t('deployments.flash.success.deleted', :list =>
deployment.name, :count => 1)
- rescue
- flash[:error] = t('deployments.flash.error.not_deleted', :list =>
deployment.name, :count => 1)
- end
- else
- flash[:error] = t('deployments.flash.error.not_deleted', :list =>
deployment.name, :count => 1)
+ cant_stop = false
+ errors = []
+ begin
+ require_privilege(Privilege::MODIFY, deployment)
+ deployment.stop_instances_and_destroy!
+ rescue Aeolus::Conductor::Base::NotStoppableDeployment
+ cant_stop = true
+ errors = deployment.not_stoppable_or_destroyable_instances.map {|i|
+ t('deployments.errors.instance_state',
+ :name => i.name,
+ :state => i.state)}
+ rescue
+ errors = t('deployments.flash.error.not_deleted',
+ :name => deployment.name,
+ :error => $!.message)
end
+
respond_to do |format|
format.js do
load_deployments
render :partial => 'list'
end
- format.html { redirect_to pools_url(:view => 'filter', :details_tab =>
'deployments') }
- format.json { render :json => {:success => destroyed, :errors => failed}
}
+
+ format.html do
+ if errors.empty?
+ flash[:success] = t('deployments.flash.success.deleted',
+ :list => deployment.name, :count => 1)
+ elsif cant_stop
+ flash[:error] = {:summary => t('deployments.errors.cannot_stop',
+ :name => deployment.name),
+ :failures => errors}
+ else
+ flash[:error] = errors
+ end
+ redirect_to pools_url(:view => 'filter', :details_tab => 'deployments')
+ end
+
+ format.json { render :json => {:success => errors.empty?,
+ :errors => errors} }
end
end
def multi_destroy
destroyed = []
- failed = []
+ errors = []
- Deployment.find(params[:deployments_selected] || []).each do |deployment|
- if check_privilege(Privilege::MODIFY, deployment)
- begin
- deployment.stop_instances_and_destroy!
- destroyed << deployment.name
- rescue
- failed << deployment.name
- end
- else
- failed << deployment.name
+ ids = Array(params[:deployments_selected])
+ Deployment.find(ids).each do |deployment|
+ begin
+ require_privilege(Privilege::MODIFY, deployment)
+ deployment.stop_instances_and_destroy!
+ destroyed << deployment.name
+ rescue
+ errors << t('deployments.flash.error.not_deleted',
+ :name => deployment.name,
+ :error => $!.message)
end
end
- # If nothing is selected, display an error message:
- flash[:error] = t('deployments.flash.error.none_selected') if failed.blank?
&& destroyed.blank?
- flash[:success] = t('deployments.flash.success.deleted', :list =>
destroyed.to_sentence, :count => destroyed.size) if destroyed.present?
- flash[:error] = t('deployments.flash.error.not_deleted', :list =>
failed.to_sentence, :count => failed.size) if failed.present?
respond_to do |format|
- format.html { redirect_to params[:backlink] || pools_url(:view => 'filter',
:details_tab => 'deployments') }
+ format.html do
+ if ids.empty?
+ flash[:error] = t('deployments.flash.error.none_selected')
+ elsif errors.present?
+ flash[:error] = errors
+ end
+ flash[:success] = t('deployments.flash.success.deleted',
+ :list => destroyed.to_sentence,
+ :count => destroyed.size) if destroyed.present?
+
+ redirect_to params[:backlink] ||
+ pools_url(:view => 'filter', :details_tab => 'deployments')
+ end
+
format.js do
load_deployments
render :partial => 'list'
end
- format.json { render :json => {:success => destroyed, :errors => failed}
}
+ format.json { render :json => {:success => destroyed, :errors => errors}
}
end
end
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 6e5e5d7..5fe72f6 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -77,6 +77,7 @@ class Deployment < ActiveRecord::Base
validates_presence_of :owner_id
validate :pool_must_be_enabled
before_destroy :destroyable?
+ before_destroy :destroy_deployment_config
before_create :inject_launch_parameters
before_create :generate_uuid
before_create :set_pool_family
@@ -164,28 +165,28 @@ class Deployment < ActiveRecord::Base
[STATE_RUNNING, STATE_INCOMPLETE].include?(self.state)
end
+ def not_stoppable_or_destroyable_instances
+ instances.find_all {|i| !(i.destroyable? or
+ i.state == Instance::STATE_RUNNING)}
+ end
+
def stop_instances_and_destroy!
- if destroyable?
- destroy_deployment_config
- destroy
- return
+ unless not_stoppable_or_destroyable_instances.empty?
+ raise Aeolus::Conductor::Base::NotStoppableDeployment,
+ I18n.t("deployments.errors.all_stopped")
end
- if instances.all? {|i| i.destroyable? or i.state == Instance::STATE_RUNNING}
+ if destroyable?
+ destroy
+ else
self.state = Deployment::STATE_SHUTTING_DOWN
- destroy_deployment_config
# The deployment will be destroyed from an InstanceObserver callback
once
# all instances are stopped.
self.scheduled_for_deletion = true
self.save!
# stop all deployment's instances
- instances.each do |instance|
- next unless instance.state == Instance::STATE_RUNNING
- instance.stop(instance.owner)
- end
- else
- raise I18n.t("deployments.errors.all_stopped")
+ instances.running.each {|instance| instance.stop(instance.owner)}
end
end
diff --git a/src/app/views/layouts/_new_notification.html.haml b/src/app/views/layouts/_new_notification.html.haml
index d807020..3150ef2 100644
--- a/src/app/views/layouts/_new_notification.html.haml
+++ b/src/app/views/layouts/_new_notification.html.haml
@@ -61,11 +61,8 @@
=image_tag 'flash_error_icon.png', :alt => 'Errors'
%ul.flashes
-if !flash[:error][:failures].blank?
- -if flash[:error][:failures].length > 1
- -flash[:error][:failures].each do |k, v|
- %li= [k, v.present? ? ": #{v}" : '']
- -else
- %li= flash[:error][:failures]
+ -flash[:error][:failures].each do |k, v|
+ %li= v.present? ? "#{k}: #{v}" : k.to_s
-else
-flash[:error].each do |e|
%li= e
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 0faa9db..e8cda15 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -393,9 +393,7 @@ en:
error:
not_launched: "Some Assemblies will not be launched:"
failed_to_launch_assemblies: "Failed to launch following Assemblies:"
- not_deleted:
- one: "The Deployment %{list} could not be deleted."
- other: "The Deployments %{list} could not be deleted."
+ not_deleted: "The deployment %{name} could not be deleted: %{error}"
none_selected: "You must select one or more Deployments."
not_updated:
one: "The Deployment %{list} could not be updated."
@@ -411,7 +409,8 @@ en:
other: "The Deployments %{list} were successfully updated."
errors:
config_server_connection: 'Cannot connect to the Config Server'
- cannot_stop: "Stop cannot be performed on this instance."
+ cannot_stop: "The deployment %{name} can not be deleted because following
instances can not be stopped:"
+ instance_state: "The instance %{name} is in state %{state}."
all_stopped: "All Instances must be stopped or running"
not_valid_deployable_xml: "seems to be not valid Deployable XML: %{msg}"
match_not_found: "Match not found: %{errors}"
diff --git a/src/lib/exceptions.rb b/src/lib/exceptions.rb
index d56ddb3..200aa3f 100644
--- a/src/lib/exceptions.rb
+++ b/src/lib/exceptions.rb
@@ -47,6 +47,7 @@ module Aeolus
module Base
class ImageNotFound < StandardError; end
class BlankImageId < StandardError; end
+ class NotStoppableDeployment < StandardError; end
end
module MultiError
diff --git a/src/spec/models/deployment_spec.rb
b/src/spec/models/deployment_spec.rb
index aa78f50..17b0892 100644
--- a/src/spec/models/deployment_spec.rb
+++ b/src/spec/models/deployment_spec.rb
@@ -335,6 +335,16 @@ describe Deployment do
lambda { Instance.find(inst1.id) }.should
raise_error(ActiveRecord::RecordNotFound)
lambda { Instance.find(inst2.id) }.should
raise_error(ActiveRecord::RecordNotFound)
end
+
+ it "should raise an exception if some instances can not be stopped" do
+ @deployment.save!
+ inst1 = Factory.create(:mock_running_instance,
+ :state => Instance::STATE_PENDING,
+ :deployment_id => @deployment.id)
+ @deployment.reload
+ lambda { @deployment.stop_instances_and_destroy!}.should
+ raise_error(Aeolus::Conductor::Base::NotStoppableDeployment)
+ end
end
describe ".any_instance_running?" do
