[PATCH] Pool's default enabled value is true
by Jirka Tomasek
From: Jiri Tomasek <jtomasek(a)redhat.com>
---
...9140916_change_enabled_default_value_in_pool.rb | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
create mode 100644 src/db/migrate/20120719140916_change_enabled_default_value_in_pool.rb
diff --git a/src/db/migrate/20120719140916_change_enabled_default_value_in_pool.rb b/src/db/migrate/20120719140916_change_enabled_default_value_in_pool.rb
new file mode 100644
index 0000000..562346f
--- /dev/null
+++ b/src/db/migrate/20120719140916_change_enabled_default_value_in_pool.rb
@@ -0,0 +1,9 @@
+class ChangeEnabledDefaultValueInPool < ActiveRecord::Migration
+ def self.up
+ change_column :pools, :enabled, :boolean, :default => true
+ end
+
+ def self.down
+ change_column :pools, :enabled, :boolean, :default => false
+ end
+end
--
1.7.7.6
11 years, 9 months
RM 3634 round - 1
by Tomas Hrcka
I am sending this patch because checking all error messages is taking longer then I expected. More patches are comming.
11 years, 9 months
Explanatory guide text in Conductor?
by Matt Wagner
Hi folks,
I've had an idea in the back of my mind for a while, and thought I'd
mention it here to see if it was actually a good idea or not.
A lot of web apps, especially the ones that are user-friendly, tend to
include text at the top of the main content section of each page walking
you through what you're expected to do on the page. After you upload a
photo to a social site, you might get a message along the lines of,
"Great photo, $user! Now, you can tag your friends in the photo, or add
a caption below!"
In that case, it's probably semi-intuitive what you're expected to do,
but the text at the top of the page makes it abundantly clear if you
weren't sure.
Aeolus does something pretty complicated, and a user must step through a
bunch of forms as they build an image, create a deployable, and go to
launch it. We've talked about working on making the flow intuitive and
the forms easy-to-understand, but I wonder if it might also be nice to
have some text guiding the user along.
As a random example, here is what the "New Deployable" page looks like
today:
http://dvmw6cjgjm3k7.cloudfront.net/wp-content/uploads/2012/08/Screenshot...
It's not necessarily a complicated form, but if you stop someone on the
street and ask them what it's all about, they're going to have no clue.
I took a really rough stab at writing some text at the top of the page:
http://dvmw6cjgjm3k7.cloudfront.net/wp-content/uploads/2012/08/Screenshot...
The text probably needs some work, and we should probably have a link or
something like that to explain _what_ a "Deployable XML file" is and how
to create one. But I like the idea of having some text telling me what
I'm doing on the page, and what will happen next.
Ignoring my mediocre example text and the fact that it's just an
unstyled paragraph tag, I wonder what people think about this general
concept. Would it be helpful? Or in an app like ours, do we expect the
user to know what they're doing? (Unlike, say, sharing a photo with some
friends, managing cloud deployments isn't necessarily you should learn
on the fly.)
-- Matt
11 years, 9 months
[PATCH conductor] RM 3635 - added better error message if deployment deletion failed
by Jan Provazník
From: Jan Provaznik <jprovazn(a)redhat.com>
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 277f9b3..6dcab0c 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 9ae82e4..27fa473 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -392,9 +392,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."
@@ -410,7 +408,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 9b2a37e..34fb413 100644
--- a/src/spec/models/deployment_spec.rb
+++ b/src/spec/models/deployment_spec.rb
@@ -357,6 +357,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
--
1.7.10.4
11 years, 9 months