[PATCH 2/2] 838726 (2) - restrict what deployments can be stopped/removed
jprovazn at redhat.com
jprovazn at redhat.com
Fri Jul 13 14:27:09 UTC 2012
From: Jan Provaznik <jprovazn at redhat.com>
Only running deployemnts can be stopped and only deployments
with destroyable or running instances can be destroyed. This
patch makes sure that deployemnt doesn't transit from new/pending state
directly to shutting down state.
---
src/app/controllers/deployments_controller.rb | 5 ++++
src/app/controllers/instances_controller.rb | 2 +-
src/app/models/deployment.rb | 22 +++++---------------
src/app/models/instance.rb | 16 +++++++++++++++
src/config/locales/en.yml | 1 +
src/features/deployment.feature | 10 +++++++++
src/features/step_definitions/deployment_steps.rb | 4 +++
7 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/src/app/controllers/deployments_controller.rb b/src/app/controllers/deployments_controller.rb
index 77b7cb8..5acbc48 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -302,6 +302,11 @@ class DeploymentsController < ApplicationController
@deployments_to_stop.each do |deployment|
# TODO: move all this logic to model
+ unless deployment.can_stop?
+ errors << t('deployments.flash.error.cant_stop',
+ :name => deployment.name)
+ next
+ end
deployment.state = Deployment::STATE_SHUTTING_DOWN
deployment.save!
deployment.instances.each do |instance|
diff --git a/src/app/controllers/instances_controller.rb b/src/app/controllers/instances_controller.rb
index ac78096..9322b21 100644
--- a/src/app/controllers/instances_controller.rb
+++ b/src/app/controllers/instances_controller.rb
@@ -160,7 +160,7 @@ class InstancesController < ApplicationController
notices << "#{instance.name}: #{t('instances.flash.notice.stop')}"
end
rescue Exception => err
- errors << "#{instance.name}: " + err
+ errors << "#{instance.name}: " + err.message
logger.error err.message
logger.error err.backtrace.join("\n ")
end
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index ac63509..974ab65 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -159,6 +159,10 @@ class Deployment < ActiveRecord::Base
instances.all? {|i| i.destroyable? }
end
+ def can_stop?
+ [STATE_RUNNING, STATE_INCOMPLETE].include?(self.state)
+ end
+
def stop_instances_and_destroy!
if destroyable?
destroy_deployment_config
@@ -166,9 +170,8 @@ class Deployment < ActiveRecord::Base
return
end
- self.state = Deployment::STATE_SHUTTING_DOWN
- save!
if instances.all? {|i| i.destroyable? or i.state == Instance::STATE_RUNNING}
+ self.state = Deployment::STATE_SHUTTING_DOWN
destroy_deployment_config
# The deployment will be destroyed from an InstanceObserver callback once
# all instances are stopped.
@@ -680,20 +683,7 @@ class Deployment < ActiveRecord::Base
error_occured = false
instances.running.each do |instance|
- begin
- instance.stop(nil)
- rescue
- error_occured = true
- self.events << Event.create(
- :source => self,
- :event_time => DateTime.now,
- :status_code => 'instance_stop_failed',
- :summary => "Failed to stop instance #{instance.name}",
- :description => $!.message
- )
- logger.error $!.message
- logger.error $!.backtrace.join("\n ")
- end
+ error_occured = true unless instance.stop_with_event(nil)
end
if error_occured
cleanup_failed_launch
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 5ca080a..14681b1 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -465,6 +465,22 @@ class Instance < ActiveRecord::Base
do_operation(user, 'stop')
end
+ def stop_with_event(user)
+ stop(user)
+ true
+ rescue
+ self.events << Event.create(
+ :source => self,
+ :event_time => DateTime.now,
+ :status_code => 'instance_stop_failed',
+ :summary => "Failed to stop instance #{self.name}",
+ :description => $!.message
+ )
+ logger.error $!.message
+ logger.error $!.backtrace.join("\n ")
+ false
+ end
+
def reboot(user)
do_operation(user, 'reboot')
end
diff --git a/src/config/locales/en.yml b/src/config/locales/en.yml
index 1a1c69e..1f458ed 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -329,6 +329,7 @@ en:
one: "The deployment %{list} could not be updated."
other: "The deployments %{list} could not be updated."
no_hwp_permission: "You do not have sufficient permission to access the %{hwp} hardware profile."
+ cant_stop: "The deployment %{name} could not be stopped - only running deployments can be stopped."
success:
deleted:
one: "The deployment %{list} was scheduled for deletion."
diff --git a/src/features/deployment.feature b/src/features/deployment.feature
index a5ebf73..15e54c5 100644
--- a/src/features/deployment.feature
+++ b/src/features/deployment.feature
@@ -82,8 +82,18 @@ Feature: Manage Deployments
And I press "stop_button"
Then I should see "testdeployment"
+ Scenario: Stop pending deployment
+ Given there is a deployment named "testdeployment" belonging to "testdeployable" owned by "testuser"
+ And deployment "testdeployment" is "pending"
+ When I go to the deployments page
+ Then I should see "testdeployment"
+ When I check "testdeployment" deployment
+ And I press "stop_button"
+ Then I should see "deployment testdeployment could not be stopped"
+
Scenario: Stop inaccessible deployments
Given there is a deployment named "testdeployment" belonging to "testdeployable" owned by "testuser"
+ And deployment "testdeployment" is "incomplete"
And "testdeployment" deployment's provider is not accessible
When I go to the deployments page
Then I should see "testdeployment"
diff --git a/src/features/step_definitions/deployment_steps.rb b/src/features/step_definitions/deployment_steps.rb
index 4ebab17..d9e80bc 100644
--- a/src/features/step_definitions/deployment_steps.rb
+++ b/src/features/step_definitions/deployment_steps.rb
@@ -38,6 +38,10 @@ Given /^the deployment "([^"]*)" has an instance named "([^"]*)"$/ do |d_name, i
deployment.instances << FactoryGirl.create(:instance, :name => i_name, :pool => deployment.pool)
end
+Given /^deployment "([^"]*)" is "([^"]*)"$/ do |arg1, arg2|
+ Deployment.find_by_name(arg1).update_attribute(:state, arg2)
+end
+
Given /^"([^"]*)" deployment's provider is not accessible$/ do |arg1|
# FIXME: didn't find a way how to create an inaccessible provider
# cleanly
--
1.7.7.6
More information about the aeolus-devel
mailing list