[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