[PATCH conductor 2/2] 838726 (2) - restrict what deployments can be stopped/removed
jprovazn at redhat.com
jprovazn at redhat.com
Thu Jul 12 18:45:10 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 6dc2c70..99cee49 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -311,6 +311,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 121e7d2..f34a58e 100644
--- a/src/app/controllers/instances_controller.rb
+++ b/src/app/controllers/instances_controller.rb
@@ -152,7 +152,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 3b4e9c1..be60802 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -160,6 +160,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
@@ -167,9 +171,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.
@@ -700,20 +703,7 @@ class Deployment < ActiveRecord::Base
def send_rollback_requests
error_occured = false
instances.running.each do |instance|
- begin
- instance.stop(nil)
- rescue
- error_occured = true
- self.events << Event.create(
- :source => instance,
- :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 8b8021e..fad0d03 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -447,6 +447,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 9295373..519d4c6 100644
--- a/src/config/locales/en.yml
+++ b/src/config/locales/en.yml
@@ -390,6 +390,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.10.4
More information about the aeolus-devel
mailing list