[PATCH conductor] Fixed deployment rollback

jprovazn at redhat.com jprovazn at redhat.com
Thu Jun 7 13:38:00 UTC 2012


From: Jan Provaznik <jprovazn at redhat.com>

When rollback was initiated, 'stop' request was sent to instances
in 'pending' state too. But it seems that 'stop' is not valid action
for 'pending' state. Now we wait until pending instances are running
and send 'stop' request then.

This is fix for already pushed task:
https://www.aeolusproject.org/redmine/issues/3320
---
 src/app/models/deployment.rb        |   14 +++++++++-----
 src/app/models/instance.rb          |    1 +
 src/app/models/instance_observer.rb |    2 +-
 src/spec/models/deployment_spec.rb  |    2 ++
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 0cf1137..7b66e56 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -654,6 +654,8 @@ class Deployment < ActiveRecord::Base
     if Instance::ACTIVE_FAILED_STATES.include?(instance.state)
       # if this instance stop failed, whole deployment rollback failed
       self.state = STATE_ROLLBACK_FAILED
+    elsif instance.state == Instance::STATE_RUNNING
+      deployment_rollback
     elsif instances.all? {|i| i.inactive?}
       # some other instances might be failed (because their
       # launch failed), but it shouldn't be a problem if all
@@ -663,17 +665,19 @@ class Deployment < ActiveRecord::Base
   end
 
   def deployment_rollback
-    stoppable_instances = instances.stoppable
-    if stoppable_instances.empty?
+    unless self.state == STATE_ROLLBACK_IN_PROGRESS
+      self.state = STATE_ROLLBACK_IN_PROGRESS
+      save!
+    end
+
+    if instances.all? {|i| i.inactive?}
       self.state = STATE_ROLLBACK_COMPLETE
       save!
       return
     end
 
-    self.state = STATE_ROLLBACK_IN_PROGRESS
-    save!
     error_occured = false
-    stoppable_instances.each do |instance|
+    instances.running.each do |instance|
       begin
         instance.stop(nil)
       rescue
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index aee01a1..3806d16 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -128,6 +128,7 @@ class Instance < ActiveRecord::Base
   scope :deployed,  :conditions => { :state => [STATE_RUNNING, STATE_SHUTTING_DOWN] }
   # FIXME: "pending" is misleading as it doesn't just cover STATE_PENDING
   scope :pending,   :conditions => { :state => [STATE_NEW, STATE_PENDING] }
+  scope :running,   :conditions => { :state => [STATE_RUNNING] }
   scope :pending_or_deployed,   :conditions => { :state => [STATE_NEW, STATE_PENDING, STATE_RUNNING, STATE_SHUTTING_DOWN] }
   # FIXME: "failed" is misleading too...
   scope :failed,    :conditions => { :state => FAILED_STATES }
diff --git a/src/app/models/instance_observer.rb b/src/app/models/instance_observer.rb
index 77b2e12..d42fbe6 100644
--- a/src/app/models/instance_observer.rb
+++ b/src/app/models/instance_observer.rb
@@ -95,7 +95,6 @@ class InstanceObserver < ActiveRecord::Observer
     # This can get stale, so reload it -- if it exists
     instance.deployment.reload if instance.deployment
     if instance.state_changed?
-      instance.deployment.update_state(instance) if instance.deployment
       event = Event.new(:source => instance, :event_time => DateTime.now,
                         :summary => "state changed to #{instance.state}",
                         :change_hash => instance.changes, :status_code => instance.state)
@@ -126,6 +125,7 @@ class InstanceObserver < ActiveRecord::Observer
           :status_code => 'vanished', :summary => "Instance disappeared from cloud provider"
         )
       end
+      instance.deployment.update_state(instance) if instance.deployment
 
     end
   end
diff --git a/src/spec/models/deployment_spec.rb b/src/spec/models/deployment_spec.rb
index 3ffbae9..48df0ad 100644
--- a/src/spec/models/deployment_spec.rb
+++ b/src/spec/models/deployment_spec.rb
@@ -428,6 +428,8 @@ describe Deployment do
         end
 
         it "should stop all running instances if an instance fails" do
+          @inst1.state = Instance::STATE_RUNNING
+          @inst1.save!
           Instance.any_instance.stub(:stop).and_return(true)
           Instance.any_instance.should_receive(:stop)
           @inst2.state = Instance::STATE_ERROR
-- 
1.7.7.6




More information about the aeolus-devel mailing list